-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
PR: Fix Codesigning for the Installers and Run Workflow on Schedule #21334
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mrclary
force-pushed
the
codesign
branch
3 times, most recently
from
September 13, 2023 21:42
d13be2f
to
a9bd9fa
Compare
mrclary
force-pushed
the
codesign
branch
3 times, most recently
from
September 13, 2023 22:40
6a8d376
to
cb22852
Compare
When ssh is selected in a workflow-dispatch, this will open the ssh session at the beginning of the workflow but allow the workflow to continue; no need for an error to occur.
This seems to correct the issue where codesign would hang while replacing signature for conda.exe. I suspect that a gui prompt is launched, waiting for user input and giving the appearance of hanging. I think this may have occurred because set-key-partition-list may have registered the wrong codesign and that is why the incorrect codesign must be moved before set-key-partition-list is called.
This also seems to fix the issue of hanging during codesigning conda.exe. I suppose the hypothesis here is that the keychain locks before the signing event and a gui likely appears prompting the user for access. The fact that both this and the location of set-key-partition-list appear to resolve the issue _independently_ (I tried each commit in isolation) is strange. I could believe the keychain locks before signing, but then that may be intermittent: sometimes it locks sooner than others, but shouldn't it be required that set-key-partition-list be set after the incorrect codesign is moved? In other words, why do these work independently? Shouldn't they each be necessary but insufficient? Also, I still don't understand how napari avoids this issue. They do not use set-key-partition-list at all, just the timeout.
build-subrepos workflow should also run if the workflow file is updated. Only run installer workflow on pull request if relevant files are changed. Run installer workflow daily at 06:30 UTC. Use subrepos in installer on scheduled events.
Do not run on pull requests, rather have installers-conda.yml run on limited PRs and call build-subrepos.yml. build-subrepos.yml will run on push to master if relevant files have changed, providing caches for any PR based off of master. PRs that trigger the installer workflow will trigger build-subrepos from within the installer workflow, ensuring that it is completed before the build installers step. If caches already exist and the subrepos do not need to be rebuilt, build-subrepos should complete rapidly.
dalthviz
approved these changes
Sep 14, 2023
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.
Thanks @mrclary !
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of Changes
set-key-partition-list
after correcting thecodesign
executable pathThe first two changes listed above seem to correct the issue where
codesign
would hang while replacing the signature forconda.exe
, independently.I suspect that a gui prompt is launched, waiting for user input and giving the appearance of hanging.
I think this may occur under two conditions:
set-key-partition-list
may have registered.../spy-inst/bin/codesign
instead of/usr/bin/codesign
, resulting in a gui prompt requesting user input (password).The fact that both the keychain timeout and the location of
set-key-partition-list
appear to resolve the issue independently is strange (I tried each commit in isolation).I would think that they each should be necessary but insufficient. 🤷🏼
Also, I still don't understand how napari/packaging avoids this issue.
They do not use
set-key-partition-list
at all, just the timeout. In my investigations, the absence ofset-key-partition-list
does not work.Issue(s) Resolved
Fixes #21302