Test that the stdlib rule does not insert build ids in archives #1454
Conversation
|
cc @steeve |
|
|
||
| def _stdlib_files_impl(ctx): | ||
| files = ctx.attr._stdlib[GoSource].stdlib.files | ||
| runfiles = ctx.runfiles(files = files) |
There was a problem hiding this comment.
ah, that's what I missed.
wouldn't it be simpler to add it to the //:stdlib rule ?
There was a problem hiding this comment.
I'd rather keep this in the GoStdLib provider for now, but I don't have a strong opinion about it, since this isn't a public interface. Fewer things to keep in sync though.
tests/core/stdlib/buildid_test.go
Outdated
| } | ||
| return filepath.Walk(path, visit) | ||
| } | ||
| if filepath.Base(path) == "fmt.a" { |
There was a problem hiding this comment.
unfortunately fmt is not a good candidate because pure go packages where producing the same build ids. (even though we're checking that they are absent I know)
that said, we should check 3 types of packages:
- the ones pure go (
fmtlooks like one) - the ones with asm (
crypto/*are in that case I think) - the ones with cgo (
os/usercomes to mind)
You can find the list at #1391 (comment), perhaps check fmt and the ones from the list ?
There was a problem hiding this comment.
it's likely it'll be the same but for the sake of correctness and future proofing...
There was a problem hiding this comment.
also, we should probably check for the sandbox path by binary search for bazel-sandbox for cgo packages because of f4201a5
but that may out of the scope of this PR though
There was a problem hiding this comment.
Very good advice. I've expanded the test to check fmt.a, crypto/aes.a, and runtime/cgo.a. That should cover those cases.
About searching for bazel-sandbox, tests/legacy/reproducible_binary checks for common reproducibility problems in compiled binaries, but I wrote it before bazel-sandbox appeared it paths, so it should be updated (in another PR).
tests/core/stdlib/buildid_test.go
Outdated
| } | ||
| return filepath.Walk(path, visit) | ||
| } | ||
| if filepath.Base(path) == "fmt.a" { |
There was a problem hiding this comment.
also, we should probably check for the sandbox path by binary search for bazel-sandbox for cgo packages because of f4201a5
but that may out of the scope of this PR though
Related #1440