Some changes to build helpers, such as transitioning to lib.extendMkDerivation, changes the indentation of a large block of code, and splitting the reformatting into its own commits helps emphasize the effective changes and make reviewing easier.
Should these reformatting commits be added to .git-blame-ignore-revs to make git blame more informative? If so, should we sort it alphabetically to prevent merge conflicts?
Here are two examples of such transition:
NixOS:master ← ShamrockLee:build-rust-package-finalattrs
opened 12:30PM - 16 Feb 25 UTC
This PR enables `buildRustPackage` to take `finalAttrs: { ... }` with the help o… f `lib.extendMkDerivation` introduced in #234651.
The large diff block is due to indentation changes, while the effective diff is minimal. Commit-by-commit review would be much easier.
I cherry-picked the `uiua` package transition from #354999 to validate the implementation. Other transition examples in #194475 and #354999 seems no longer relevant.
This PR doesn't address the existing overriding issues of the `buildRustPackage` arguments (such as `cargoHash`). They will be addressed in subsequent PRs with the help of `finalAttrs`, just like #225051 and #379602.
## Things done
- Built on platform(s) (Ne rebuilds or clean-ups.)
- [x] x86_64-linux
- [ ] aarch64-linux
- [ ] x86_64-darwin
- [ ] aarch64-darwin
- For non-Linux: Is sandboxing enabled in `nix.conf`? (See [Nix manual](https://nixos.org/manual/nix/stable/command-ref/conf-file.html))
- [ ] `sandbox = relaxed`
- [ ] `sandbox = true`
- [ ] Tested, as applicable:
- [NixOS test(s)](https://nixos.org/manual/nixos/unstable/index.html#sec-nixos-tests) (look inside [nixos/tests](https://github.com/NixOS/nixpkgs/blob/master/nixos/tests))
- and/or [package tests](https://github.com/NixOS/nixpkgs/blob/master/pkgs/README.md#package-tests)
- or, for functions and "core" functionality, tests in [lib/tests](https://github.com/NixOS/nixpkgs/blob/master/lib/tests) or [pkgs/test](https://github.com/NixOS/nixpkgs/blob/master/pkgs/test)
- made sure NixOS tests are [linked](https://github.com/NixOS/nixpkgs/blob/master/pkgs/README.md#linking-nixos-module-tests-to-a-package) to the relevant packages
- [x] 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](https://github.com/Mic92/nixpkgs-review#usage)
- [ ] Tested basic functionality of all binary files (usually in `./result/bin/`)
- [25.05 Release Notes](https://github.com/NixOS/nixpkgs/blob/master/nixos/doc/manual/release-notes/rl-2505.section.md) (or backporting [24.11](https://github.com/NixOS/nixpkgs/blob/master/nixos/doc/manual/release-notes/rl-2411.section.md) and [25.05](https://github.com/NixOS/nixpkgs/blob/master/nixos/doc/manual/release-notes/rl-2505.section.md) 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
- [x] Fits [CONTRIBUTING.md](https://github.com/NixOS/nixpkgs/blob/master/CONTRIBUTING.md).
<!--
To help with the large amounts of pull requests, we would appreciate your
reviews of other pull requests, especially simple package updates. Just leave a
comment describing what you have tested in the relevant package/service.
Reviewing helps to reduce the average time-to-merge for everyone.
Thanks a lot if you do!
List of open PRs: https://github.com/NixOS/nixpkgs/pulls
Reviewing guidelines: https://github.com/NixOS/nixpkgs/blob/master/pkgs/README.md#reviewing-contributions
-->
---
Add a :+1: [reaction] to [pull requests you find important].
[reaction]: https://github.blog/2016-03-10-add-reactions-to-pull-requests-issues-and-comments/
[pull requests you find important]: https://github.com/NixOS/nixpkgs/pulls?q=is%3Aopen+sort%3Areactions-%2B1-desc
NixOS:master ← JohnRTitor:buildNpmPackage-finalAttrsv2
opened 10:46AM - 15 Mar 25 UTC
Inspired from https://github.com/NixOS/nixpkgs/pull/382550.
Nixfmt is done on… the 2nd commit to allow easier commit by commit review.
1 Like
“Hide whitespace” should suffice for reviewing IMO?
How amazing! Thank you for sharing with me!
github
Splitting the reformatting still benefits Git workflows like rebasing and merging during PR reviewing process or during the staging flow.
1 Like
Yes, they should be added. I’m not sure it is worth it to keep the file alphabetically sorted. Very few PRs touch it.
Just glanced through the Nixpkgs Reference Manual. As build helpers are adapting lib.extendMkDerivation, we could expect ~30 new entries from language- and framework-specific build helper transition PRs.
1 Like