Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

dep ensure fails on subpackage with a revision constraint #705

Closed
ellotheth opened this issue Jun 2, 2017 · 11 comments
Closed

dep ensure fails on subpackage with a revision constraint #705

ellotheth opened this issue Jun 2, 2017 · 11 comments

Comments

@ellotheth
Copy link

ellotheth commented Jun 2, 2017

What version of Go are you using (go version)/operating system and processor architecture?

$ go version
go version go1.8.3 linux/amd64

What did you do?

I've been using Glide, and I'm trying to migrate to dep. I don't want to upgrade my dependencies in the project, so I'm using [[constraint]] to fix them at the revision specified in my glide.lock file.

cd `mktemp -d`
export GOPATH=$PWD
mkdir -p src/github.com/ellotheth/deptest
cd src/github.com/ellotheth/deptest
cat <<EOD > main.go
package main

import (
        "fmt"
        "github.com/mikespook/gearman-go/worker"
)

func main() {
        w := worker.New(worker.OneByOne)
        fmt.Println(w)
}
EOD
cat <<EOD > Gopkg.toml
[[constraint]]
  name = "github.com/mikespook/gearman-go"
  revision = "b79fee29655dcdce7ba4a738f8388f9862b6779e"
EOD
dep ensure -v

What did you expect to see?

A successful dependency graph for the revision I requested.

What did you see instead?

Root project is "github.com/ellotheth/deptest"
 1 transitively valid internal packages
 1 external packages imported from 1 projects
(0)   ✓ select (root)
(1)     ? attempt github.com/mikespook/gearman-go with 1 pkgs; 9 versions to try
(1)         try github.com/mikespook/gearman-go@b79fee29655dcdce7ba4a738f8388f9862b6779e
(2)     ✗   github.com/mikespook/gearman-go at b79fee29655dcdce7ba4a738f8388f9862b6779e has problem subpkg(s):
(2)             github.com/mikespook/gearman-go/worker has err (*pkgtree.LocalImportsError); required by (root).
(1)         try github.com/mikespook/gearman-go@0.1.3
(2)     ✗   github.com/mikespook/gearman-go@0.1.3 not allowed by constraint b79fee29655dcdce7ba4a738f8388f9862b6779e:
(2)         b79fee29655dcdce7ba4a738f8388f9862b6779e from (root)
(1)         try github.com/mikespook/gearman-go@0.1.2
(2)     ✗   github.com/mikespook/gearman-go@0.1.2 not allowed by constraint b79fee29655dcdce7ba4a738f8388f9862b6779e:
(2)         b79fee29655dcdce7ba4a738f8388f9862b6779e from (root)
(1)         try github.com/mikespook/gearman-go@0.1.1
(2)     ✗   github.com/mikespook/gearman-go@0.1.1 not allowed by constraint b79fee29655dcdce7ba4a738f8388f9862b6779e:
(2)         b79fee29655dcdce7ba4a738f8388f9862b6779e from (root)
(1)         try github.com/mikespook/gearman-go@0.1
(2)     ✗   github.com/mikespook/gearman-go@0.1 not allowed by constraint b79fee29655dcdce7ba4a738f8388f9862b6779e:
(2)         b79fee29655dcdce7ba4a738f8388f9862b6779e from (root)
(1)         try github.com/mikespook/gearman-go@0.0.1
(2)     ✗   github.com/mikespook/gearman-go@0.0.1 not allowed by constraint b79fee29655dcdce7ba4a738f8388f9862b6779e:
(2)         b79fee29655dcdce7ba4a738f8388f9862b6779e from (root)
(1)         try github.com/mikespook/gearman-go@master
(2)     ✗   github.com/mikespook/gearman-go@master not allowed by constraint b79fee29655dcdce7ba4a738f8388f9862b6779e:
(2)         b79fee29655dcdce7ba4a738f8388f9862b6779e from (root)
(1)         try github.com/mikespook/gearman-go@0.1-testing
(2)     ✗   github.com/mikespook/gearman-go@0.1-testing not allowed by constraint b79fee29655dcdce7ba4a738f8388f9862b6779e:
(2)         b79fee29655dcdce7ba4a738f8388f9862b6779e from (root)
(1)         try github.com/mikespook/gearman-go@0.2-dev
(2)     ✗   github.com/mikespook/gearman-go@0.2-dev not allowed by constraint b79fee29655dcdce7ba4a738f8388f9862b6779e:
(2)         b79fee29655dcdce7ba4a738f8388f9862b6779e from (root)
(1)       ← no more versions of github.com/mikespook/gearman-go to try; begin backtrack
  ✗ solving failed

Solver wall times by segment:
  b-source-exists: 2.541066715s
      b-list-pkgs:  38.964791ms
          satisfy:    131.412µs
      select-root:     100.39µs
         new-atom:     90.703µs
       b-pair-rev:     38.656µs
  b-list-versions:     27.778µs
        b-matches:      9.972µs
            other:      1.576µs

  TOTAL: 2.580431993s

ensure Solve(): No versions of github.com/mikespook/gearman-go met constraints:
        b79fee29655dcdce7ba4a738f8388f9862b6779e: Could not introduce github.com/mikespook/gearman-go@b79fee29655dcdce7ba4a738f8388f9862b6779e, as its subpackage github.com/mikespook/gearman-go/worker does not contain usable Go code (*pkgtree.LocalImportsError).. (Package is required by (root).)
        0.1.3: Could not introduce github.com/mikespook/gearman-go@0.1.3, as it is not allowed by constraint b79fee29655dcdce7ba4a738f8388f9862b6779e from project github.com/ellotheth/deptest.
        0.1.2: Could not introduce github.com/mikespook/gearman-go@0.1.2, as it is not allowed by constraint b79fee29655dcdce7ba4a738f8388f9862b6779e from project github.com/ellotheth/deptest.
        0.1.1: Could not introduce github.com/mikespook/gearman-go@0.1.1, as it is not allowed by constraint b79fee29655dcdce7ba4a738f8388f9862b6779e from project github.com/ellotheth/deptest.
        0.1: Could not introduce github.com/mikespook/gearman-go@0.1, as it is not allowed by constraint b79fee29655dcdce7ba4a738f8388f9862b6779e from project github.com/ellotheth/deptest.
        0.0.1: Could not introduce github.com/mikespook/gearman-go@0.0.1, as it is not allowed by constraint b79fee29655dcdce7ba4a738f8388f9862b6779e from project github.com/ellotheth/deptest.
        master: Could not introduce github.com/mikespook/gearman-go@master, as it is not allowed by constraint b79fee29655dcdce7ba4a738f8388f9862b6779e from project github.com/ellotheth/deptest.
        0.1-testing: Could not introduce github.com/mikespook/gearman-go@0.1-testing, as it is not allowed by constraint b79fee29655dcdce7ba4a738f8388f9862b6779e from project github.com/ellotheth/deptest.
        0.2-dev: Could not introduce github.com/mikespook/gearman-go@0.2-dev, as it is not allowed by constraint b79fee29655dcdce7ba4a738f8388f9862b6779e from project github.com/ellotheth/deptest.

Notes

If I use github.com/mikespook/gearman-go/worker as the constrained package, I get the correct revision for that subpackage, but a different revision for the parent:

cat <<EOD > Gopkg.toml
[[constraint]]
  name = "github.com/mikespook/gearman-go/worker"
  revision = "b79fee29655dcdce7ba4a738f8388f9862b6779e"
EOD
dep ensure -v

produces

Root project is "github.com/ellotheth/deptest"
 1 transitively valid internal packages
 1 external packages imported from 1 projects
(0)   ✓ select (root)
(1)     ? attempt github.com/mikespook/gearman-go/worker with 1 pkgs; 9 versions to try
(1)         try github.com/mikespook/gearman-go/worker@b79fee29655dcdce7ba4a738f8388f9862b6779e
(1)     ✓ select github.com/mikespook/gearman-go/worker@b79fee29655dcdce7ba4a738f8388f9862b6779e w/1 pkgs
(2)     ? attempt github.com/mikespook/gearman-go with 1 pkgs; 8 versions to try
(2)         try github.com/mikespook/gearman-go@0.1.3
(2)     ✓ select github.com/mikespook/gearman-go@0.1.3 w/2 pkgs
(3)     ? attempt github.com/mikespook/golib with 1 pkgs; 1 versions to try
(3)         try github.com/mikespook/golib@master
(3)     ✓ select github.com/mikespook/golib@master w/1 pkgs
  ✓ found solution with 4 packages from 3 projects

with the following lock file:

# This file is autogenerated, do not edit; changes may be undone by the next 'dep ensure'.


[[projects]]
  name = "github.com/mikespook/gearman-go"
  packages = ["client","common"]
  revision = "d6c6bfe9f011021c28ca173d8a4f82faf3607cdf"
  version = "0.1.3"

[[projects]]
  name = "github.com/mikespook/gearman-go/worker"
  packages = ["."]
  revision = "b79fee29655dcdce7ba4a738f8388f9862b6779e"

[[projects]]
  branch = "master"
  name = "github.com/mikespook/golib"
  packages = ["autoinc"]
  revision = "38fe6917d34b267a79c3432369e7dcc379e2d5b3"

[solve-meta]
  analyzer-name = "dep"
  analyzer-version = 1
  inputs-digest = "5198c4ccc70d8f6df11ed15e0596e90e43d11a4993949265b82866fd3de4032c"
  solver-name = "gps-cdcl"
  solver-version = 1
@ellotheth ellotheth changed the title dep ensure fails on subpackage with a revision constraint dep ensure fails on subpackage with a revision constraint Jun 2, 2017
@sdboyer
Copy link
Member

sdboyer commented Jun 2, 2017

hi, welcome! thanks for the detailed issue 🎉

I've been using Glide, and I'm trying to migrate to dep. I don't want to upgrade my dependencies in the project, so I'm using [[constraint]] to fix them at the revision specified in my glide.lock file.

This is exactly what we're hoping for folks not to do 😢. I thought we had an FAQ entry for this, but apparently not.

When you do this, it turns Gopkg.toml into a duplicate of Gopkg.lock, and you end up missing out on a lot of the benefit of using dep. Avoiding upgrades should be a matter of not passing -update to dep ensure - not changing anything in Gopkg.toml.

That said, until we make a bit more progress on #277 (hopefully in the next week or so), dep still suffers from the same problem that glide does where you can't update only a single dependency at a time. If that's what you're guarding against, then I get it. But once we get that fixed, I hope you'll be able to stop using dep this way.

Anyway, on to your problem. The central thing is this:

ensure Solve(): No versions of github.com/mikespook/gearman-go met constraints:
        b79fee29655dcdce7ba4a738f8388f9862b6779e: Could not introduce github.com/mikespook/gearman-go@b79fee29655dcdce7ba4a738f8388f9862b6779e, as its subpackage github.com/mikespook/gearman-go/worker does not contain usable Go code (*pkgtree.LocalImportsError).. (Package is required by (root).)

That's happening because there's a relative import in one of the tests in that package. Some explanations:

  1. Right now, we have rather draconian denial rules about relative imports, originally derived from the compiler's wariness about them.
  2. This case, though we could probably admit it, as it's probably possible for us to know that the .. is not only lexically within the project, but structurally as well, as we control the whole directory tree.
  3. More importantly, this is a straight up bug - it should be admissible because that import was in a test file, which we were never going to use anyway. This demonstrates that we need to differentiate between errors in normal package files and test files - or just, more generally, parameterize (Parameterization: OS/Arch/build tags #291) errors.

Almost certainly this is more than you're looking for, @ellotheth, but it points to issues we need to resolve, so I have to highlight them 😄

If I use github.com/mikespook/gearman-go/worker as the constrained package, I get the correct revision for that subpackage, but a different revision for the parent:

This is...weird. Very weird.

Constraints cannot be applied to arbitrary packages - only the root of a project. Or at least, they should not be able to be applied to arbitrary packages. The fact that it worked for you above is a happy accident, but it's undefined behavior. We need errors to guard against it, and are working on them in e.g. #697 (though that's slightly different).

It's even weirder, though, that the error with the relative import didn't re-occur. Definitely needs some investigating.

@ellotheth
Copy link
Author

Thank you enormously for the comprehensive response. I appreciate your time!

This is exactly what we're hoping for folks not to do 😢.

Ok so before this causes anyone a sad, I swear there's method to my madness:

  1. My project has a few dependencies that don't use versioning, or that use very limited versioning. My Glide config asks for the master branch for these projects, so they're always out of date.
  2. I don't want my Migrate to dep task to become Migrate to dep and oops update all the dependencies, because updating the dependencies means incurring all the overhead of checking for backwards-compatibility breaks. If I lock the initial revisions in dep, I can do the migration without worrying about the content of my build.
  3. Once the migration to dep is working, I can go back to Gopkg.toml and update my constraints to use branches or versions, or remove the constraints completely, as appropriate.

(I could also have updated all my dependencies with Glide and started moving to dep afterwards, but I wasn't sure how long it would take or if it would work.)

That's happening because there's a relative import in one of the tests in that package.

ARGH, you know what, I saw a couple other issues related to relative imports, so I checked all the non-test files in that mikespook/gearman-go/worker package, and didn't find any. I should have checked the test files!

Almost certainly this is more than you're looking for

Not at all! dep is trying to solve a hard problem, and the process is fascinating.

This is...weird. Very weird.

Ha! I'm glad it's not just me. If you end up needing more information, let me know!

@carolynvs
Copy link
Collaborator

I don't want my Migrate to dep task to become Migrate to dep and oops update all the dependencies

@ellotheth We just merged a feature that will read your glide configuration during dep init and use it to generate the Gopkg.toml and Gopkg.lock files. Is the repo you are working with public? I am interested in testing out how the migration looks for your case.

@ellotheth
Copy link
Author

@carolynvs Unfortunately the repo is not public; it's a backend service for WonderNetwork. That said, I'd be happy to test it out and report back!

@ellotheth
Copy link
Author

@carolynvs Gist here: https://gist.github.com/ellotheth/e96f8f2c5fdd1b9e09e183970cfb276b. It breaks on the mgo package, maybe because it's expecting semver-compatible versioning?

@carolynvs
Copy link
Collaborator

Thanks! I think the problem is that I'm converting "v2" to a semver instead of a branch when importing your glide.yaml. I'll poke around a bit more and see if that's right.

@sdboyer
Copy link
Member

sdboyer commented Jun 2, 2017

Thanks! I think the problem is that I'm converting "v2" to a semver instead of a branch when importing your glide.yaml. I'll poke around a bit more and see if that's right.

Probably is - I think I stumbled across this code last night. One of the main things we have to do with glide is figure out which type a string like that - which COULD be semver, OR a string-literal match to e.g. a branch, should be.

@sdboyer
Copy link
Member

sdboyer commented Jun 2, 2017

@ellotheth

My project has a few dependencies that don't use versioning, or that use very limited versioning. My Glide config asks for the master branch for these projects, so they're always out of date.

Even this should fall under the same general rubric as what I described above - even if you have to follow a branch, what's in Gopkg.lock will almost never change on a dep ensure, on adding new dependencies, etc.

It'll normally only change on a dep ensure -update - and, once we make the changes to it in #489, typically only if you specifically request that that project be updated: dep ensure -update path/to/project/root

Not at all! dep is trying to solve a hard problem, and the process is fascinating.

❤️ ❤️

@ellotheth
Copy link
Author

@sdboyer

Even this should fall under the same general rubric as what I described above - even if you have to follow a branch, what's in Gopkg.lock will almost never change on a dep ensure, on adding new dependencies, etc.

That works once I've got dep into the state I want it, but how do I get to that state?

I'm using Glide to track the master branch of gearman-go, because it stopped using semantic versions four years ago. Say I did a production deploy three weeks ago, and what was current master then is now master~14. I want to switch to dep, but I don't want to update my dependency.

I'm using Glide, so the gearman-go source is in my project vendor/ directory, not in my $GOPATH. Running dep init (with -skip-tools now!) gives me the most recently tagged version, which is four years old. I can't ask dep for master in Gopkg.toml, because that will give me current master instead of master~14. How should I get Gopkg.lock into the correct state to reproduce my production build?

@sdboyer
Copy link
Member

sdboyer commented Jun 3, 2017

That works once I've got dep into the state I want it, but how do I get to that state?

Ahh, gotcha. i believe that dep init (without -skip-tools - that would skip the glide converter!) should read your current glide.yaml and glide.lock, and translate them pretty directly to Gopkg.toml and Gopkg.lock, respectively. That is, if the SHA1 of master~14 is recorded in glide.lock, it should be translated over to Gopkg.lock, and dep init shouldn't move it.

@fractalawareness
Copy link

fractalawareness commented Apr 17, 2019

3. More importantly, this is a straight up bug - it should be admissible because that import was in a test file, which we were never going to use anyway. This demonstrates that we need to differentiate between errors in normal package files and test files - or just, more generally, parameterize (#291) errors.

Is there any workaround for this bug? We use many packages that contain test files like that.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants