Skip to content

Force codesign to replace existing signatures#6975

Closed
voodoos wants to merge 2 commits intoocaml:mainfrom
voodoos:force-codesign
Closed

Force codesign to replace existing signatures#6975
voodoos wants to merge 2 commits intoocaml:mainfrom
voodoos:force-codesign

Conversation

@voodoos
Copy link
Copy Markdown
Collaborator

@voodoos voodoos commented Feb 1, 2023

We add -f to the list of flags passed to codesign. In some cases, the binary already has a signature so the codesign tool from Apple prints some error messages on stderr. We filter out these error messages as they are innocuous. In addition, this ensures that the test suite has the same output on macos and Linux.

Fixes #6265

@emillon emillon self-assigned this Feb 1, 2023
@emillon
Copy link
Copy Markdown
Collaborator

emillon commented Feb 1, 2023

@voodoos I pushed some changes. Can you check that the test suite passes on M1? (the "replacing existing signature" messages should be gone)

@emillon emillon added this to the 3.7.0 milestone Feb 1, 2023
@voodoos
Copy link
Copy Markdown
Collaborator Author

voodoos commented Feb 1, 2023

Indeed, the messages are gone ✅

We add `-f` to the list of flags passed to `codesign`.
In some cases, the binary already has a signature so the `codesign` tool
from Apple prints some error messages on stderr. We filter out these
error messages as they are innocuous. In addition, this ensures that the
test suite has the same output on macos and Linux.

Fixes ocaml#6265

Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
Signed-off-by: Etienne Millon <me@emillon.org>
Co-authored-by: Etienne Millon <me@emillon.org>
@emillon
Copy link
Copy Markdown
Collaborator

emillon commented Feb 1, 2023

@anmonteiro we implemented the fix you suggested. can you try it and let me know if it works for you?

let mac_codesign_hook ~codesign path =
Process.run ~display:!Clflags.display Strict codesign
[ "-s"; "-"; Path.to_string path ]
Temp.with_temp_file ~dir:Path.root ~prefix:"codesign" ~suffix:"stderr"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure it's the right API to call / the right dir to put the tempfile.

@emillon emillon changed the title Force codesign Force codesign to replace existing signatures Feb 1, 2023
Signed-off-by: Etienne Millon <me@emillon.org>
@anmonteiro
Copy link
Copy Markdown
Collaborator

@anmonteiro we implemented the fix you suggested. can you try it and let me know if it works for you?

I don’t actually have access to a m1 machine anymore, and won’t have for at least another 2 weeks. If you don’t mind waiting I could try then.

@emillon
Copy link
Copy Markdown
Collaborator

emillon commented Feb 1, 2023

No problem. Since I think the issue only affects m1+nix users I think that we can move that to 3.8. Thanks!

@emillon emillon modified the milestones: 3.7.0, 3.8.0 Feb 1, 2023
@anmonteiro
Copy link
Copy Markdown
Collaborator

anmonteiro commented Feb 17, 2023

I've now confirmed this works on arm64 (M2) + nix.

@emillon
Copy link
Copy Markdown
Collaborator

emillon commented Apr 20, 2023

Closing in favor of #6975.

@emillon emillon closed this Apr 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nix + macOS codesigning: -f flag needed

3 participants