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

Don't leave temporary directory around when building extensions to improve build reproducibility #4610

Merged

Conversation

baloo
Copy link
Contributor

@baloo baloo commented May 19, 2021

What was the end-user or developer problem that led to this PR?

rubygems was leaving empty directories like:

lib/ruby/gems/2.7.0/gems/racc-1.5.2/ext/racc/cparse/.gem.20210503-6-csuqdb

Because those files contains the date, this hinders build-reproducibility efforts.

This is related to the issue NixOS/nixpkgs#123718 over at NixOS.

What is your fix for the problem, implemented in this PR?

tmp_dest is rewritten which makes it impossible to collect.

fixup: b97ec2a use relative path for siteconf in case of space and update relevant tests

Make sure the following tasks are checked

@welcome
Copy link

welcome bot commented May 19, 2021

Thanks for opening a pull request and helping make RubyGems and Bundler better! Someone from the RubyGems team will take a look at your pull request shortly and leave any feedback. Please make sure that your pull request has tests for any changes or added functionality.

We use GitHub Actions to test and make sure your change works functionally and uses acceptable conventions, you can review the current progress of GitHub Actions in the PR status window below.

If you have any questions or concerns that you wish to ask, feel free to leave a comment in this PR or join our #rubygems or #bundler channel on Slack.

For more information about contributing to the RubyGems project feel free to review our CONTRIBUTING guide

@deivid-rodriguez
Copy link
Member

Thank you! Could you add a test to cover the fix?

@baloo
Copy link
Contributor Author

baloo commented May 20, 2021

I can't run tests easily on my setup, the bundler in rubygems conflicts with bundler on my operating system.

Feel free to contribute tests if it works for you.

@deivid-rodriguez deivid-rodriguez marked this pull request as draft May 20, 2021 16:13
@deivid-rodriguez
Copy link
Member

Sure, I'll leave this PR open for anyone who would like to wrap it up with a test.

Because the temporary directory used is named like '.gem.YYMMDD-RANDOM'
it causes reproduciblity problems.
tmp_dest is rewritten which makes it impossible to collect.

fixup: b97ec2a use relative path for siteconf in case of space and update relevant tests

Signed-off-by: Arthur Gautier <baloo@superbaloo.net>
@baloo baloo force-pushed the baloo/reproducibility-tmpdir branch from 71057bb to 2c2ffde Compare May 20, 2021 16:57
@baloo baloo marked this pull request as ready for review May 20, 2021 16:57
@baloo
Copy link
Contributor Author

baloo commented May 20, 2021

I hacked together a test-runner.

@deivid-rodriguez
Copy link
Member

Awesome, thanks for your work!

Copy link
Member

@deivid-rodriguez deivid-rodriguez left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@deivid-rodriguez deivid-rodriguez changed the title Ensure temporary directory is cleaned up Don't leave temporary directory around when building extensions to improve build reproducibility May 21, 2021
@deivid-rodriguez deivid-rodriguez merged commit 2cbd904 into rubygems:master May 22, 2021
74 checks passed
deivid-rodriguez added a commit that referenced this pull request May 25, 2021
Don't leave temporary directory around when building extensions to improve build reproducibility

(cherry picked from commit 2cbd904)
deivid-rodriguez added a commit that referenced this pull request May 25, 2021
Don't leave temporary directory around when building extensions to improve build reproducibility

(cherry picked from commit 2cbd904)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants