Conversation
oliverchang
left a comment
There was a problem hiding this comment.
thanks! some surface level / style comments for now, will take a closer look!
Can you please also use yapf to format these files (using
https://github.com/google/oss-fuzz/blob/master/.style.yapf for the style config?)
oss-fuzz/arvo_reproducer.py
Outdated
| 'BuildData', ['project_name', 'engine', 'sanitizer', 'architecture']) | ||
| warnings.filterwarnings("ignore", category=UserWarning, module="google.auth._default") | ||
| logging.basicConfig(level=logging.INFO, format='%(asctime)s - %(levelname)s - %(message)s') | ||
| class DfTool(): |
There was a problem hiding this comment.
nit: I assume this is a Df is short for Dockerfile?
Could we please rename this DockerfileModifier to be clearer?
There was a problem hiding this comment.
nit: I assume this is a
Dfis short for Dockerfile?Could we please rename this
DockerfileModifierto be clearer?
I fixed the naming issue at c499d6
oss-fuzz/arvo_reproducer.py
Outdated
| return new/self.name | ||
| except: | ||
| return False | ||
| def getRecentCommit(self,commits_list,time_seconds=3600*24*(2.5)): |
There was a problem hiding this comment.
can we consistently rename functions to have snake_case ?
There was a problem hiding this comment.
can we consistently rename functions to have
snake_case?
The scripts' naming style is changed in b2541b
oss-fuzz/arvo_reproducer.py
Outdated
| else: | ||
| return False | ||
| return tmp | ||
| class GitTool(): |
There was a problem hiding this comment.
nit: this is more like a VersionControlTool instead of just GitTool right?
There was a problem hiding this comment.
nit: this is more like a
VersionControlToolinstead of justGitToolright?
The naming issue is fixed at 2f3f6e
oss-fuzz/arvo_reproducer.py
Outdated
| if res['project'] == "NOTFOUND": | ||
| res['project'] = res['job_type'].split("_")[-1] | ||
| return res | ||
| def fetchIssue(localId): |
There was a problem hiding this comment.
can you add a TODO here to replace this with proper issue tracker API calls?
There was a problem hiding this comment.
can you add a TODO here to replace this with proper issue tracker API calls?
Issue was fixed in a1c8d5
oss-fuzz/arvo_reproducer.py
Outdated
| if not file.exists(): | ||
| return True | ||
| dft = DfTool(file) | ||
| if pname == "uwebsockets": |
There was a problem hiding this comment.
does it make sense to put this inside arvo_data.py instead so we have a single place for these one-off replacements?
oss-fuzz/arvo_reproducer.py
Outdated
| dft.replaceOnce(r'RUN apt',"RUN apt update -y && apt install git ca-certificates -y && git config --global http.sslVerify false && git config --global --add safe.directory '*'\nRUN apt") | ||
| dft.strReplaceAll(globalStrReplace) | ||
|
|
||
| if project == "lcms": |
There was a problem hiding this comment.
same comment here -- can we add these per-project hacks in arvo_data.py ?
…emove unecessary funcs
Thanks so much for the code review; I really appreciate it! And I am sorry for the mental damage caused by my code since I just made the code run, but not for reading. I was kind of too busy to refactor it, but I cleaned it a bit now, and I'll reply to each comment with related updates. For the style issue, I fixed it at 10a0b0. |
|
fix: an obsolete feat
There was a problem hiding this comment.
thank you for addressing the comments!
I have some more suggestions to make the code a bit more readable and maintainable.
Do you also want to move this PR over to the main oss-fuzz repo ?
I think we can create it under infra/experimental/contrib/arvo.
This adds fuzzing support for Arvo under the experimental contrib directory. Original work from: n132/ARVO#6
|
Thanks for the review! I've moved this PR to the main oss-fuzz repo at google/oss-fuzz#13897. Signed the CLA form. Please Let us know if there are more action items needed from our end. P.S. I had some trouble figuring out email problems with CLA form due to commit history. Inspired by this, I decided to change all my commit's username/email to a single one. No files changed for the last push. |
"Add ARVO fuzzing infrastructure This adds ARVO under the experimental contrib directory. Original work from: n132/ARVO#6 Paper: https://arxiv.org/abs/2408.02153" --------- Co-authored-by: Oliver Chang <oliverchang@users.noreply.github.com>
New Feat:
ARVO reproducer for OSS-Fuzz
Done:
Todo: