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

Add a simple reproducibility test command. #13689

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

jpakkane
Copy link
Member

No description provided.

@jpakkane jpakkane force-pushed the reprotest branch 2 times, most recently from 98c0cb5 to 9ba04e5 Compare September 19, 2024 08:29
Copy link

@fortysixandtwo fortysixandtwo left a comment

Choose a reason for hiding this comment

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

pretty cool, I just gave it a little spin.

Currently ccache in my PATH is not detected in which case it should also abort.

🚀

## Simple tool to test build reproducibility

Meson now ships with a command for testing whether your project can be
[built reprodicibly](https://reproducible-builds.org/). It can be used

Choose a reason for hiding this comment

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

typo: s/reprodicibly/reproducibly

Comment on lines +15 to +16
# Note: when adding arguments, please also add them to the completion
# scripts in $MESONSRC/data/shell-completions/

Choose a reason for hiding this comment

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

Have I missed these?


Builds the current project with its default settings.

meson reprotest --intermediates -- --buildtype=debugoptimized

Choose a reason for hiding this comment

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

typo: it should be --intermediaries

self.cleanup()
return len(self.issues)

def check_ccache(self) -> None:

Choose a reason for hiding this comment

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

I have /usr/lib/ccache in my PATH
and that makes me currently not abort with sys.exit(1) even though I should.

> $ ccache -z 
Statistics zeroed

> $ time ~/git/meson/meson.py reprotest --intermediaries 
The Meson build system
Version: 1.5.99
Source dir: /home/fortysixandtwo/git/calls
Build dir: /home/fortysixandtwo/git/calls/buildrepro
Build type: native build

[...]

[422/422] Linking target tests/application
No differences detected.
~/git/meson/meson.py reprotest --intermediaries  94,72s user 21,80s system 491% cpu 23,705 total

> $ ccache -s
Cacheable calls:    550 / 633 (86.89%)
  Hits:             550 / 550 (100.0%)
    Direct:         548 / 550 (99.64%)
    Preprocessed:     2 / 550 ( 0.36%)
  Misses:             0 / 550 ( 0.00%)
Uncacheable calls:   83 / 633 (13.11%)
Local storage:
  Cache size (GiB): 1.3 / 5.0 (25.31%)
  Hits:             550 / 550 (100.0%)
  Misses:             0 / 550 ( 0.00%)

Copy link
Member Author

Choose a reason for hiding this comment

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

Detecting all ways ccache could be injected is tricky, especially since actual compiler detection code is elsewhere and not easily callable. I guess we could check if PATH contains the string ccache, but that might have false positives.

Copy link
Member

Choose a reason for hiding this comment

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

Simplest approach is to just use $CCACHE_DISABLE in the environment to tell ccache that even when run, it should simply forward directly to the compiler without performing any actions itself.

Copy link
Member

Choose a reason for hiding this comment

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

This would also mean we could simply support the ccache users, as their testing would work by telling ccache to bypass the actual cache, instead of just refusing to perform the reproducibility testing at all.

)

class ReproTester:
def __init__(self, options: T.Any):
Copy link
Member

Choose a reason for hiding this comment

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

A Protocol can get rid of the need for Any here, it looks like all it would need is:

if T.TYPE_CHECKING:
    from typing_extensions import Protocol

    class Arguments(Protocol):
         intermediaries: bool
         mesonargs: T.List[str]

Copy link
Member

Choose a reason for hiding this comment

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

Eventually I'd like to get to the point that we don't have Any anywhere in the codebase, my experience is that it basically always hides issues.

Comment on lines +77 to +81
subprocess.check_call(setup_command)
subprocess.check_call(build_command)
self.builddir.rename(self.storagedir)
subprocess.check_call(setup_command)
subprocess.check_call(build_command)
Copy link
Member

Choose a reason for hiding this comment

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

Does it make sense to catch the exceptions that check_call would generate, print a more helpful message, and return a non 1 return code?

Comment on lines +61 to +66
for evar in ('CC', 'CXX'):
evalue = os.environ.get(evar, '')
if 'ccache' in evalue:
print(f'Environment variable {evar} set to value "{evalue}".')
print('This implies using a compiler cache, which is incompatible with reproducible builds.')
sys.exit(1)
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't take into account the approach of linking ln -s ccache gcc. If that's a symlink I guess we can resolve that, though I don't know what to do if it's a hardlink.

Maybe a a better solution is to set CCACHE_DISABLE=1 before calling Meson, which causes ccache to skip reading and writing to the cache?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants