Skip to content

perfect_dark: un-vendor gzip; refactor derivation builder; add updateScript#440718

Merged
SigmaSquadron merged 3 commits intomasterfrom
unknown repository
Sep 7, 2025
Merged

perfect_dark: un-vendor gzip; refactor derivation builder; add updateScript#440718
SigmaSquadron merged 3 commits intomasterfrom
unknown repository

Conversation

@ghost
Copy link

@ghost ghost commented Sep 6, 2025

As a follow-up to #306767, this PR improves upon the derivation further:

  • Un-vendors gzip in the source output which allows for dropping the lib.licenses.gpl3Plus attribute from meta.licenses
  • Misc improvements to the builder.
  • Adds a passthru.updateScript that works with the modified fetcher.

No obvious regressions detected when running or building perfect dark.

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

Add a 👍 reaction to pull requests you find important.

Jasi added 3 commits September 6, 2025 13:23
- Use sdl2-compat attribute instead of SDL2
- Remove `hardeningEnable` attribute
- Limit `patchShebangs` to `tools/assetmgr`
@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Sep 6, 2025

preConfigure = ''
patchShebangs --build .
patchShebangs --build tools/assetmgr
Copy link
Contributor

Choose a reason for hiding this comment

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

why specify this path? doesn't this makes the process less robust to future changes?

Copy link
Author

Choose a reason for hiding this comment

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

We generally should only patch files which need to be patched. I'd rather the build fail because of an unpatched file that we can review and add than have a new patched file execute without us knowing when updating.

Copy link
Author

Choose a reason for hiding this comment

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

But I can drop this change if it's really not worth it, we can see what @SigmaSquadron says.

Copy link
Contributor

@PaulGrandperrin PaulGrandperrin Sep 6, 2025

Choose a reason for hiding this comment

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

I think in this particular case, it's safe to assume that any script is either part of the build process (and will need the patching) or will not be used at runtime (because we install only one binary in /bin/).

In any case, it's not important.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @normalcea here. It's best if we have absolute control over which files in the source are executable.

@SigmaSquadron SigmaSquadron merged commit ff43694 into NixOS:master Sep 7, 2025
32 of 35 checks passed
@ghost ghost deleted the perfect-dark-unvendor branch September 7, 2025 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants