Skip to content
This repository has been archived by the owner on Jun 20, 2022. It is now read-only.

Add script/bootstrap to install dependencies in ./_vendor #67

Closed
wants to merge 1 commit into from

Conversation

mislav
Copy link
Contributor

@mislav mislav commented Sep 12, 2014

This helps newcomers who might have cloned the repo into an environment where they haven't yet set up proper GOPATH structure, and enables them to run tests and hack on the project.

It is also a step towards vendoring all dependencies in git; hence the cleanup of unnecessary files. Ideally no bootstraping should be required when someone clones this project.

/cc @jingweno

This helps newcomers who might have cloned the repo into an environment
where they haven't yet set up proper GOPATH structure, and enables them
to run tests and hack on the project.
@owenthereal
Copy link
Contributor

Your script is doing what Godep is trying to? We use Godep for Hub as well.

@mislav
Copy link
Contributor Author

mislav commented Sep 12, 2014

Yeah, I'm reiterating on that approach. I purposely not using a package manager because I feel one is not needed if go get is enough for our needs right now.

@owenthereal
Copy link
Contributor

I agree that vendoring dependencies is the best approach for a Go library. But I think instead of providing a bootstrap script, it'd be better to version control all dependencies:

  • Create a vendor folder (not _vendor since we want to refer to the library in the folder, Go will skip folder starting with _)
  • go get all dependencies to the vendor folder and remove unnecessary files, e.g., tests
  • Refer to the library from this vendor folder like import github.com/octokit/go-octokit/vendor/github.com/lostisland/go-sawyer etc.

This approach will make go build work without running any bootstrap script. An example of this approach is docker. We'll just need to provide similar script to make the vendoring step reproducible.

The only drawback with this IMO is the long package name. But it's not a show stopper.

@mislav
Copy link
Contributor Author

mislav commented Sep 14, 2014

Refer to the library from this vendor folder like import github.com/octokit/go-octokit/vendor/github.com/lostisland/go-sawyer etc.

Interesting approach. If I understand correctly, you also want us to load these vendored dependencies at runtime as well (when go-octokit is included in a larger project), not just in development? Do you have examples of other libraries taking the same approach?

I went for _vendor just to ease development for now. Even Godep does Godep/_workspace so that commands such as go test ./... doesn't try to run tests within all dependencies.

@owenthereal
Copy link
Contributor

Interesting approach. If I understand correctly, you also want us to load these vendored dependencies at runtime as well (when go-octokit is included in a larger project), not just in development?

That's right, the dependency tree would look something like this:

hub
  - go-octokit
    - vendor/go-sawyer
    - ...
  - ...

And hub wouldn't depend on go-octokit's version of go-sawer directly. But of course it could by using path like import github.com/octokit/go-octokit/vendor/github.com/lostisland/go-sawyer. hub could also has its own version of go-sawyer if it imports like import github.com/lostisland/go-sawyer.

Do you have examples of other libraries taking the same approach?

There's a GopherCon talk about this earlier (skip to the dependency manager section). docker is actually adopting similar approach with this script. Any app develops on top of docker would have similar tree structure. Also Google is adopting it internally from what I heard. I haven't yet seen another popular open source library developed this way. I'm guessing that's because the Go philosophy is to always depend on the latest version and never break the API. In practice, I find not breaking the APIs very hard to achieve, especially for a WIP library.

I went for _vendor just to ease development for now. Even Godep does Godep/_workspace so that commands such as go test ./... doesn't try to run tests within all dependencies.

Yup, I see what you're trying to accomplish there. Using vendor instead of _vendor, we would need to remove all the _test files so that go test won't be confused and run the tests from the dependencies. In the end, go build and go test will work seamlessly. Back to docker, they're actually using a wrapper script to run unit test for each folder (they don't remove _test files). But I think either approach would work.

Note that this is the first library I'm trying it. No worry if it sounds too "untraditional", we could go with using _vendor or Godep.

@owenthereal
Copy link
Contributor

Pulling in @technoweenie and @pengwynn for the discussion

@mislav
Copy link
Contributor Author

mislav commented Oct 12, 2014

After talking to some Soundcloud engineers who seem to do a lot of Go for their (mosly internal) toolset, I was convinced that vendoring dependencies and rewriting imports is the way to go. So basically you were right.

I'm gonna redo this PR when I have time with the new approach. Ideally only maintainers (i.e. us) should ever have to use a tool like Godep or a custom script when bumping up dependencies, but any contributors to the library should be good to go after just cloning this repository.

@pengwynn
Copy link
Collaborator

pengwynn commented May 3, 2015

@mislav @jingweno Any interest in moving this forward? If not I can see if any of our new collaborators might want to take a crack.

@owenthereal
Copy link
Contributor

@pengwynn I'm not working on this currently. Feel free to move ahead.

There's a on-going discussion about dependency & vendoring. For a library like go-octokit, it's not recommended to vendor dependencies. Vendoring is only used for binaries, e.g., hub. We'd still need to maintain the API stability for go-octokit.

@feiranchen
Copy link
Contributor

@pengwynn @jingweno
Thanks for the clear explanation and providing sources on the issue. I just have a couple questions for clarification:

  • Suppose we try the vender solution and we do the following steps as discussed above:
  • Create a vendor folder (not _vendor since we want to refer to the library in the folder, Go will skip folder starting with _)
  • go get all dependencies to the vendor folder and remove unnecessary files, e.g., tests
  • Refer to the library from this vendor folder like import github.com/octokit/go-octokit/vendor/github.com/lostisland/go-sawyer etc.

If I'm understanding correctly, we want to write a script that does exactly the above, correct? This involves adding vender/ to GOPATH, calling go get for all the dependencies, and cleaning up, correct? If so, do open source developers always need to run this script manually after they clone? Or would it be a good idea to automate this like what's currently been done to the bootstrap script?

  • Problem: we want to avoid the situation where go test ./... run tests within all dependencies
    You mentioned:

    Back to docker, they're actually using a wrapper script to run unit test for each folder (they don't remove _test files)

    After looking into the script, I got this idea: How about we move all octokit _test files in one subfolder called tests/ (parallel to fixtures/) and instead of go test ./... we run go test ./tests/...? Is this a sensible solution?
    Otherwise we can just remove test files in the dependencies as part of the clean up step, in this script we are going to write?

Thanks for reading and please let me know what you think!

@owenthereal
Copy link
Contributor

@feiranchen Sorry if I didn't make it more clear. But let me try to explain.

Disclaimer: Vendoring is not yet implemented in Go and there's a WIP spec for vendoring tools.

go-octokit is a package (library) but not a binary program. So from the discussion, "Vendoring should be used only for the dependencies of binaries, not for the dependencies of packages". The reason is if we vendored dependencies for a package, other packages or binary programs that depend on this package will also need to vendor the dependencies of this package. If there're conflicting dependencies, e.g., pkg A & pkg B depend on different versions of pkg C, only one version of the depedencies will be picked. This is due to Go resolves dependencies as a list, not as a tree. It'd be a disaster without a tool to properly prompt the user about potential version conflicts (you can see Ruby bundler or similar tools have this).

That being said. I would recommend not to vendor dependencies for packages like go-octokit for now. And let's wait for the next mature vendoring tool (shameless plug, it could be this one 😸).

This PR could be closed.

@pengwynn
Copy link
Collaborator

pengwynn commented May 8, 2015

Thanks @jingweno. Gonna close this and we can revisit vendoring 🔜.

@pengwynn pengwynn closed this May 8, 2015
@pengwynn pengwynn deleted the gopath branch May 8, 2015 15:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants