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

how to rewrite import paths #127

Closed
kr opened this Issue Oct 4, 2014 · 3 comments

Comments

Projects
None yet
4 participants
@kr
Member

kr commented Oct 4, 2014

There is a problem with the current design of godep save -r.

Background

Some projects use godep save -r to rewrite import statements in files that import their dependencies. This makes the repo self-contained—having no external dependencies—so it can be built with an ordinary “go get” command while retaining reproducibility.

Now consider a repo r that contains two packages: command r/c and non-main package r/p. Command r/c imports r/p, which imports an external dependency d. There are (at least) two ways one might rewrite import statements when copying d into r:

  1. Copy d to r/Godeps/_workspace/src/d. Change any import statement in the tree rooted at r from “d” to “r/Godeps/_workspace/src/d”.
  2. Copy d to r/Godeps/_workspace/src/d and copy r/p to r/Godeps/_workspace/src/r/p. Change any import statement in r/c and the tree rooted at r/Godeps/_workspace/src from “d” to “r/Godeps/_workspace/src/d” and from “r/p” to “r/Godeps/_workspace/src/r/p”. Note that import statements in r/p remain unchanged.

The first method above is the current behavior of godep save -r.

Problem

Consider another package u that imports both r/p and d. The current behavior means that r/p no longer imports d, it imports r/Godeps/_workspace/src/d. Package u cannot interchange types defined in d with types from r/Godeps/_workspace/src/d—they are distinct packages.

(Attempting to require u to import r/Godeps/_workspace/src/d instead of d does not eliminate the problem, because u may depend on d indirectly, through yet another repo q, which may or may not itself have copied and renamed d to q/Godeps/_workspace/src/d. There is nothing u can do to resolve the difference between r/Godeps/_workspace/src/d and q/Godeps/_workspace/src/d.)

This is not hypothetical. Users of godep have occasionally encountered this situation already.

The second method above would avoid this problem, but introduce a new one: the author of r needs to rerun godep save -r every time they edit r/p and want to test the results in r/c. This makes the development workflow more cumbersome.

Questions

What do you think of the second method? We have not yet tried it, so we don’t have a good idea of how cumbersome it would be in practice.

Is there another approach that avoids both of these problems?

@rgarcia

This comment has been minimized.

Show comment
Hide comment
@rgarcia

rgarcia Apr 17, 2015

What about a third method?

  • Copy d to r/c/Godeps/_workspace/src/d and copy r/p to r/c/Godeps/_workspace/src/r/p. Change any import statement in r/c and the tree rooted at r/c from "d" to “r/c/Godeps/_workspace/src/d” and from “r/p” to “r/c/Godeps/_workspace/src/r/p”. Note that import statements in r/p remain unchanged.

Put another way, basically treat r/c as if it were a totally different repository. Still has the problem of having to run godep save -r to pull in changes to r/p in r/c, but drives home the fact that only package mains should vendor.

rgarcia commented Apr 17, 2015

What about a third method?

  • Copy d to r/c/Godeps/_workspace/src/d and copy r/p to r/c/Godeps/_workspace/src/r/p. Change any import statement in r/c and the tree rooted at r/c from "d" to “r/c/Godeps/_workspace/src/d” and from “r/p” to “r/c/Godeps/_workspace/src/r/p”. Note that import statements in r/p remain unchanged.

Put another way, basically treat r/c as if it were a totally different repository. Still has the problem of having to run godep save -r to pull in changes to r/p in r/c, but drives home the fact that only package mains should vendor.

@freeformz

This comment has been minimized.

Show comment
Hide comment
@freeformz

freeformz Sep 11, 2015

Contributor

vendor/ provides some of this as you can have multiple vendor folders at different levels. I'm not yet sure we should support that and instead only make a top level vendor/ folder with everything flattened down into it with a way smarter system for package resolution.

Contributor

freeformz commented Sep 11, 2015

vendor/ provides some of this as you can have multiple vendor folders at different levels. I'm not yet sure we should support that and instead only make a top level vendor/ folder with everything flattened down into it with a way smarter system for package resolution.

@freeformz freeformz closed this Sep 15, 2015

@colemickens

This comment has been minimized.

Show comment
Hide comment
@colemickens

colemickens Nov 5, 2015

How are people handling this in practice?

How are people handling this in practice?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment