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 --profile command line option #73

Merged
merged 1 commit into from
May 21, 2023

Conversation

jpgrayson
Copy link
Contributor

When expanding an asset path, currently only the "target/release/" prefix could be remapped. By comprehending the build profile, path expansion can happen for "target/". This enables use of both alternative target-triple and build profiles.

For example, given the following asset configuration in Cargo.toml:

[package.metadata.generate-rpm]
assets = [
  { source = "target/my-profile/my-exe", dest = "/usr/bin/", mode = "755" },
]

rpms can be generated for multiple target triples, such as:

cargo generate-rpm --profile my-profile --target x86_64-unknown-linux-musl
cargo generate-rpm --profile my-profile --target i668-unknown-linux-musl

with the paths for the "my-exe" executable expanded correctly to:

target/x86_64-unknown-linux-musl/my-profile/my-exe
target/i686-unknown-linux-musl/my-profile/my-exe

This behavior is consistent with cargo-deb, which also has a --profile option for the same purpose.

@cat-in-136 cat-in-136 added the enhancement New feature or request label May 20, 2023
@cat-in-136
Copy link
Owner

Thank you for proposing new feature.


In your PR, source in package.metadata.generate-rpm.assets is supposed to be written in with the value of target specified as an argument instead of release, like target/my-profile/my-exe. I think the source in package.metadata.generate-rpm.assets should be written as target/release/my-exe and it should be expanded to target/my-profile/my-exe or target/x 86_64-unknown-linux-musl/my-profile/my-exe.
That is, I would prefer that profile be treated the same as target.

What do you think?

Could you please update the README.md in addition to the source code? A short addition to the "Cross compilation" clause would be sufficient.

When expanding a source asset path, the "target/release/" prefix is now
remapped using profile in addition to target and target-dir. This enables
use of both alternative target-triple and build profiles.

For example, given the following asset configuration in Cargo.toml:

    [package.metadata.generate-rpm]
    assets = [
      { source = "target/release/my-exe", dest = "/usr/bin/", mode = "755" },
    ]

rpms can be generated for multiple target triples, such as:

    cargo generate-rpm --profile my-profile --target x86_64-unknown-linux-musl
    cargo generate-rpm --profile my-profile --target i668-unknown-linux-musl

with the paths for the "my-exe" executable expanded correctly to:

    target/x86_64-unknown-linux-musl/my-profile/my-exe
    target/i686-unknown-linux-musl/my-profile/my-exe

This behavior is consistent with cargo-deb, which also has a --profile
option for the same purpose.

Signed-off-by: Peter Grayson <pete@jpgrayson.net>
@jpgrayson
Copy link
Contributor Author

Yes, good suggestion.

I've updated the patch so that the "target/release/" prefix is preferentially remapped while still also looking for "target/{prefix}". This is how cargo-deb does it.

And I've added a paragraph about the '--profile` option to README.md.

Thanks for reviewing this PR. Much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants