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

Hardcode paths to Apple's system tools #543

Merged
merged 2 commits into from
Aug 15, 2022

Conversation

jaimergp
Copy link
Contributor

@jaimergp jaimergp commented Aug 12, 2022

Description

We ran into this issue in the napari/packaging#11. Apparently some package in the dependency tree includes a codesign binary too, with a different CLI. This can get in the way of our signing process, resulting in a 109 error.

This happened because we let subprocess pick the first codesign in PATH. The fix is to hardcode the location to /usr/bin/codesign, as conda itself does.

I also applied the same patch to productbuild and productsign, just in case.

Checklist - did you ...

  • Add a file to the news directory (using the template) for the next release's release notes?
  • Add / update necessary tests?
  • Add / update outdated documentation?

@jaimergp jaimergp requested a review from a team as a code owner August 12, 2022 09:41
@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Aug 12, 2022
isuruf
isuruf previously approved these changes Aug 12, 2022
constructor/osxpkg.py Outdated Show resolved Hide resolved
goanpeca
goanpeca previously approved these changes Aug 12, 2022
@goanpeca
Copy link
Contributor

Maybe we can use a variable for the PREFIX.

APPLE_TOOLS_PREFIX + "/codesign"

or something 🤷🏼

hoechenberger
hoechenberger previously approved these changes Aug 12, 2022
Copy link
Contributor

@hoechenberger hoechenberger left a comment

Choose a reason for hiding this comment

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

The changes look good to me.

@jaimergp jaimergp dismissed stale reviews from hoechenberger, goanpeca, and isuruf via 7b718d0 August 15, 2022 07:40
@jaimergp
Copy link
Contributor Author

Thanks @hoechenberger - this automated review dismissal is a bit inconvenient :(

@hoechenberger
Copy link
Contributor

Yep, can this be disabled?

We could also set up automerge for when reviews are in and CI passes. WDYT?

@jaimergp
Copy link
Contributor Author

Agree, that'd be nice. I think we need to ask someone with more permissions though. Let's create an issue.

@jaimergp jaimergp merged commit 7e76a57 into conda:main Aug 15, 2022
@github-actions github-actions bot added the locked [bot] locked due to inactivity label Aug 16, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla-signed [bot] added once the contributor has signed the CLA locked [bot] locked due to inactivity
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants