robfig / glock

MIT License
230 stars 27 forks source link

Consider adding GO15VENDOREXPERIMENT support #28

Open kardianos opened 9 years ago

kardianos commented 9 years ago

In 1.5 the go command will read the environment variable GO15VENDOREXPERIMENT. If enabled, it will use the local "/vendor/" folder for dependencies. This replaces the need to manage GOPATH variables as dependencies can be fetched directly into the vendor folder and resolved there when built. This allows the GOPATH to not be modified. The contents of the vendor folder can be ignored.

Will you consider supporting this approach?

robfig commented 9 years ago

To clarify what you're suggesting.. if the GO15VENDOREXPERIMENT environment variable is set, then use /vendor/ as the root of the GOPATH for "go get" invocations instead of the system GOPATH when fetching dependencies. The benefit of that is that multiple glock-managed projects could be present in a single GOPATH without having to agree on one version of everything or having to sync when switching between them. The vendor directory could still be git ignored, avoiding checking everything in.

I like the idea. Thinking about the main operations that need to be supported:

Doesn't seem so bad. Is that what you had in mind?

kardianos commented 9 years ago

Yes, that is exactly what I was suggesting.

On Tue, Jun 30, 2015 at 7:49 AM Rob Figueiredo notifications@github.com wrote:

To clarify what you're suggesting.. if the GO15VENDOREXPERIMENT environment variable is set, then use /vendor/ as the root of the GOPATH for "go get" invocations instead of the system GOPATH when fetching dependencies. The benefit of that is that multiple glock-managed projects could be present in a single GOPATH without having to agree on one version of everything or having to sync when switching between them. The vendor directory could still be git ignored, avoiding checking everything in.

I like the idea. Thinking about the main operations that need to be supported:

  • glock save -- this calculates the set of dependencies used by all packages under the managed project using "go list", running "go get" if packages are missing. It seems like it would have to get the full set of dependencies (whether in /vendor or in GOPATH) and then "go get" them with GOPATH set to the /vendor directory to ensure we have everything there. Any existing stuff in /vendor would be seen by "go list" and have the right revision used for the dependency calculation.
  • glock sync -- as long as all dependencies are found under /vendor, the build.Import should find the right one and the VCS checkout command will work regardless. I believe no change is necessary here, unless we want to enforce that deps are under /vendor and not GOPATH

Doesn't seem so bad. Is that what you had in mind?

— Reply to this email directly or view it on GitHub https://github.com/robfig/glock/issues/28#issuecomment-117214690.

robfig commented 9 years ago

I pushed a version on the vendor branch. Unfortunately, I realized that putting everything within the vendor directory breaks the ability to build commands -- afaik there is no easy way to build commands from within the vendor directory. I'll think about ways to arrange it...

kardianos commented 9 years ago

@robfig I could be mistaken, but I think commands within the vendor folder should build as usual. They will first pull from the vendor dir then from the GOPATH.

robfig commented 9 years ago

I think that only works for library dependencies. For example, my work glockfile includes "cmd github.com/gogo/protobuf/protoc-gen-gogo", which causes glock to "go install github.com/gogo/protobuf/protoc-gen-gogo" whenever dependencies update. However, the go install command does not build it from $COMPANY/vendor/github.com/gogo/protobuf/protoc-gen-gogo -- it couldn't know which vendor folder to look in, and running go install from within the vendor folder does not help. I guess vendoring doesn't really have this use case in mind.

kardianos commented 9 years ago

@robfig I did a test this morning:

.
└── vendor
    ├── cmd1
    │   ├── cmd1
    │   └── cmd.go
    └── dep1
        └── dep.go

daniel@conk ~/s/test4> cat vendor/dep1/dep.go 
package dep1

func Help() string {
    return "Hello"
}

daniel@conk ~/s/test4> cat vendor/cmd1/cmd.go 
package main

import "dep1"

func main() {
    println(dep1.Help())
}

After setting "GO15VENDOREXPERIMENT=1" I was able to compile cmd1 just fine. Is what you were saying different?

pkieltyka commented 9 years ago

hey guys.. yea, the go tools check for the GO15VENDOREXPERIMENT variable and will use the vendor/ directory for its go path if it exists.. I also believe go get might support this too now, to help build the vendor directory.. what is missing from what Go provides is a Glockfile that builds the vendor/ directory. I think it makes a lot of sense for glock to provide this.

pkieltyka commented 9 years ago

https://github.com/golang/go/commit/cd3a5edb0433e6f3cafd997794aaea3083ae5b92

pkieltyka commented 9 years ago

@robfig nice work on the vendor branch, I just tried it work it works well. I have a few questions..

1. In the vendor/, should the source packages contain .git (or other vcs files)? I think the vendor files should be VC'd for the app but should exclude the dependency vcs metadata (.git, .hg etc directories). I can see why you need them there right now because that is how you can sync to the particular version very quickly and check for updates. What are your thoughts..?

some ideas..

A. projects should know to put vendor/**/.git and vendor/**/.hg in their .gitignore files of their project, in which case, this is solved for distribution and locally, once run it will sync up .. etc..? NOTE: I tried this, and if you clone a repo without the .git / .hg's .. the next sync will fail as contents already exist..

B. change fundamentally how Glock works.. supporting both GOPATH sync/locking and vendoring might be different problems..

2. Since vendor/ has just the source files, a build with GO15VENDOREXPERIMENT is slower as it has to rebuild each dep. For doing a clean dist build, that is fine and normal, but if someone is developing off the vendor/ directory, then it should be nice to have vendor/src and vendor/pkg .. im not sure if GO15VENDOREXPERIMENT will work with the src and pkg tho..

3. would be nice to have some kind of update command to update one or all deps .. effectively go get -u for a repo in vendor/ (or $GOPATH)

pkieltyka commented 9 years ago

4. .. glock sync should ignore vendor/ directories (maybe also with a Glockfile in there?) when searching for dependencies.. this happens if I have a vendor/ directory, but I want to sync the deps in $GOPATH, say for development.

pkieltyka commented 9 years ago

btw, should glock save skip the vendor/ directory when generating the Glockfile? it will be using ./... of the project's import to find deps, in which case, it will stumble on vendor/ and traverse that too.. I don't think GO15VENDOREXPERIMENT should be required for this either, it should just always work like this. The idea is that the vendor/ directory is like internal/ in that it should be skipped for vendoring cases.. thought of as separate