benchexec: 3.21 → 3.27

[lorenzleutgeb]

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:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• 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.

[Adda0]

Suggestion: I think this should be replace-fail to let us know when this can be removed if the project moves to a different build system, updates this line themselves, etc. Is there a reason you used "only" replace-warn?

Suggested change

	--replace-warn 'setuptools ==' 'setuptools >='
	--replace-fail 'setuptools ==' 'setuptools >='

[lorenzleutgeb]

If they change the version requirement and/or build system, and BenchExec can be built even with the replacement not being successful, I'd rather get a warning, rather than a failure. In some sense it would be an unnecessary failure. The build would just fail because some string cannot be replaced in a file, not because BenchExec actually failed to build. If the replacement is crucial, and without it the build actually fails, we find out very soon after, and if even that is not enough, there's a version test that is relatively cheap also, which would catch actual build failure just another bit later in the process.

So, I wanted to be lax, because there are more, better, verification mechanisms in place.

However, if this turns out to block the PR, I am happy to go with failure.

[Adda0]

It is a question between convenience and precision, I would say. It is convenient to have it as -warn, and it will force us to fix the package immediately when it fails to perform the replacement.

Your arguments are good enough for me. Not sure what the generally preferred option for nixpkgs would be in this case. We can leave this conversation open and someone might chime in.

[nbraud]

I would also lean towards --replace-fail, if only because otherwise this would likely remain in the package long after it stops being necessary.

[Adda0]

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 374278

---

x86_64-linux

✅ 2 packages built:

• benchexec
• benchexec.dist

Reviewed points

✅ 14 | 🔴 0

• Package name fits the guidelines.
• Package version fits the guidelines.
• The commit text fits the guidelines.
• Package maintainers are notified.
  • The continuous integration system will make GitHub notify users based on the submitted changes, but it can happen that it misses some of the package maintainers.
• meta field information fits the guidelines and contain the correct information:
  • License can change with version updates, so it should be checked to match the upstream license.
  • If the package has no maintainer, a maintainer must be set. This can be the update submitter or a community member that accepts to take maintainership of the package.
• The code contains no typos.
• The package builds locally on x86_64-linux.
• All executables run on x86_64-linux.
• All executables tested on x86_64-linux.
• All depending packages build.
• Patches have a comment describing either the upstream URL or a reason why the patch wasn't upstreamed.
• Patches that are remotely available are fetched rather than vendored.