search

volks73 / cargo-wix

A cargo subcommand to build Windows installers for rust projects using the WiX Toolset
https://volks73.github.io/cargo-wix
Apache License 2.0
306 stars 25 forks source link

[RFC] cargo-dist wix orchestration #207

Closed Gankra closed 1 year ago

Gankra commented 1 year ago

Hey there! My work in cargo-dist to use cargo-wix for msi's (https://github.com/axodotdev/cargo-dist/pull/370) is now far enough along that I think I have a pretty good picture of what it should look like.

Notably:

volks73 commented 1 year ago

Thank you for the Request For Comment (RFC). My thoughts:

  • Because we've had a lot of problems with users desync'ing their settings when we let them generate hand-editable artifacts, we're planning to discourage hand-editing, and invoke cargo wix init constantly to ensure the file is up to date with the values stored in the Cargo.toml. We would add an allow-dirty to our own config to return to the "vanilla" cargo-wix experience. I'd argue it would be beneficial to usptream this design to cargo-wix itself, but that would be a breaking change so I understand if it's not interesting.

This appears to be very similar to the often requested and desired "Ephemeral" workflow, #85 and #88. Would the cargo wix print invocation be more useful? The main.wxs file would be printed to STDOUT or the --output=<FILE> destination, which could be a temporary directory. Nothing would need to be re-synced as the needed files would be created on-the-fly.

Although, it looks like cargo wix is not STDIN aware, i.e., the cargo wix print | cargo wix would not work.

  • Rerunning cargo wix init would mess up GUIDs, so we also generate the GUIDs once and add [package.metadata.wix] to our user's Cargo.tomls. Is this functionality you'd like to have upstreamed into your project (not sure if it would make sense)?

I believe this is already implemented. There is the --upgrade-guid option for the cargo wix init command, but there is also the upgrade-guid field for the [package.metadata.wix] section in the Cargo.toml file. It appears the upgrade-guid field is not documented in the Configuration section, but it is recognized and used if it exists (#116).

This is also true for the --path-guid, where support for the path-guid field in the Cargo.toml file exists, but alast it is not documented, too (#117).

I am only aware of these two GUIDs in the default main.wxs template. The rest of the GUIDs appear to use the wildcard, *, but am I missing any others?

It looks like the Configuration documentation needs to be updated.

  • Although I originally planned to use --no-build, and implemented Add a --profile flag to be more general than --debug-build #198 to support this, I've realized it's probably not the right solution (see feat(msi): add msi installer axodotdev/cargo-dist#370 for detailed discussion of that). I think ideally we'd just have an interface for saying "hey all the binaries (and maybe other static assets to include in the MSI?) are in this directory, no need to build". Does that sound reasonable?

Is this handled by the -B,--binary option in combination with the --no-build flag?

This option can be used multiple times to define multiple binaries to include in the installer. The value is a path to a binary file. The file stem (file name without extension) is used as the binary name within the WXS file. A relative or absolute path is acceptable.

The issue is that the binaries and assets to include in an installer must be defined in the WXS file, which is part of the cargo wix init or cargo wix print commands/stage. When the WXS file is generated, it has to guess at the binary location and assumes it is the release destination. However, the -B,--binary option can be used to override the guess. It does not support a folder/directory (#140 and #161) as that is not directly supported by WiX. WiX really wants everything explicitly defined in the WXS file. There is the heat tool from WiX, but I have considered a wrapper of that tool out-of-scope for this project.

Taking all of these thoughts into account, an example invocation that would address all bullet points:

[package.metadata.wix]
upgrade-guid = "<GUID>"
path-guid = "<GUID>"

where <GUID> is replaced with unique GUIDs for the two fields and:

C:\> cargo wix init -B C:\tmp\build-123456\hello.exe -B C:\tmp\build-123456\world.exe -B C:\tmp\build-123456\greeting.exe
C:\> cargo wix --no-build

There would be the problem that all of the commands assume the Current Working Directory (CWD) contains the Cargo.toml file. There is #160 to address. The Ephemeral workflow is possible but there is some improvement needed. For example, the --manifest-path option, STDIN support for the cargo wix command, and overriding the default wix\main.wxs would be useful.

Gankra commented 1 year ago

This appears to be very similar to the often requested and desired "Ephemeral" workflow, https://github.com/volks73/cargo-wix/issues/85 and https://github.com/volks73/cargo-wix/issues/88

Thanks for the links! This is potentially the right solution, but it seemed like a "best of both worlds" if I could check it in and allow the user to opt into manual edits If They Know What They're Doing (and also they could still mess around with cargo-wix on its own without getting locked into cargo-dist, I guess?).

There is the --upgrade-guid option for the cargo wix init command, but there is also the upgrade-guid field for the [package.metadata.wix] section in the Cargo.toml file.

Yes, these fields are already being relied on by cargo-dist. What I'm saying is cargo is that cargo dist init will add something like this to all packages that want msi's, so that rerunning cargo wix init will be stable:

[package.metadata.wix]
upgrade-guid = "ABA64E5C-D137-4570-B8BD-C2254D6DAD68"
path-guid = "9B907AD9-B437-47DA-957E-A73DBDD05342"

The functionality I was proposing upstreaming was the code that patches this stuff into a Cargo.toml for the user instead of just baking the generated GUIDs into the output file.

The issue is that the binaries and assets to include in an installer must be defined in the WXS file, which is part of the cargo wix init or cargo wix print commands/stage. When the WXS file is generated, it has to guess at the binary location and assumes it is the release destination. However, the -B,--binary option can be used to override the guess. It does not support a folder/directory (https://github.com/volks73/cargo-wix/issues/140 and https://github.com/volks73/cargo-wix/issues/161) as that is not directly supported by WiX. WiX really wants everything explicitly defined in the WXS file. There is the heat tool from WiX, but I have considered a wrapper of that tool out-of-scope for this project.

Aha, this was key context I was missing, thanks! But yes to be clear I don't necessarily need to be able to say "dynamically read this dir", but rather just a way to for me to set CargoTargetBinDir, instead of letting wix compute it based on profile/target-triple.

volks73 commented 1 year ago

The functionality I was proposing upstreaming was the code that patches this stuff into a Cargo.toml for the user instead of just baking the generated GUIDs into the output file.

Thank you for clarifying. I am not opposed to adding this as a flag to the cargo wix init command, something like cargo wix init --guid (or something), where --guid would add the GUIDs to the Cargo.toml file. Alternative names for the flag could be --toml-guid, --cargo-guid, --manifest-guid, etc. I am open to suggestions on the flag name.

Gankra commented 1 year ago

Ok, I'll start queuing up some PRs with initial impls.

volks73 commented 1 year ago

@Gankra Has this been addressed with v0.3.5 release?

Gankra commented 1 year ago

I'll need #219 resolved before I can ship this design, but yeah this issue can be closed in favour of that one.