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

Enforce go-licenses v1.0.0 #396

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

matzew
Copy link
Member

@matzew matzew commented Sep 3, 2024

This should fixes issues we are getting with go.mod files containing the toolchain directive.

See google/go-licenses#128.

This should fixes issues we are getting with `go.mod` files containing
the `toolchain` directive.

See google/go-licenses#128.

Signed-off-by: Matthias Wessendorf <mwessend@redhat.com>
Copy link

knative-prow bot commented Sep 3, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: matzew
Once this PR has been reviewed and has the lgtm label, please assign dprotaso for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

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

@knative-prow knative-prow bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Sep 3, 2024
@matzew
Copy link
Member Author

matzew commented Sep 3, 2024

Tekton was doing similar before:
tektoncd/plumbing#2042

I was getting a lot of

E0903 10:38:10.600391   51037 library.go:117] Package crypto/aes does not have module info. Non go modules projects are no longer supported. For feedback, refer to https://github.com/google/go-licenses/issues/128.
W0903 10:38:10.600398   51037 library.go:101] "golang.org/x/crypto/chacha20poly1305" contains non-Go code that can't be inspected for further dependencies:
/home/matzew/go/src/knative.dev/serving/vendor/golang.org/x/crypto/chacha20poly1305/chacha20poly1305_amd64.s
W0903 10:38:10.616318   51037 library.go:101] "golang.org/x/crypto/internal/poly1305" contains non-Go code that can't be inspected for further dependencies:
/home/matzew/go/src/knative.dev/serving/vendor/golang.org/x/crypto/internal/poly1305/sum_amd64.s
W0903 10:38:10.623869   51037 library.go:101] "golang.org/x/sys/cpu" contains non-Go code that can't be inspected for further dependencies:
/home/matzew/go/src/knative.dev/serving/vendor/golang.org/x/sys/cpu/cpu_x86.s
E0903 10:38:11.100804   51037 library.go:117] Package net/rpc does not have module info. Non go modules projects are no longer supported. For feedback, refer to https://github.com/google/go-licenses/issues/128.
F0903 10:38:16.129641   51037 main.go:77] some errors occurred when loading direct and transitive dependency packages

hence I am proposing this

@dsimansk
Copy link
Contributor

dsimansk commented Sep 3, 2024

@matzew what's the exact error related to toolchain directive? The warning about native files are present for a long time in go-licenses check.

@matzew
Copy link
Member Author

matzew commented Sep 4, 2024

I personally find this a bit annoying:

F0904 08:18:20.345646   54599 main.go:77] some errors occurred when loading direct and transitive dependency packages
exit status 1
--- FAIL: go-licenses failed the license check
Command '__go_update_deps_for_module' failed in module /home/matzew/go/src/knative.dev/eventing: 1

Am I the only one w/ the exit status and FAIL:?

@@ -918,7 +918,7 @@ function run_kntest() {
# Run go-licenses to check for forbidden licenses.
function check_licenses() {
# Check that we don't have any forbidden licenses.
go_run github.com/google/go-licenses@v1.6.0 \
go_run github.com/google/go-licenses@v1.0.0 \
Copy link

@skonto skonto Sep 5, 2024

Choose a reason for hiding this comment

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

We are going back? 🤔 Ok I see:

In v1.1, we made a breaking change of no longer supporting non go modules managed projects

Tekton has it (and go license have breaking changes) but is it a failure or warning? Does it happen on the CI too or is it local? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

There are warning in various CI logs, but I haven't found exit status 1.

@dsimansk
Copy link
Contributor

dsimansk commented Sep 5, 2024

I personally find this a bit annoying:

F0904 08:18:20.345646   54599 main.go:77] some errors occurred when loading direct and transitive dependency packages
exit status 1
--- FAIL: go-licenses failed the license check
Command '__go_update_deps_for_module' failed in module /home/matzew/go/src/knative.dev/eventing: 1

Am I the only one w/ the exit status and FAIL:?

What's the command you are running it through here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants