kornelski / cargo-deb

A cargo subcommand that generates Debian packages from information in Cargo.toml
https://lib.rs/cargo-deb
MIT License
404 stars 48 forks source link

feat. Allow assets to be merged by dest path #115

Closed juliusl closed 9 months ago

juliusl commented 9 months ago

Address #114

kornelski commented 9 months ago

This exposed a design ambiguity: I've assumed the source file would be the key for the merge (so you can have variant-specific names in the package for the same source file), but you've made the destination the key for merge.

I'm not sure which one is better. Would it make sense to have both behaviors, and merge based on either path? Or maybe it's all too clever and it should just append assets array?

juliusl commented 9 months ago

This exposed a design ambiguity: I've assumed the source file would be the key for the merge (so you can have variant-specific names in the package for the same source file), but you've made the destination the key for merge.

I'm not sure which one is better. Would it make sense to have both behaviors, and merge based on either path? Or maybe it's all too clever and it should just append assets array?

Interesting, I think adding an option for both behaviors is reasonable. Let me take a look at implementing that.

juliusl commented 9 months ago

@kornelski I made changes to allow for merging by dest, src, or by appending. I updated the example with these variants as well.

Here is a snippet of what usage looks like:

[package.metadata.deb]
assets = [
    # binary
    ["target/release/example", "usr/bin/", "755"],
    # assets
    ["assets/*", "var/lib/example", "644"],
    ["target/release/assets/*", "var/lib/example", "644"],
    ["3.txt", "var/lib/example/3.txt", "644"],
    ["3.txt", "var/lib/example/merged.txt", "644"],
]

[package.metadata.deb.variants.mergeappend]
merge-assets.append = [
    ["4.txt", "var/lib/example/appended/4.txt", "644"]
]

[package.metadata.deb.variants.mergedest]
merge-assets.by.dest = [
    ["4.txt", "var/lib/example/merged.txt", "644"]
]

[package.metadata.deb.variants.mergesrc]
merge-assets.by.src = [
    ["3.txt", "var/lib/example/merged-2.txt", "644"]
]

Also, it is an error to try and set both, here is what that looks like --

...
[package.metadata.deb.variants.shoulderror]
merge-assets.by.src = [
    ["3.txt", "var/lib/example/merged-2.txt", "644"]
]
merge-assets.by.dest = [
    ["4.txt", "var/lib/example/merged.txt", "644"]
]
...

1 ❯ ../target/debug/cargo-deb --variant shoulderror                                                                                                                                                                                example -> pr/merge-assets !
cargo-deb: Unable to parse /home/juliusl/rs/cargo-deb/example/Cargo.toml
  because: TOML parse error at line 52, column 1
   |
52 | [package.metadata.deb.variants.mergesrcdest]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
wanted exactly 1 element, more than 1 element

  because: TOML parse error at line 52, column 1
   |
52 | [package.metadata.deb.variants.mergesrcdest]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
wanted exactly 1 element, more than 1 element
kornelski commented 9 months ago

Thanks for the pr. This looks more fancy than I expected.

juliusl commented 9 months ago

@kornelski No problem, will you update the docs or should I update them w/ this pr?

Edit Went ahead and updated the docs. Let me know if I need to add anything else.

kornelski commented 9 months ago

Thank you