Skip to content

Conversation

@cyyever
Copy link
Contributor

@cyyever cyyever commented Oct 15, 2025

Description

This PR contains the below changes:

  1. Fixes [Bug] ONNX_OPERATOR_SCHEMA macro incompatible with new OpSchemaRegisterOnce definition (causes type conversion error) #7388 by removing explicit decoration from class OpSchemaRegisterOnce.
  2. Adds a unit test to the CMake integration test.
  3. Fix other errors detected by the unit test. Most importantly, because some protobuf generated classes are used by OpSchemaRegisterOnce, and that there is no easy way to selectively export these symbols, we have to export all protobuf symbols.

Motivation and Context

To keep backward compatibility.

@codecov
Copy link

codecov bot commented Oct 15, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 54.45%. Comparing base (a5a638d) to head (5cfa004).
⚠️ Report is 29 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7390   +/-   ##
=======================================
  Coverage   54.45%   54.45%           
=======================================
  Files         511      511           
  Lines       31876    31876           
  Branches     2868     2868           
=======================================
  Hits        17359    17359           
  Misses      13734    13734           
  Partials      783      783           

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

@cyyever cyyever force-pushed the fix_bug4 branch 6 times, most recently from fad2547 to ff54e8d Compare October 15, 2025 08:22
@github-project-automation github-project-automation bot moved this from In progress to Reviewer approved in PR Tracker Oct 15, 2025
@justinchuby
Copy link
Member

Can we have a test for this?

@cyyever
Copy link
Contributor Author

cyyever commented Oct 15, 2025

@justinchuby Sadly it is not covered. I cannot conceive an OpSchema for the unit test, any idea?

@justinchuby
Copy link
Member

Do you mean you need a way to create a custom schema?

@cyyever
Copy link
Contributor Author

cyyever commented Oct 15, 2025

@justinchuby Yes, do you have an example?

@justinchuby
Copy link
Member

@cyyever cyyever requested a review from a team as a code owner October 15, 2025 23:49
@cyyever
Copy link
Contributor Author

cyyever commented Oct 16, 2025

A case has been added to CMake test.

@cyyever cyyever force-pushed the fix_bug4 branch 5 times, most recently from 89da743 to d17b5bc Compare October 16, 2025 02:29
@justinchuby justinchuby added this to the 1.20 milestone Oct 16, 2025
@cyyever cyyever force-pushed the fix_bug4 branch 2 times, most recently from 7199536 to 859aaa9 Compare October 16, 2025 03:16
@cyyever cyyever added the run release CIs Use this label to trigger release tests in CI label Oct 16, 2025
@titaiwangms
Copy link
Contributor

Updating the branch and enable auto-merge

@titaiwangms
Copy link
Contributor

Some CIs won't run.

@titaiwangms titaiwangms disabled auto-merge October 16, 2025 17:56
Signed-off-by: Yuanyuan Chen <cyyever@outlook.com>
Signed-off-by: Yuanyuan Chen <cyyever@outlook.com>
Signed-off-by: Yuanyuan Chen <cyyever@outlook.com>
Signed-off-by: Yuanyuan Chen <cyyever@outlook.com>
@cyyever cyyever added run release CIs Use this label to trigger release tests in CI and removed run release CIs Use this label to trigger release tests in CI labels Oct 17, 2025
@cyyever
Copy link
Contributor Author

cyyever commented Oct 17, 2025

@titaiwangms There was a syntactic error in main.yaml. It is fixed and CI is green.

@titaiwangms titaiwangms enabled auto-merge (squash) October 17, 2025 00:16
@titaiwangms titaiwangms merged commit 595a069 into onnx:main Oct 17, 2025
87 checks passed
@github-project-automation github-project-automation bot moved this from Reviewer approved to Done in PR Tracker Oct 17, 2025
@cyyever cyyever deleted the fix_bug4 branch October 17, 2025 00:30
@titaiwangms titaiwangms added the module: utility Helper modules label Oct 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module: utility Helper modules run release CIs Use this label to trigger release tests in CI

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Bug] ONNX_OPERATOR_SCHEMA macro incompatible with new OpSchemaRegisterOnce definition (causes type conversion error)

3 participants