Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

failure to generate lock file due to conflicting package versions #564

Open
timoreimann opened this issue Aug 19, 2016 · 11 comments
Open

failure to generate lock file due to conflicting package versions #564

timoreimann opened this issue Aug 19, 2016 · 11 comments

Comments

@timoreimann
Copy link

timoreimann commented Aug 19, 2016

When trying to glide upgrade -v with the latest HEAD version (commit 8ec4e94) and no glide.lock file existing, my run finishes with an error:

[INFO]  --> Exporting github.com/prometheus/client_model
[INFO]  --> Exporting github.com/pmezard/go-difflib
[INFO]  --> Exporting golang.org/x/sys
[INFO]  --> Exporting bitbucket.org/ww/goautoneg
[INFO]  --> Exporting k8s.io/kubernetes
[INFO]  --> Exporting gopkg.in/yaml.v2
[INFO]  Replacing existing vendor dependencies
[ERROR] Failed to generate lock file: Generating lock produced conflicting versions of github.com/prometheus/client_model. import (), testImport (fa8ad6fec33561be4280a8f0514318c79d7f6cb6)

The package in question was (correctly) identified as a testImport in my glide.yaml file (initially populated through glide create with the version specifier manually added by me):

testImport:
- package: github.com/prometheus/client_model
  version: fa8ad6fec33561be4280a8f0514318c79d7f6cb6
  subpackages:
  - go

My code doesn't use the package in non-test code anywhere. I don't see client_model being referenced elsewhere in glide.yaml either (i.e., there's no duplicate entry in the importsection).

The state of my vendor/ folder is also somewhat messy: I had it vendor-stripped and committed before but after the failed run, git status is reporting a huge number of changes (reminding me of #367). What I can also see is a number of nested vendor folders, with some of them referencing the offending prometheus package. My guess is that this is where the conflict stems from, even though the error message apparently claims that the set of (non-test) imports is empty.

The run does not produce a lock file.

What I'm double-checking right now: AFAIR, I haven't seen this problem with 0.11.1 and not even with versions of glide a few commits ago (some of them certainly past the last 0.11.1 release).

@mattfarina
Copy link
Member

@timoreimann this is a good question.

  • This is happening now rather than in earlier versions because test imports are now managed by Glide.
  • A transitive dependency (dependency of a dependency) is asking to use github.com/prometheus/client_model. That's why it ends up in the imports section.
  • A fix for this, right now, is to list this package in your import section with the version you want to use and remove from your testImport.

Any thoughts on a better UX here?

@timoreimann
Copy link
Author

timoreimann commented Aug 19, 2016

@mattfarina thanks for the quick feedback.

It seems to me that the user might actually not be able to reliably determine when a particular dependency can be considered test-only or not -- at least not over time as the number of dependencies evolve. I feel like it should be up to glide exclusively to make that judgement by means of analyzing the direct and transitive dependencies and "shifting" packages between the the import and test import groups as appropriate. Technically, this could mean that the testImport configuration key from glide.yaml disappears in future versions, and classification decisions are recorded in the lock file only.
Does that seem conceivable and reasonable to do?

Apart from that, I wonder if we can improve the way glide deals with this error in the current implementation:

  • The import version in the error message is empty. Shouldn't that be something non-empty? Perhaps the error message should be completely different, saying something along the lines of "the package is requested as both an import and test-import; move the test-import into the import section to resolve the issue"?
  • My understanding was that glide works on a temporary directory when updating dependencies and only swaps vendor folders when it's clear there's no error. As mentioned earlier, my vendor folder looked pretty dirty after the failing update run. Is that a bug we need to tackle possibly?

@sdboyer
Copy link
Member

sdboyer commented Aug 20, 2016

@timoreimann all of this that you've described is already done, working, and tested in gps :)

Well, almost all. We don't annotate packages in the resulting lock file as being test-only or not. It's not a trivial refactor, but it's not a terrible one, either...though I'm not sure how important it is as an immediate, practical matter. Issue for it is sdboyer/gps#44. I've also noted this on the list on #565

@timoreimann
Copy link
Author

timoreimann commented Aug 20, 2016

@sdboyer is it reasonable to consider backporting the working solution from gps to glide? Or are the two fundamentally too different to justify such an effort (which is my assumption)?

The least we should do from my perspective is to help the user understand what's going on and make sure the vendor folder isn't touched in case of this error (which summarizes my two bullet points). Having to ask the user for manual resolution is not ideal but hopefully rather cheap to implement and better than what we have now, at least until glide/gps is able to deal with this autonomously.

By the way, what's the actual benefit of classifying into build and test imports in glide currently? I suppose you want to enable a cleaner separation between installing dependencies required for production vs. testing. Is there already a glide work flow supporting this model, along with a set of commands / switches?
(Why I am pondering about this: If testImport isn't really leveraged yet, it might be worth thinking about withdrawing the parameter again or hiding it behind a flag until the feature is fully ready for prime time.)

Side note: My expectations of gps are so high by now, if this thing won't be able to resolve dependencies fully automatically while also brewing my morning coffee at the same time, I'll be heavily disappointed. ;-)

@sdboyer
Copy link
Member

sdboyer commented Aug 20, 2016

@timoreimann

is it reasonable to consider backporting the working solution from gps to glide? Or are the two fundamentally too different to justify such an effort (which is my assumption)?

correct, it's not feasible. fundamentally different algorithms. best thing to do is get gps merged in 💯

The least we should do from my perspective is to help the user understand what's going on and make sure the vendor folder isn't touched in case of this error (which summarizes my two bullet points). Having to ask the user for manual resolution is not ideal but hopefully rather cheap to implement and better than what we have now, at least until glide/gps is able to deal with this autonomously.

The topic of how much "automated" resolution is good is one that's come up quite frequently in the community discussions in this space. I expect the committee that's forming will deal with it quite extensively.

By the way, what's the actual benefit of classifying into build and test imports in glide currently?

The main one I always think of is the ability to conditionally download only those dependences that are needed for the task at hand. (The same applies to tagging for os/arch).

My expectations of gps are so high by now, if this thing won't be able to resolve dependencies fully automatically while also brewing my morning coffee at the same time, I'll be heavily disappointed. ;-)

I am truly loathe to oversell things, but when people bring up specific cases that I've addressed, I feel OK saying something about it :) There are holes in gps, and I'm sure we'll find more as time goes on...but this isn't one of them.

@timoreimann
Copy link
Author

Any chance we can move forward with this issue in a way that doesn't require waiting for gps to get merged? (Unless that's a thing close to completion, which it wasn't last time I checked.)

As mentioned, I see two possible short term solutions:

  1. Remove/disable testImport until gps is in. My assumption here is that features like conditional downloads aren't part of glide yet, so we'd lose nothing.
  2. Improve the log message and prevent glide from messing up the vendor folder when the problem occurs.

I'd be happy to try to implement either of two approaches or any third one that might be preferred.

Thanks.

@xh3b4sd
Copy link

xh3b4sd commented Dec 11, 2016

I was just hit by this and would like to see some progress here as well.

@stevenroose
Copy link
Contributor

I think the best solution is to don't have a version constraint on your test dependencies.

cfchou added a commit to cfchou/go-gentle that referenced this issue May 9, 2017
@clee
Copy link

clee commented May 30, 2017

@stevenroose I don't think that "solution" works for some of us. For example, I have a testImport for github.com/stretchr/testify with no version specified at all, but because one of my other imports already imports stretch/testify (via Godep, and with a hardcoded revision number, thanks Godep), I get this error message from glide:

[ERROR]	Failed to generate lock file: Generating lock produced conflicting versions of github.com/stretchr/testify. import (089c7181b8c728499929ff09b62d3fdd8df8adff), testImport ()

@jiangytcn
Copy link

same issue and no version specified at all

testImport:
- package: github.com/onsi/ginkgo
- package: github.com/onsi/gomega
  subpackages:
  - gbytes
  - gexec
- package: gopkg.in/DATA-DOG/go-sqlmock.v1
Failed to generate lock file: Generating lock produced conflicting versions of github.com/onsi/ginkgo. import (43e2af1f01ace55adbb6d7d0f30416476db1baae), testImport ()

@ykumar-rb
Copy link

ykumar-rb commented Apr 9, 2019

Getting similar issue:

Failed to generate lock file: Generating lock produced conflicting versions of github.com/golang/mock. import (), testImport (v1.1.1).

I have specified version in glide.yaml
Any suggestion ?

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

No branches or pull requests

9 participants