Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a number of testing & dependency issues #24

Merged
merged 8 commits into from
Mar 6, 2017

Conversation

erikh
Copy link
Contributor

@erikh erikh commented Jan 30, 2017

Please see the individual commits. Until this was done, I was unable to test
locally.

The virtualbox flip in particular is important if you have the kvm module
running but no qemu or libvirt vagrant provider, which doesn't install on
modern vagrant without a rubygems error. So, I think it's better to keep the
safe, easy-to-install version preferred, especially because vagrant was made to
run virtualbox.

I also incorporated github.com/LK4D4/vndr to manage dependencies.

I think it might be a good idea to setup a monorepo for all the containers/
projects as a submodule to vendor/. This will help quite a bit with conflicts
I think. cc @runcom @mtrmac

The integration-cli target doesn't exist because the dir doesn't exist. I don't
know if that's an intentional thing or not, but as an open source project it's
pretty much impossible to contribute from the outside without this information.

go as installed from package (at least on fedora) does not work to compile
golint and a few other things, so I decided it was best to get official 1.7.4
in there -- it'll also help if the debian/fedora packages mismatch.

Again, I couldn't test at all until these patches were made. All of them except
the .gitignore changes were required.

Erik Hollensbe added 6 commits January 30, 2017 08:43
Signed-off-by: Erik Hollensbe <github@hollensbe.org>
Signed-off-by: Erik Hollensbe <github@hollensbe.org>
Signed-off-by: Erik Hollensbe <github@hollensbe.org>
…r does not exist.

Signed-off-by: Erik Hollensbe <github@hollensbe.org>
Signed-off-by: Erik Hollensbe <github@hollensbe.org>
…ths.

Signed-off-by: Erik Hollensbe <github@hollensbe.org>
@erikh erikh force-pushed the fix-vendor branch 2 times, most recently from 93ced02 to 350b440 Compare February 2, 2017 02:28
@nalind
Copy link
Member

nalind commented Feb 13, 2017

I love just about everything in here.
I'm a bit worried about bringing our own version of the compiler into VMs for running the tests, since this package gets vendored into other projects where the compiler version is not something we dictate. Fedora 23 had 1.5.4, while Fedora 25, if we tweaked the Vagrantfile to switch to it, and Debian testing both get 1.7.5. Travis tests with multiple versions, but none of those are the packaged versions of the compiler, either, so we wouldn't be testing with the OS-provided compiler anywhere.

.travis.yml Outdated
- sudo env AUTO_GOPATH=1 PATH="$PATH" ./hack/make.sh test-unit
- AUTO_GOPATH=1 make docs
- make install.tools
- ./hack/make.sh validate-gofmt validate-pkg validate-lint validate-test validate-toml validate-vet validate-vendor
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Travis may only be failing because this patch removes the 'validate-vendor' script, but leaves it in this list.

@erikh
Copy link
Contributor Author

erikh commented Feb 13, 2017

the unfortunate problem with jumping between pre-1.7 and 1.7 is the context package. It typically works with one or the other, and most of the libs have already ported to the 1.7 context package (as opposed to the golang.org third-party one)

@nalind
Copy link
Member

nalind commented Feb 13, 2017

Ugh. IIRC we've got some targets (I think CentOS 7 and its RHEL upstream) that are still on 1.6. I don't think we're using ioutils.CancelReadCloser anywhere in storage itself, so if it isn't being used by containers/image, skopeo, or cri-o (buildah doesn't), I'd be tempted to drop the implementation of that type to side-step the issue.

Signed-off-by: Erik Hollensbe <github@hollensbe.org>
@erikh
Copy link
Contributor Author

erikh commented Feb 16, 2017

I've refreshed the branch and will monitor it to ensure tests pass.

.travis.yml Outdated
- make .gitvalidation
- make build-binary
- ./hack/make.sh cross
- sudo ./hack/make.sh test-unit
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently the go compiler's not in the default $PATH in Travis.

@erikh
Copy link
Contributor Author

erikh commented Feb 17, 2017

some pathing issue; I'll have to deal with this tomorrow, sorry

Signed-off-by: Erik Hollensbe <github@hollensbe.org>
@erikh
Copy link
Contributor Author

erikh commented Feb 27, 2017

This should be passing now; the fundamental issue was some GOPATH muckery I missed and travis's sudo config has env_keep PATH set I think, so the PATH needs to be injected directly.

All in the last commit. PTAL.

@erikh
Copy link
Contributor Author

erikh commented Mar 1, 2017

Sorry to press but the lint branch is getting pretty expansive, I was hoping we could discuss or finalize this somehow.

@erikh erikh mentioned this pull request Mar 4, 2017
@nalind
Copy link
Member

nalind commented Mar 6, 2017

I'm still going to need to get builds going on non-1.7 versions of go, but that's not a good reason to hold this up, as the tests pass in CI and on my box.

@nalind nalind merged commit 7c6ff42 into containers:master Mar 6, 2017
@erikh
Copy link
Contributor Author

erikh commented Mar 6, 2017

Ok, I will put in a ticket for that and try to address it this week.

@nalind
Copy link
Member

nalind commented Mar 7, 2017

That should be sorted by #30, assuming nothing in there is too outlandish.

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

Successfully merging this pull request may close these issues.

2 participants