removeReferencesToVendoredSources: sign aarch64-darwin binaries

[simonzkl]

Motivation

Fixes #417

On aarch64-darwin we need to sign binaries. Usually this is done automatically, but it seems the signature is invalidated when we patch binaries using removeReferencesToVendoredSources.

Checklist

• added tests to verify new behavior
• added an example template or updated an existing one
• updated docs/API.md (or general documentation) with changes
• updated CHANGELOG.md

[ipetkov]

Hi @simonzkl thanks for the PR and the test case!

I think there's a simpler way of solving this problem: removing references later in the build (after signing has had a chance to run)! It looks like nixpkgs will perform signing as a fixup hook. Currently references are removed as a postInstall step but if we changed that to a post fixup hook it might work out?

[simonzkl]

I could be wrong but my understanding is that signing runs as part of the clang build when you run cargo build, not as a hook of the buildPackage derivation. So I'm not sure if we actually have any control over this.

[simonzkl]

@ipetkov actually you're right. Moving removeReferencesToVendoredSources to fixupHooks does solve the issue. I updated the PR.

[ipetkov]

Looking good, thanks again for the fix!

[ipetkov]

Uh oh, we've got a regression! Actually it looks like the hook isn't running at all, maybe we need to specify postFixupHooks?

[simonzkl]

Ah sorry, it seems I fixed the issue by not running the hook at all. Changing it to preFixupHooks/postFixupHooks fixes that, but reintroduces the issue. I think my initial understanding that the signing part runs as part of cargo build was correct so this is not going to work. If you look at remove-references-to in nixpkgs, they manually run the signing hook, so I think we need to do the same.

[simonzkl]

I reverted back to the old approach, but I'm happy to do it differently if there's a better idea. Should we revert postFixupHooks back to postInstallHooks? Technically this is closer to a fixup hook, but it is a breaking change I suppose.

[ipetkov]

Ah I apologize for sending you down the wrong path, I had mistakenly assumed the signing was always being done by the stdenv and we were somehow clobbering it, when in reality we have to explicitly sign because we patch the binary afterwards?

Anyway this seems like the appropriate path (at least it fixes the issues even if I'm still fuzzy on the details)

Should we revert postFixupHooks back to postInstallHooks? Technically this is closer to a fixup hook, but it is a breaking change I suppose.

Yes let's revert this (also yay no longer a breaking change!) we can circle back to the change log when this is ready to merge

[ipetkov]

Gonna restart the workflow once the darwin examples cache (looks like we're stuck compiling gcc atm): https://github.com/ipetkov/crane/actions/runs/6525453459/job/17718153027