makeBinaryWrapper change, 70586 affected packages

Birdee · April 11, 2025, 2:33am

Edit: This PR has been closed, and replaced by a new one which, rather than changing an existing flag, just adds an --add-flag so that at least makeBinaryWrapper can accept flags with spaces in some way.

Old post:

This affects a massive portion of nixpkgs and I figured it deserves a post here.

(when I say massive, I mean Package: added 0, removed 0, changed 70586, Rebuild: linux 60153, darwin 53012 )

This pr needs review.

github.com/NixOS/nixpkgs

makeBinaryWrapper: --add-flags arguments now properly escaped

NixOS:staging ← BirdeeHub:fix_makeBinaryWrapper_add-flags

opened 07:26AM - 10 Apr 25 UTC

BirdeeHub

+148 -21

arguments passed to --add-flags were ALWAYS split by spaces regardless of how yo…u passed them. This PR also applies to --append-flags as well This splits them correctly as if they were shell arguments. Addresses https://github.com/NixOS/nixpkgs/issues/330471 --add-flags is useless for makeBinaryWrapper for a ton of programs because of the inability to accept arguments with spaces regardless of escaping. I also hope to introduce a --run in a future PR (Edit: although, unsure if I will be able to in a way that makes sense to add) such that makeBinaryWrapper becomes truly a drop in replacement for makeWrapper, but for now, this should be fixed. ## Things done - Built on platform(s) - [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` - [x] 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 - [ ] 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) - [x] 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 - [ ] Fits [CONTRIBUTING.md](https://github.com/NixOS/nixpkgs/blob/master/CONTRIBUTING.md).  --- 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

–add-flags and --append-flags used to not accept arguments with spaces properly, it ALWAYS split on them, regardless of quoting.

This fixes that to be in line with pkgs.makeWrapper

Also:

It needs some mac users to run the tests for makeBinaryWrapper against the PR commit hash

For that you may use

nix build -v github:nixos/nixpkgs/5368af0a086372d4fd52bd015db5749274469a5e#tests.makeBinaryWrapper

And then let us know how that goes.