Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

Update clap to remove suppressions #2856

Merged
merged 2 commits into from
Feb 19, 2023
Merged

Update clap to remove suppressions #2856

merged 2 commits into from
Feb 19, 2023

Conversation

Porges
Copy link
Member

@Porges Porges commented Feb 17, 2023

A Friday afternoon jaunt.

To be merged after #2855; does the remaining work to close #2277 (and closes off some things that Dependabot was trying to upgrade).

Tested by manually running the commands; we don't have good coverage for this kind of stuff. OTOH, most of these commands are for the experimental local fuzzing mode, which is not fully supported yet. I did specifically test the onefuzz-task managed command which is the one used in production.

Details

  • Bump clap to 4.1.6
    • Remove structopt as this is subsumed by clap now
  • Bump envlogger to 0.10 (removes problematic dependency)
  • Set default-features=false on proc-maps (removes a feature which is only needed to support FreeBSD), and bump it to 0.3

The main changes migrating clap are:

  • value_t! is gone; now use matches.get_one::<T>. If T is not String then a parser must have been registered on the Arg when it was created, with arg.value_parser(value_parser!(T)).
  • Command::with_name and Arg::with_name are now called new.
  • Command and Subcommand were unified, and App is removed.
  • arg.takes_value(true) is gone; it is the default. For flags use arg.action(ArgAction::SetTrue) and then retrieve the flag value with matches.get_flag.

This code would be simplified a lot by using the clap::Parser on structs, but that requires reworking the code significantly as we cannot dynamically add/remove arguments the way that this is currently done.

Also found

Found one bug while manually testing the onefuzz-task local commands; see comment below.

@codecov-commenter
Copy link

codecov-commenter commented Feb 17, 2023

Codecov Report

Merging #2856 (3f6edba) into main (1b07b7d) will increase coverage by 0.00%.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##             main    #2856   +/-   ##
=======================================
  Coverage   28.09%   28.09%           
=======================================
  Files         302      302           
  Lines       35629    35627    -2     
=======================================
  Hits        10011    10011           
+ Misses      25618    25616    -2     
Impacted Files Coverage Δ
...c/agent/dynamic-library/src/bin/dynamic-library.rs 2.32% <0.00%> (-0.06%) ⬇️
src/agent/onefuzz-agent/src/main.rs 0.42% <ø> (+<0.01%) ⬆️
src/agent/onefuzz-task/src/local/cmd.rs 0.00% <0.00%> (ø)
src/agent/onefuzz-task/src/local/common.rs 0.00% <0.00%> (ø)
src/agent/onefuzz-task/src/local/coverage.rs 0.00% <0.00%> (ø)
...c/agent/onefuzz-task/src/local/generic_analysis.rs 0.00% <0.00%> (ø)
...ent/onefuzz-task/src/local/generic_crash_report.rs 0.00% <0.00%> (ø)
.../agent/onefuzz-task/src/local/generic_generator.rs 0.00% <0.00%> (ø)
src/agent/onefuzz-task/src/local/libfuzzer.rs 0.00% <0.00%> (ø)
...t/onefuzz-task/src/local/libfuzzer_crash_report.rs 0.00% <0.00%> (ø)
... and 22 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

src/deny.toml Show resolved Hide resolved
@Porges Porges requested a review from chkeita February 17, 2023 02:29
Base automatically changed from bump-clap-proxy to main February 19, 2023 21:17
@Porges Porges force-pushed the remove-cargo-denies branch from b299509 to 3f6edba Compare February 19, 2023 21:18
@Porges Porges enabled auto-merge (squash) February 19, 2023 21:25
@Porges
Copy link
Member Author

Porges commented Feb 19, 2023

Passed check-pr.

@Porges Porges merged commit cd18c60 into main Feb 19, 2023
@Porges Porges deleted the remove-cargo-denies branch February 19, 2023 21:45
@mgreisen mgreisen mentioned this pull request Mar 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace uses of ansi_term
3 participants