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

Reproducible builds for native compiled binaries #16860

Merged
merged 1 commit into from Mar 13, 2024
Merged

Conversation

Bo98
Copy link
Member

@Bo98 Bo98 commented Mar 8, 2024

This makes Clang, GCC and ld64 produce reproducible builds, for at least simple cases. It tells them to strip out build prefixes, which we randomise on each build.

This also drops support for Xcode < 7.3, which include Clang compilers which don't support this flag. El Capitan users are able to run Xcode up to 8.2.1, so this hopefully regresses nobody.

This doesn't cover __FILE__ substitutions should any build tools pass absolute paths to the compiler. This can be fixed by -ffile-prefix-map but this is only supported on GCC 8, LLVM Clang 10 and Apple Clang 12, and trying to detect these dynamically is awkward at best.

To be clear: this doesn't guarantee the same build output across different compiler/linker versions so we're not getting all bottles from this.

Comment on lines 314 to 315
# Ideally this would be -ffile-prefix-map, but that requires a minimum of GCC 8, LLVM Clang 10 or Apple Clang 12
# and detecting the version dynamcally based on what `HOMEBREW_CC` may have been rewritten to point to is awkward
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could just take a page from a configure script and see if passing the flag doesn't error...

Copy link
Member Author

@Bo98 Bo98 Mar 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably don't want to do that on every cc run and doing it once is tricky because HOMEBREW_CC can and does change during the build (e.g. swift wrapper).

I guess ideally what we'd do is query all compiler versions ahead of time and check the appropriate one within the shim. Not really sure...

Copy link
Member

@carlocab carlocab Mar 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, yea. What cases will doing this for macOS with Xcode >= 12 cause problems?

Copy link
Member Author

@Bo98 Bo98 Mar 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's probably OK for clang on macOS, though it's a bit messier on the GCC side (in particular Linux) where we technically still support GCC 4.9 and later in brew and you can have multiple installed.

@@ -310,6 +311,9 @@ class Cmd
args += path_flags("-isystem", isystem_paths) + path_flags("-I", include_paths)
# Add -nostdinc when building against glibc@2.13 to avoid mixing system and brewed glibc headers.
args << "-nostdinc" if @deps.include?("glibc@2.13")
# Ideally this would be -ffile-prefix-map, but that requires a minimum of GCC 8, LLVM Clang 10 or Apple Clang 12
# and detecting the version dynamcally based on what `HOMEBREW_CC` may have been rewritten to point to is awkward
args << "-fdebug-prefix-map=#{formula_buildpath}=." if formula_buildpath
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reason why we want . here?

Copy link
Member Author

@Bo98 Bo98 Mar 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No real reason - this can be anything we want it to be. Though an empty string doesn't seem to work.

Debian uses .. If we want more info we could add the formula name or something, though you probably know that from the binary name itself.

Smaller paths in theory will reduce binary size too

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @Bo98! One small typo fix needed then good to ship.

Library/Homebrew/shims/super/cc Outdated Show resolved Hide resolved
@Bo98 Bo98 merged commit c8214fd into master Mar 13, 2024
33 checks passed
@Bo98 Bo98 deleted the reproducible-builds branch March 13, 2024 03:55
@github-actions github-actions bot added the outdated PR was locked due to age label Apr 13, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants