-
Notifications
You must be signed in to change notification settings - Fork 55
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
tuftool: Add clone
subcommand
#404
Conversation
c277a55
to
4257c49
Compare
^ Fix the clippy complaint - I didn't see it when I ran the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice testing.
fn clone_base_command<'a>(cmd: &'a mut Command, repo_paths: &RepoPaths) -> &'a mut Command { | ||
cmd.args(&[ | ||
"clone", | ||
"--root", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice tests! Should we have one for --allow-root-download
as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point - I'll add one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:( Bad news everyone! We use reqwest
to get the root.json
, and reqwest
doesn't support file:///
URLs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think what we normally do is add a branch, where if url.scheme() is file, we just std::fs::copy it instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened a new issue to track this: #406
This change adds a `download_root` module, which abstracts the logic of downloading `root.json` out of `download` (the subcommand). It also makes the HTTP request a bit more robust by handling the initial request error as well as a bad response. Additionally, it avoids creating the file until after the request has succeeded to avoid creating cruft on the user's system.
This change updates the `download` module/subcommand to make use of the previously added `download_root` function. It also defines a default of "1" for the `root_version` argument. Previously, we effectively had this default in code by using `1.root.json` in the event the argument wasn't passed. It also has the nice side effect of not needing to deal with an `Option` for this argument.
4257c49
to
37b3596
Compare
^ addressed all the comments |
This adds a `clone` subcommand to `tuftool`, allowing a user to download a fully functioning TUF repository. A user has the option to download a full repository, a subset of the targets, or just metadata.
37b3596
to
e62cfee
Compare
^ removed a dangling |
Issue #, if available:
Fixes #401
Description of changes:
This PR does a small amount of refactoring before the main event, a new
clone
subcommand! This subcommand gives the user the opportunity to clone a repository and it's metadata, including all, none or some of the targets. It can be considered a bit more "low-level" thantuftool download
as the files end up on disk identically named as they are in the TUF repository, including the SHA256 prefix.tuftool download
also does not give the user the opportunity to download metadata.Reading through by commit is probably easiest as they're fairly small.
Testing:
root.json
and with the flag--allow-root-download
)./foo
(withroot.json
and with the flag--allow-root-download
)manifest.json
) (withroot.json
and with the flag--allow-root-download
)--metadata-only
flagBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.