CI: Add check for SPDX license headers by jodh-intel · Pull Request #242 · kata-containers/tests

jodh-intel · 2018-04-17T13:46:22Z

Add a check to ensure that all files where possible contain the
required SPDX license header.

Fixes #240.

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

jodh-intel · 2018-04-17T13:47:10Z

jodh-intel · 2018-04-17T13:58:29Z

.ci/static-checks.sh

+		--exclude="vendor/*" \
+		--exclude="VERSION" \
+		--exclude="*.md" \
+		-EL "\<${pattern}\>" \


This list of excludes works reasonably well, but I wonder if we want to add .pullapprove.yml to it too? We could of course instead add the SPDX header to that file ;)

what about exclude yml and toml files in general?

I guess we could, although for example the versions database does have the SPDX tag:

https://github.com/kata-containers/runtime/blob/master/versions.yaml#L4

But yes, we'd need to add it to configuration.toml (and probably should ;)

I vote maybe we should add them if that does not upset any of the relevant processing tools or bots

Ok - added.

jcvenegas

lgtm! I would exclude yaml and toml files too

jcvenegas · 2018-04-17T14:40:58Z

.ci/static-checks.sh

+	files=$(git diff-tree \
+		--name-only \
+		--no-commit-id \
+		--diff-filter="ACMRUX" \


new to me - went to look it up - still digesting... https://git-scm.com/docs/git-diff-tree#git-diff-tree---diff-filterACDMRTUXB82308203

Yeah, it's a bit opaque :) I've updated the file to make it a bit clearer ;)

jcvenegas · 2018-04-17T14:42:34Z

.ci/static-checks.sh

+		--exclude="vendor/*" \
+		--exclude="VERSION" \
+		--exclude="*.md" \
+		-EL "\<${pattern}\>" \


what about exclude yml and toml files in general?

grahamwhaley

Even if we are going to fix all the .yaml and other files that are either missing or have a non-SPDX header, I think this will still work fine, so:
lgtm

grahamwhaley · 2018-04-17T15:39:58Z

and thanks @jodh-intel for writing that :-)

Add a check to ensure that all files where possible contain the required SPDX license header. Fixes kata-containers#240. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

GabyCT · 2018-04-17T19:43:06Z

lgtm

jcvenegas · 2018-04-17T20:03:00Z

@GabyCT @chavafg "jenkins-ci-ubuntu-16-04 Expected — Waiting for status to be reported" I see ubuntu did not reported, is this a known issue?

chavafg · 2018-04-17T20:07:09Z

Don't know what happened. The job finished successfully: http://kata-jenkins-ci.westus2.cloudapp.azure.com/job/kata-containers-tests-ubuntu-16-04-PR/325/
Restarted the job.

jodh-intel · 2018-04-18T06:17:19Z

@grahamwhaley - assuming this was a CI hiccup, do you want to merge?

grahamwhaley · 2018-04-18T08:35:21Z

afaict, the CI did pass - at least as well as any other PR. @chavafg , looking at a few of the logs for the 16.04 builds, they seem to have a number of error-ish type lines (around yaml not found and coverage not reported etc.). I wonder if we can make the scripts a little smarter to reduce some of the noise in the future?
merging..

bergwolf · 2018-04-18T09:52:06Z

It seems that this PR is breaking all CIs. I'm seeing

ERROR: Required license identifier ('SPDX-License-Identifier: Apache-2.0') missing from following files:

for PRs that do not add any new files. Do we want to block all PRs until the switch is done?

grahamwhaley · 2018-04-18T09:59:08Z

@bergwolf (or anybody) - do you have a link to a PR that has the failing CI so we can have a look?

bergwolf · 2018-04-18T10:03:29Z

@grahamwhaley https://travis-ci.org/kata-containers/runtime/builds/368056793

grahamwhaley · 2018-04-18T10:07:43Z

Ah, I'm afraid the tool did exactly what it was meant to. The bit you did not paste is:

virtcontainers/agent.go
virtcontainers/api.go
virtcontainers/api_test.go
virtcontainers/hyperstart_agent.go
virtcontainers/implementation.go
virtcontainers/interfaces.go
virtcontainers/kata_agent.go
virtcontainers/noop_agent.go
virtcontainers/pkg/vcmock/mock.go
virtcontainers/pkg/vcmock/mock_test.go
virtcontainers/pkg/vcmock/sandbox.go
virtcontainers/pkg/vcmock/types.go
virtcontainers/sandbox.go

From the files in the PR: https://github.com/bergwolf/kata-runtime/blob/6eb92805dd13ba4bd9e37cbe8a48f1b53c363065/virtcontainers/agent.go#L1-L15

See kata-containers/runtime#227
The correct fix might be to update/fix the license headers in those files...

bergwolf · 2018-04-18T10:26:46Z

@grahamwhaley That's what I meant by "block all PRs until the switch is done", because it is now required to update the license header for any PR that touches the improperly licensed files.

I think the sequence to move forward is to send separate PRs to fix the license headers and get them merged first, and then merge this PR to make sure new any added files are properly licensed.

Right now, it seems that we are merging tests first and catching up with implementations.

bergwolf · 2018-04-18T10:29:13Z

And CI for any PR in runtime repo will fail until kata-containers/runtime#227 is fixed.

grahamwhaley · 2018-04-18T10:32:42Z

Well, I understand :-) Yes, we are currently checking any files that are modified as well as newly added.
options:

fix the files that are touched in PRs right now whilst we prep the PR for vc SPDX headers
temporarily change the check so it only checks newly added files
turn the check off completely
write and land the vc SPDX update PR asap

I favour maybe a combination of 2 and 4, if we cannot do 4 very quickly... I can jump on (4) in a few hours time....

bergwolf · 2018-04-18T10:38:54Z

@grahamwhaley I'm in favor of any option that can unblock other PRs quickly ;) Thanks!

grahamwhaley · 2018-04-18T12:45:57Z

See kata-containers/runtime#231

jodh-intel added the review label Apr 17, 2018

jodh-intel force-pushed the check-spdx-license-headers branch from cbe2405 to be93644 Compare April 17, 2018 13:54

jodh-intel commented Apr 17, 2018

View reviewed changes

jcvenegas approved these changes Apr 17, 2018

View reviewed changes

jodh-intel force-pushed the check-spdx-license-headers branch 3 times, most recently from 23769da to 2e479d4 Compare April 17, 2018 15:39

grahamwhaley approved these changes Apr 17, 2018

View reviewed changes

CI: Add check for SPDX license headers

f5db778

Add a check to ensure that all files where possible contain the required SPDX license header. Fixes kata-containers#240. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

jodh-intel force-pushed the check-spdx-license-headers branch from 2e479d4 to f5db778 Compare April 17, 2018 15:41

grahamwhaley mentioned this pull request Apr 17, 2018

virtcontainers: move to using SPDX headers kata-containers/runtime#227

Closed

grahamwhaley merged commit a7ee85b into kata-containers:master Apr 18, 2018

grahamwhaley removed the review label Apr 18, 2018

grahamwhaley mentioned this pull request Apr 18, 2018

license: SPDX: update all vc files to use SPDX style kata-containers/runtime#231

Merged

Conversation

jodh-intel commented Apr 17, 2018

Uh oh!

jodh-intel commented Apr 17, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcvenegas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

grahamwhaley left a comment

Choose a reason for hiding this comment

Uh oh!

grahamwhaley commented Apr 17, 2018

Uh oh!

GabyCT commented Apr 17, 2018 • edited by amshinde Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jcvenegas commented Apr 17, 2018

Uh oh!

chavafg commented Apr 17, 2018

Uh oh!

jodh-intel commented Apr 18, 2018

Uh oh!

grahamwhaley commented Apr 18, 2018

Uh oh!

bergwolf commented Apr 18, 2018

Uh oh!

grahamwhaley commented Apr 18, 2018

Uh oh!

bergwolf commented Apr 18, 2018

Uh oh!

grahamwhaley commented Apr 18, 2018

Uh oh!

bergwolf commented Apr 18, 2018

Uh oh!

bergwolf commented Apr 18, 2018

Uh oh!

grahamwhaley commented Apr 18, 2018

Uh oh!

bergwolf commented Apr 18, 2018

Uh oh!

grahamwhaley commented Apr 18, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

GabyCT commented Apr 17, 2018 •

edited by amshinde

Loading