Dep: ProjectRoot name validation should be just a Warn, not a Failure

Created on 25 Jul 2018  Â·  8Comments  Â·  Source: golang/dep

What version of dep are you using (dep version)?


v0.4.1-308-gba8d7503

What dep command did you run?

$ dep ensure -v

With the following Gopkg.toml content:

[[constraint]]
  name = "gitlab.com/organization/version/dependent-api"
  source = "https://gitlab.com/organization/version/dependent-api.git"
  version = "0.11.6"

What did you expect to see?

Root project is "gitlab.com/organization/some-tests"
 9 transitively valid internal packages
 4 external packages imported from 4 projects
(0)   ✓ select (root)
(1)     ? attempt golang.org/x/oauth2 with 1 pkgs; 1 versions to try
(1)         try golang.org/x/oauth2@master
(1)     ✓ select golang.org/x/oauth2@master w/2 pkgs
[...]
(8)     ? attempt gitlab.com/organization/version/dependent-api (from https://gitlab.com/organization/version/dependent-api.git) with 1 pkgs; 60 versions to try
(9)         try gitlab.com/organization/version/dependent-api (from https://gitlab.com/organization/version/dependent-api.git)@v0.11.6
(9)     ✓ select gitlab.com/organization/version/dependent-api (from https://gitlab.com/organization/version/dependent-api.git)@v0.11.6 w/1 pkgs
[...]
  TOTAL: 26.514353756s

(1/15) Wrote github.com/googleapis/[email protected]
(2/15) Wrote gitlab.com/organization/version/dependent-api (from https://gitlab.com/organization/version/dependent-api.git)@v0.11.6
(3/15) Wrote golang.org/x/oauth2@master
[...]

It could be releveant to display a Warning on this kind of case but not an error.

What did you see instead?

The following issues were found in Gopkg.toml:

  ✗ the name for "gitlab.com/organization/version/dependent-api" should be changed to "gitlab.com/organization/version"

ProjectRoot name validation failed

All 8 comments

It's gotta be a failure, actually - dep's current design can't work properly unless it points at a repository root.

The wording on the error message could be improved to more clearly indicate this, though.

I'm not sure to understand well, but we just talk about the name here, not the repository.

As far as I guess, we could do this check only if there is no source property in the Gopkg.toml file for this constraint.

To be more precise, I completely agree with you to raise an error in this case, because we are not pointg to a repository root:

[[constraint]]
  name = "gitlab.com/organization/version/dependent-api"
  version = "0.11.6"

But not in this case, because we specify the source:

[[constraint]]
  name = "gitlab.com/organization/version/dependent-api"
  source = "https://gitlab.com/organization/version/dependent-api.git"
  version = "0.11.6"

For information, I already tried to replace this code block in manifest.go file:

        origPR, err := sm.DeduceProjectRoot(string(pr))
        if err != nil {
            errorCh <- err
        } else if origPR != pr {
            errorCh <- fmt.Errorf("the name for %q should be changed to %q", pr, origPR)
        }

With:

        _, err := sm.DeduceProjectRoot(string(pr))
        if err != nil {
            errorCh <- err
        }

And everything worked well (for my case) but I don't know which can be the impact for other use cases.

@sdboyer ?

hiya @lhauspie - wow, you charged further down this path than anyone else has!

i'm going to try to take a look at your PR later this week. This has been a thorn in our side for a while. tbqh, i'm not optimistic - this could cause confusion about the canonicality of names on disk, when we're reliant on having a particular bit of info from Gopkg.toml - but it's not impossible.

Thanks a lot @sdboyer to take time to review my PR.

Don't hesitate.

@sdboyer Did you get a chance to look at @lhauspie PR yet? I see that it does not have much activity, and this would solve so many problems!

Dep was officially deprecated earlier this year, and the proposal to archive this repository was accepted. As such, I'm closing outstanding issues before archiving the repository. For any further comments, please use the proposal thread on the Go issue tracker. Thanks!

Was this page helpful?
0 / 5 - 0 ratings