Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

versions: Add xurls utility #855

Closed
wants to merge 1 commit into from

Conversation

jodh-intel
Copy link
Contributor

Add the xurls utility to the
versions database. This tool is used by the CI scripts in the tests
repo to check URLs for validity. However, the latest version of xurls
requires golang 1.10.3+, but we don't need to use the latest version and
doing so is causing us problems (see
kata-containers/kata-containers#32).

Fixes #854.

Signed-off-by: James O. D. Hunt james.o.hunt@intel.com

Copy link
Contributor

@grahamwhaley grahamwhaley left a comment

Choose a reason for hiding this comment

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

I think pinning this, particularly for older stable branches (so they don't rot as the tools move forwards), is the right thing to do...
lgtm

@jodh-intel
Copy link
Contributor Author

This fix will allow the tests CI to be updated to install the correct version of xurls for a particular stable branch. However, this YAML could arguably could be added the tests versions database instead:

Either way, we need to use a pinned version of xurls in the stable branches to fix the CI failures on these PRs:

... or we have to backport the golang 1.10.4 fix to all stable branches:

@egernst, @gnawux, @grahamwhaley, @chavafg - any thoughts on what the best approach is here?

@grahamwhaley
Copy link
Contributor

The principle of backports to stable branches for new versions of golang (be that in our code or code and tools we import), feels wrong. They are stable, so we should not be injecting a bunch of flux.
Pinning items feels like a more sustainable fix to me (at some upfront cost of doing some config setup)

Add the [`xurls`](https://mvdan.cc/xurls/cmd/xurls) utility to the
versions database. This tool is used by the CI scripts in the `tests`
repo to check URLs for validity. However, the latest version of `xurls`
requires golang 1.10.3+, but we don't need to use the latest version and
doing so is causing us problems (see
kata-containers/kata-containers#32).

Fixes kata-containers#854.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
@chavafg
Copy link
Contributor

chavafg commented Oct 25, 2018

@jodh-intel I would prefer that this xurls version should go in the versions.yaml of the tests repository as it is a tool only used for testing purposes.

@jodh-intel
Copy link
Contributor Author

@chavafg - wfm. I'll close this PR and probably handle it on kata-containers/tests#856...

@jodh-intel jodh-intel closed this Oct 25, 2018
egernst pushed a commit to egernst/runtime that referenced this pull request Feb 9, 2021
Handle PCI paths consistently and more generally
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