Experimental subcommand-based interface #185
Conversation
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #185 +/- ##
===========================================
+ Coverage 47.89% 74.43% +26.53%
===========================================
Files 4 7 +3
Lines 666 841 +175
===========================================
+ Hits 319 626 +307
+ Misses 347 215 -132 ☔ View full report in Codecov by Sentry. |
|
Nice job |
src/commands/extract.rs
Outdated
| #[clap(flatten)] | ||
| trust: Trust, | ||
| // | ||
| // TODO: add flag for additionally exporting unknown ingredients (ingredients that |
There was a problem hiding this comment.
Nice, this will be insanely useful.
There was a problem hiding this comment.
With the unstable API we should be able to support this functionality. Separate PR coming soon.
| trust, | ||
| } => { | ||
| if !path.exists() { | ||
| bail!("Input path does not exist") |
There was a problem hiding this comment.
What do you think about having distinct exit codes per error that the command encounters? We have another CLI tool for internal clients named adobe_c2patool that uses a similar subcommand model. In that tool, we have distinct exit codes so clients can easily key off those exit codes to influence logic on their end without parsing strings.
This could be separate ticket - just curious what you think.
There was a problem hiding this comment.
That's a good idea. Another option is to introduce an argument such as --format, similar to cargo's --message-format=json. It can provide detailed information about command success/failure, e.g. paths that successfully signed and paths that failed to sign. I think this would pair well with exit codes?
Here's a real example of how cargo outputs with --message-format=json (there are multiple outputs like this as the command runs):
{
"reason":"compiler-artifact",
"package_id":"registry+https://github.com/rust-lang/crates.io-index#[email protected]",
"manifest_path":"/Users/me/.cargo/registry/src/index.crates.io-6f17d22bba15001f/mockall-0.12.1/Cargo.toml",
"target":{
"kind":[
"lib"
],
"crate_types":[
"lib"
],
"name":"mockall",
"src_path":"/Users/me/.cargo/registry/src/index.crates.io-6f17d22bba15001f/mockall-0.12.1/src/lib.rs",
"edition":"2021",
"doc":true,
"doctest":true,
"test":true
},
"profile":{
"opt_level":"0",
"debuginfo":2,
"debug_assertions":true,
"overflow_checks":true,
"test":false
},
"features":[
],
"filenames":[
"/Users/me/Documents/repos/c2patool/target/debug/deps/libmockall-7bdbc60eb0277649.rlib",
"/Users/me/Documents/repos/c2patool/target/debug/deps/libmockall-7bdbc60eb0277649.rmeta"
],
"executable":null,
"fresh":true
}
There was a problem hiding this comment.
Also, it may help to restructure the program to remove its dependence on anyhow. If we provide detailed errors we can support detailed output formats ^ and assign error codes to errors via Termination.
* Foundation for migrating to unstable API * Extract resources using unstable API and introduce --unknown flag * Use unstable API for viewing * More unstable API during extraction * Use c2pa-rs branch w/ wip features * Set ingredient base paths * Use `Manifest::signature_info` to output certs * Migrate `view tree` command, rename `debug` flag to `detailed`, and update dependencies * Fix `--unknown` flag when extracting resources * Normalize URIs when extracting ingredients * Rearrange * Add `--no-embed` flag to signing` * Small changes to tests and fix comments * Use Reader in tests * More strict uri normalization and fix typo
Changes in this pull request
Rework the CLI to use subcommands rather than optional arguments.
c2patool sign- sign manifestc2patool extract- extract datac2patool extract resources- extract resourcesc2patool extract ingredient- extract ingredient json and binary manifestc2patool extract manifest- extract json manifest (can extract .c2pa manifest with--binary)c2patool view- view info about manifestc2patool view manifest- view user-friendly json manifest (can specify--debug)c2patool view ingredient- view json ingredientc2patool view info- view general info about a manifestc2patool view tree- view manifest treec2patool view certs- view manifest certsc2patool i.jpg --info --certs --tree --detailed --manifest manifest.json --output o.jpg trust, but what's actually happening?clapwill output a user-friendly error w/ suggestions)--helpmenus are much shorter and navigable, describing what subcommands/args are valid to specifycargo,rustup,cross,git, etc.Additional Changes
--binaryflag to extract manifest as .c2pa binary manifest--no-verifyflag to allow extracting invalid binary manifests for debugging--unknownflag to additionally extract unstandardized labeled resources--no-embedflag when signing to not embed a manifest in the asset.--verboseflag to specify verbosity via flags (rather than only env vars)--no-verify-signingto--no-verify--sidecaris specifiedingredient_pathsto be merged intoingredientsfield, specifying path or inline ingredient in .json--remoteto--manifest-urland applied the same suffix-style pattern to trust argsextract manifest --binary)Removed--remotein favor of specifying path or url in--manifest(similar to trust args)Uncertain Changes
ingredient.jsonif directory specified as ingredient path--configExamples
Screenshots
c2patoolc2patool extractc2patool viewc2patool view manifest -hc2patool sign -hChecklist
TO DOitems (or similar) have been entered as GitHub issues and the link to that issue has been included in a comment.