ponylang / corral

:horse: Pony dependency manager tool
https://ponylang.io
BSD 2-Clause "Simplified" License
194 stars 20 forks source link

Misleading error on update where dependency bundle is not found #99

Open Theodus opened 4 years ago

Theodus commented 4 years ago

corral.json:

{
  "deps": [
    {
      "locator": "github.com/ponylang/peg.git",
      "version": "0.1.0"
    }
  ],
  "info": {}
}
$ corral update
update: updating from /home/theodus/src/wip/changelog-tool
git cloning github.com/ponylang/peg.git into /home/theodus/src/wip/changelog-tool/_repos/github_com_ponylang_peg_git
update: will load dep: github.com/ponylang/peg @ 0.1.0
git checking out @0.1.0 into /home/theodus/src/wip/changelog-tool/_corral/github_com_ponylang_peg
Error loading dep bundle: github_com_ponylang_peg

The corresponding warning for the fetch command should also mark itself as a warning:

$ corral fetch
fetch: fetching from /home/theodus/src/wip/changelog-tool
git fetching github.com/ponylang/peg.git into /home/theodus/src/wip/changelog-tool/_repos/github_com_ponylang_peg_git
fetch: fetching dep: github.com/ponylang/peg @ 0.1.0
git checking out @0.1.0 into /home/theodus/src/wip/changelog-tool/_corral/github_com_ponylang_peg
No dep bundle for: github.com/ponylang/peg.git
SeanTAllen commented 4 years ago

The problem with this comes down to the relationship between a project and a bundle.

Corral internally allows for the concept of a project without a bundle, but the update code will always try to load and then consider it an error. The lack of bundle.json for transitive dependencies shouldn't be an error in my mind.

The specific error is here:

https://github.com/ponylang/corral/blob/master/corral/bundle/project.pony#L62

The big question here is...

should there be an error message when you use a project that doesn't have a corral.json?

I think maybe no and that we don't output anything. (Nothing because I think warnings are dumb, things should either be an error or not).

The argument in favor of making this an error is that it will help push everyone to start using corral.json in all their projects.

But if an error it should stop processing and let you know it will not continue. This would put pressure on the Pony community to bring libraries up to date with working with corral. It might mean that in order to use a dependency, that you have to fork it to add a bundle.json, but if a library is abandoned, that might not be the worst idea.

adri326 commented 4 years ago

If an error is kept, it should say that the specific dependency is missing a corral.json file:

Error loading dep bundle: github_com_ponylang_peg is missing a corral.json config file
CandleCandle commented 4 years ago

I am in favour of encouraging people to use the same patterns, however, I would not want to block a developer from using a 3rd party dependency if it is the correct tool for the job.

If a dependency does not have a corral.json then corral should behave as if it had no dependencies. The message can be something like:

Warning: loading dep bundle: github_com_ponylang_peg is missing a corral.json, no transitive dependencies will be added.

Informing the user that this is something that should be fixed and may have detrimental effects if it is not fixed, however, it should not be a terminal error.

An example:

MainProject
  |- CorralBasedA
  | \- CorralBasedC 
  |- CorralBasedB
  \- NonCorralBased

In the above example, NonCorralBased may have dependencies, but it is role of the developer of MainProject to add those dependencies to MainProject

niclash commented 4 years ago

I am with @SeanTAllen that warnings are bad, especially when they refer to something that I can't fix. Having a dependency on another project that is lacking corral.json (and bundle.json) is such a case. Consume it as if no dependencies and give no warnings is IMHO the right way.

CandleCandle commented 4 years ago

Consume it as if no dependencies

This is the key point, feedback to the user is optional.