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

Import external configuration from dependencies during init #821

Closed
chriswhelix opened this issue Jul 15, 2017 · 7 comments
Closed

Import external configuration from dependencies during init #821

chriswhelix opened this issue Jul 15, 2017 · 7 comments

Comments

@chriswhelix
Copy link

Use case

We have about 20 projects currently using Glide for dependency management. Many of these projects rely on a shared internal framework library (let's call this repo "framework"), that itself relies on specific versions of various external libraries. Let's consider one project, "projectA" that I want to migrate to dep.

Assume my initial state is like:

projectA/glide.yaml:

  • package: github.com/myhelix/framework
    version: ^2.8.1

framework/glide.yaml:

  • package: github.com/dgrijalva/jwt-go
    version: ^2.7.0
  • package: github.com/ansel1/merry
    version: c60dfce5f3ad230b54625a81b2bb65a7e4f1e64b
    repo: git@github.com:myhelix/merry.git

In other words, our framework requires jwt-go 2.x (not 3.0 in master), and requires a specific revision of our forked version of merry.

When we run "glide up" on projectA, these constraints are imported from framework/glide.yaml, and we get the correct revisions of jwt-go and merry.

Now when I migrate projectA to dep, the dep init can pick up my requirement for framework ^2.8.1, but neither dep init nor dep ensure pay any attention to the constraints in framework/glide.yaml; this means that dep will retrieve the incompatible master versions of jwt-go and ansel1/merry.

This leaves me between a rock and a hard place. If I migrate "framework" to dep, that would fix projectA, but the 19 other projects still using glide would get the wrong versions next time they update, unless I also continue to maintain framework/glide.yaml. Maintaining both glide and dep versions of dependency information in "framework" would present a great deal of opportunity for error/confusion. So there's no practicable migration path for my organization from glide to dep.

Proposed solution

Have "dep ensure" recursively check dependencies for glide.yaml files, and include the constraints in those files in addition to those in Gopkg.toml.

Open questions

What to do when there are conflicting constraints?

One option would be to just fail. But it might be nicer to allow a constraint specified in Gopkg.toml to override any constraint imported from dependencies; that way if various dependencies have conflicting constraints, you can manually tiebreak by specifying what you want directly in Gopkg.toml.

I'm not sure if this has been addressed already in the context of support for recursive Gopkg.toml parsing, but it's worth noting that glide treats such conflicts as warnings, not errors, and moves on by some automatic rules of precedence. Example output:

[WARN] Conflict: golang.org/x/net rev is currently release-branch.go1.6, but github.com/gin-gonic/gin wants f315505cf3349909cdf013ea56690da34e96a451

These seem fairly common in my experience using glide, so a smooth transition would require some reasonable mechanism for dealing with it.

Related

#222 seems to be a more general case of this request.

@carolynvs
Copy link
Collaborator

@chriswhelix You started working on this at Gophercon, right?

@chriswhelix
Copy link
Author

@carolynvs yes, waiting to submit PR until I get our CLA issues squared away. But I didn't get much farther than turning it on as you suggested, and seeing that it works, anyway.

@drafnel
Copy link

drafnel commented Jul 24, 2017

@chriswhelix did you push your code anywhere that I can fetch and test?

@chriswhelix
Copy link
Author

@drafnel you can see my WIP here:
https://github.com/myhelix/dep/pull/1/files

So far I'm focused on adding test cases for "dep init" -- however, my initial testing did NOT appear to exhibit the behavior we're after in "dep ensure". Once I'm satisfied I have a complete set of tests for init, I will try to produce all the equivalent cases for ensure, and go from there.

@carolynvs
Copy link
Collaborator

carolynvs commented Jul 25, 2017

@chriswhelix How about submitting that branch here as a WIP PR? 😀 I promise to not nitpick while you work!

By the way, I noticed that you fixed a bug in the glide importer (using any constraints instead of latest) and that would be great to split off into a quickie PR that we could get merged quickly.

UPDATE: I think the fix for the any constraint may need to be made in InferConstraint itself. Working on that now...

@carolynvs
Copy link
Collaborator

I've submitted #901 to fix this for all the importers.

@chriswhelix
Copy link
Author

Cool, thanks, will rebase my WIP on that and open a PR for it shortly.

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

Successfully merging a pull request may close this issue.

4 participants