Build AppX version in CI #643

mokagio · 2024-11-04T00:55:21Z

Related to pesfPa-19a-p2

Proposed Changes

Add automation to generate two AppX packages, one signed for local testing and one unsigned for upload to the Windows Store
Call automation in the Windows Dev|Release build CI step (the Release branch was tested bypassing the tag check in Test PR for AppX build #613)
Store packages as CI artifacts

Testing Instructions

Download the signed version and run in on a Windows machine or VM.

Pre-merge Checklist

Have you checked for TypeScript, React or other console errors? – N.A.

mokagio · 2024-11-04T03:04:15Z

.windows-10-sdk-version

@@ -0,0 +1 @@
+20348


There are later versions than this, but we know this works in CI. We can try newer versions later on.

mokagio · 2024-11-04T03:07:32Z

scripts/package-appx.mjs

+} );
+
+const appxOutputPathSigned = path.resolve( outPath, `${ appxName }-signed` );
+console.log( `~~~ Creating signed .appx for local testing at ${ appxOutputPathSigned }...` );


If one tries to launch the unsigned AppX, Windows refuses to enable the "Install" button:

The flow with the signed version still results in multiple warnings, but clicking through them one can eventually run the app:

I figured given the packaging process is relatively fast, it might be useful to generate both versions just in case we need to test something.

Alternatively, we can abstract this logic and generate the unsigned AppX only when building for release.

Is there something we could do to avoid the extra warning alerts?

Alternatively, we can abstract this logic and generate the unsigned AppX only when building for release.

If it's not a big effort , limiting the AppX only for releases would be ideal. Thanks for the suggestion!

@sejas this warning suppose to alert Mac users who run Intel binary on Apple Silicon chip, and I think it shouldn't pop up on Windows. Should we check it?

Created the issue to investigate it further:
https://github.com/Automattic/dotcom-forge/issues/9846

The message is indeed launched by Studio:

studio/src/renderer.ts

Lines 85 to 102 in 335c634

// Show warning if running an ARM64 translator

if ( appGlobals.arm64Translation && ! localStorage.getItem( 'dontShowARM64Warning' ) ) {

const showARM64MessageBox = async () => {

const { response, checkboxChecked } = await getIpcApi().showMessageBox( {

type: 'warning',

message: __( 'This version of Studio is not optimized for your computer' ),

detail:

window.appGlobals.platform === 'darwin'

? __(

'Downloading the Mac with Apple Silicon Chip version of Studio will provide better performance.'

)

: __(

'Downloading the optimized version of Studio will provide better performance.'

),

checkboxLabel: __( "Don't show this warning again" ),

buttons: [ __( 'Download' ), __( 'Not now' ) ],

cancelId: 1,

} );

mokagio · 2024-11-04T04:18:38Z

scripts/package-appx.mjs

+	...sharedOptions,
+	// See details at https://partner.microsoft.com/en-us/dashboard/products/<id>/identity
+	publisher: 'CN=E2E5A157-746D-4B04-9116-ABE5CB928306',
+	devCert: 'nil', // skip code signing for Store upload


The Microsoft Store code signs on upload. This setup is successful

@matt-west

Kindly provided by @matt-west See pesfPa-1cz-p2#comment-763

sejas

This looks fantastic! Thank you!

I left a comment about the electron2appx library, but it's not blocking this PR.

If it’s not too much effort, it would be ideal to limit the AppX generation to betas and final releases. As I don't think we need this format in the rest of PRs. This could be addressed in a follow-up.

sejas · 2024-11-12T22:18:56Z

.buildkite/commands/build-for-windows-dev.ps1

 Get-ChildItem -Path $artifactsPath -Recurse -Include "*.nupkg" | Rename-Item -NewName "studio-update.nupkg"

 If ($LastExitCode -ne 0) { Exit $LastExitCode }
-


Do we need to remove this empty line?

It does not make any difference functionally. But it was an unnecessary trailing new line.

sejas · 2024-11-12T23:00:41Z

package.json

 		"cross-port-killer": "^1.4.0",
 		"date-fns": "^3.3.1",
 		"electron-squirrel-startup": "^1.0.0",
+		"electron2appx": "^2.1.2",


I noticed that PocketCasts also uses the electron2appx library. I’m surprised that the official guide https://www.electronjs.org/docs/latest/tutorial/windows-store-guide suggests one library and we actually need to use a fork of it.

It is odd right? And what are the odds that no one run into the need for disabling code signing when building the appx? (I haven't actually investigated existing issues in the repo, maybe there are open ones in that regard. TODO)

We need this fork because we need to disable code signing. But it's possible the issue is with us having to disable code signing. That is, we should not have to do that.

The reason we have to do it is that the cert we use for code signing comes from a publisher, CN='Automattic, Inc.', O='Automattic, Inc.', S=California, C=US that is different from the one in the Store, CN=E2E5A157-746D-4B04-9116-ABE5CB928306. My team has an open task to add processes and tooling around the certificate management, so hopefully we'll discover more about this soon.

mokagio force-pushed the mokagio/build-appx branch from 3efa7d1 to 9f5c2c6 Compare November 4, 2024 02:52

mokagio added 4 commits November 4, 2024 14:00

Remove a couple of trailing whitespaces

c1b3a65

Add script to install Windows 10 SDK

2009d01

Add script to package AppX version for Windows

a37949c

Update Windows 10 SDK requirement documentation

b157488

mokagio force-pushed the mokagio/build-appx branch from 9f5c2c6 to 738075d Compare November 4, 2024 03:02

mokagio commented Nov 4, 2024

View reviewed changes

mokagio added 2 commits November 4, 2024 14:40

Also build signed version so we can test the AppX locally

1864628

Disable makePri

76501a2

mokagio force-pushed the mokagio/build-appx branch from c6751ad to 76501a2 Compare November 4, 2024 03:40

mokagio marked this pull request as ready for review November 4, 2024 04:01

mokagio requested review from sejas and wojtekn November 4, 2024 04:06

mokagio commented Nov 4, 2024

View reviewed changes

Update AppX Square and Wide assets

a06ac02

Kindly provided by @matt-west See pesfPa-1cz-p2#comment-763

This was referenced Nov 5, 2024

Test PR for AppX build #613

Closed

Add all AppX assets from Microsoft spec #647

Merged

sejas approved these changes Nov 12, 2024

View reviewed changes

mokagio merged commit fca15ed into trunk Nov 13, 2024
13 checks passed

mokagio deleted the mokagio/build-appx branch November 13, 2024 03:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Build AppX version in CI #643

Build AppX version in CI #643

Uh oh!

mokagio commented Nov 4, 2024 •

edited

Loading

Uh oh!

mokagio Nov 4, 2024

Uh oh!

mokagio Nov 4, 2024 •

edited

Loading

Uh oh!

sejas Nov 12, 2024 •

edited

Loading

Uh oh!

wojtekn Nov 13, 2024

Uh oh!

sejas Nov 15, 2024

Uh oh!

mokagio Nov 4, 2024

Uh oh!

sejas left a comment

Uh oh!

sejas Nov 12, 2024

Uh oh!

mokagio Nov 13, 2024

Uh oh!

sejas Nov 12, 2024 •

edited

Loading

Uh oh!

mokagio Nov 13, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	// Show warning if running an ARM64 translator
	if ( appGlobals.arm64Translation && ! localStorage.getItem( 'dontShowARM64Warning' ) ) {
	const showARM64MessageBox = async () => {
	const { response, checkboxChecked } = await getIpcApi().showMessageBox( {
	type: 'warning',
	message: __( 'This version of Studio is not optimized for your computer' ),
	detail:
	window.appGlobals.platform === 'darwin'
	? __(
	'Downloading the Mac with Apple Silicon Chip version of Studio will provide better performance.'
	)
	: __(
	'Downloading the optimized version of Studio will provide better performance.'
	),
	checkboxLabel: __( "Don't show this warning again" ),
	buttons: [ __( 'Download' ), __( 'Not now' ) ],
	cancelId: 1,
	} );

		Get-ChildItem -Path $artifactsPath -Recurse -Include "*.nupkg" \| Rename-Item -NewName "studio-update.nupkg"

		If ($LastExitCode -ne 0) { Exit $LastExitCode }

		@@ -0,0 +1 @@
		20348 No newline at end of file

Build AppX version in CI #643

Build AppX version in CI #643

Uh oh!

Conversation

mokagio commented Nov 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

Testing Instructions

Pre-merge Checklist

Uh oh!

mokagio Nov 4, 2024

Choose a reason for hiding this comment

Uh oh!

mokagio Nov 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sejas Nov 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wojtekn Nov 13, 2024

Choose a reason for hiding this comment

Uh oh!

sejas Nov 15, 2024

Choose a reason for hiding this comment

Uh oh!

mokagio Nov 4, 2024

Choose a reason for hiding this comment

Uh oh!

sejas left a comment

Choose a reason for hiding this comment

Uh oh!

sejas Nov 12, 2024

Choose a reason for hiding this comment

Uh oh!

mokagio Nov 13, 2024

Choose a reason for hiding this comment

Uh oh!

sejas Nov 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mokagio Nov 13, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mokagio commented Nov 4, 2024 •

edited

Loading

mokagio Nov 4, 2024 •

edited

Loading

sejas Nov 12, 2024 •

edited

Loading

sejas Nov 12, 2024 •

edited

Loading