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

Proposal: handle vendor symlink redirect gracefully #930

Closed clgroft closed 5 years ago

clgroft commented 7 years ago

Many Go repositories, instead of having a directory at vendor, have a symlink to a directory such as .vendor or _vendor so that the go toolchain will skip vendored code. At present dep fails on such repos entirely with a source is not a directory error; it uses lstat instead of stat on vendor, lest it get caught in an infinite loop.

Possible solutions:

My preferred solution is 2 before the release of 1.9, 3 afterward, but I'm happy to implement any of these or any modification.

What version of Go (go version) and dep (git describe --tags) are you using?

go version go1.7.5 darwin/amd64
v0.1.0-313-g44a454b
sdboyer commented 7 years ago

hi! thanks for the issue, and giving us a menu of options to choose from 😄 🎉

i'd say that 2.ii and 2.iii aren't actually mutually exclusive, and we probably want both. the implementation for them lives in very different places, especially because we have to be able to deal with legacy code from before this fix was made.

2.ii is the harder one, with further-reaching implications. so, notes...

At present dep fails on such repos entirely with a source is not a directory error

this is actually just the tip of the iceberg. that may need fixing as well (i'm guessing that error is only occurring during dep init?) but the hairier thing is updating the way we build a virtual representation of the filesystem to accommodate this reality.

if dep finds a symlink at vendor, explicitly follow it once, verify it points to a directory

we have to be a little more restrictive than this, as we've been all up and down this kind of path before. symlinks are 🤕 🔫 . so, more rules:

  1. symlink MUST be relative
  2. symlink MUST point to a sibling node (no subdirs, and definitely no ../)
  3. symlink MUST not point to another symlink (encompassed by your "must be dir" req, but just to be 💯 clear)

OTTOMH, these'll need to be applied in that ListPackages() func, the probably mirrored elsewhere. further, we'll need to expand ListPackages() to understand that the referent of the vendor symlink is also a directory that should be skipped. with the way that filepath.Walk() works, this is likely to require keeping some state outside of the giant walkFn.

glasser commented 6 years ago

Another reason I'd like to be able to make vendor be a symlink: if we don't check vendor into git and want to store vendor in a cache in CI. It would be nice to be able to just run ln -s /cache/go-vendor ./vendor at the top of my CI setup and assume that dep ensure will only write to vendor via my symlink.

kduda commented 6 years ago

We have a complicated hybrid perforce/git environment where it would help me if /go/src/arista.com/vendor could be a symlink. I would prefer option one, where the go toolchain just assumes I know what I'm doing.

Generally, it would be nice if go treated symlinks as first-class within /go/src. ("go install arista.com/..." does not do what I want because it disregards my symlink forest in /go/src/arista.com). I may have to resort to bind mounts to get my go workspace to behave the way I want while preserving interoperability with my existing source control hierarchy. I suppose if I have to bind mount other things anyway, why not bind-mount /go/src/arista.com/vendor too. :-/

Thanks, -Ken

Kenneth Duda Arista Networks