Move godeps to vendor/ #24242

thockin · 2016-04-14T07:13:59Z

This is a first-step towards glide support, maybe we don't want or need to take this, but it was easy to try.

This fails to compile, not sure why:

# k8s.io/kubernetes/pkg/apis/extensions/v1beta1
_output/local/go/src/k8s.io/kubernetes/pkg/apis/extensions/v1beta1/conversion_generated.go:2703: undefined: extensions.ClusterAutoscaler
_output/local/go/src/k8s.io/kubernetes/pkg/apis/extensions/v1beta1/conversion_generated.go:2703: undefined: ClusterAutoscaler
_output/local/go/src/k8s.io/kubernetes/pkg/apis/extensions/v1beta1/conversion_generated.go:2719: undefined: extensions.ClusterAutoscaler
_output/local/go/src/k8s.io/kubernetes/pkg/apis/extensions/v1beta1/conversion_generated.go:2719: undefined: ClusterAutoscaler
_output/local/go/src/k8s.io/kubernetes/pkg/apis/extensions/v1beta1/conversion_generated.go:2723: undefined: extensions.ClusterAutoscalerList
_output/local/go/src/k8s.io/kubernetes/pkg/apis/extensions/v1beta1/conversion_generated.go:2723: undefined: ClusterAutoscalerList
_output/local/go/src/k8s.io/kubernetes/pkg/apis/extensions/v1beta1/conversion_generated.go:3468: Convert_extensions_JobSpec_To_v1beta1_JobSpec redeclared in this block
    previous declaration at _output/local/go/src/k8s.io/kubernetes/pkg/apis/extensions/v1beta1/conversion.go:328
_output/local/go/src/k8s.io/kubernetes/pkg/apis/extensions/v1beta1/conversion_generated.go:3845: Convert_extensions_ScaleStatus_To_v1beta1_ScaleStatus redeclared in this block
    previous declaration at _output/local/go/src/k8s.io/kubernetes/pkg/apis/extensions/v1beta1/conversion.go:98
_output/local/go/src/k8s.io/kubernetes/pkg/apis/extensions/v1beta1/conversion_generated.go:4737: Convert_v1beta1_JobSpec_To_extensions_JobSpec redeclared in this block
    previous declaration at _output/local/go/src/k8s.io/kubernetes/pkg/apis/extensions/v1beta1/conversion.go:380
_output/local/go/src/k8s.io/kubernetes/pkg/apis/extensions/v1beta1/conversion_generated.go:5186: Convert_v1beta1_ScaleStatus_To_extensions_ScaleStatus redeclared in this block
    previous declaration at _output/local/go/src/k8s.io/kubernetes/pkg/apis/extensions/v1beta1/conversion.go:120
_output/local/go/src/k8s.io/kubernetes/pkg/apis/extensions/v1beta1/conversion_generated.go:2723: too many errors
!!! Error in /home/thockin/tmp/godep-vendor/src/k8s.io/kubernetes/hack/lib/golang.sh:417

thockin · 2016-04-14T07:14:11Z

@krousey

krousey · 2016-04-14T15:42:02Z

hack/verify-govet.sh

@@ -58,7 +58,7 @@ for i in $targets
    #
    # The intended result is that each incantation of this line returns
    # either 0 (pass) or 1 (fail).
-    godep go vet "${goflags[@]:+${goflags[@]}}" "$i" 2>&1 \
+    go vet "${goflags[@]:+${goflags[@]}}" "$i" 2>&1 \


On line 42 above, you might want to add something to filter out vendor since go list will list vendored packages (unlike Godeps/_workspace).

thockin · 2016-04-19T04:51:34Z

Rebased. 'go build' works but Make still fails.

thockin · 2016-04-19T04:57:46Z

I know why. It's because we're trying to build ginkgo, but unlike _workspace/src/, vendor/ is not considered for build. Easy fix, testing now.

thockin · 2016-04-19T04:59:25Z

One reference to _workspace is left: hack/update-godep-licenses.sh

thockin · 2016-04-19T05:14:42Z

Uggh. We have deps that do import-rewriting. We godep saveing them, godep does no un-rewrite them. Until we update all those deps, this PR is hung.

glide, conveniently, DOES un-rewrite them.

$ find vendor/ -name _workspace
vendor/k8s.io/heapster/Godeps/_workspace
vendor/github.com/opencontainers/runc/Godeps/_workspace
vendor/github.com/appc/spec/Godeps/_workspace
vendor/github.com/appc/cni/Godeps/_workspace
vendor/github.com/coreos/rkt/Godeps/_workspace
vendor/github.com/google/cadvisor/Godeps/_workspace
vendor/github.com/mesos/mesos-go/examples/Godeps/_workspace

thockin · 2016-04-20T05:15:15Z

I can't, for the life of me, figure out how to get godep to update github.com/appc/spec/schema, which is needed to fix the rewriting mess,.

$ go get -u github.com/appc/spec/schema
# cd /home/thockin/tmp/godep-vendor/src/github.com/appc/spec; git pull --ff-only
You are not currently on a branch.
Please specify which branch you want to merge with.
See git-pull(1) for details.

    git pull <remote> <branch>

package github.com/appc/spec/schema: exit status 1

hmm, ok...

$ rm -rf $GOPATH/src/github.com/appc/spec/
$ go get github.com/appc/spec/schema
$ godep update github.com/appc/spec/schemagodep: Package (github.com/appc/spec/Godeps/_workspace/src/github.com/camlistore/go4/errorutil) not found
$ rm -rf vendor/github.com/appc/spec/schema/
$ godep update github.com/appc/spec/schema
godep: Package (github.com/appc/spec/Godeps/_workspace/src/github.com/camlistore/go4/errorutil) not found
$ vi Godeps/Godeps.json # remove camlistore/go4
$ godep update github.com/appc/spec/schema
godep: no packages can be updated

Yet again, godep is just inscrutable. I have no idea what it wants.

bprashanth · 2016-04-20T05:20:42Z

cd /home/thockin/tmp/godep-vendor/src/github.com/appc/spec; git checkout master
godep restore

usually works

thockin · 2016-04-20T05:23:05Z

$ vi Godeps/Godeps.json # remove camlistore/go4
$ godep update github.com/appc/spec/...
godep: Package (github.com/tobi/airbrake-go) not found

This is referenced from logrus in some code we don't use. If I go get that dep and the retry, it spins for 30 seconds and gives me another. And another. And another. Good grief, this is a pain in the butt. What am I doing wrong?

thockin · 2016-04-20T05:24:56Z

Ahh, and it ends in failure:

$ godep update github.com/appc/spec/...
godep: Package (code.google.com/p/go-charset/charset) not found
$ go get code.google.com/p/go-charset/charset
package code.google.com/p/go-charset/charset: unable to detect version control system for code.google.com/ path

thockin · 2016-04-20T05:28:16Z

# cd /home/thockin/tmp/godep-vendor/src/k8s.io/kubernetes/pkg/api/resource;
git checkout e310e619fc1ac4f3238bf5ebe9e7033bf5d47ee2
error: Your local changes to the following files would be overwritten by
checkout:
Godeps/Godeps.json
vendor/github.com/appc/spec/schema/image.go
vendor/github.com/appc/spec/schema/pod.go
vendor/github.com/appc/spec/schema/types/isolator_resources.go
vendor/github.com/appc/spec/schema/types/semver.go
Please, commit your changes or stash them before you can switch branches.
Aborting
godep: error restoring dep (k8s.io/kubernetes/pkg/api/resource): exit
status 1
godep: Error restoring some deps. Aborting check.

That is trying to restore a specific version of kubernetes.

On Tue, Apr 19, 2016 at 10:21 PM, Prashanth B notifications@github.com
wrote:

cd /home/thockin/tmp/godep-vendor/src/github.com/appc/spec; git checkout
master
godep restore

usually works

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#24242 (comment)

thockin · 2016-04-20T05:38:53Z

Awesome, now Go just barfs.

$ go get golang.org/x/net/trace
# cd /home/thockin/tmp/godep-vendor/src/golang.org/x/net; git pull --ff-only
You are not currently on a branch.
Please specify which branch you want to merge with.
See git-pull(1) for details.

    git pull <remote> <branch>

package golang.org/x/net/trace: exit status 1

Nuke from orbit and start over. Have I mentioned today how much I loathe these tools. Please just tell me WTF you are doing...please...

The build is now fast enough to not need them.

Go mistreats "testdata" and can't find vendor/ dirs.

a) it doesn't need it b) changing CWD to a path with symlinks breaks deep within ginkgo, where it crafts a relative path to ../../../../../../platforms/amd64/whatever which then traverses the physical path not the symlinked one, and breaks.

thockin · 2016-05-09T03:32:20Z

Rebased and re-pushed.

@k8s-oncall WAW team, consider a manual merge, please? I rebase this daily :(

k8s-github-robot · 2016-05-09T04:31:26Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

k8s-bot · 2016-05-09T05:11:16Z

GCE e2e build/test passed for commit 2100016.

k8s-github-robot · 2016-05-09T05:20:09Z

Automatic merge from submit-queue

thockin · 2016-05-09T15:53:49Z

W00t!

eparis · 2016-05-09T15:56:44Z

It's hard to know if the merge queue ran really well over the weekend or if this PR just caused every single other PR to need a rebase, so the number in the queue dropped dramtically. (ok, actually you can tell, this PR seemingly kicked 3 others out of the merge queue because they now need a rebase, so really not that bad)

smarterclayton · 2016-05-09T16:00:02Z

The merge queue ran well on Sunday.

On Mon, May 9, 2016 at 11:57 AM, Eric Paris notifications@github.com
wrote:

It's hard to know if the merge queue ran really well over the weekend or
if this PR just caused every single other PR to need a rebase, so the
number in the queue dropped dramtically. (ok, actually you can tell, this
PR seemingly kicked 3 others out of the merge queue because they now need a
rebase, so really not that bad)

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#24242 (comment)

MHBauer · 2016-05-09T16:54:21Z

wh00! good job!

thockin · 2016-05-09T23:18:53Z

FWIW I tried the same treatment on contrib/ (which has many _workspace dirs is various sub-dirs) and godep basically crapped the bed. First it gave me errors about dirty directories that were not dirty. Then it decided to vendor the entire contrib directory into a subdir of the contrib directory. As usual, no error messages, no justification.

bprashanth · 2016-05-09T23:21:40Z

I tried doing it for contrib/ingress this morning, I got

package k8s.io/contrib/ingress/controllers/gce
    imports k8s.io/contrib/ingress/controllers/gce/controller
    imports k8s.io/kubernetes/vendor/google.golang.org/api/compute/v1: use of vendored package not allowed
package k8s.io/contrib/ingress/controllers/gce
    imports k8s.io/contrib/ingress/controllers/gce/controller
...

At which point I decided to de-prioritize, just rev godeps till right before your commit, and hope someone else gets to the bottom of it :)

smarterclayton · 2016-05-09T23:22:26Z

Sounds helpful

eparis · 2016-05-10T14:54:20Z

So I tried in contrib. Interesting results. One godep issue and now one golang issue.

First, if you rm -rf Godeps your tree is no longer 'clean' so godep save won't run.
Ok, no problem, just git save then godep save ./...; then git add --all; git commit -a --amend

But then I ran into:

GOBIN=/storage/gopath/src/k8s.io/contrib/mungegithub CGO_ENABLED=0 GOOS=linux go install -installsuffix cgo -ldflags '-w'
# k8s.io/contrib/mungegithub
./mungegithub.go:109: cannot use "k8s.io/kubernetes/pkg/util/flag".WordSepNormalizeFunc (type func(*"k8s.io/kubernetes/vendor/github.com/spf13/pflag".FlagSet, string) "k8s.io/kubernetes/vendor/github.com/spf13/pflag".NormalizedName) as type func(*"k8s.io/contrib/mungegithub/vendor/github.com/spf13/pflag".FlagSet, string) "k8s.io/contrib/mungegithub/vendor/github.com/spf13/pflag".NormalizedName in argument to root.SetGlobalNormalizationFunc
Makefile:22: recipe for target 'mungegithub' failed
make: *** [mungegithub] Error 2

Which looks to me that golang is complaining that k8s.io/kubernetes/vendor/github.com/spf13/pflag != k8s.io/contrib/mungegithub/vendor/github.com/spf13/pflag

eparis · 2016-05-10T15:30:46Z

Found the solution. I had k8s.io/kubernetes/pkg/util vendored in but did not have k8s.io/kubernetes/pkg/util/flag so go build was looking in $GOPATH/src/k8s.io/pkg/util/flag instead of in vendor/k8s.io/pkg/util/flag and was upset about the results. Which makes a tee-tiny bit of sense I guess.

In /.gitignore we have lines like `kubernetes/`. Because that line is not anchored with `/` it ignores any directory called `kubernetes` anywhere in the repo. This caused a problem in kubernetes#24242 because the user didn't realize that the directory in `vendor/` was being ignored.

corentone · 2016-05-10T21:30:41Z

@thockin You mention Glide in the PR and related issue. I was wondering if the choice of keeping Godep versus moving to Glide was documented somewhere?
Maybe the move to vendor was big enough you didn't want to also change the dependency manager? I was also considering both and would definitely love to hear your views on it. Thanks!

thockin · 2016-05-12T02:59:52Z

I think glide is a better tool,and intend to move to it as soon as possible.

On Tue, May 10, 2016 at 2:31 PM, Corentin Debains notifications@github.com
wrote:

@thockin https://github.com/thockin You mention Glide in the PR and
related issue. I was wondering if the choice of keeping Godep versus moving
to Glide was documented somewhere?
Maybe the move to vendor was big enough you didn't want to also change the
dependency manager? I was also considering both and would definitely love
to hear your views on it. Thanks!

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#24242 (comment)

googlebot added the cla: yes label Apr 14, 2016

thockin mentioned this pull request Apr 14, 2016

Support vendor/ directories for vendoring code #21098

Closed

k8s-github-robot assigned eparis Apr 14, 2016

k8s-github-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. release-note-label-needed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Apr 14, 2016

krousey reviewed Apr 14, 2016
View reviewed changes

thockin force-pushed the godep_vendor_dir branch from b412e10 to 96dcfc0 Compare April 19, 2016 04:51

k8s-github-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 19, 2016

thockin force-pushed the godep_vendor_dir branch from 96dcfc0 to fd761fd Compare April 19, 2016 04:58

thockin force-pushed the godep_vendor_dir branch from fd761fd to bf95f3f Compare April 19, 2016 05:15

thockin added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Apr 20, 2016

thockin force-pushed the godep_vendor_dir branch from bf95f3f to c766ee8 Compare April 20, 2016 04:44

thockin force-pushed the godep_vendor_dir branch from c766ee8 to f6584c7 Compare April 20, 2016 06:27

k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Apr 20, 2016

thockin added 5 commits May 8, 2016 20:30

Fix update-generated-swagger-docs

bfabdb0

Fix verify-godeps

833770d

Get rid of hack/after-build scripts

18e7a3e

The build is now fast enough to not need them.

Rename codecgen "testdata" dir so ugorji works

7aff0f1

Go mistreats "testdata" and can't find vendor/ dirs.

Don't set the golang env for ginkgo

2100016

a) it doesn't need it b) changing CWD to a path with symlinks breaks deep within ginkgo, where it crafts a relative path to ../../../../../../platforms/amd64/whatever which then traverses the physical path not the symlinked one, and breaks.

thockin force-pushed the godep_vendor_dir branch from ed74ff9 to 2100016 Compare May 9, 2016 03:32

thockin added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels May 9, 2016

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 9, 2016

k8s-github-robot merged commit fa95788 into kubernetes:master May 9, 2016

MHBauer mentioned this pull request Jun 7, 2016

Sort out $GOPATH dependencies in hack/* scripts #17139

Closed

thockin deleted the godep_vendor_dir branch July 2, 2016 22:27

enoodle mentioned this pull request Mar 16, 2017

adding vendor directory openshift/image-inspector#20

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move godeps to vendor/ #24242

Move godeps to vendor/ #24242

thockin commented Apr 14, 2016

thockin commented Apr 14, 2016

krousey Apr 14, 2016

thockin commented Apr 19, 2016

thockin commented Apr 19, 2016

thockin commented Apr 19, 2016

thockin commented Apr 19, 2016

thockin commented Apr 20, 2016

bprashanth commented Apr 20, 2016

thockin commented Apr 20, 2016

thockin commented Apr 20, 2016

thockin commented Apr 20, 2016

thockin commented Apr 20, 2016

thockin commented May 9, 2016

k8s-github-robot commented May 9, 2016

k8s-bot commented May 9, 2016

k8s-github-robot commented May 9, 2016

thockin commented May 9, 2016

eparis commented May 9, 2016

smarterclayton commented May 9, 2016

MHBauer commented May 9, 2016

thockin commented May 9, 2016

bprashanth commented May 9, 2016

smarterclayton commented May 9, 2016 via email

eparis commented May 10, 2016

eparis commented May 10, 2016

corentone commented May 10, 2016

thockin commented May 12, 2016

Move godeps to vendor/ #24242

Move godeps to vendor/ #24242

Conversation

thockin commented Apr 14, 2016

thockin commented Apr 14, 2016

krousey Apr 14, 2016

Choose a reason for hiding this comment

thockin commented Apr 19, 2016

thockin commented Apr 19, 2016

thockin commented Apr 19, 2016

thockin commented Apr 19, 2016

thockin commented Apr 20, 2016

bprashanth commented Apr 20, 2016

thockin commented Apr 20, 2016

thockin commented Apr 20, 2016

thockin commented Apr 20, 2016

thockin commented Apr 20, 2016

thockin commented May 9, 2016

k8s-github-robot commented May 9, 2016

k8s-bot commented May 9, 2016

k8s-github-robot commented May 9, 2016

thockin commented May 9, 2016

eparis commented May 9, 2016

smarterclayton commented May 9, 2016

MHBauer commented May 9, 2016

thockin commented May 9, 2016

bprashanth commented May 9, 2016

smarterclayton commented May 9, 2016 via email

eparis commented May 10, 2016

eparis commented May 10, 2016

corentone commented May 10, 2016

thockin commented May 12, 2016