Skip to content

Conversation

@abhinavdahiya
Copy link
Contributor

Glide will continue to be supported for some time but is considered to be in a state of support rather than active feature development.

From https://github.com/Masterminds/glide#golang-dep

  • For root vendor directory
$ rm -rf tests
$ dep init
// edit Gopkg.toml to add.
// [prune]
//   non-go = true
//   go-tests = true
//   unused-packages = true
$ dep ensure
$ git checkout tests
  • For vendor in tests/smoke
$ dep init
// edit Gopkg.toml to add.
// [prune]
//   non-go = true
//   go-tests = true
//   unused-packages = true
$ dep ensure

@openshift-ci-robot openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 29, 2018
@abhinavdahiya
Copy link
Contributor Author

dep keeps tests/smoke/vendor/k8s.io/kubernetes/Godeps/LICENSES that adds around 80k lines :P @crawford

@wking
Copy link
Member

wking commented Sep 29, 2018

While I like the vendor/ updates you have in this PR, note that dep also has:

The Go toolchain, as of 1.11, has (experimentally) adopted an approach that sharply diverges from dep. As a result, we are continuing development of dep, but gearing work primarily towards the development of an alternative prototype for versioning behavior in the toolchain.

So we may want to stick with Glide until we can bump to Go 1.11 here (which we may be able to do already? openshift/release#1365).

@abhinavdahiya
Copy link
Contributor Author

abhinavdahiya commented Oct 2, 2018

@wking
dep has better semantics around constraints and overrides.

Also there is not a reason to believe that we can vendor kubernetes libraries with experimental go mod in go1.11. kubernetes/kubernetes#63607
I don't think go mod are going to be default until https://github.com/golang/go/milestone/65 late Jan 2019.

@wking
Copy link
Member

wking commented Oct 2, 2018

dep has better semantics around constraints and overrides.

Yeah, I'm not arguing Glide vs. dep long-term. I'm just not sure it's worth spending time on dep if we are going to switch to modules in the next few months. But...

Also there is not a reason to believe that we can vendor kubernetes libraries with experimental go mod in go1.11. kubernetes/kubernetes#63607

This issues seems to be about Kubernetes supporting modules cleanly. Which would be awesome. But in the meantime, I expect we can pin to specific commits. For example, see this in my very-WIP go-modules branch from my initial poking around back in August.

I don't think go mod are going to be default until https://github.com/golang/go/milestone/65 late Jan 2019.

No, but the Go folks have committed to forward compat.

So I don't have enough experience with the modules approach to know if it will work out or not. But I'm not yet convinced it's not workable ;).

@crawford
Copy link
Contributor

crawford commented Oct 2, 2018

I see the following when I run dep ensure:

# Gopkg.lock is out of sync with Gopkg.toml and project imports:
k8s.io/apimachinery/pkg/apis/meta/v1/unstructured: imported or required, but missing from Gopkg.lock's input-imports
k8s.io/apimachinery/pkg/util/errors: imported or required, but missing from Gopkg.lock's input-imports
k8s.io/client-go/kubernetes: imported or required, but missing from Gopkg.lock's input-imports
k8s.io/client-go/pkg/api/v1: imported or required, but missing from Gopkg.lock's input-imports
k8s.io/client-go/tools/clientcmd: imported or required, but missing from Gopkg.lock's input-imports
k8s.io/kubernetes/pkg/kubectl/cmd/util: imported or required, but missing from Gopkg.lock's input-imports
k8s.io/kubernetes/pkg/kubectl/resource: imported or required, but missing from Gopkg.lock's input-imports

@crawford
Copy link
Contributor

crawford commented Oct 2, 2018

I'm in favor of switch to dep, even if we end up switching to go modules relatively soon.

@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 2, 2018
@openshift-bot openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 2, 2018
$ rm -rf tests
$ dep init
// edit Gopkg.toml
```toml
[prune]
  non-go = true
  go-tests = true
  unused-packages = true
```
$ dep ensure
$ dep init
// edit Gopkg.toml
```toml
[prune]
  non-go = true
  go-tests = true
  unused-packages = true
```
$ dep ensure
@crawford
Copy link
Contributor

crawford commented Oct 2, 2018

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 2, 2018
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, crawford

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

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [abhinavdahiya,crawford]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@abhinavdahiya
Copy link
Contributor Author

/retest

@openshift-merge-robot openshift-merge-robot merged commit 444d2c6 into openshift:master Oct 3, 2018
wking added a commit to wking/openshift-installer that referenced this pull request Oct 20, 2018
These escaped the great purge of 0c6d53b (*: remove bazel,
2018-09-24, openshift#342).  kubernetes/BUILD.bazel snuck in with 70ea0e8
(tests/smoke/vendor: switch from glide to dep, 2018-09-28, openshift#380), and
tectonic/BUILD.bazel snuck in with e2d9fd3 (manifests: make tectonic/
flat dir, 2018-09-25, openshift#330).  I'd guess both were due to rebases from
commits originally made before openshift#342 landed.
wking added a commit to wking/openshift-installer that referenced this pull request Nov 8, 2018
This reverts commit 8ff1cee
(2018-09-28, openshift#370).

We moved from Glide to dep in 1f45543 (vendor: switch from glide to
dep, 2018-09-28, openshift#380), so we no longer need to worry about yamllint
vs. Glide.yaml.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants