search

golang / dep

Go dependency management tool experiment (deprecated)
https://golang.github.io/dep/
BSD 3-Clause "New" or "Revised" License
12.85k stars 1.05k forks source link

dep can't upgrade from client-go release-4.0 to release-5.0 #1265

Closed davecheney closed 6 years ago

davecheney commented 7 years ago

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

v0.3.1-122-g98f625e

What dep command did you run?

The scenario is this.

  1. check out https://github.com/davecheney/dep-k8s-example
  2. dep ensure && go build

  3. Edit Gopkg.toml and apply this patch to upgrade from the release-4.0 branch to release-5.0. (The revision for apimachinery is taken from https://github.com/kubernetes/client-go/blob/release-5.0/Godeps/Godeps.json)

    lucky(~/src/github.com/davecheney/dep-k8s-example) % git diff
diff --git a/Gopkg.toml b/Gopkg.toml
index 58ae22c..1b91f62 100644
--- a/Gopkg.toml
+++ b/Gopkg.toml
@@ -8,11 +8,11 @@

[[constraint]]
name="k8s.io/client-go"
-  branch="release-4.0"
+  branch="release-5.0"

[[override]]
name="k8s.io/apimachinery"
-  revision="917740426ad66ff818da4809990480bcc0786a77"
+  revision="9d38e20d609d27e00d4ec18f7b9db67105a2bde0"

[[override]]
name="github.com/ugorji/go"
  4. Run dep ensure to fetch the new dependencies.

What did you expect to see?

I expected dep ensure to complete successfully.

What did you see instead?

lucky(~/src/github.com/davecheney/dep-k8s-example) % dep ensure                                                                                        
ensure Solve(): No versions of k8s.io/client-go met constraints:
        release-4.0: Could not introduce k8s.io/client-go@release-4.0, as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        v4.0.0: Could not introduce k8s.io/client-go@v4.0.0, as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        v3.0.0: Could not introduce k8s.io/client-go@v3.0.0, as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        v2.0.0: Could not introduce k8s.io/client-go@v2.0.0, as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        v1.5.1: Could not introduce k8s.io/client-go@v1.5.1, as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        v1.5.0: Could not introduce k8s.io/client-go@v1.5.0, as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        v1.4.0: Could not introduce k8s.io/client-go@v1.4.0, as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        v4.0.0-beta.0: Could not introduce k8s.io/client-go@v4.0.0-beta.0, as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        v3.0.0-beta.0: Could not introduce k8s.io/client-go@v3.0.0-beta.0, as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        v2.0.0-alpha.1: Could not introduce k8s.io/client-go@v2.0.0-alpha.1, as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        v2.0.0-alpha.0: Could not introduce k8s.io/client-go@v2.0.0-alpha.0, as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        master: Could not introduce k8s.io/client-go@master, as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        release-1.4: Could not introduce k8s.io/client-go@release-1.4, as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        release-1.5: Could not introduce k8s.io/client-go@release-1.5, as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        release-2.0: Could not introduce k8s.io/client-go@release-2.0, as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        release-3.0: Could not introduce k8s.io/client-go@release-3.0, as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        release-5.0: Could not introduce k8s.io/client-go@release-5.0, as its subpackage k8s.io/client-go/pkg/api/v1 is missing. (Package is required by (root).)
        revert-14-1.5: Could not introduce k8s.io/client-go@revert-14-1.5, as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.

dep cannot update to the release-5.0 branch because it would fail to compile. Superficially that is helpful, but unless dep upgrades the client-go dependency, I cannot fix the build breakage from the major api bump.

How can I use dep to fetch release-5.0 so I can fix the build breakage?

sdboyer commented 7 years ago

man, you just keep on bumping into these fundamental questions 😜 gotta love k8s, drawing out all the warts.

the simple answer for what you can do right now would be that you add k8s.io/client-go/pkg/api/v1 to the ignored list in Gopkg.toml. i think that'll let you make it through - it just won't care that that package doesn't exist. you can then remove that ignored directive once you've eliminated all instances of that import. please do let me know if that doesn't work.

this is a tip of a bigger iceberg, though. these sort of additional checks that dep does give it the characteristics of a strongly mean-reverting system - one where wild fluctuations tend to be dampened out, back towards some (working) midpoint. this tends to be good the overwhelming majority of the time, but can put the user in an awkward position when they need to make large transitions, as you do here.

the biggest original problem in this class this was the "how do i add a new dependency?" problem chicken-or-egg problem, now addressed by dep ensure -add. but your issue here is a good example of that same sort of "how do i get from here to there?" gap. and, if we end up adding type analysis, that gap will become much more pronounced.

my plan has been that, if we ever get to doing type analysis, it would absolutely 💯 need to come with some sort of -version-checks-only flag, which can be included in order to tell the solver to disable anything other than the essential version satisfiability checks. i don't think it's strictly necessary now, for cases like this, as recourse exists with ignored. but eventually, such a flag would let users live in these "in between" places, both for this case, and the likely much more common case of creating some temporary type mismatches while making slightly larger changes.

justinsb commented 7 years ago

Does this imply that the versions determined by dep depend not just on Gopkg.toml, but on application code as well?

sdboyer commented 7 years ago

yes.

On October 14, 2017 4:04:16 PM EDT, Justin Santa Barbara notifications@github.com wrote:

Does this imply that the versions determined by deps depend not just on Gopkg.toml, but on application code as well?

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/golang/dep/issues/1265#issuecomment-336663291

justinsb commented 6 years ago

Can you explain what we're trying to do here? I guess that for dependencies that don't define their dependencies we have to do this to infer their dependencies somehow. But for a user application using dep, we should make them declare their direct dependencies.

I'd suggest we should also warn about dependencies that are not good go-citizens, and do not include a Gopkg.toml.

Taking that to extremes (if we're trying to "fill in the gaps" until all libraries include a Gopkg.toml): disable all additional analysis entirely by default (including for transitive dependencies); we expect all libraries to declare a Gopkg.toml file. Then we have a flag --legacy which must be passed to enable dependency inference, and it suggests filing a github issue against the legacy library. Eventually --legacy inference would be removed from dep, likely moved to a plugin.

ianlewis commented 6 years ago

As an aside I had similar issues recently with apimachinery and it turns out that having a copy of it in your GOPATH could also cause issues. Albeit in my case, it might have been a poorly or partially installed version. Once I cleared it out it worked.

I think yours is a separate problem but it just might change things with a clean GOPATH.

sdboyer commented 6 years ago

@justinsb even if we were to make the changes you're proposing, it would have little practical bearing on this issue - but making such changes is not on the table:

Can you explain what we're trying to do here? I guess that for dependencies that don't define their dependencies we have to do this to infer their dependencies somehow.

they do define their dependencies - that's what import statements in the source code are. asking the user to duplicate that information into Gopkg.toml is tedious and invites the possibility of conflicting information, so we don't do it. the package management committee settled on this last year.

consider just how much information we would have to record in e.g. k8s' Gopkg.toml if we wanted to make it possible for a user to import a single package within k8s, and get only the direct external dependencies of that package, and of all the other internal k8s packages transitively imported by that package. (we'd have to duplicate both the package structure and the import statements in their entirety)

But for a user application using dep, we should make them declare their direct dependencies.

we literally can't make anyone do anything. there is no central point of control, of publishing, that could be used to enforce such a rule - and intentionally excluding segments of the ecosystem is a non-starter.

I'd suggest we should also warn about dependencies that are not good go-citizens, and do not include a Gopkg.toml.

we're a long way off from this, if we ever do it. and we're unlikely to, as if your library relies solely on stdlib, there's no reason at all to use dep.

@ianlewis if accurate, this is very concerning - the only time that GOPATH should matter in any way to dep is if you dep init -gopath. outside of that, i can't even conceive of how cleaning out GOPATH/src could change dep's behavior.

ianlewis commented 6 years ago

@sdboyer Yah, I though so too. Drove me bonkers trying to figure it out. I got errors like this:

$ mkdir -p $GOPATH/src/test
$ cd $GOPATH/src/test
$ curl -o main.go https://raw.githubusercontent.com/kubernetes/client-go/release-4.0/examples/out-of-cluster-client-configuration/main.go
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  2535  100  2535    0     0  14419      0 --:--:-- --:--:-- --:--:-- 14485

$ dep init
No versions of k8s.io/apimachinery met constraints:
    master: unable to update checked out version: : command failed: [git checkout 3b05bbfa0a45413bfa184edbf9af617e277962fb]: exit status 1
    release-1.6: unable to update checked out version: : command failed: [git checkout 35be0db31bd6e5635ef6366708dc2b137392f6a2]: exit status 1
    release-1.7: unable to update checked out version: : command failed: [git checkout 8ab5f3d8a330c2e9baaf84e39042db8d49034ae2]: exit status 1
    release-1.8: unable to update checked out version: : command failed: [git checkout 9d38e20d609d27e00d4ec18f7b9db67105a2bde0]: exit status 1

I'll see if I can find some time soon to repo it in a more meaningful way.

sdboyer commented 6 years ago

@ianlewis oooh ooh cache corruption cache corruption. could you try compiling dep from #1279 and seeing if that magically fixes it?

could you tar up $GOPATH/pkg/dep/sources/https---github.com-kubernetes-apimachinery and make it available so i can see the state of the repo? or, just running git status in that dir and seeing if anything seems odd/there's a dirty tree or untracked files, etc. do that, then #1279 😄

this is getting off-topic, though - let's please carry that discussion on in slack, or another issue. @davecheney, did the ignored approach work for you, at least for now?

ncdc commented 6 years ago

@sdboyer I've been trying the ignored approach too (for https://github.com/heptio/ark). I initially did what you suggested, and reran dep ensure -update k8s.io/client-go k8s.io/apimachinery k8s.io/kubernetes, which then listed another package that it had trouble with, so I kept adding to the ignored list until dep would run without complaining. But now I can't actually get it to pull in dependencies that are new to client-go v5.0.0. If I comment out the ignored list, it continues to complain about packages via transitive dependencies, e.g.:

v5.0.0: "k8s.io/client-go/discovery" transitively (through 1 packages) imports "k8s.io/client-go/pkg/api/v1", which contains malformed code: no package exists at "k8s.io/client-go/pkg/api/v1"

I don't have any references in the Ark repo to k8s.io/client-go/pkg/api* anywhere. Help?

sdboyer commented 6 years ago

@ncdc yeah, this story still needs improving 😢 it's not a common case, but it's definitely exacerbated by the complexity of k8s.

to be clear, though:

which then listed another package that it had trouble with,

these are packages from release-4.0 that no longer exist in release-5.0, right? those are the only ones i was imagining should go into ignored. if so, this:

But now I can't actually get it to pull in dependencies that are new to client-go v5.0.0.

confuses me...oh, wait! that'd just be because you don't actually import those yet. right.

OK, here's my suggestion for a virtuous loop that probably still won't be pleasant, but should at least get you through this with less hair-pulling:

  1. identify a package that has disappeared in the new version, and add it to the ignored list
  2. if that package has a corresponding package(s) in the new version, add those to the required list, including a comment about the old one it's replacing
  3. repeat 1-2 until solving goes through
  4. work through the list of each ignored package in your code, searching for instances of the import statement pointing at it and refactoring the corresponding file(s) to point to the new package(s), which you can handily reference b/c of the comments you made
  5. remove the entry from ignored, and corresponding required entries
  6. repeat 4-5 until you're totally converted

so, i'm imagining your WIP Gopkg.toml looks something like this:

ignored = [
  "something/we/actually/ignore/all/the/time",
# START REFACTORING LIST, ALL THESE ARE GONE WHEN WE'RE DONE
  "importpath/to/old/pkg1",
  "importpath/to/old/pkg2",
]

required = [
  "importpath/to/new/pkg1", # from "importpath/to/old/pkg1"
  "importpath/to/new/pkg2a", # from "importpath/to/old/pkg2"
  "importpath/to/new/pkg2b", # from "importpath/to/old/pkg2"
]
ncdc commented 6 years ago

these are packages from release-4.0 that no longer exist in release-5.0, right? those are the only ones i was imagining should go into ignored

Actually, no. Some of the packages listed are no longer in 5.0, but others are. Weird...

I'll give your suggestion a try and see how it goes. Will report back later. Thanks!

ncdc commented 6 years ago

@sdboyer something was messed up with my cache. Once I removed ~/go/pkg/dep/sources/https---github.com-kubernetes-client--go, I was able to get past my issues and have it download the new transitive dependencies. I'm still fixing compilation issues, but I'm optimistic I got past my main problem.

sdboyer commented 6 years ago

@ncdc uuuugh. hopefully #1279 will make the incidence of these cache issues much, much less frequent. but that's still us fighting blind in the dark, after the fact - i would love to figure out a way to better manage/report/something when these problems occur, rather than waiting for them to blow up. which is tricky, given that they almost entirely occur as a result of handling signals.

i want to merge that PR soon so that we can have people test it in the wild for a while before release. it's an adaptive failure mechanism, so the most we can really test it for is that it doesn't have unintended consequences on the happy path, or misbehaviors on certain known failure paths.

ncdc commented 6 years ago

@sdboyer in my case, I'm pretty sure the cached copy of client-go wasn't dirty. I'm not 100% positive, but I know I didn't cd in there and muck with it. It had the v5.0.0 tag checked out, but it most definitely did not match a manual clone & checkout of that tag. Very odd.

sdboyer commented 6 years ago

@ncdc any chance that upstream folks had force-pushed that tag at some point?

ncdc commented 6 years ago

@sdboyer can't say for sure, but we don't think so.

ianlewis commented 6 years ago

@sdboyer Sounds good. Will follow up.

sdboyer commented 6 years ago

i'm gonna close this one out, as we at least have an adequate workaround (even if not ideal)