-
Notifications
You must be signed in to change notification settings - Fork 11
Fix signed build test issues #178
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
Conversation
zooba
commented
Oct 7, 2025
- disables automatic updating while running tests
- adds timeouts for test stages (in case they block endlessly)
- ensures that test signing overrules real signing
- removes use of stored secret tokens
Strictly speaking, most of these changes weren't necessary, but they're all general goodness that I've been meaning to do anyway (and at least I proved that they weren't necessary by trying them) |
$p | ||
Set-AppxPackageAutoUpdateSettings $p.PackageFamilyName -CheckOnLaunch $false | ||
Set-AppxPackageAutoUpdateSettings $p.PackageFamilyName -ShowPrompt $false | ||
Set-AppxPackageAutoUpdateSettings $p.PackageFamilyName -PauseUpdates -HoursToPause 1 |
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.
👍🏻
- ${{ elseif eq(parameters.Sign, 'true') }}: | ||
- group: CPythonSign |
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.
Why the order change?
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.
Because then if you select both TestSign and Sign, you'll get TestSign. Previously, you'd get both (I also changed if
to elseif
), which ought to have caused an error (multiple defined variables), but probably just caused confusion.
scriptType: 'ps' | ||
scriptLocation: 'inlineScript' | ||
inlineScript: | | ||
"##vso[task.setvariable variable=AZURE_CLIENT_ID;issecret=true]${env:servicePrincipalId}" |
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.
Can't say I'm familiar with this syntax. That's setting $(AZURE_CLIENT_ID)
to what was in ${env:servicePrincipalId}
? Couldn't we use ${env:servicePrincipalId}
directly below?
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.
This entire task logs in using the service connection (temporary token obtained via OIDC), runs the inline script, and then clears the environment (this last bit is the main answer to your question). We need to smuggle out the connection info and use it to obtain a new temporary token (the az login
below) that lasts for the subsequent steps. It's apparently just an annoying design.
The alternative is to turn every PowerShell step below that does signing into an AzureCLI task and add lines to rename the environment variables into the AZURE_...
ones expected by the sign
command. It turns out, that really isn't any better than this approach, though both kinda suck.
The best solution would be a dedicated task for signing that knows how to use the service connection directly, but as far as I can tell that task doesn't exist without adding extensions to Azure DevOps (you can't just pull them directly from public repos like in GitHub Actions), and it wasn't clear that we could do the filtering needed either. So I preferred this approach.