-
Notifications
You must be signed in to change notification settings - Fork 94
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
codesign binaries on osx-arm64 #257
Conversation
This has been tested locally but as there is no Apple Silicon Github runner yet, we won't be able to test it in CI. |
subprocess.run( | ||
["/usr/bin/codesign", "-s", "-", "-f", path], capture_output=True | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the output be logged if the command failed? Something like:
subprocess.run( | |
["/usr/bin/codesign", "-s", "-", "-f", path], capture_output=True | |
) | |
process = subprocess.run( | |
["/usr/bin/codesign", "-s", "-", "-f", path], capture_output=True, text=True, | |
) | |
if process.returncode: | |
log.debug("codesign failed for '%s'.\nstdout:\n%s\nstderr:\n%s", path, process.stdout, process.stderr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm keeping the same behaviour here as we do in conda: https://github.com/conda/conda/blob/a3648d7b58175fd505b3176e45e3485c6d2ea571/conda/core/portability.py#L87
I can add that logging but it will probably never pop to the user with being on debug level.
Co-authored-by: jaimergp <jaimergp@users.noreply.github.com>
@conda/constructor This is ready for review. |
@dbast Can you have a look at this again? |
Fixes #238
Description
Checklist - did you ...
news
directory (using the template) for the next release's release notes?