glide icon indicating copy to clipboard operation
glide copied to clipboard

failure to generate lock file due to conflicting package versions

Open timoreimann opened this issue 8 years ago • 11 comments

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).

timoreimann avatar Aug 19 '16 17:08 timoreimann

@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?

mattfarina avatar Aug 19 '16 17:08 mattfarina

@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?

timoreimann avatar Aug 19 '16 20:08 timoreimann

@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

sdboyer avatar Aug 20 '16 00:08 sdboyer

@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. ;-)

timoreimann avatar Aug 20 '16 09:08 timoreimann

@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.

sdboyer avatar Aug 20 '16 17:08 sdboyer

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.

timoreimann avatar Aug 27 '16 12:08 timoreimann

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

xh3b4sd avatar Dec 11 '16 15:12 xh3b4sd

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

stevenroose avatar Mar 28 '17 16:03 stevenroose

@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 ()

clee avatar May 30 '17 23:05 clee

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 ()

jiangytcn avatar Aug 02 '17 08:08 jiangytcn

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 ?

ykumar-rb avatar Apr 09 '19 07:04 ykumar-rb