Skip to content

buildPgrxExtension: restructure with lib.extendMkDerivation#394089

Merged
wolfgangwalther merged 3 commits intoNixOS:masterfrom
diogotcorreia:buildpgrxextension-extendmkderivation
Apr 13, 2025
Merged

buildPgrxExtension: restructure with lib.extendMkDerivation#394089
wolfgangwalther merged 3 commits intoNixOS:masterfrom
diogotcorreia:buildpgrxextension-extendmkderivation

Conversation

@diogotcorreia
Copy link
Member

@diogotcorreia diogotcorreia commented Mar 28, 2025

This PR enables buildPgrxExtension to take finalAttrs: { ... } with the help of lib.extendMkDerivation introduced in #234651.

I migrated the pgvecto-rs extension to validate the implementation. All cargo-pgrx extensions have been migrated

The PR should yield no rebuilds.

Related discussion: #392710 (comment)
cc @wolfgangwalther

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 review from Ma27 and thoughtpolice March 28, 2025 19:44
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Mar 28, 2025
@wolfgangwalther
Copy link
Contributor

The large diff block is due to indentation changes, while the effective diff is minimal. Commit-by-commit review is much easier.

If you change GitHub's diff settings to "hide whitespace", the second commit becomes empty. Thus, you don't need to split the first two - reviewing the whole PR with "hide whitespace" works very well.

@diogotcorreia diogotcorreia force-pushed the buildpgrxextension-extendmkderivation branch from c474d09 to 6b9ebe5 Compare March 29, 2025 22:03
@diogotcorreia
Copy link
Member Author

@wolfgangwalther TIL, thanks for telling me, squashed the two commits
I had copied the idea from a similar PR for buildRustPackage 😅

@wolfgangwalther
Copy link
Contributor

I plan to look at this, but I am not familiar with lib.extendMkDerivation, yet. So I will have to look into that first - my plan is, to refactor postgresqlBuildExtension to use it as well, and then review here with my gained knowledge. Once I find the time..!

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 2, 2025
Copy link
Contributor

@wolfgangwalther wolfgangwalther left a comment

Choose a reason for hiding this comment

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

Let's refactor the remaining 3-4 pgrx packages to use finalAttrs, too - shouldn't be too much effort.

In #398194, I refactored all those with postgresqlBuildExtension, combined with this we'd have all of them :)

@diogotcorreia diogotcorreia force-pushed the buildpgrxextension-extendmkderivation branch from 6b9ebe5 to b50641e Compare April 13, 2025 12:15
@diogotcorreia
Copy link
Member Author

Done :)
And fixed merge conflicts

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 13, 2025
@wolfgangwalther wolfgangwalther merged commit be15e0c into NixOS:master Apr 13, 2025
41 of 43 checks passed
@diogotcorreia diogotcorreia deleted the buildpgrxextension-extendmkderivation branch April 13, 2025 13:00
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: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants