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

feat: Experimental subcommand-based interface (via @ok-nick) #729

Open
wants to merge 62 commits into
base: main
Choose a base branch
from

Conversation

scouten-adobe
Copy link
Collaborator

Changes in this pull request

Rework the CLI to use subcommands rather than optional arguments.

  • Introduces subcommands as follows
    • c2patool sign - sign manifest
    • c2patool extract - extract data
      • c2patool extract resources - extract resources
      • c2patool extract ingredient - extract ingredient json and binary manifest
      • c2patool extract manifest - extract json manifest (can extract .c2pa manifest with --binary )
    • c2patool view - view info about manifest
      • c2patool view manifest - view user-friendly json manifest (can specify --debug)
      • c2patool view ingredient - view json ingredient
      • c2patool view info - view general info about a manifest
      • c2patool view tree - view manifest tree
      • c2patool view certs - view manifest certs
  • More intuitive
    • With the old one, it's valid to specify c2patool i.jpg --info --certs --tree --detailed --manifest manifest.json --output o.jpg trust, but what's actually happening?
    • With the new one, it's impossible to make an ambiguous call, for each subcommand you can only specify the arguments relevant to that subcommand (otherwise clap will output a user-friendly error w/ suggestions)
    • Because of this, --help menus are much shorter and navigable, describing what subcommands/args are valid to specify
    • Subcommands also follow a familiar interface, similar to existing tools such as cargo, rustup, cross, git, etc.
  • Easier to modularize
    • Subcommands can be broken down into logical units, making it significantly easier to add new subcommands and modify existing ones
    • Subcommands can be constructed individually and tested individually
  • Migrated to the (new) c2pa-rs unstable API

Additional Changes

  • Added insta-rs snapshot integration tests (asserts outputs against a reference value)
  • Added --binary flag to extract manifest as .c2pa binary manifest
  • Added --no-verify flag to allow extracting invalid binary manifests for debugging
  • Added --unknown flag to additionally extract unstandardized labeled resources
  • Added --no-embed flag when signing to not embed a manifest in the asset.
  • Added glob as input path to sign multiple assets w/ the same manifest
  • Added --verbose flag to specify verbosity via flags (rather than only env vars)
  • Changed args to kebab-case (default) rather than a mix of kebab-case and snake_case
  • Changed --no-verify-signing to --no-verify
  • Changed check to allow different file extensions for input/output if --sidecar is specified
  • Changed ingredient_paths to be merged into ingredients field, specifying path or inline ingredient in .json
  • Changed --remote to --manifest-url and applied the same suffix-style pattern to trust args
  • Removed extracting ingredient .json also extracts .c2pa binary manifest (use extract manifest --binary)
  • Removed --remote in favor of specifying path or url in --manifest (similar to trust args)

Uncertain Changes

  • Removed checking for child ingredient.json if directory specified as ingredient path
  • Removed passing manifest as string via --config
  • Removed displaying manifest after signing (what would happen if multiple files are signed?)

Examples

# old
c2patool some_dir/C.jpg
# new - exactly the same
c2patool some_dir/C.jpg
# new - the above command redirects to below
c2patool view manifest some_dir/C.jpg
# new - let's view a detailed (debug) manifest
c2patool view manifest some_dir/C.jpg --debug

# old - let's view the certs
c2patool some_dir/C.jpg --certs
# new - the view subcommand contains: manifest, ingredient, info, tree, certs
c2patool view certs some_dir/C.jpg

# old - let's sign a manifest
c2patool some_dir/C.jpg --manifest some_dir/ingredient_test.json --output some_dir/C-signed.jpg
# new - very similar, it's now locked behind a subcommand
c2patool sign some_dir/C.jpg --manifest some_dir/ingredient_test.json --output some_dir/C-signed.jpg
# new - let's sign jpg files recursively
c2patool sign some_dir/**/*.jpg --manifest some_dir/ingredient_test.json --output some_dir/

# old - let's extract resources
c2patool some_dir/C.jpg --output some_dir
# new - now our intention is clear
c2patool extract resources some_dir/C.jpg --output some_dir
# new - I want to extract resources for all jpg files in some_dir
c2patool extract resources some_dir/*.jpg --output some_dir

Screenshots

c2patool
c2patool extract
c2patool view
c2patool view manifest -h
c2patool sign -h

Checklist

  • This PR represents a single feature, fix, or change.
  • All applicable changes have been documented.
  • Any TO DO items (or similar) have been entered as GitHub issues and the link to that issue has been included in a comment.

ok-nick added 30 commits June 6, 2024 10:55
ok-nick and others added 25 commits June 13, 2024 16:32
* 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
@scouten-adobe
Copy link
Collaborator Author

Reopened from contentauth/c2patool#185 by @ok-nick. We'll need to clean up the conflicts, especially in the .github workflows section before merging.

@scouten-adobe scouten-adobe changed the title Experimental subcommand-based interface feat: Experimental subcommand-based interface Dec 11, 2024
@scouten-adobe scouten-adobe changed the title feat: Experimental subcommand-based interface feat: Experimental subcommand-based interface (via @oknick) Dec 12, 2024
@scouten-adobe scouten-adobe changed the title feat: Experimental subcommand-based interface (via @oknick) feat: Experimental subcommand-based interface (via @ok-nick) Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants