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

Remove uses of gzip that might capture build time #21306

Merged
merged 3 commits into from
Dec 20, 2016

Conversation

joachifm
Copy link
Contributor

More work on extracting non-controversial stuff from #2281

No idea if this is worthwhile but for completeness ... if this is something we want I have a few more queued up (some that also fix determinism issues; none of the ones I've included in the initial batch seem make a difference ...)

gzip is originally called as 'gzip -9 -c'

This is a port of
NixOS@a8e7ddd

Note that it does not seem to make a difference to `nix-build --check`.
Note that it does not seem to make a difference to `nix-build --check`.
Note that it does not seem to make a difference to `nix-build --check`.
@mention-bot
Copy link

@joachifm, thanks for your PR! By analyzing the history of the files in this pull request, we identified @obadz, @zimbatm and @edolstra to be potential reviewers.

@zimbatm
Copy link
Member

zimbatm commented Dec 20, 2016

Is this solving an issue we have? If it's really an issue it seems fragile to depend on patching all the invocation of gzip, it might be easier to patch gzip to remove that feature and provide that gzip flavor at build time.

@joachifm
Copy link
Contributor Author

Some invocations of gzip do seem to cause non-reproducible outputs, so yes, in some cases it does solve a problem. I also thought it might make more sense to do this some other way ...

@dezgeg
Copy link
Contributor

dezgeg commented Dec 20, 2016

We could instead set the GZIP environment variable in stdenv to add the n flag to all invocations.

@edolstra edolstra merged commit edd5bab into NixOS:master Dec 20, 2016
@edolstra
Copy link
Member

Unfortunately, the gzip manpage says about GZIP that "This feature will be removed in a future release of gzip".

@7c6f434c
Copy link
Member

7c6f434c commented Dec 20, 2016 via email

@joachifm
Copy link
Contributor Author

@7c6f434c the determinism PR contains a fake date utility, I've been tinkering with wrappers along the same vein, so I think that'd be a good route to go (I was a little surprised the current PR was merged, tbh :)). That said, the question is really if somebody will commit to writing, testing, and getting a better solution merged ... If we have to wait 2 more years for a perfect solution to materialize I'd rather go ahead with patching stuff.

It also occurs to me that many uses of gzip are to compress man pages, which I expect would otherwise be handled properly by the compress-manpages phase hook ...

@joachifm joachifm deleted the gzip-9n branch December 20, 2016 22:11
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.

None yet

6 participants