-
-
Couldn't load subscription status.
- Fork 673
Go: compile with trimpath for reproducible output #22749
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
base: main
Are you sure you want to change the base?
Conversation
As a point of order, is this a regression? That is, did it work in an earlier release? |
AFAIK no it has not worked before. |
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 use go, but I've never had a reason to go looking for the -trimpath option. Nice!
Are there any other GoSdkProcess instances that need this? Maybe the go build calls in pants.backend.go.util_rules
.build_pkg?
For anyone else that's curious here's the description of -trimpath in the docs.
The longest description of -trimpath is on the go build docs.
go build [-o output] [build flags] [packages]
...
-trimpath
remove all file system paths from the resulting executable.
Instead of absolute file system paths, the recorded file names
will begin either a module path@version (when using modules),
or a plain import path (when using the standard library, or GOPATH).
From https://pkg.go.dev/cmd/go#hdr-Compile_packages_and_dependencies
This PR touches on 2 go tool invocations:
go tool compile [flags] file...
...
-trimpath prefix
Remove prefix from recorded source file paths.
From https://pkg.go.dev/cmd/compile#hdr-Command_Line
go tool cgo [cgo options] [-- compiler options] gofiles...
...
-trimpath rewrites
Apply trims and rewrites to source file paths.
From https://pkg.go.dev/cmd/cgo#hdr-Using_cgo_directly
And it looks like the GoSdkProcess calls to these tools already include the -trimpath option:
go tool asmgo tool link
| description=f"Compile Go package: {request.import_path}", | ||
| output_files=("__pkg__.a", *([asm_header_path] if asm_header_path else [])), | ||
| env={"__PANTS_GO_COMPILE_ACTION_ID": action_id_result.action_id}, | ||
| replace_sandbox_root_in_args=True, |
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 was curious to see how this works, and it's pretty straight forward:
pants/src/python/pants/backend/go/util_rules/sdk.py
Lines 158 to 159 in f61a952
| if request.replace_sandbox_root_in_args: | |
| env[GoSdkRunSetup.SANDBOX_ROOT_ENV] = "1" |
pants/src/python/pants/backend/go/util_rules/sdk.py
Lines 98 to 103 in f61a952
| if [ -n "${GoSdkRunSetup.SANDBOX_ROOT_ENV}" ]; then | |
| export __PANTS_SANDBOX_ROOT__="$sandbox_root" | |
| args=("${{@//__PANTS_SANDBOX_ROOT__/$sandbox_root}}") | |
| set -- "${{args[@]}}" | |
| fi | |
| exec "{goroot.path}/bin/go" "$@" |
| description=f"Generate Go and C files from CGo files ({request.import_path})", | ||
| input_digest=cgo_input_digest, | ||
| output_directories=(obj_dir_path,), | ||
| replace_sandbox_root_in_args=True, |
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.
Interesting. I see this process was partially implemented, and just had a TODO to use -trimpath.
| "-trimpath", | ||
| "__PANTS_SANDBOX_ROOT__", |
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.
Does the other GoSdkProcess in this file need -trimpath as well? (The GoSdkProcess that calls go tool cgo -dynimport in _dynimport(...))
Adds
-trimpathon go compilation to strip sandbox paths from the output binary.It would be great to cherry-pick this as currently builds are not reproducible which impacts cache and downstream.
Fixes #16835