Address feedback on new delete-nonreduced-fuzz-inputs script in rust#270
Conversation
|
|
||
| let all_inputs_dir = move_fuzz_inputs()?; | ||
| git_commit_all(QA_ASSETS_PATH, "Delete fuzz inputs")?; | ||
| // git commit -a does not add untracked files so we use git add manually |
There was a problem hiding this comment.
| // git commit -a does not add untracked files so we use git add manually | |
| // git commit does not commit untracked removed files |
I think this can be removed, or reworded?
There was a problem hiding this comment.
Isn’t it important to specifically mention that this is about the staging behavior of -a? To me, this reads confusingly, since git commit does not commit anything unless you stage it beforehand.
| // git commit -a does not add untracked files so we use git add manually | |
| // git commit -a does not commit untracked removed files |
edit: did this in b89fa64
Yeah, it is a bit long running. Though, the steps may not take equal time, as libfuzzer+sanitizers or AFL may take a longer time than the parallelized libfuzzer run without sanitizers. I usually also take a look at the current git commit (or the output of |
Addressed feedback: * use relative link in README * refactor argument parsing to be easier to extend * drops 'too many arguments' error message * Use ?; consistently * avoid temporary copy when building arguments for Command::new * replace wget with curl * also use --depth=1 when cloning AFLplusplus * use git add + git commit consistently instead of git commit -a * don't require -Znext-lockfile-bump with cargo Additionally: * fix git clone error message did not use format! macro * add .gitignore to ignore cargo target/
3f5ac2d to
b89fa64
Compare
I will run the script many more times when working on #265. I will then look into a progress bar more, and see if it's worth it
The script just finished. New data points wrt time and determinism: Previously in #269 (comment): Only 14 files less now! |
Addressed feedback:
Command::newwgetwithcurl--depth=1when cloning AFLplusplusgit add+git commitconsistently instead ofgit commit -a-Znext-lockfile-bumpwithcargoAdditionally:
git_cloneerror message did not useformat!macroAll in one commit because I think this is easy enough to review, and not "mission-critical code" anyway. Lmk if you would prefer it differently.
This is currently running, but lgtm.
Btw, I'm considering improving the UX/DX: We could pipe the current output to a file and show a progress bar on stdout (without adding a dependency). The progress bar would indicate how many fuzz targets
afl-cminhas already processed, at which sanitizer step we are, and maybe there's more useful progress information we can get by looking at the current output. This would help with knowing how long it might still take. I also always want to know how long it took to know what to expect next time, but currently, I have to remember to run the script withtimeor look at the commit timestamps to get an idea.With #265, the script will take even longer; making improvements to its long-running nature more important.