-
Notifications
You must be signed in to change notification settings - Fork 580
Windows signing fix experiments #3235
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
Closed
Closed
Conversation
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
Also updates the call to comply with the new API.
Co-authored-by: Kat Hagan <[email protected]> Co-authored-by: Olivier Halligon <[email protected]>
Co-authored-by: Gio Lodi <[email protected]>
* Don't pass a variable to require, it makes webpack mad * remove get-config.js because only webpack can use fs * update electron-updater due to security vulnerability * keep target set as default * add release notes assuming this is going to constitute a new 2.22.1 version
Otherwise, `package.json` would have been considered the source of the configurations.
See docs at https://www.electron.build/configuration/mac#:~:text=mergeASARs%20%3D%20true,for%20%E2%80%9Cuniversal%E2%80%9D%20arch. and discussion at electron-userland/electron-builder#6735 Apparently, the option needs to be disabled at this time when building for universal Mac.
This is to avoid errors such as https://buildkite.com/automattic/simplenote-electron/builds/148#0190c9fe-7dba-4f28-912d-c20b1ca2f011 However, I wonder why the `appx` is being built using `electron-builder.json` when there's a dedicated config file for it that should be used instead...
...The default configuration should build it already
From the docs at https://www.electron.build/configuration/appx#appx-package-code-signing > If the AppX package is meant for Windows Store distribution, no need > to sign the package with any certificate. The Windows Store will take > care of signing it with a Microsoft certificate during the submission > process.
d3ff059 was based on what's written in the docs
I guess "no need to sign" might be different from "don't pass any parameter"? |
See failure at #3235 (comment) I doubt this will result in a pass, but I'm curious to see how it will change the validation failure.
See https://buildkite.com/automattic/simplenote-electron/builds/156#0190d7d2-d094-40ca-97a4-e5ec076182bc [2024-07-22T00:31:18Z] ⨯ AppX Application.Id can’t start with numbers: "22490Automattic.Simplenote" [2024-07-22T00:31:18Z] Please set appx.applicationId (or correct appx.identityName or name)
Will this be enough to get the code signing to succeed? See failure at https://buildkite.com/automattic/simplenote-electron/builds/157#0190d7dc-d943-42f8-99e3-b97389c5b382
From the docs applicationId String - The application id. Defaults to identityName. This string contains alpha-numeric fields separated by periods. Each field must begin with an ASCII alphabetic character. https://www.electron.build/configuration/appx
electron-userland/electron-builder#2144 (comment) With the previous version, we got AppX Application.Id can’t start with numbers
This reverts commit 96b2d30.
55ab919
to
0071b13
Compare
This reverts commit 5899382.
ab50e01
to
295f5dd
Compare
This reverts commit eca23e4. It made the build fail with the usual code sign related error: Error information: "Error: SignerSign() failed." (-2147024885/0x8007000b) https://buildkite.com/automattic/simplenote-electron/builds/171#0190e313-197a-43c2-95b7-0d708f061ba9
It should now be the same as how it was for 2.22.0, with the difference that the `appx` settings are those coming from the redundant dedicated `-appx.json` config file
The `appx` was built before the `exe`, but with the latest config changes it failed to code sign. https://buildkite.com/automattic/simplenote-electron/builds/173#0190e360-f70d-4ee9-8fd7-15d8cff032d4 At the same time, with the config before those changes, the `exe` was not signed, or not signed properly. Regardless, Windows did not want to open that file. Removing `appx` to get to the `exe` build in CI and see if it signs okay with these settings
Notice that this build failed silently on `npm install`: https://buildkite.com/automattic/simplenote-electron/builds/174#0190e36e-b92e-4c7d-a6aa-7c26a02197b0/233-243
It had been deleted via f684f9d.
But in an improved version where the default one does not attempt to build the Appx as well.
9ea9952
to
a7647c0
Compare
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.
No description provided.