switch to glide #17391

deads2k · 2017-11-20T18:08:17Z

No description provided.

deads2k · 2017-11-20T21:57:58Z

@liggitt alright, this builds locally for me and it creates symlinks for the staging folders when you do hack/update-deps.sh

liggitt · 2017-11-21T07:54:58Z

it creates symlinks for the staging folders when you do hack/update-deps.sh

running that left a diff in my working dir... that expected?

$ git diff --numstat
4       2       glide.lock
0       2       vendor/github.com/getsentry/raven-go/docs/_sentryext/.gitignore
0       13      vendor/github.com/getsentry/raven-go/docs/_sentryext/README.rst
0       708     vendor/github.com/getsentry/raven-go/docs/_sentryext/sentryext.py
0       113     vendor/github.com/getsentry/raven-go/docs/_sentryext/verify-docs.py
14      8       vendor/google.golang.org/appengine/.travis.yml
26      26      vendor/google.golang.org/appengine/README.md
4       0       vendor/google.golang.org/appengine/aetest/instance.go
3       0       vendor/google.golang.org/appengine/aetest/instance_test.go
23      17      vendor/google.golang.org/appengine/aetest/instance_vm.go
37      0       vendor/google.golang.org/appengine/appengine.go
1       37      vendor/google.golang.org/appengine/appengine_vm.go
1       1       vendor/google.golang.org/appengine/capability/capability.go
4       0       vendor/google.golang.org/appengine/channel/channel.go
1       1       vendor/google.golang.org/appengine/cloudsql/cloudsql.go
1       1       vendor/google.golang.org/appengine/cloudsql/cloudsql_vm.go
2       2       vendor/google.golang.org/appengine/cmd/aebundler/aebundler.go
24      216     vendor/google.golang.org/appengine/cmd/aedeploy/aedeploy.go
3       2       vendor/google.golang.org/appengine/datastore/datastore.go
277     32      vendor/google.golang.org/appengine/datastore/datastore_test.go
26      16      vendor/google.golang.org/appengine/datastore/doc.go
135     40      vendor/google.golang.org/appengine/datastore/load.go
4       5       vendor/google.golang.org/appengine/datastore/metadata.go
81      45      vendor/google.golang.org/appengine/datastore/prop.go
99      156     vendor/google.golang.org/appengine/datastore/prop_test.go
11      0       vendor/google.golang.org/appengine/datastore/query.go
46      19      vendor/google.golang.org/appengine/datastore/save.go
25      5       vendor/google.golang.org/appengine/delay/delay.go
106     0       vendor/google.golang.org/appengine/delay/delay_test.go
1       1       vendor/google.golang.org/appengine/demos/guestbook/app.yaml
1       1       vendor/google.golang.org/appengine/demos/guestbook/guestbook.go
1       1       vendor/google.golang.org/appengine/demos/helloworld/helloworld.go
2       1       vendor/google.golang.org/appengine/internal/aetesting/fake.go
46      9       vendor/google.golang.org/appengine/internal/api.go
35      3       vendor/google.golang.org/appengine/internal/api_classic.go
35      13      vendor/google.golang.org/appengine/internal/api_common.go
24      25      vendor/google.golang.org/appengine/internal/api_test.go
39      9       vendor/google.golang.org/appengine/internal/identity_classic.go
5       1       vendor/google.golang.org/appengine/internal/identity_vm.go
0       34      vendor/google.golang.org/appengine/internal/internal.go
0       58      vendor/google.golang.org/appengine/internal/internal_test.go
580     219     vendor/google.golang.org/appengine/internal/search/search.pb.go
6       0       vendor/google.golang.org/appengine/internal/search/search.proto
2       2       vendor/google.golang.org/appengine/log/log.go
1       1       vendor/google.golang.org/appengine/mail/mail.go
19      15      vendor/google.golang.org/appengine/memcache/memcache_test.go
0       52      vendor/google.golang.org/appengine/namespace_test.go
40      20      vendor/google.golang.org/appengine/remote_api/client.go
19      0       vendor/google.golang.org/appengine/remote_api/client_test.go
1       1       vendor/google.golang.org/appengine/remote_api/remote_api.go
2       2       vendor/google.golang.org/appengine/runtime/runtime.go
8       4       vendor/google.golang.org/appengine/search/doc.go
108     28      vendor/google.golang.org/appengine/search/search.go
378     14      vendor/google.golang.org/appengine/search/search_test.go
12      6       vendor/google.golang.org/appengine/search/struct.go
43      2       vendor/google.golang.org/appengine/search/struct_test.go
1       1       vendor/google.golang.org/appengine/socket/doc.go
2       2       vendor/google.golang.org/appengine/socket/socket_vm.go
45      0       vendor/google.golang.org/appengine/taskqueue/taskqueue.go
57      0       vendor/google.golang.org/appengine/taskqueue/taskqueue_test.go
11      2       vendor/google.golang.org/appengine/user/user_classic.go

mfojtik · 2017-11-21T09:13:57Z

hack/update-deps.sh

+for pkg in vendor/k8s.io/kubernetes/staging/src/k8s.io/*; do
+  dir=$(basename $pkg)
+  unlink vendor/k8s.io/$dir || true
+  rm -rf vendor/k8s.io/$dir || true


@deads2k will this leave the deps broken when the glide below fails?

@deads2k will this leave the deps broken when the glide below fails?

Yeah. We'll call that something to iterate on.

mfojtik · 2017-11-21T09:17:42Z

glide.yaml

+- package: k8s.io/kubernetes
+  repo:    git@github.com:openshift/kubernetes
+  version: release-1.8.1
+# only used by pkg/build/builder


i thought pkg/build/builder is excluded?

i thought pkg/build/builder is excluded?

they have an incomplete vendor folder. Go them. Another thing to fix up post-this.

mfojtik · 2017-11-21T09:19:47Z

glide.yaml

+  version: a8ee86b1cce0c13bd541a99140682a92635ba9f7
+- package: github.com/containers/image
+  repo:    git@github.com:openshift/containers-image
+  version: openshift-3.8


so does this mean that if I want to bump this I will have to bump the fork branch and then re-run update-deps ?

so does this mean that if I want to bump this I will have to bump the fork branch and then re-run update-deps ?

Yeah. Or you can find the person who made that ridiculous carry and make them fix it.

mfojtik · 2017-11-21T09:44:14Z

I ended up with same diff as @liggitt but it seems like a progress! :)

UPDATE: The diff is not the same, I don't see changes in vendor/github.com/getsentry/raven-go just the appengine.

mfojtik · 2017-11-21T12:01:35Z

@deads2k @liggitt this fixed the appengine diff: deads2k@e9ee6ff

tnozicka · 2017-11-21T12:01:39Z

hack/update-deps.sh

+
+source "$(dirname "${BASH_SOURCE}")/lib/init.sh"
+
+# fail early if any of the staging dirs is checked out


why do we care about this in update script?

how is GOPATH relevant to symlinks which are in vendor folder? vendor takes precedence

how is GOPATH relevant to symlinks which are in vendor folder? vendor takes precedence

We symlink into vendor. This is pre-existing and it looks correct to me.

to me it seems unrelated to the task. If the bellow lines are not affected by stuff present in GOPATH why does it check for whats there? If this is preexisting, that's likely because of Godeps specifics when it worked with your GOPATH. Can't see any relations to glide.

We used to have hard to debug re-vendoring errors where the GOPATH version was picked up and the result differed depending on the developer's GOPATH. This was added as a sanity check to avoid this situation.

tnozicka · 2017-11-21T12:04:03Z

hack/update-deps.sh

+#  "github.com/jteeuwen/go-bindata/go-bindata"
+#)
+
+# remove symlinks for glide update


glide switches vendor/ and tmp/vendor/ as far as I recall so this shouldn't be needed or am I wrong?

tnozicka · 2017-11-21T12:04:41Z

hack/update-deps.sh

+  dir=$(basename $pkg)
+  rm -rf vendor/k8s.io/$dir
+  ln -s kubernetes/staging/src/k8s.io/$dir vendor/k8s.io/$dir
+done


tnozicka · 2017-11-21T12:05:23Z

hack/update-deps.sh

+for pkg in vendor/k8s.io/kubernetes/staging/src/k8s.io/*; do
+  dir=$(basename $pkg)
+  rm -rf vendor/k8s.io/$dir
+  ln -s kubernetes/staging/src/k8s.io/$dir vendor/k8s.io/$dir


I think ln can do this in one step, it was something like ln -sfn

I think ln can do this in one step, it was something like ln -sfn

tried, does not work

tnozicka · 2017-11-21T12:08:00Z

glide.yaml

+  repo:    git@github.com:openshift/containers-image
+  version: openshift-3.8
+- package: github.com/vjeantet/ldapserver
+  version: v1.0


tnozicka · 2017-11-21T12:38:05Z

+100 for switching to glide.

Since I have been there I don't think the symlinking can ever work properly. The issue is that glide has to resolve all the dependencies in glide update step and if there are some files missing, it fails. So you need have the publishing repository updated first. Putting your faith into probability you could hope that the changes were so small and not changing dependency resolution that you will be fine but even a simple file addition/move/backport in upstream will cause it to fail. But since that establishes the need for the repository being published upfront you can include that repository directly and there is no point for symlinking.

The appropriate staging repo commits for kubernetes commit f6401 can be found by

REPO=client-go && \
git --git-dir=${GOPATH}/src/k8s.io/${REPO}/.git --work-tree=${GOPATH}/src/k8s.io/${REPO} log --all --follow -- kubernetes-sha | grep -B4 f6401

soltysh · 2017-11-21T12:43:06Z

The only worrying thing for me is the huge diff here, but I assume it's godep vs glide thing, I'll be looking more into it after tomorrow.

mfojtik · 2017-11-21T13:00:46Z

@soltysh the diff is due to appengine version in pkg/build/builder I have commit that pin the version to the sha256 used in our level, so we should be fine?

deads2k · 2017-11-21T14:04:40Z

running that left a diff in my working dir... that expected?

Actually yes. I didn't explicitly pin levels. We'll have to decide whether we're really going to pin everything, whether we'll try to find branches, whether we'll try to match kube (which I think happens by inspection of their godep.json), or whether we'll try to stay current when possible.

Pinning levels is heavy, but possible if we want to. I did go ahead and pin that one.

deads2k · 2017-11-21T18:51:41Z

@liggitt @mfojtik alright, see if you can live with what I've done. I disabled some tests we weren't running before, removed the oadm completions a bit early (they looked like the only ones that were broken), and updated some openapi generation excludes.

openshift-merge-robot · 2017-11-22T13:38:43Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, mfojtik

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

~~OWNERS~~ [deads2k,mfojtik]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

sttts · 2017-11-22T13:52:35Z

hack/update-deps.sh

+# Some things we want in godeps aren't code dependencies, so ./...
+# won't pick them up.
+# TODO seems like this should be failing something somewhere
+#REQUIRED_BINS=(


rm -rf vendor/k8s.io/code-generator && git clone --branch master git@github.com:kubernetes/code-generator.git vendor/k8s.io/code-generator && pushd vendor/k8s.io/code-generator &>/dev/null && git checkout <SHA> && rm -rf .git && popd >/dev/null

liggitt · 2017-11-22T16:04:03Z

vendor/k8s.io/kubernetes/cmd/kube-apiserver/app/testing/server_test.go

@@ -152,7 +152,7 @@ func TestCRDShadowGroup(t *testing.T) {
 	}
 }

-func TestCRD(t *testing.T) {
+func ETestCRD(t *testing.T) {


?

Disables a test (when I thought I had one to two) without making a large diff to deal with. I think its a lack of entropy creating new certs problem.

…ports

deads2k · 2017-11-22T18:51:23Z

/retest

deads2k · 2017-11-22T19:09:35Z

/retest

openshift-merge-robot · 2017-11-22T19:32:38Z

Automatic merge from submit-queue.

openshift-ci-robot · 2017-11-22T19:45:48Z

@deads2k: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
ci/openshift-jenkins/extended_conformance_install_update	`9fa7abd`	link	`/test extended_conformance_install_update`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 20, 2017

openshift-merge-robot assigned soltysh and liggitt Nov 20, 2017

openshift-merge-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. vendor-update Touching vendor dir or related files labels Nov 20, 2017

deads2k force-pushed the glide-01 branch from f5c29c8 to fb9a409 Compare November 20, 2017 20:57

deads2k changed the title ~~[wip] switch to glide~~ switch to glide Nov 20, 2017

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 20, 2017

deads2k force-pushed the glide-01 branch 2 times, most recently from ef18ff0 to 0e3c8e2 Compare November 20, 2017 21:57

mfojtik reviewed Nov 21, 2017

View reviewed changes

tnozicka reviewed Nov 21, 2017

View reviewed changes

deads2k force-pushed the glide-01 branch 3 times, most recently from 5d1f6de to 386e904 Compare November 21, 2017 18:50

deads2k force-pushed the glide-01 branch 2 times, most recently from 33dccde to 02dcece Compare November 21, 2017 20:41

sttts reviewed Nov 22, 2017

View reviewed changes

deads2k added 3 commits November 22, 2017 08:56

use script to link to staging folder for patches

e373c53

bump(*): glide

4de2dc5

UPSTREAM: <carry>: disable failing etcd test for old level

98568e6

deads2k force-pushed the glide-01 branch from 3557492 to bfdad40 Compare November 22, 2017 13:56

deads2k added lgtm Indicates that a PR is ready to be merged. queue/multiple-rebases and removed lgtm Indicates that a PR is ready to be merged. labels Nov 22, 2017

deads2k force-pushed the glide-01 branch from bfdad40 to de79976 Compare November 22, 2017 14:22

openshift-merge-robot removed the lgtm Indicates that a PR is ready to be merged. label Nov 22, 2017

deads2k added lgtm Indicates that a PR is ready to be merged. and removed lgtm Indicates that a PR is ready to be merged. labels Nov 22, 2017

liggitt reviewed Nov 22, 2017

View reviewed changes

deads2k added 4 commits November 22, 2017 11:05

update openapi generation script to exclude dir

7a98e50

UPSTREAM: 55974: Allow constructing spdy executor from existing trans…

2187923

…ports

minor completion changes

50b62dc

prevent k8s.io/kubernetes/cmd since we didn't run them before

9fa7abd

deads2k force-pushed the glide-01 branch from 2749463 to 9fa7abd Compare November 22, 2017 16:05

deads2k added lgtm Indicates that a PR is ready to be merged. and removed lgtm Indicates that a PR is ready to be merged. labels Nov 22, 2017

openshift-merge-robot merged commit ef87687 into openshift:master Nov 22, 2017

smarterclayton mentioned this pull request Nov 22, 2017

break: test-cmd broke because of image for ppc64le arch no longer being imported from docker hub #17436

Closed

soltysh mentioned this pull request Nov 28, 2017

Remove hardcoded fields for parameter substitution configuration in Cluster Loader #17072

Merged

deads2k deleted the glide-01 branch January 24, 2018 14:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

switch to glide #17391

switch to glide #17391

deads2k commented Nov 20, 2017

deads2k commented Nov 20, 2017

liggitt commented Nov 21, 2017

mfojtik Nov 21, 2017

deads2k Nov 21, 2017

mfojtik Nov 21, 2017

deads2k Nov 21, 2017

mfojtik Nov 21, 2017

deads2k Nov 21, 2017

mfojtik commented Nov 21, 2017 •

edited

Loading

mfojtik commented Nov 21, 2017

tnozicka Nov 21, 2017

mfojtik Nov 21, 2017

tnozicka Nov 21, 2017

deads2k Nov 21, 2017

tnozicka Nov 21, 2017

sttts Nov 22, 2017

tnozicka Nov 21, 2017

tnozicka Nov 21, 2017

tnozicka Nov 21, 2017

deads2k Nov 21, 2017

tnozicka Nov 21, 2017

tnozicka commented Nov 21, 2017

soltysh commented Nov 21, 2017

mfojtik commented Nov 21, 2017

deads2k commented Nov 21, 2017

deads2k commented Nov 21, 2017

openshift-merge-robot commented Nov 22, 2017

sttts Nov 22, 2017

liggitt Nov 22, 2017

deads2k Nov 22, 2017

deads2k commented Nov 22, 2017

deads2k commented Nov 22, 2017

openshift-merge-robot commented Nov 22, 2017

openshift-ci-robot commented Nov 22, 2017


		source "$(dirname "${BASH_SOURCE}")/lib/init.sh"

		# fail early if any of the staging dirs is checked out

switch to glide #17391

switch to glide #17391

Conversation

deads2k commented Nov 20, 2017

deads2k commented Nov 20, 2017

liggitt commented Nov 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mfojtik commented Nov 21, 2017 • edited Loading

mfojtik commented Nov 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tnozicka commented Nov 21, 2017

soltysh commented Nov 21, 2017

mfojtik commented Nov 21, 2017

deads2k commented Nov 21, 2017

deads2k commented Nov 21, 2017

openshift-merge-robot commented Nov 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deads2k commented Nov 22, 2017

deads2k commented Nov 22, 2017

openshift-merge-robot commented Nov 22, 2017

openshift-ci-robot commented Nov 22, 2017

mfojtik commented Nov 21, 2017 •

edited

Loading