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

Revisit the need to bundle the LICENSE files in third_party folder #1441

Closed
cardil opened this issue Sep 27, 2023 · 11 comments · Fixed by knative/hack#376
Closed

Revisit the need to bundle the LICENSE files in third_party folder #1441

cardil opened this issue Sep 27, 2023 · 11 comments · Fixed by knative/hack#376
Assignees

Comments

@cardil
Copy link
Contributor

cardil commented Sep 27, 2023

Background

We run google/go-licenses tool's save command in every repo. It bundles the LICENSE files of the dependencies, or their code, depends on the license itself. This has been the practice of the project since Knative inception. The requirements were coming from Google's OSPO office.

The CNCF has those 2 pages on the licenses:

Despite that, the CNCF Allowlist License Policy mentions that non-apache deps should be held in a designated third-party folder, such practice is very uncommon among the CNCF projects.

I did find only the https://github.com/kubernetes/kubernetes actually holds some third-party components in a designated folder (not only the LICENSE files)

Question

Is the current practice really required by CNCF? Maybe we can drop it like most projects do?

Links

This follows up the discussion in knative/hack#315
Related to cncf/foundation#642

@evankanderson
Copy link
Member

/project Steering Committee Backlog

Copy link

knative-prow bot commented Nov 21, 2023

@evankanderson: You must be a member of the knative/community github team to set the project and column.

In response to this:

/project Steering Committee Backlog

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@evankanderson
Copy link
Member

See discussion in cncf/foundation#642

@dprotaso
Copy link
Member

Following up here - I'm inclined to drop this practice given we've gotten no guidance from CNCF. Clearly it's not an issue for them if we don't do this.

@rhuss
Copy link
Contributor

rhuss commented Jan 16, 2024

I would drop our license handling, too. The sooner, the better. After the upgrade to go 1.21 that introduced "toolchain", the license checker is broken for any toolchain not bundled with the installed go installation. See google/go-licenses#244 . And there are tons of other peculiarities which took ages to analyze. Please drop the license check.

@dprotaso
Copy link
Member

After the upgrade to go 1.21 that introduced "toolchain",

Is there a reason to specify the toolchain directive over go directive?

@rhuss
Copy link
Contributor

rhuss commented Jan 17, 2024

Is there a reason to specify the toolchain directive over go directive?

I don't think so. We haven't added toolchain manually to go.mod, but it was introduced by ./hack/update-deps.sh for some reason. We don't actively use toolchain (and nailing everything down to the patch-level is also something we should not do via this mechanism)

@cardil
Copy link
Contributor Author

cardil commented Mar 27, 2024

Okay. Can we agree that we could at least remove the need to bundle all those LICENSE files?

Personally, I would keep the check as it, from time to time, actually finds some invalid libs, like knative/client-pkg#166

@dprotaso
Copy link
Member

Okay. Can we agree that we could at least remove the need to bundle all those LICENSE files?

Yeah let's update hack to stop saving the licenses - then each repo can delete them

@cardil
Copy link
Contributor Author

cardil commented Mar 27, 2024

This PR does that knative/hack#376

@dprotaso
Copy link
Member

Serving PRs are out that drop the vendored files

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

Successfully merging a pull request may close this issue.

4 participants