Skip to content
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

Remove redundant signing op for mas and mas-dev targets #4321

Merged
merged 1 commit into from
Oct 22, 2019
Merged

Remove redundant signing op for mas and mas-dev targets #4321

merged 1 commit into from
Oct 22, 2019

Conversation

oNaiPs
Copy link
Contributor

@oNaiPs oNaiPs commented Oct 15, 2019

I noticed that running the packager on a basic vanilla repo was yielding two signing operations for mas/mas-dev targets:

mkdir test && cd test
yarn init
yarn add --dev electron electron-builder
echo "console.log('hello world')" > index.js
yarn electron-builder -m mas
  • electron-builder  version=21.2.0 os=18.7.0
  • description is missed in the package.json  appPackageFile=/Users/onaips/test/package.json
  • author is missed in the package.json  appPackageFile=/Users/onaips/test/package.json
  • writing effective config  file=dist/builder-effective-config.yaml
  • packaging       platform=mas arch=x64 electron=6.0.12 appOutDir=dist/mas
  • default Electron icon is used  reason=application icon is not set
  • signing         file=dist/mas/test.app identityName=Developer ID Application: -redacted- identityHash=-redacted- provisioningProfile=none
  • signing         file=dist/mas/test.app identityName=3rd Party Mac Developer Application:-redacted- identityHash=-redacted- provisioningProfile=none

I think this is an artifact from this PR after this other one was implemented

This PR removes the unneeded intermediate package signing.

@oNaiPs
Copy link
Contributor Author

oNaiPs commented Oct 15, 2019

It looks like these signing operation are indeed needed, since the signApp is called with any MAS options.
Perhaps way to go would be to instead skip the regular signing called on doPack from platformPackager.ts?

@oNaiPs
Copy link
Contributor Author

oNaiPs commented Oct 15, 2019

OK should be fixed now.

@tflack
Copy link

tflack commented Oct 15, 2019

OK should be fixed now.

CI is failing because of an extra semi colon...

@oNaiPs
Copy link
Contributor Author

oNaiPs commented Oct 15, 2019

@tflack should be fixed now.

@develar develar merged commit 5692281 into electron-userland:master Oct 22, 2019
@develar
Copy link
Member

develar commented Oct 22, 2019

Thanks!

@patrickmichalina
Copy link

FYI: This new code has broken "DMG" signing.

@quanglam2807
Copy link
Contributor

I agree, @patrickmichalina. This PR has regression. It disables signing altogether if mas is in targets. So if targets include non-mas like dmg, zip together with mas, non-mas builds won't be signed. Would you mind providing a fix, @oNaiPs?

@gaodeng
Copy link
Contributor

gaodeng commented Feb 26, 2020

please revert this commit 5692281 @develar

@develar
Copy link
Member

develar commented Mar 3, 2020

Reverted.

develar added a commit to develar/electron-builder that referenced this pull request Mar 3, 2020
develar added a commit to develar/electron-builder that referenced this pull request Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants