Skip to content
This repository has been archived by the owner on Feb 26, 2019. It is now read-only.

Should I commit _workspace? #131

Closed
hurrymaplelad opened this issue Oct 13, 2014 · 11 comments
Closed

Should I commit _workspace? #131

hurrymaplelad opened this issue Oct 13, 2014 · 11 comments

Comments

@hurrymaplelad
Copy link
Contributor

Fuller question on StackOverflow.

I'm confused whether I should commit Godeps.json, _workspace, both, or it's up to me. Whatever the answer, we should update the docs to clear it up. This paragraph under Getting Started was particularly confusing:

This will save a list of dependencies to the file Godeps/Godeps.json, and copy their source code into Godeps/_workspace. Read over its contents and make sure it looks reasonable. Then commit the file to version control.

@mariokostelac
Copy link

Just add Godeps.json file to the repo, and _workspace to the .gitignore list :).

While your code should be fully included in your repo, dependencies have to be just referenced somehow (godep.json, package.json, git submodule... you choose), and nothing more than that.
The same tactics apply to npm, bower, apt and all other package managers.

Your repo - your things + references to vendor libs (of course, when it is possible, you cannot reference sourceforge zip file).

@hurrymaplelad
Copy link
Contributor Author

Looks to me like the heroku buildpack expects _workspace to be committed. If it sees Godeps, it will only godep go install, not godep go get or godep restore. Is this intentional, or just a PR waiting to happen?

@mariokostelac
Copy link

I do not develop Godep, but I was taught that vendor code should not be included in the repo (we know the reasons). @kr should say what he thinks about that and make it clear is that a bug (I think it is not) or he thinks that we should deploy to heroku o some other way.

Edit: it seems that http://mmcgrana.github.io/2012/09/getting-started-with-go-on-heroku.html says we should include dependencies to the repo. It seems wierd from my point of view.

@rlmcpherson
Copy link

The intended use of godep is to vendor dependencies and commit the _workspace directory to version control. See the proposal document by @kr linked in #123 (proposal: http://goo.gl/RpYs8e) As discussed in that proposal, godep used to have a mode (-copy=false) that supported not vendoring the dependencies. My guess is that the ambiguous language in the Readme may be due to that. This mode has been removed as documented in #123.

@hurrymaplelad
Copy link
Contributor Author

Thanks @rlmcpherson, that clears things up quite a bit.

I'm still wrapping my head around go dependencies. Mind walking me through a scenario to convince me committing _workspace is reasonable?

Say I've got a server project foo that depends on library bar, and both depend on library baz. Both foo and bar manage their dependencies with godeps.

When I go get http://bar will I end up with foo/Godeps/_workspace/src/bar/_workspace/src/baz and foo/Godeps/_workspace/src/baz, with foo and bar finding their respective versions?

hurrymaplelad added a commit to hurrymaplelad/godep that referenced this issue Oct 16, 2014
I understand the [docs need lots of work](tools#122 (comment)) but I think this line should be updated soon to include the decision made in tools#123, even if a big rewrite is coming.  It has [confused me](tools#131 (comment)) and [several other people](http://stackoverflow.com/questions/26334220/should-i-commit-godeps-workspace-or-is-godeps-json-enough/26342342#26342342).
@bithavoc
Copy link

Please merge the referenced commit by @hurrymaplelad, this is very confusing.

@antonholmquist
Copy link

Comitting the Godep directory may not always be the right thing to do. Although I agree apps should generally do it, it is probably not the right way to go for frameworks and other external packages.

@bithavoc
Copy link

@antonholmquist check this comment by @kr heroku/heroku-buildpack-go#65 (comment)

@kr kr closed this as completed in e28d568 Nov 24, 2014
@kr
Copy link
Member

kr commented Nov 24, 2014

Thanks for all this discussion folks. It's super helpful! 💖

@colonelpanic8
Copy link

The fact that the official way to manage dependencies is to commit the whole _workspaces directory deeply offends my sensibilities. Its really hard for me to imagine a good reason for this. It seems like the dependencies should be part of a submodule at the very least to keep the repo history clean.

Also the comment/issue from heroku buildpack is gone. It would be great if the README explained the rationale for thsi practice.

@mariokostelac
Copy link

That way is called vendoring so please watch https://www.youtube.com/watch?v=c3dW80eO88I (Dave Cheney on reproducible builds) and try to think about that again. I also thought it's stupid, now I think different (keep in mind that there is no real package provider like npm or cargo that would keep packages there forever).

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

No branches or pull requests

7 participants