Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Force codesign to replace existing signatures #6975

Closed
wants to merge 2 commits into from

Conversation

voodoos
Copy link
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
Copy link
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
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
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?

@@ -52,8 +52,22 @@ type conf =
}

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
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
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
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
Collaborator

anmonteiro commented Feb 17, 2023

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

@emillon
Copy link
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