-
-
Notifications
You must be signed in to change notification settings - Fork 113
ci(windows): pin msstore CLI to v0.4.2 now that #162 is fixed #5388
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -235,7 +235,7 @@ On a `v*` tag push the workflow: | |||||||||
| 1. Builds `AetherSDR.exe`, runs `windeployqt`, packages the MSIX, and creates | ||||||||||
| the `.msixupload` (existing steps). | ||||||||||
| 2. The pinned `microsoft/microsoft-store-apppublisher` action puts the pinned | ||||||||||
| `msstore` CLI v0.4.1 on PATH and the workflow logs `msstore --version`. | ||||||||||
| `msstore` CLI v0.4.2 on PATH and the workflow logs `msstore --version`. | ||||||||||
| 3. `msstore reconfigure` authenticates from the four GitHub secrets. | ||||||||||
| 4. `packaging/windows/publish-store.ps1` finds the `.msixupload` and runs | ||||||||||
| `msstore publish <pkg>.msixupload -id <ProductId> --uploadTimeout 300 | ||||||||||
|
|
@@ -257,18 +257,28 @@ If the MSIX packaging step (which is `continue-on-error`) produced no | |||||||||
| `.msixupload`, `publish-store.ps1` warns and exits 0 rather than turning an | ||||||||||
| otherwise-successful release red. | ||||||||||
|
|
||||||||||
| The upload timeout is deliberately explicit. `msstore` CLI v0.4.0 and v0.4.1 | ||||||||||
| have a regression where omitting `--uploadTimeout` supplies a zero-second Azure | ||||||||||
| blob network timeout, producing the characteristic `Uploading Bundle to Azure | ||||||||||
| blob: 0%` failure and exit code `-1` | ||||||||||
| The upload timeout is deliberately explicit, and stays that way — but it is no | ||||||||||
| longer a bug workaround. `msstore` CLI v0.4.0 and v0.4.1 had a regression where | ||||||||||
| omitting `--uploadTimeout` supplied a zero-second Azure blob network timeout, | ||||||||||
| producing the characteristic `Uploading Bundle to Azure blob: 0%` failure and | ||||||||||
| exit code `-1` | ||||||||||
| ([microsoft/msstore-cli#162](https://github.com/microsoft/msstore-cli/issues/162)). | ||||||||||
| The script uses the documented workaround of 300 seconds. It deliberately | ||||||||||
| leaves verbose logging disabled because Actions logs are public and expanded | ||||||||||
| authentication or upload diagnostics could expose derived credentials that | ||||||||||
| GitHub cannot mask by their registered secret values. The affected CLI is | ||||||||||
| pinned to prevent `latest` from silently changing publish behavior; advance | ||||||||||
| that pin only after validating a released version containing | ||||||||||
| [microsoft/msstore-cli#163](https://github.com/microsoft/msstore-cli/pull/163). | ||||||||||
| That is fixed by | ||||||||||
| [microsoft/msstore-cli#163](https://github.com/microsoft/msstore-cli/pull/163) | ||||||||||
| and released in v0.4.2, which the pin above now names, so omitting the option | ||||||||||
| would correctly yield the documented 100 s default. | ||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Verified all three claims in this sentence against the upstream repo at tag
Worth noting for anyone re-reading this later: the fix's own source comment spells out the precondition — the option's arity is |
||||||||||
|
|
||||||||||
| 300 seconds is kept because 100 s is genuinely too short for this package. The | ||||||||||
| CLI sets no `StorageTransferOptions`, so a `.msixupload` under 256 MiB is | ||||||||||
| uploaded as a **single PUT** and the network timeout has to cover the whole | ||||||||||
| transfer rather than an individual chunk. AetherSDR's upload is ~200 MB, which | ||||||||||
| at 100 s would demand a sustained ~2 MB/s for the entire request. | ||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Both halves of this check out: One caveat for future readers rather than for this PR: the single-PUT property holds only while the package stays under the SDK's 256 MiB |
||||||||||
|
|
||||||||||
| The workflow deliberately leaves verbose logging disabled because Actions logs | ||||||||||
| are public and expanded authentication or upload diagnostics could expose | ||||||||||
| derived credentials that GitHub cannot mask by their registered secret values. | ||||||||||
| The CLI stays pinned to prevent `latest` from silently changing publish | ||||||||||
| behavior. | ||||||||||
|
Comment on lines
+280
to
+281
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The sentence this replaces ended with an instruction aimed at the next maintainer, not just a statement about today:
Retiring the #163-specific half is right. But "validate a release before advancing the pin" is a standing rule that this PR is itself a clean instance of, and it has gone from the doc along with it. Suggest keeping it in general form:
Suggested change
Non-blocking. |
||||||||||
|
|
||||||||||
| ### One-time setup (maintainer, outside the repo) | ||||||||||
|
|
||||||||||
|
|
||||||||||
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.
Right split — the action stays pinned by SHA while only the CLI version it fetches moves. I confirmed the action's
action.ymlatcc9910a8takesversionas a free-form input (defaulting tolatest), so an arbitrary newer tag is plausible; the actual resolution happens in the bundleddist/index.js, which I did not audit, so that last step is read rather than verified.Blast radius if
v0.4.2ever failed to resolve: this step has nocontinue-on-errorand runs after thesoftpropsrelease step above, so the run would go red with the release assets already published. Pre-existing structure from #5345, not introduced here — naming it because this bump is the first thing to exercise the new value, and it will do so on a livev*tag with no CI lane ahead of it.