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

adding vendor directory #20

Closed
wants to merge 1 commit into from

Conversation

enoodle
Copy link

@enoodle enoodle commented Jun 29, 2016

This is a suggestion to move to the way Go 1.6 is handling vendoring. I just did:

mkdir vendor
cp -r Godeps/_workspace/src/* vendor/

I have seen this done in kubernetes too. I think we can keep the Godeps directory for a while and then remove it if we see that it works well for us.

@simon3z @pweil-

@simon3z
Copy link

simon3z commented Jun 29, 2016

This is a suggestion to move to the way Go 1.6 is handling vendoring

Is this backward compatible with 1.4/1.5? (Can you build on those versions?)

@enoodle
Copy link
Author

enoodle commented Jun 30, 2016

@simon3z This is not backward compatible but eventually we will have to do the change according to those warnings that I get while compiling using godep:

godep: WARNING: Godep workspaces (./Godeps/_workspace) are deprecated and support for them will be removed when go1.8 is released.
godep: WARNING: Go version (go1.6) & $GO15VENDOREXPERIMENT= wants to enable the vendor experiment, but disabling because a Godep workspace (Godeps/_workspace) exists

To compile this way we just need to use:

go build cmd/image-inspector.go

@simon3z
Copy link

simon3z commented Jun 30, 2016

@enoodle we can't break builds on 1.4 and 1.5 just to suppress warnings in 1.6.
(Anyway I have the feeling that you can use this vendor path with 1.4 and 1.5 if you change the build configuration).

@enoodle enoodle force-pushed the vendor_directory_go_16 branch 2 times, most recently from 11109ae to 610f854 Compare March 12, 2017 14:40
@enoodle
Copy link
Author

enoodle commented Mar 12, 2017

@simon3z Can you please take another look at this? I think as we prepare for a new version of this tool we might be willing to let go of go1.4 to adopt the new "vendor/" directory and use better package management tools than Godep (which I personally really don't like, trying to add the github.com/containers/image repo as a dependency was a nightmare)

The interesting files will be Makefile, vendor.conf, hack/test-go.sh, hack/common.sh, hack/install-tools.sh, .gitignore and Readme

@enoodle enoodle force-pushed the vendor_directory_go_16 branch from 610f854 to 1c890b6 Compare March 12, 2017 14:49
@simon3z
Copy link

simon3z commented Mar 13, 2017

@enoodle have you refreshed all the dependencies with newer versions?

@enoodle
Copy link
Author

enoodle commented Mar 13, 2017

@simon3z
Copy link

simon3z commented Mar 13, 2017

@enoodle as far as I understood by the conversation we just had you're still polishing this. Let me know when it's really settled.

@enoodle
Copy link
Author

enoodle commented Mar 16, 2017

I see that kubernetes has started to move towards this, but I guess moving such a big project is harder. It is present in contianers/image [2] (They upgraded to a similar tool which is better because it does caching [3], I will try it out too). Openshift is also using the vendor directory but I couldn't find where it was introduced.

[1]kubernetes/kubernetes#24242
[2]containers/image#240
[3]containers/image#244

@enoodle enoodle force-pushed the vendor_directory_go_16 branch 2 times, most recently from 77d70c6 to f3b9c86 Compare March 20, 2017 16:13
@enoodle enoodle force-pushed the vendor_directory_go_16 branch 3 times, most recently from 5361b5d to bae7de7 Compare April 9, 2017 16:36
@simon3z
Copy link

simon3z commented Apr 11, 2017

@ilackarms can you test / review this?
cc @pweil-

@ilackarms
Copy link

ilackarms commented Apr 11, 2017

this compiles without issue, but i would suggest using godep save to generate Godeps file and vendor directory rather than manually rearranging deprecated structure.

edit: ignore above comment.

I'm unfamiliar with trash, but I'm in favor of anything that's not godeps. Tested locally and it looks good.
The only issue is that you need to add

RUN go get -u github.com/rancher/trash && \
	trash

to the Dockerfile as well

removing Godeps/

GO15VENDOREXPERIMENT set to 1
@enoodle enoodle force-pushed the vendor_directory_go_16 branch from bae7de7 to a552eca Compare April 12, 2017 12:34
@enoodle
Copy link
Author

enoodle commented Apr 12, 2017

@ilackarms Trash is similar to glide, it can also use the glide configuration file. I added a make deps line to the Dockerfile to do exactly what you advised, Thank you.

@enoodle
Copy link
Author

enoodle commented May 22, 2017

ping @ilackarms @pweil- @simon3z Do you want to take another look? This make it much easier to manage dependencies.

@simon3z
Copy link

simon3z commented Jun 22, 2017

Moved to #57

@simon3z simon3z closed this Jun 22, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants