Skip to content

compress-man-pages: optimize, use multiple cores#406922

Merged
Mic92 merged 1 commit intoNixOS:stagingfrom
DavHau:manpages
May 15, 2025
Merged

compress-man-pages: optimize, use multiple cores#406922
Mic92 merged 1 commit intoNixOS:stagingfrom
DavHau:manpages

Conversation

@DavHau
Copy link
Member

@DavHau DavHau commented May 14, 2025

Decreases the time spent on gzipping man pages.

Decreases the number of processes launched per file from 2 to 1.
Launches multiple processes in parallel via xargs -P.

The behavior of the hook is unchanged.
gzip -f is now needed to retain the behavior of compressing hardlinkgs.
Previously '-f' was not needed because gzip compressed to stdout.

It removes the check checking if gzip failed, because there os no reason it should ever fail.
Even if it fails we probably want to fix the issue instead of silently not gzipping.
This check has been introduced via c06046e.
No comment was given on why it would be necessary.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@nix-owners nix-owners bot requested a review from Ericson2314 May 14, 2025 02:56
@github-actions github-actions bot added 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. labels May 14, 2025
@siraben
Copy link
Member

siraben commented May 14, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 406922 --package automake


x86_64-linux

✅ 1 package built:
  • automake

Decreases the time spent on gzipping man pages.

Decreases the number of processes launched per file from 2 to 1.
Launches multiple processes in parallel via xargs -P.

The behavior of the hook is unchanged.
gzip -f is now needed to retain the behavior of compressing hardlinkgs.
Previously '-f' was not needed because gzip compressed to stdout.

It removes the check checking if gzip failed, because there os no reason it should ever fail.
Even if it fails we probably want to fix the issue instead of silently not gzipping.
This check has been introduced via c06046e.
No comment was given on why it would be necessary.
@DavHau
Copy link
Member Author

DavHau commented May 14, 2025

Pushed another change with further optimization. It now calls gzip directly instead of going through an extra shell.
As discussed with @Mic92, this should not affect the behavior (see commit message).

@Mic92
Copy link
Member

Mic92 commented May 15, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 406922 --package automake


x86_64-linux

✅ 1 package built:
  • automake

@Mic92 Mic92 merged commit c5252e1 into NixOS:staging May 15, 2025
26 of 27 checks passed
raboof added a commit that referenced this pull request Aug 24, 2025
without `-n`, gzip leaks the file timestamp into the compressed file,
which is likely to leak the build timestamp into the output.

This fixes #434930, a regression introduced in c5252e1 / #406922
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants