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

feat(msi): add fileAssociation support for MSI target #6530

Conversation

aplum
Copy link
Contributor

@aplum aplum commented Jan 6, 2022

Also fix iconId sometimes containing invalid characters, and iconId config option being ignored (was added in #6247, but it doesn't seem to have been used).

Marking this as a draft and targeting v23.0.0-alpha because I think there's some more related changes that should be made:

  • I'm not sure why the iconId config option was added, and I think it should be removed. I don't see why anyone would want to manually configure it when we should be able to generate it ourselves. Even if the product name contains all invalid-for-MSI-Id characters, we could still generate a valid unique Id somehow (base64, seeded RNG, whatever).
    • Even if we do want to keep the option to manually configure it, it's now too specific – we also need a product-specific Id prefix for ProgId Ids with this PR, and possibly others in the future.
  • I didn't support FileAssociation.icon yet, but I'd be willing to if someone wants to lend a hand. The WiX side should be easy (add more Icon elements as needed, reference their Ids here), but I can't justify the time to figure out getting/converting the icon file since I don't need this feature at my job.

Also, I didn't use the FileAssociation.name config. I'm not sure what the expected values look like here, but the given example ("PNG") doesn't match up with the recommended format where it's used in NsisTarget – in the docs the example value is "myapp.textfile", which matches the format used in WiX ProgId Id examples. So in NSIS it looks like we should generate that value ourselves, as done in MSI ProgId here. Furthermore, on Mac targets name is used for CFBundleTypeName, which I'm not familiar with but this example uses "Adobe PDF" and iMovie.app on my Mac uses values like "Color Preset"… so I'm thinking maybe the name and description options should be merged?

Closes #3694, #6488

@changeset-bot
Copy link

changeset-bot bot commented Jan 6, 2022

🦋 Changeset detected

Latest commit: e3fafcf

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages
Name Type
app-builder-lib Major
dmg-builder Major
electron-builder-squirrel-windows Major
electron-builder Major
electron-forge-maker-appimage Major
electron-forge-maker-nsis-web Major
electron-forge-maker-nsis Major
electron-forge-maker-snap Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@mmaietta
Copy link
Collaborator

mmaietta commented Jan 7, 2022

Since we're targeting the v23 alpha branch, perhaps we can remove the iconId field and generate it ourselves as you mentioned? That way we can avoid this:

it's now too specific – we also need a product-specific Id prefix for ProgId Ids with this PR, and possibly others in the future.

Not sure regarding the other items, the subject kind of flew over my head. Never worked with File Associations in the past

@aplum
Copy link
Contributor Author

aplum commented Jan 7, 2022

I've removed the iconId config option and added a better fallback value than the "ElectronApp" I was using.

Supporting FileAssociation.icon could be done in a minor release in the future, and my rambling paragraph about FileAssociation.name would be more appropriate as a separate PR anyway (if it holds any merit) (FYI don't expect a PR about it from me), so I'm marking this as ready for review now.

@aplum aplum marked this pull request as ready for review January 7, 2022 23:44
@mmaietta
Copy link
Collaborator

Can you please merge in latest master? That should enable latest test configuration to be run

@aplum
Copy link
Contributor Author

aplum commented Jan 12, 2022

Done, I think?

@mmaietta mmaietta changed the base branch from v23.0.0-alpha to master January 12, 2022 19:32
@mmaietta mmaietta changed the base branch from master to v23.0.0-alpha January 12, 2022 19:32
@mmaietta
Copy link
Collaborator

Not sure what happened, but the merge triggered much more "file changes" than what was originally in the PR.

Could you try rebasing your feature on top of v23.0.0-alpha branch? I just want to clean up the PR to only have your changes.

Also fix iconId sometimes containing invalid characters, and iconId config option being ignored.

Closes electron-userland#3694, electron-userland#6488
Also change the fallback value for generated MSI Ids to a unique string for the product.

BREAKING CHANGE: remove MSI option `iconId`
@aplum
Copy link
Contributor Author

aplum commented Jan 12, 2022

That seems to have fixed it.

@mmaietta mmaietta merged commit 04a3f14 into electron-userland:v23.0.0-alpha Jan 12, 2022
@mmaietta mmaietta mentioned this pull request Jan 16, 2022
mmaietta added a commit that referenced this pull request Jan 16, 2022
* Adding INPUTxxx and OUTPUTxxx CHARSETS to makensis
Fixes: #4898 #6232 #6259

* Adding additional details to error console logging

* Breaking change: Removing Bintray support since it was sunset. Ref: https://jfrog.com/blog/into-the-sunset-bintray-jcenter-gocenter-and-chartcenter/

* fix: force strip path separators for backslashes on Windows

* fix: Force authentication for local Mac Squirrel update server

* Breaking Change: Fail-fast for signature verification failures. Adding `-LiteralPath` to update file for injected wildcards

* Adding changeset and eslint

* Fix error: `-OUTPUTCHARSET is disabled for non Win32 platforms.`

* feat(mac): ElectronAsarIntegrity in electron@15 (#6511)

* feat(mac): ElectronAsarIntegrity in electron@15
See: electron/electron#30667
Fix: #6506
Fix: #6507

* fix(msi): MSI fails to install when deployed machine-wide via GPO (#6514)

* fix(msi): MSI fails to install when deployed machine-wide via GPO
* Disable advertised shortcuts, since MSIs with advertised Start Menu shortcuts that have a
Shortcut Property fails to install when deployed machine-wide via GPO but works fine in all
other contexts. This might be a bug in Windows or a misdiagnosis; see #6508 for more details.
Closes #6508
* BREAKING CHANGE: Admins using advertisement must apply an MST to re-enable it. See #6508.

* Don't set GitHub Releases draft title since it automatically pulls it from tag name. Fixes #3683

* feat(snap): add lzo to Snap compression options (also as new default) (#6201)

* feat(msi): add fileAssociation support for MSI target (#6530)

* fix(win): iconId sometimes containing invalid characters, and iconId config option being ignored.
* fix(msi): change the fallback value for generated MSI Ids to a unique string for the product.

* BREAKING CHANGE: remove MSI option `iconId`

* fix: stabilizing tests by moving updater tests to its own node to explicitly segment env.___TOKEN integration tests from other standard unit tests

* chore: synchronizing docs and schema plus prettier

* Adding changset to set as alpha

* Updating changeset documentation

* feat(msi): support assisted installer for MSI target (#6550)

* Add basic support for assisted installer, with UI to choose between per-user and per-machine. Supported config settings: runAfterFinish, perMachine, oneClick. Not supported: license (EULA), allowToChangeInstallationDirectory, etc. Also prevent oneClick's runAfterFinish from executing when installed silently.

Co-authored-by: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com>
Co-authored-by: Alex Plumley <alex+git@aplum.ca>
Co-authored-by: Mike Maietta <mmaietta@outlook.com>
Co-authored-by: Omer Akram <om26er@gmail.com>
Co-authored-by: Maximilian Federle <max.federle@gmail.com>
@mmaietta
Copy link
Collaborator

Published in 23.0.0-alpha.0. Enjoy!

mergify bot pushed a commit to enso-org/enso that referenced this pull request Apr 21, 2022
[ci no changelog needed]

[Task link](https://www.pivotaltracker.com/story/show/181944234).

It fixes the build issue on Mac OS 12.3.1 that is caused by removed `/usr/bin/python` executable.

Also applied `enso-formatter` to the sources.

# Important Notes
We're basically updating for one major `electron-builder` release - from `v22` to `v23`. I didn't spot anything in the changelog that could affect us. See features + breaking changes excerpt:

```
Features:

- feat(msi): add fileAssociation support for MSI target (electron-userland/electron-builder#6530)
- feat(mac): ElectronAsarIntegrity in electron@15 - See: electron/electron#30667 (electron-userland/electron-builder#6506 electron-userland/electron-builder#6507)
- feat(snap): add lzo to Snap compression options (also as new default) (electron-userland/electron-builder#6201) Upgraded app-builder-bin dependency required newer version of Go
- feat(msi): support assisted installer for MSI target (electron-userland/electron-builder#6550)

Breaking changes:

- Removing Bintray support since it was sunset. Ref: https://jfrog.com/blog/into-the-sunset-bintray-jcenter-gocenter-and-chartcenter/
- Fail-fast for windows signature verification failures. Adding -LiteralPath to update file path to disregard injected wildcards
- Force strip path separators for backslashes on Windows during update process
- Authentication for local mac squirrel update server
- Disabled advertised shortcuts, since MSIs with advertised Start Menu shortcuts that have a
Shortcut Property fails to install when deployed machine-wide via GPO but works fine in all
other contexts. Admins using advertisement must apply an MST to re-enable it. See electron-userland/electron-builder#6508.
- Removing optional NSIS icon ID from config and generating it automatically to synchronize IDs with Advertised Shortcuts and future features
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants