{{ message }}
Fuzzer Migration Follow-ups#1903
Merged
Byron merged 10 commits intogitpython-developers:mainfrom Apr 18, 2024
Merged
Conversation
Prefer executing these files using the OSS-Fuzz or `python` command methods outlined in the `fuzzing/README`. Based on feedback and discussion on: gitpython-developers#1901
This script is meant to be sourced by the OSS-Fuzz file of the same name, rather than executed directly. The shebang may lead to the incorrect assumption that the script is meant for direct execution. Replacing it with this directive instructs ShellCheck to treat the script as a Bash script, regardless of how it is executed. Based @EliahKagan's suggestion and feedback on: gitpython-developers#1901
This script is executed directly, not sourced as is the case with `build.sh`, so it should have an executable bit set to avoid ambiguity. Based @EliahKagan's suggestion and feedback on: gitpython-developers#1901
- Make the link text for the OSS-Fuzz test status URL more descriptive - Fix capitalization of GitPython repository name Based @EliahKagan's suggestion and feedback on: gitpython-developers#1901
Replaces the null character delimiter `-d $'\0'` with the simpler empty string `-d ''` in the fuzzing harness build loop. This changes leverages the Bash `read` builtin behavior to avoid unnecessary complexity and improving script readability. Based @EliahKagan's suggestion and feedback on: gitpython-developers#1901
Based @EliahKagan's suggestion and feedback on: gitpython-developers#1901
A misspelling in the https://github.com/gitpython-developers/qa-assets repository is still present here. It will need to be fixed in that repository first. "corpora" is a difficult word to spell consistently I guess. This made for a good opportunity to improve the phrasing of two other comments at at least. Based @EliahKagan's suggestion and feedback on: gitpython-developers#1901
EliahKagan
approved these changes
Apr 17, 2024
Member
EliahKagan
left a comment
EliahKagan
left a comment
There was a problem hiding this comment.
This looks good.
I wonder if corpra should be renamed to corpora in that path as well, or if that would cause problems.
If and only if that is changed, this line would then need to be changed accordingly:
Unrelated to that, there is a tiny style nit commented on below (which you may even decide to leave as-is).
Contributor
Author
Also makes come structural improvements to how the local instructions for running OSS-Fuzz are presented now that only the single `address` sanitizer is a valid option. The `undefined` sanitizer was removed from GitPython's `project.yaml` OSS-Fuzz configuration file at the request of OSS-Fuzz project reviewers in google/oss-fuzz#11803. The `undefined` sanitizer is only useful in Python projects that use native exstensions (such as C, C++, Rust, ect.), which GitPython does not currently do. This commit updates the `fuzzing/README` reference to that sanitizer accoirdingly. See: - google/oss-fuzz@b210fb2 - google/oss-fuzz#11803 (comment)
EliahKagan
approved these changes
Apr 18, 2024
Byron
approved these changes
Apr 18, 2024
Member
Byron
left a comment
Byron
left a comment
There was a problem hiding this comment.
Thanks a lot for this follow-up and for sharing your plan of the additional improvements.
Not using zip blobs stored in the qa-assets repository is my favorite :).
DaveLak
added a commit
to DaveLak/oss-fuzz
that referenced
this pull request
Apr 18, 2024
The upstream GitPython repository merged a change that set the executable bit on `container-environment-bootstrap.sh` in Git, so the `chmod` is no longer needed and the `RUN` instruction can execute the script directly. See: - gitpython-developers/GitPython#1903 - gitpython-developers/GitPython@b0a5b8e
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR addresses most unresolved review comments from #1901. It also updates the README
Addressed in This PR
build.shwith ShellCheck directive (Comment)fuzzing/oss-fuzz-scripts/container-environment-bootstrap.shto mark it executable in Git (Comment)fuzzing/README.mdfor the OSS-Fuzz test status URL more descriptive (Comment)fuzzing/README.md(Comment)build.sh's build fuzz harness loop (Comment)fuzzing/oss-fuzz-scripts/container-environment-bootstrap.shfor consistent script formatting (Comment)Misc
undefinedas a valid option in google/oss-fuzz@b210fb2(#11803) (Context: Comment in OSS-Fuzz PR #11803)TODO / Pending Further Discussion
I felt that these items are better addressed in separate PRs.
Add GitPython's standard license header comments to non-Apache 2.0 licensed files (Comment)Done in Add GitPython's Standard License Header Comments to Shell Scripts #1907fuzzing/fuzz-targets/fuzz_tree.pyrefactoring (Comment 1 & Comment 2)$SRC,$OUT, &$WORK) infuzzing/oss-fuzz-scripts(Comment)