SFT-6948: added "add-secrets" to devshell#637
Conversation
|
Sound, with two minor cargo-cult issues from copy-pasting cosign.nix: But the new add-secrets.nix carries forward both: These were copy-pasted from cosign.nix (where they're correct — cosign actually uses OpenSSL). Here they're dead weight: harmless to the build, but they pull openssl into the closure of add-secrets for no reason and lie about its dependencies. Should be: Or just omit those keys entirely. |
There was a problem hiding this comment.
Blocking bug — Darwin build will fail
add-secrets.c:65:
buffer = (uint8_t)calloc(1, info.st_size + sizeof(ulong));
ulong is a non-portable typedef. It's defined in Linux's sys/types.h but not in Apple's Darwin headers (Darwin defines u_long, not ulong). And flake.nix:23-28 declares the
package for both Darwin systems:
systems = [
"aarch64-darwin"
"x86_64-darwin"
"aarch64-linux"
"x86_64-linux"
];
So this PR promotes add-secrets into every devshell on every advertised platform, but the source won't compile on the two Darwin platforms. A Foundation dev with an M-series Mac running nix develop will see a build failure on a tool they probably didn't even ask for.
Fix options
Several valid ways out, in order of "least scope":
- Source fix: replace sizeof(ulong) with sizeof(unsigned long) (or sizeof(uint64_t) if the original intent was a fixed-size pad, or sizeof(size_t) if it's a buffer alignment thing). This is a one-character change in add-secrets.c and makes the source portable.
- Platform-gate the package: wrap add-secrets.nix import in a platform check (if pkgs.stdenv.isLinux then ... else ...) so it only builds on Linux. Cleaner than option 1 if add-secrets is only meant for Linux production-line use anyway.
- Remove Darwin from the flake: too aggressive — would break other tools too.
Option 1 is the right answer if Mac users on the team might ever want to run add-secrets manually; option 2 is the right answer if Foundation only provisions devices on Linux.
Other observations
- NIX_CFLAGS_COMPILE = "-Wno-error=int-conversion" is cargo-culted and doesn't appear necessary for add-secrets.c. Drop it rather than mask real future warnings.
- The flake.nix system removal in the cosign import is fine — no need to split it out.
No description provided.