Fix Flutter's reference to the arch binary on macOS

[madsmtm]

Description of changes

darwin.adv_cmds contains no arch binary, the correct binary is instead provided via. stdenv.hostPlatform.darwinArch, for an example of how this is done elsewhere see moarvm. This mistake was introduced in #286750 (comment), CC @reckenrode.

I'm with limited internet today, so have done this through the GitHub UI and haven't tested this locally (as I'd have to clone the nixpkgs repo). Feel free to squash the changes with a proper Git commit message, I think I failed to follow the guidelines on that.

Pinging package maintainers @babariviere @ericdallo @FlafyDev @hacker1024.

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/)
• 24.05 Release Notes (or backporting 23.05 and 23.11 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.

[reckenrode]

Sorry about causing the issue. I don’t know how that worked for me when the binary clearly isn’t there.

[wegank]

Wait, how can this work? stdenv.hostPlatform.darwinArch is just a string that evaluates to either "x86_64" or "arm64", i.e. the result of $(/usr/bin/arch), not the binary itself.

[madsmtm]

Wait, how can this work? stdenv.hostPlatform.darwinArch is just a string that evaluates to either "x86_64" or "arm64", i.e. the result of $(/usr/bin/arch), not the binary itself.

Yeah, you're completely correct, I overlooked that the other places where this is used, the replaced command is `/usr/bin/arch`, with the backticks, which makes the shell it's executed in execute the command and replace with the result.

I'm not sure what the best way to fix this is then? Is there some other place where Nix provides an arch binary? How much does it matter that Flutter is using the system path for that binary, and would a better approach be to change this to refer to just arch taken from the current PATH instead?

[wegank]

Is there some other place where Nix provides an arch binary?

I am not aware of any. It should theoretically reside in darwin.system_cmds, but that is still at OS X 10.11 in Nixpkgs, so at least too old to have an aarch64-darwin build. Packaging a separate unixtools.arch, however, is probably not difficult, since its source is a single file written in C.

[reckenrode]

Instead of packaging a separate one, can we fix the system_cmds derivation to provide the missing command? At worst, it ignores the Xcode project and builds the command directly.

I plan to update the _cmds derivations eventually (once I can recover my Meson build files but also after the SDK and other stuff I need to do), but that should be an easy fix.

[wegank]

There's a trivial fix, without Rosetta support (apple-oss-distributions/system_cmds@8579bd4), though.

diff --git a/arch.tproj/arch.c b/arch.tproj/arch.c
index 4152bf0..c2c24b5 100644
--- a/arch.tproj/arch.c
+++ b/arch.tproj/arch.c
@@ -75,9 +75,10 @@ static const CPUTypes knownArchs[] = {
 #if defined(__i386__) || defined(__x86_64__)
     {"i386", CPU_TYPE_I386},
     {"x86_64", CPU_TYPE_X86_64},
+#elif defined(__arm64__)
+    {"arm64", CPU_TYPE_ARM64},
 #elif defined(__arm__)
     {"arm", CPU_TYPE_ARM},
-#endif
 #else
 #error "Unsupported architecture"
 #endif
@@ -99,10 +100,12 @@ extern char **environ;
 #if defined(__i386__) || defined(__x86_64__)
 #define NATIVE_32	"i386"
 #define NATIVE_64	"x86_64"
+#elif defined(__arm64__)
+#define NATIVE_64	"arm64"
+#define NATIVE_32	NULL
 #elif defined(__arm__)
 #define NATIVE_32	"arm"
 #define NATIVE_64	NULL
-#endif
 #else
 #error "Unsupported architecture"
 #endif

[reckenrode]

I was able to recover my work on updating system_cmds to 970.0.4. I’m going to open an update PR this weekend once I confirm the stdenv bootstrap builds for both Darwin platforms. I was hoping to wait until after the newer SDKs were added, but it would be better to land it now instead of adding workarounds that are fixed with an update.

[reckenrode]

#296321 is the PR to update system_cmds. It builds arch on aarch64-darwin (and x86_64-darwin if it’s overridden to use a newer SDK).

[madsmtm]

I've gotten the project cloned locally, but just running nix-build -A darwin.system_cmds doesn't seem to provide the arch binary, so this still doesn't work.

Would you mind a lot if I just changed this PR to use /usr/bin/arch, until this is resolved fully?

[wegank]

I've gotten the project cloned locally, but just running nix-build -A darwin.system_cmds doesn't seem to provide the arch binary, so this still doesn't work.

That PR was landed on staging, it'll take weeks for it to land on master.

Would you mind a lot if I just changed this PR to use /usr/bin/arch, until this is resolved fully?

I'm OK with that.

[madsmtm]

That PR was landed on staging, it'll take weeks for it to land on master.

Ah, makes sense.

I'm OK with that.

Cool, I've done that then.