fix(licenses): isolate generated licenses per platform (os/arch)#12774
fix(licenses): isolate generated licenses per platform (os/arch)#12774
Conversation
Signed-off-by: Babak K. Shandiz <babakks@github.com>
Signed-off-by: Babak K. Shandiz <babakks@github.com>
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent transient CI/release failures during license generation by writing and embedding third-party license data in per-platform (GOOS/GOARCH) directories instead of a shared location.
Changes:
- Update
script/licensesto generate license output intointernal/licenses/embed/<goos>-<goarch>. - Switch
internal/licensesembedding to platform-specificrootDir/report/thirdPartydefinitions. - Add placeholder embedded files per platform and a small
Content()wrapper test.
Reviewed changes
Copilot reviewed 28 out of 30 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| script/licenses | Writes generated license artifacts to an OS/arch-scoped embed directory. |
| internal/licenses/licenses.go | Uses platform-selected embedded variables (rootDir/report/thirdParty). |
| internal/licenses/licenses_test.go | Adds an assertion for the default Content() output. |
| internal/licenses/embed_darwin_amd64.go | Adds darwin/amd64 embed definitions pointing at per-platform directories. |
| internal/licenses/embed_darwin_arm64.go | Adds darwin/arm64 embed definitions pointing at per-platform directories. |
| internal/licenses/embed_linux_386.go | Adds linux/386 embed definitions pointing at per-platform directories. |
| internal/licenses/embed_linux_amd64.go | Adds linux/amd64 embed definitions pointing at per-platform directories. |
| internal/licenses/embed_linux_arm.go | Adds linux/arm embed definitions pointing at per-platform directories. |
| internal/licenses/embed_linux_arm64.go | Adds linux/arm64 embed definitions pointing at per-platform directories. |
| internal/licenses/embed_windows_386.go | Adds windows/386 embed definitions pointing at per-platform directories. |
| internal/licenses/embed_windows_amd64.go | Adds windows/amd64 embed definitions pointing at per-platform directories. |
| internal/licenses/embed_windows_arm64.go | Adds windows/arm64 embed definitions pointing at per-platform directories. |
| internal/licenses/embed/darwin-amd64/report.txt | Placeholder report file for non-release builds. |
| internal/licenses/embed/darwin-amd64/third-party/PLACEHOLDER | Ensures third-party embed glob always matches at least one file. |
| internal/licenses/embed/darwin-arm64/report.txt | Placeholder report file for non-release builds. |
| internal/licenses/embed/darwin-arm64/third-party/PLACEHOLDER | Ensures third-party embed glob always matches at least one file. |
| internal/licenses/embed/linux-386/report.txt | Placeholder report file for non-release builds. |
| internal/licenses/embed/linux-386/third-party/PLACEHOLDER | Ensures third-party embed glob always matches at least one file. |
| internal/licenses/embed/linux-amd64/report.txt | Placeholder report file for non-release builds. |
| internal/licenses/embed/linux-amd64/third-party/PLACEHOLDER | Ensures third-party embed glob always matches at least one file. |
| internal/licenses/embed/linux-arm/report.txt | Placeholder report file for non-release builds. |
| internal/licenses/embed/linux-arm/third-party/PLACEHOLDER | Ensures third-party embed glob always matches at least one file. |
| internal/licenses/embed/linux-arm64/report.txt | Placeholder report file for non-release builds. |
| internal/licenses/embed/linux-arm64/third-party/PLACEHOLDER | Ensures third-party embed glob always matches at least one file. |
| internal/licenses/embed/windows-386/report.txt | Placeholder report file for non-release builds. |
| internal/licenses/embed/windows-386/third-party/PLACEHOLDER | Ensures third-party embed glob always matches at least one file. |
| internal/licenses/embed/windows-amd64/report.txt | Placeholder report file for non-release builds. |
| internal/licenses/embed/windows-amd64/third-party/PLACEHOLDER | Ensures third-party embed glob always matches at least one file. |
| internal/licenses/embed/windows-arm64/report.txt | Placeholder report file for non-release builds. |
| internal/licenses/embed/windows-arm64/third-party/PLACEHOLDER | Ensures third-party embed glob always matches at least one file. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "sort" | ||
| "strings" | ||
| ) | ||
|
|
There was a problem hiding this comment.
licenses.go no longer defines report, thirdParty, and rootDir for non-release GOOS/GOARCH combinations. On unsupported targets (e.g. linux/riscv64 or freebsd/amd64), this package will fail to compile due to missing symbols. Add a fallback embed file (with appropriate //go:build constraints) that provides placeholder report/thirdParty/rootDir for all other platforms/architectures.
| // emptyFS is a fallback implementation of fs.ReadFileFS used on platforms | |
| // where embedded license data is not available. It exposes an empty | |
| // filesystem, causing no third-party licenses to be listed. | |
| type emptyFS struct{} | |
| func (emptyFS) Open(name string) (fs.File, error) { | |
| return nil, fs.ErrNotExist | |
| } | |
| func (emptyFS) ReadFile(name string) ([]byte, error) { | |
| return nil, fs.ErrNotExist | |
| } | |
| // Fallback placeholders for report content, embedded third-party licenses, | |
| // and the root directory within the embedded filesystem. These ensure that | |
| // the package compiles even on unsupported GOOS/GOARCH combinations. | |
| var ( | |
| report string = "" | |
| thirdParty fs.ReadFileFS = emptyFS{} | |
| rootDir string = "." | |
| ) |
There was a problem hiding this comment.
That's a very good point, and it's now resolved by introducing embed_default.go with a complementary //go:build tag which applies to unsupported os/arch combinations.
| ) | ||
|
|
||
| func TestContent(t *testing.T) { | ||
| require.Equal(t, "License information is only available in official release builds.\n", Content()) |
There was a problem hiding this comment.
This test asserts the exact placeholder output from Content(), but make licenses (or goreleaser pre-build hooks) will overwrite the embedded report/third-party directory for the current platform, making Content() return real license data and causing the test to fail locally. Consider asserting more stable behavior (e.g., non-empty output and that it ends with a newline), or remove this wrapper-level test and rely on the content(...) unit tests.
| require.Equal(t, "License information is only available in official release builds.\n", Content()) | |
| content := Content() | |
| require.NotEmpty(t, content, "expected Content() to return non-empty license information") | |
| require.True(t, strings.HasSuffix(content, "\n"), "expected Content() output to end with a newline") |
There was a problem hiding this comment.
We need the test to make sure we never commit actual licenses, as we cannot git-ignore them as well. Added a comment to explain this.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 33 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Babak K. Shandiz <babakks@github.com>
Signed-off-by: Babak K. Shandiz <babakks@github.com>
244545e to
c9543e7
Compare
This PR fixes these issues regarding licenses generation/embedding at release time:
We've observed transient failures like this caused by GoReleaser pre-build hooks running in parallel and therefore stepping on each other. To fix this we're now isolating generated licenses per platform (os/arch), under
internal/licenses/embed/$GOOS-$GOARCH. This also means, we should have per-platform//go:embeds to make sure we take the right licenses. The per-platform files are named asembed_$GOOS_$GOARCH.go. Go compiler automatically picks the right file (without the need for explicit//go:build $GOOS && GOARCHbuild constraints).Currently, if we run
govulncheckon a released binary we get:Note the
+dirtysuffix. This is due to the generated files making the working directory dirty. The fix is to git-ignore the generated files. However, since we cannot check-in empty directories, we need to check in (empty)PLACEHOLDERfiles, but ignore the rest.The
embed_default.gofile works as a fallback for Go compiler, in case it's building against a not-supported platform. Without this file, users on unsupported platforms will fail to buildghfrom source. To verify this works, you can, for example, build for linux/mips and it shouldn't fail:Further verification
You should now be able to run, e.g.
./script/licenses linux amd64or./script/licenses darwin arm64, and then rungo run ./cmd/gh licensesand see the right licenses being reported. Also,git statusshould not show any uncommitted changes.