fix(signingscript): Bug 2034166 reduce notarization requests#1426
Open
fix(signingscript): Bug 2034166 reduce notarization requests#1426
Conversation
Restructure apple_notarize_stacked to notarize and staple .pkgs before touching .apps, then probe each .app with a staple attempt to see if it was transitively validated by its parent .pkg. Only .apps that fail the probe go through a full notarize/wait/staple fallback. When no .pkg is in the batch, the probe runs a single attempt (no retry) before falling through.
hneiva
force-pushed
the
hneiva/smart-notarize
branch
from
4c1bf58 to
c2ca693
Compare
hneiva
changed the title
Contributor
Author
|
I was able to test this locally with a build from maple:
The important bits:
|
bhearsum
reviewed
Apr 27, 2026
Contributor
bhearsum
left a comment
bhearsum
left a comment
There was a problem hiding this comment.
Given how things are set-up today, and we notarize .app and .pkg files in the same job, this is OK. In an ideal world I think we'd adjust the CI flow to go "sign .app" -> "notarize .app" -> "create .pkg" -> "sign .pkg" -> "notarize .pkg".
That is clearly out of scope for a short term improvement though.
| # .pkgs can be notarized in separate tasks with an app->pkg dependency | ||
| # in CI, so by the time the .app task runs its parent .pkg is already | ||
| # stapled and the probe succeeds on the first try. | ||
| probe_kwargs = STAPLE_PROBE_RETRY_KWARGS if pkg_paths else {"attempts": 1} |
Contributor
There was a problem hiding this comment.
I understand why this is as it is, but this override based on pkg_paths is a code smell. Even for cases where we have both .pkg and .app inputs to notarize, this breaks down if an .app is not contained within a .pkg.
Rather than relying on retries, can we do something a bit more intelligent here? Perhaps we can keep track of which .app packages are in a .pkg and do the right thing based on that instead?
(This issue also goes away if we rework the order things are done in CI, of course.)
Contributor
Author
There was a problem hiding this comment.
Mmmmm that's a good point.
Unfortunately I don't know of an easy way of checking what's inside the pkg without having to unpack the whole thing just to find out which locale is it.
I think the best long-term solution is to:
- merge https://phabricator.services.mozilla.com/D291641 which separates the pkg into its own workflow.
- add the dependency via CI/kind configuration (as stated in the comment above)
- do a staple probe for all .apps
It also begs the question, should we always staple probe before sending for notarization? (even on pkgs) Reruns on chunked tasks are wasteful, since they always submit everything to notarization even if previous attempts have already notarized.
Or maybe this task should be "run-aware"? If it's the first run, don't probe pkgs?
In an ideal world I think we'd adjust the CI flow to go "sign .app" -> "notarize .app" -> "create .pkg" -> "sign .pkg" -> "notarize .pkg"
I think we should avoid 2 notarizations here.
What we can do instead (the patch I liked gets us close to this):
sign .app -> create .pkg -> notarize+staple .pkg -> only staple the .app (which becomes the .dmg)
The .app inside the .pkg doesn't have to be notarized before it's packed inside the pkg.
Contributor
There was a problem hiding this comment.
Unfortunately I don't know of an easy way of checking what's inside the pkg without having to unpack the whole thing just to find out which locale is it.
Fair enough -- and unpacking just for this is probably expensive enough that it may not be worthwhile.
It also begs the question, should we always staple probe before sending for notarization? (even on pkgs) Reruns on chunked tasks are wasteful, since they always submit everything to notarization even if previous attempts have already notarized.
When you say "staple probe", do you mean inspection of local artifacts, or querying the remote server? If it's the latter, I think that's an excellent idea (more on that below).
Or maybe this task should be "run-aware"? If it's the first run, don't probe pkgs?
This is an excellent idea that might be good to take up later/elsewhere. On runs > 0 we could have no, some, or all packages already notarized...and ideally we'd handle that gracefully. We do a similar thing in landoscript where we dump out status when submitting a job, but before polling its status. And when we first start up, we look for status from an earlier run when run_id is > 0.
In the case of notarization, I think there's one or more URLs we'd want to write out and use in a similar fashion?
sign .app -> create .pkg -> notarize+staple .pkg -> only staple the .app (which becomes the .dmg)
The .app inside the .pkg doesn't have to be notarized before it's packed inside the pkg.
Are we sure this doesn't have negative consequences? Presumably the .app still has its notarization status checked (whether through talking to apple, or checking for a stapled ticket)?
Or does it get implicitly notarized when the pkg is submitted? If that's the case, my view is that it's more comprehensible to sign and notarize the .app first, and then use the notarized app to make the pkg. The idea of making the app, then stuffing it into a pkg, then notarizing that, then pulling the app out of it and stapling it afterwards seems awkward to me. It also seems like it risks running into update issues -- if the .app in the pkg and the dmg are not identical, partial mars will fail for one or the other.
I'd be happy to hop on Zoom to hash this out more at some point if that's useful; we might be reaching the limits of what we can hash out here.
In any case, if there's plans to improve and rework this in the near future I'm a little less fussed about what's happening here if it gives us a short term win.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Restructure apple_notarize_stacked to notarize and staple .pkgs before touching .apps, then probe each .app with a staple attempt to see if it was transitively validated by its parent .pkg. Only .apps that fail the probe go through a full notarize/wait/staple fallback. When no .pkg is in the batch, the probe runs a single attempt (no retry) before falling through.