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

add --target-bin-dir to execute #209

Merged
merged 1 commit into from
Sep 1, 2023
Merged

Conversation

Gankra
Copy link
Contributor

@Gankra Gankra commented Aug 31, 2023

This sets CargoTargetBinDir in the wix template, allowing something orchestrating cargo-wix to take the reigns on building/gathering binaries completely. Setting this an --no-build supresses the warning (as at this point there's a pretty clear signal the user is doing this intentionally and not as a temp thing).

No integration tests were added for this functionality because it wasn't clear to me how to reasonable test this, but the functionality is boring enough that it's easy to trust with just the unit tests.

As compensation some backlog tests/docs were added.


Also does some cleanups to factor target-triple/profile logic so that things aren't recomputed in a bunch of places.

Also adds the arguably-forgotten-to-be-added package.metadata.wix.profile (for projects that always want to build with a special profile).

Note that --target-bin-dir this time is intentionally without a package.metadata pair, because it's pretty exceptional to want to fully bake the target dir.

@Gankra
Copy link
Contributor Author

Gankra commented Aug 31, 2023

This is the most important part of #207, everything else is bonus.

Copy link
Owner

@volks73 volks73 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good. Thank you for the PR and additional tests.

One minor change request before merging, could you please add a doc comment?

This sets CargoTargetBinDir in the wix template, allowing something orchestrating
cargo-wix to take the reigns on building/gathering binaries completely. Setting
this an --no-build supresses the warning (as at this point there's a pretty
clear signal the user is doing this intentionally and not as a temp thing).

No integration tests were added for this functionality because it wasn't clear
to me how to reasonable test this, but the functionality is boring enough that
it's easy to trust.

As compensation some backlog tests/docs were added.

-----

Also does some cleanups to factor target-triple/profile logic so that things
aren't recomputed in a bunch of places.

Also adds the arguably-forgotten-to-be-added package.metadata.wix.profile
(for projects that always want to build with a special profile).

Note that --target-bin-dir this time is *intentionally* without a package.metadata
pair, because it's pretty exceptional to want to fully bake the target dir.
@volks73 volks73 self-requested a review September 1, 2023 18:37
Copy link
Owner

@volks73 volks73 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@volks73 volks73 merged commit 6b3fafe into volks73:main Sep 1, 2023
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