Skip to content

Conversation

@ToastCheng
Copy link
Contributor

@ToastCheng ToastCheng commented Sep 1, 2025

After adding the Cirq to OSS-Fuzz project list, we'll need to configure GitHub action to run the CI fuzzer. This PR basically follows https://google.github.io/oss-fuzz/getting-started/continuous-integration/ to enable the workflow.

Partially implements #7515

@ToastCheng ToastCheng requested review from a team and vtomole as code owners September 1, 2025 14:38
@ToastCheng ToastCheng requested a review from maffoo September 1, 2025 14:38
@github-actions github-actions bot added the size: S 10< lines changed <50 label Sep 1, 2025
@codecov
Copy link

codecov bot commented Sep 1, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.57%. Comparing base (34cddd4) to head (d09f430).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7625   +/-   ##
=======================================
  Coverage   99.57%   99.57%           
=======================================
  Files        1102     1102           
  Lines       98425    98425           
=======================================
  Hits        98005    98005           
  Misses        420      420           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Collaborator

@pavoljuhas pavoljuhas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you for setting this up.

@pavoljuhas pavoljuhas enabled auto-merge November 13, 2025 20:07
@pavoljuhas pavoljuhas disabled auto-merge November 13, 2025 20:08
@ToastCheng
Copy link
Contributor Author

Hi @pavoljuhas! From my local test, the project name needs to be in lowercase, since it's used as a Docker image name, which has a lowercase restriction. I also updated my OSS-Fuzz commit accordingly, sorry about the confusion!

(running on OSS-Fuzz repo root)
python infra/helper.py shell Cirq
INFO:__main__:Running: docker build -t gcr.io/oss-fuzz/Cirq --file /usr/local/google/home/toastcheng/dev/qc/oss-fuzz/projects/Cirq/Dockerfile /usr/local/google/home/toastcheng/dev/qc/oss-fuzz/projects/Cirq.
[+] Building 0.0s (0/0)                                                                                                                                                                          docker:default
ERROR: failed to build: invalid tag "gcr.io/oss-fuzz/Cirq": repository name must be lowercase
ERROR:__main__:Docker build failed.

Note: this command is from OSS-Fuzz debugging doc

Change project name back to lowercase as it needs to work
as (lowercase-only) Docker image name.

Refs:

- quantumlib#7625 (comment)
- ToastCheng/oss-fuzz@7deb0b7

This reverts commit 31f8647.
@pavoljuhas pavoljuhas requested a review from mhucka November 25, 2025 02:22
@pavoljuhas
Copy link
Collaborator

Hi @pavoljuhas! From my local test, the project name needs to be in lowercase, since it's used as a Docker image name, which has a lowercase restriction. I also updated my OSS-Fuzz commit accordingly, sorry about the confusion!
...

Thank you for pointing this out! I have reverted back to the lowercase name.

On a side note, I have reassigned the review to @mhucka who has more domain knowledge with CI workflows.

Copy link
Contributor

@mhucka mhucka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ToastCheng Thank you very much for your work! I'm sorry for the number of comments in my review. I tried to provide detailed explanations for why the changes are requested; I also realize that you used the suggested workflow from the cifuzz docs so the shortcomings in the workflow are actually theirs and not yours. Still, this seemed to be a opportunity to provide some information that may help future workflow-writing efforts. I hope you don't mind.

@ToastCheng ToastCheng force-pushed the i7515-fuzz branch 4 times, most recently from 09af7fd to bdba2c0 Compare November 29, 2025 12:02
@ToastCheng
Copy link
Contributor Author

@mhucka Thank you for the suggestions and detailed explanation! I have updated the PR accordingly.

One thing I found tricky is that OSS-Fuzz doesn't have release/tag, so after the OSS-Fuzz side PR merged (so that oss-fuzz can recognize Cirq project), the commit sha needs to be updated.

Related issues on this topic:

@mhucka
Copy link
Contributor

mhucka commented Dec 1, 2025

@mhucka Thank you for the suggestions and detailed explanation! I have updated the PR accordingly.

One thing I found tricky is that OSS-Fuzz doesn't have release/tag, so after the OSS-Fuzz side PR merged (so that oss-fuzz can recognize Cirq project), the commit sha needs to be updated.

Related issues on this topic:

Oh, I didn't know – thanks for that info. Hmmm this is a tricky case. I'm tempted to say it's better to use @main after all, until cifuzz makes it possible to pin to a hash, but that's not ideal either. Will the PR for this simply be a trivial 2-line change? If it's anything more than a 1-time change to update the hashes, then I would say we should go back to referencing their main.

@mhucka
Copy link
Contributor

mhucka commented Dec 1, 2025

@ToastCheng Also, is the current failure in the CIFuzz job due to the hash issue? Would using @main instead of version hashes avoid that?

If yes, I propose this approach:

  1. reference main instead of the hash for this PR, so that it runs without error
  2. in a subsequent PR, change it to the appropriate hashes once those are known

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size: S 10< lines changed <50

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants