-
Notifications
You must be signed in to change notification settings - Fork 8.3k
Ensure lint workflow checks whether 3rd party license and code is up to date #11047
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
Changes from all commits
bd24865
815c557
b30101c
14c2673
98ea250
4e8d022
4d1eb59
11e8a81
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| # GitHub CLI dependencies | ||
|
|
||
| The following open source dependencies are used to build the [cli/cli][] GitHub CLI. | ||
|
|
||
| ## Go Packages | ||
|
|
||
| Some packages may only be included on certain architectures or operating systems. | ||
|
|
||
| {{ range . }} | ||
| - [{{.Name}}](https://pkg.go.dev/{{.Name}}) ([{{.LicenseName}}]({{.LicenseURL}})) | ||
| {{- end }} | ||
|
|
||
| [cli/cli]: https://github.com/cli/cli |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| # License Compliance | ||
|
|
||
| GitHub CLI complies with the software licenses of its dependencies. This document explains how license compliance is maintained. | ||
|
|
||
| ## Overview | ||
|
|
||
| When a dependency is added or updated, the license information needs to be updated. We use the [`google/go-licenses`](https://github.com/google/go-licenses) tool to: | ||
|
|
||
| 1. Generate markdown documentation listing all Go dependencies and their licenses | ||
| 2. Copy license files for dependencies that require redistribution | ||
|
|
||
| ## License Files | ||
|
|
||
| The following files contain license information: | ||
|
|
||
| - `third-party-licenses.darwin.md` - License information for macOS dependencies | ||
| - `third-party-licenses.linux.md` - License information for Linux dependencies | ||
| - `third-party-licenses.windows.md` - License information for Windows dependencies | ||
| - `third-party/` - Directory containing source code and license files that require redistribution | ||
|
|
||
| ## Updating License Information | ||
|
|
||
| When dependencies change, you need to update the license information: | ||
|
|
||
| 1. Update license information for all platforms: | ||
|
|
||
| ```shell | ||
| make licenses | ||
| ``` | ||
|
|
||
| 2. Commit the changes: | ||
|
|
||
| ```shell | ||
| git add third-party-licenses.*.md third-party/ | ||
| git commit -m "Update third-party license information" | ||
| ``` | ||
|
|
||
| ## Checking License Compliance | ||
|
|
||
| The CI workflow checks if license information is up to date. To check locally: | ||
|
|
||
| ```sh | ||
| make licenses-check | ||
| ``` | ||
|
|
||
| If the check fails, follow the instructions to update the license information. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| #!/bin/bash | ||
|
|
||
| go install github.com/google/go-licenses@latest | ||
|
|
||
| # Setup temporary directory to collect updated third-party source code | ||
| export TEMPDIR="$(mktemp -d)" | ||
| trap "rm -fr ${TEMPDIR}" EXIT | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (praise) Smart! |
||
|
|
||
| # Clear third-party source code to avoid stale content | ||
| rm -rf third-party | ||
| mkdir -p third-party | ||
|
|
||
|
Comment on lines
+9
to
+12
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: Should we check successes (exit codes) here? Are we okay with this maybe failing but then continuing?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not worried about these commands failing, but if there is a concern then we might want to |
||
| for goos in linux darwin windows ; do | ||
| # Note: we ignore warnings because we want the command to succeed, however the output should be checked | ||
| # for any new warnings, and potentially we may need to add license information. | ||
| # | ||
| # Normally these warnings are packages containing non go code, which may or may not require explicit attribution, | ||
| # depending on the license. | ||
| echo "Generating licenses for ${goos}..." | ||
| GOOS="${goos}" go-licenses save ./... --save_path="${TEMPDIR}/${goos}" --force || echo "Ignore warnings" | ||
| GOOS="${goos}" go-licenses report ./... --template .github/licenses.tmpl --ignore github.com/cli/cli > third-party-licenses.${goos}.md || echo "Ignore warnings" | ||
| cp -fR "${TEMPDIR}/${goos}"/* third-party/ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: Similar question here - should we check exit codes here or keep going even if something failed? |
||
| done | ||
|
|
||
| echo "Licenses generated for all platforms." | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| #!/bin/bash | ||
|
|
||
| go install github.com/google/go-licenses@latest | ||
|
|
||
| # Setup temporary directory for generated license reports | ||
| export TEMPDIR="$(mktemp -d)" | ||
| trap "rm -fr ${TEMPDIR}" EXIT | ||
|
|
||
| for goos in linux darwin windows ; do | ||
| # Note: we ignore warnings because we want the command to succeed, however the output should be checked | ||
| # for any new warnings, and potentially we may need to add license information. | ||
| # | ||
| # Normally these warnings are packages containing non go code, which may or may not require explicit attribution, | ||
| # depending on the license. | ||
| echo "Checking licenses for ${goos}..." | ||
| GOOS="${goos}" go-licenses report ./... --template .github/licenses.tmpl --ignore github.com/cli/cli > "${TEMPDIR}/third-party-licenses.${goos}.md" || echo "Ignore warnings" | ||
| if ! diff -s "${TEMPDIR}/third-party-licenses.${goos}.md" "third-party-licenses.${goos}.md"; then | ||
| echo "::error title=License check failed::Please update the license files by running \`make licenses\` and committing the output." | ||
| exit 1 | ||
| fi | ||
| done | ||
|
|
||
| echo "License check passed for all platforms." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: Are we comfortable with always using the latest here, or should we pin to a sha for stability/security etc?
I know that pinning comes with the "when does this get updated then?" question, and I don't know the answer to that either :P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this is especially in my mind with this script being used in actions... 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@BagToad : what do you think about conditionalizing this to skip installing this if
CI, leaving that to the workflow?otherwise, everything running the script has to manage its installation. conditionalizing it will make it easy for contributors and maintainers.