feat: Allow for NSIS windows installer to be wrapped in an MSI #7407
Conversation
🦋 Changeset detectedLatest commit: a1dde6f The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 packages
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 |
✅ Deploy Preview for car-park-attendant-cleat-11576 ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
ghost1face
changed the title
|
This is awesome! Thanks for the contribution. Super odd that the doc generation isn't working, but I can take care of that separately if need be. |
|
Ok, let me know if there's anything you need from me whether it be screenshot samples or anything else of the sort. I think I covered all the pre-requisites: schema, tests, naming conventions, etc. But if there's something I missed I'd be happy to make updates |
package.json
Outdated
| @@ -57,6 +57,7 @@ | |||
| "eslint": "8.26.0", | |||
| "eslint-config-prettier": "8.5.0", | |||
| "eslint-plugin-prettier": "4.2.1", | |||
| "fast-xml-parser": "^4.0.15", | |||
There was a problem hiding this comment.
I think this needs to be a dependency in app-builder-lib specifically. Not sure if prod or dev dependency though
There was a problem hiding this comment.
fast-xml-parser is only referenced in the unit tests. So my thought is that it's only a devDependency here
There was a problem hiding this comment.
ahhh fair enough. Does it still need to be at the electron-builder workspace level or can it be installed within the /test/package.json then?
There was a problem hiding this comment.
I went ahead and moved the dependency to the appropriate location, under test
| }) | ||
|
|
||
| // WiX doesn't support Mono, so, dontnet462 is required to be installed for wine (preinstalled in our bundled wine) | ||
| export default class MsiWrappedTarget extends Target { |
There was a problem hiding this comment.
I'm noticing a lot of copied code from MsiTarget, perhaps we should extend MsiTarget instead of Target?
This would mirror the current setup of
interface MsiWrappedOptions extends CommonWindowsInstallerConfiguration, TargetSpecificOptions { ... }
There was a problem hiding this comment.
Good point, I copied it at the start not knowing how far it may stray, let me try to clean that up
|
I did some cleanup between the targets so there's less duplication |
|
The failing tests are unrelated to this PR and seem to be an environment issue. The PR LGTM, thanks for the contribution! I'll work on getting the docs regenerated |
Hey all,
I ran into a similar issue as others where we build an .exe and .msi installer only to find out they are completely different. I looked into a few potential tools and solutions and have come to this one. Ultimately, the goal here is that we install a single application using the same Product Code regardless of installer.
The way I'm accomplishing this here is by building an msi installer that wraps and invokes an exe installer behind the scenes.
This pull request adds a new target:
msiWrappedwhich is a "dependent target" that depends on thensistarget to also be configured. Order of the configuration doesn't matter here as I use theisAsyncSupportedflag for this target to ensure thensisbuild goes down before themsiWrapped.{ "build": { ...other stuff... "win": { "target": [ "nsis", "msiWrapped" ] } }I did the following:
Take a look and let me know what you guys think. I kept the implementation basic so it definitely does have room to develop over time.