First stab at cross-compilation support

[roblabla]

My first stab at fixing #110. Not fully complete yet (need to integrate it into the CLI, write some tests, fix some others).

This PR works by adding a new optional target flag to the Create step, which contains the rust target to build for. This flag is passed as-is to the cargo invocation.

When invoking wix, we use the target to find which -arch to pass to wix. This is currently implemented by looking at the first component of the target name, and transforming it to something wix-compatible (e.g. "x86_64" => "x64", etc...). Note that this is somewhat wrong: Ideally we should extract the target spec from rustc -Z unstable-options --print target-spec-json | jq .llvm-target, but the target spec json isn't stable, so we can't easily rely on it. Using the target name is an OK approximation that should get us most of the way there.

We then pass the target name to Wix as a variable, so we can find where the resulting binaries go.

I took the opportunity to remove use of Win64 and Platform in the wxs: those attributes are deprecated according to the wix documentation, passing -arch on the command line should be prefered (which this PR now does). var.Platform is replaced by sys.BUILDARCH.

[volks73]

I took the opportunity to remove use of Win64 and Platform in the wxs: those attributes are deprecated according to the wix documentation, passing -arch on the command line should be prefered (which this PR now does).

I just want to note that the Platform attribute for the Package element is not deprecated but discouraged according to the online documentation. There is a some confusion as some values for the Platform attribute have been deprecated. I am guessing the discouraged usage of the Platform attribute is to eventually deprecate it. I am all for removing it. I just wanted to make a note to avoid future confusion.

However, I cannot find any information on the deprecation of the Win64 attribute for the Component element. It appears to be automatically set based on the -arch switch, similar to the recommended usage for the Platform attribute. So, it looks like all that is really needed is to use the -arch switch and let WiX take care of everything else. This is all good. Again, just wanting to make notes to reduce confusion in the future.

[volks73]

By the way, the approach looks good. Let me know when you are ready to merge.

I agree with implementing a workaround until target-spec-json is stabilized. Let's just create an issue after this is merged to remind us to refactor when appropriate.

[roblabla]

This took longer than expected to finish, sorry about the delay 😅. This is now ready for review. I mainly fixed all the tests and added a test to make sure cross-compilation actually worked, and added back var.Platform to avoid breaking old wxs (Its use should be avoided in the future though).

Big caveat: I have no clue about the good practices in WiX. Notably the whole CargoTargetExplicit ifdef thing feels gross to me. You can't exactly pass booleans to the preprocessor, so that was the next closest thing I could find, but if there's a better way, I'd love to know :).

[volks73]

This took longer than expected to finish, sorry about the delay 😅

No worries. It might take me some time to review, too.

...and added back var.Platform to avoid breaking old wxs (Its use should be avoided in the future though).

Without any in-depth review or extensive thought to this, I am actually okay with breaking compatibility here. If the usage should be avoided, then we just need eliminate it. I was going to make a v0.4 release after this was complete. The tick up in the minor version, pre-1.0.0, would indicate some things have changed and I would make note in the release notes. I will play around with it a little bit to see if my initial thoughts are constant.

[roblabla]

I was going to make a v0.4 release after this was complete. The tick up in the minor version, pre-1.0.0, would indicate some things have changed and I would make note in the release notes.

My thought process is, since there's no (easy) way to tell cargo install "Please install this version or any semver-compatible version", I tend to avoid breaking changes in tools installable via cargo-install as much as possible. I have some internal tooling in my CI to pin my tools to specific version, but I assume this isn't the case of most people.

So removing the var.Platform variable will likely break a lot of people's build, while keeping it only costs us a single extra line of code. So I think keeping it around is worth the trouble. If you don't agree though, well it's pretty easy to remove :).

[volks73]

I just looked at the main.wxs.mustache template, and the var.Platform is removed. Maybe the commit to add it back in was not pushed?

[roblabla]

The template doesn't use it anymore, but it's still defined in create.rs such that people using the old template (e.g. people who did a cargo wix init with the previous version) can still build.

[volks73]

The template doesn't use it anymore, but it's still defined in create.rs such that people using the old template (e.g. people who did a cargo wix init with the previous version) can still build.

Got it! The platform is now just an alias for the architecture.

[volks73]

Again, thank you for implementing this feature and the effort. I saw that the --build-plan option might be removed and I got really confused by the CargoTargetExplicit implementation. I think it might be possible to move that logic back into Rust and avoid using the preprocessor. Please see my comment/rambling about it.

Once the comments are acknowledged, I am happy to merge/continue refining this implementation.