Revert ONNX CMake changes by cyyever · Pull Request #7515 · onnx/onnx

cyyever · 2025-12-11T01:15:29Z

Motivation and Context

It turns out that the CMake object libraries have portability issues on some platforms.
Fixes #7514

codecov · 2025-12-11T02:00:35Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 55.25%. Comparing base (82c1c48) to head (700e6e6).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #7515   +/-   ##
=======================================
  Coverage   55.25%   55.25%           
=======================================
  Files         513      513           
  Lines       32213    32213           
  Branches     2895     2895           
=======================================
  Hits        17799    17799           
  Misses      13620    13620           
  Partials      794      794

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

justinchuby

Thanks - would it be possible to create a CI pipeline to test the xcode generator?

cyyever · 2025-12-11T04:46:46Z

@justinchuby We can, but should be in another PR

Signed-off-by: Yuanyuan Chen <cyyever@outlook.com>

Signed-off-by: cyy <cyyever@outlook.com>

justinchuby · 2025-12-12T00:34:02Z

One test failing?

cyyever · 2025-12-12T00:39:26Z

@justinchuby Looks like it is flaky.

Signed-off-by: cyy <cyyever@outlook.com>

Copilot

Pull request overview

This PR reverts the CMake object library approach that was causing portability issues on some platforms, as reported in issue #7514. The changes simplify the build system by returning to a more traditional library structure while maintaining the same functionality.

Key changes:

Reverted from OBJECT libraries (onnx_object, onnx_proto_object) back to regular libraries
Consolidated compile options and configuration into reusable CMake functions
Fixed loop variable types in C++ code to use size_t with explicit casting to avoid type warnings

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
`onnx/cpp2py_export.cc`	Changed loop variables from `auto` to `size_t` with explicit casting to `int` for Input/Output calls to avoid signed/unsigned comparison warnings
`cmake/summary.cmake`	Updated target references from object library targets (`onnx_object`, `onnx_proto_object`) to regular library targets (`onnx`, `onnx_proto`)
`cmake/Utils.cmake`	Added `add_onnx_compile_options` function to centralize compile options, flags, and library linking; reformatted existing functions for consistency
`CMakeLists.txt`	Removed object library definitions and simplified build to use regular libraries; updated `nanobind_add_module` to include sources directly instead of linking object libraries

justinchuby · 2025-12-12T18:54:43Z

Copilot comments look reasonable

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Yuanyuan Chen <cyyever@outlook.com>

cyyever · 2025-12-12T23:39:19Z

@justinchuby Fixed

justinchuby · 2025-12-13T01:23:02Z

Thank you

### Motivation and Context It turns out that the CMake object libraries have portability issues on some platforms. Fixes #7514 --------- Signed-off-by: Yuanyuan Chen <cyyever@outlook.com> Signed-off-by: cyy <cyyever@outlook.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

- #7505 - #7515 - #7524 - #7525 --------- Signed-off-by: cyy <cyyever@outlook.com> Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com> Signed-off-by: Yuanyuan Chen <cyyever@outlook.com> Co-authored-by: Yuanyuan Chen <cyyever@outlook.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

franramirez688 · 2026-01-21T10:18:51Z

Hi @cyyever @justinchuby

I had a local test which ran this case (similar to ONNXRUNTIME linking with ONNX):

# . . .

find_package(ONNX REQUIRED CONFIG)

add_executable(${PROJECT_NAME} test_package.cpp)
target_link_libraries(${PROJECT_NAME} PRIVATE onnx onnx_proto)

It compiled and linked fine, building as a dynamic library. But I noticed that using the 1.20.x release, using the same example and built as a dynamic one, I got this:

onnx/1.20.1 (test package): Running test()
onnx/1.20.1 (test package): RUN: ./test_package
WARNING: All log messages before absl::InitializeLog() is called are written to STDERR
E0000 00:00:1768982174.694810 6980831 descriptor_database.cc:678] File already exists in database: onnx/onnx-ml.proto
F0000 00:00:1768982174.696511 6980831 descriptor.cc:2518] Check failed: GeneratedDatabase()->Add(encoded_file_descriptor, size)
*** Check failure stack trace: ***
    @        0x100202488  absl::lts_20250814::log_internal::LogMessage::Flush()
    @        0x100d6a854  google::protobuf::DescriptorPool::InternalAddGeneratedFile()
    @        0x100e0c928  google::protobuf::internal::AddDescriptors()
    @        0x19c27acb0  ___ZNK5dyld46Loader25findAndRunAllInitializersERNS_12RuntimeStateE_block_invoke
    @        0x19c2b8730  ___ZNK5dyld313MachOAnalyzer18forEachInitializerER11DiagnosticsRKNS0_15VMAddrConverterEU13block_pointerFvjEPKv_block_invoke.204
    @        0x19c2d7540  ___ZNK6mach_o6Header14forEachSectionEU13block_pointerFvRKNS0_11SectionInfoERbE_block_invoke
    @        0x19c2d4164  mach_o::Header::forEachLoadCommand()
    @        0x19c2d59fc  mach_o::Header::forEachSection()
    @        0x19c2b8220  dyld3::MachOAnalyzer::forEachInitializer()
    @        0x19c27aa68  dyld4::Loader::findAndRunAllInitializers()
    @        0x19c2828b0  dyld4::JustInTimeLoader::runInitializers()
    @        0x19c27b214  dyld4::Loader::runInitializersBottomUp()
    @        0x19c27b1b4  dyld4::Loader::runInitializersBottomUp()
    @        0x19c27fe50  dyld4::Loader::runInitializersBottomUpPlusUpwardLinks()::$_0::operator()()
    @        0x19c27b530  dyld4::Loader::runInitializersBottomUpPlusUpwardLinks()
    @        0x19c29d04c  dyld4::APIs::runAllInitializersForMain()
    @        0x19c25f158  dyld4::prepare()
    @        0x19c25dd04  start
/bin/sh: line 1: 64127 Abort trap: 6           ./test_package

ERROR: onnx/1.20.1 (test package): Error in test() method, line 26
	self.run(bin_path, env="conanrun")
	ConanException: Error 134 while executing

I think that it could be related to the fact that onnx target now also includes the ONNX_PROTO_SRCS https://github.com/onnx/onnx/pull/7515/changes#diff-1e7de1ae2d059d21e1dd75d5812d5a34b0222cef273b7c3a2af62eb747f9d20aR418

My question is:

Is that expected? Now, should I only link against onnx, as it embeds those sources too?
If this is not expected, do you want me to open a fresh issue?

Thanks in advance!

cyyever · 2026-01-22T02:03:39Z

@franramirez688 Can you reproduce it with ONNX 1.20.1? If so, could you provide the problematic command?

franramirez688 · 2026-01-23T15:45:39Z

@franramirez688 Can you reproduce it with ONNX 1.20.1? If so, could you provide the problematic command?

@cyyever I just uploaded a little repro-case here: https://github.com/franramirez688/repro-onnx-1_20_1
It's using Conan, but it's not going to pollute your local environment; it'll be contained within that folder. Anyway, if you want me to provide another repro case without Conan, let me know.

cyyever requested a review from a team as a code owner December 11, 2025 01:15

github-project-automation Bot added this to PR Tracker Dec 11, 2025

github-project-automation Bot moved this to In progress in PR Tracker Dec 11, 2025

cyyever force-pushed the drop_onnx_object branch 4 times, most recently from 4496608 to e3bb2d3 Compare December 11, 2025 01:31

cyyever added the topic: build Issues related to ONNX builds and packages label Dec 11, 2025

cyyever force-pushed the drop_onnx_object branch 3 times, most recently from 09a9f2e to 38ee775 Compare December 11, 2025 01:46

cyyever added the run release CIs Use this label to trigger release tests in CI label Dec 11, 2025

cyyever changed the title ~~Revert ONNX CMake libraries~~ Revert ONNX CMake changes Dec 11, 2025

cyyever force-pushed the drop_onnx_object branch from 07d22d3 to 254f9f6 Compare December 11, 2025 02:28

justinchuby approved these changes Dec 11, 2025

View reviewed changes

github-project-automation Bot moved this from In progress to Reviewer approved in PR Tracker Dec 11, 2025

justinchuby requested a review from titaiwangms December 11, 2025 04:52

cyyever added 3 commits December 11, 2025 19:51

Revert ONNX CMake libraries

872a931

Signed-off-by: Yuanyuan Chen <cyyever@outlook.com>

More fixes

601ea7f

Signed-off-by: Yuanyuan Chen <cyyever@outlook.com>

More fixes

e33a42d

Signed-off-by: Yuanyuan Chen <cyyever@outlook.com>

cyyever force-pushed the drop_onnx_object branch from b099eb4 to e33a42d Compare December 11, 2025 11:51

cyyever added 3 commits December 11, 2025 20:31

More fixes

79cb1be

Signed-off-by: cyy <cyyever@outlook.com>

More fixes

8c35e3d

Signed-off-by: cyy <cyyever@outlook.com>

More fixes

490ef94

Signed-off-by: cyy <cyyever@outlook.com>

titaiwangms mentioned this pull request Dec 11, 2025

Integration with ONNX==1.20.1 microsoft/onnxruntime#26579

Merged

Merge branch 'main' into drop_onnx_object

297a8e9

Fix builds

b6a2443

Signed-off-by: cyy <cyyever@outlook.com>

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 Dec 12, 2025

Fix builds

55708f9

Signed-off-by: cyy <cyyever@outlook.com>

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 Dec 12, 2025

justinchuby approved these changes Dec 12, 2025

View reviewed changes

justinchuby requested a review from Copilot December 12, 2025 18:18

justinchuby added this to the 1.20.1 milestone Dec 12, 2025

Copilot started reviewing on behalf of justinchuby December 12, 2025 18:19 View session

Copilot AI reviewed Dec 12, 2025

View reviewed changes

Comment thread cmake/Utils.cmake Outdated

Comment thread CMakeLists.txt Outdated

cyyever and others added 3 commits December 13, 2025 07:38

Update cmake/Utils.cmake

5ff8a04

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Yuanyuan Chen <cyyever@outlook.com>

Update CMakeLists.txt

ab65df4

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Yuanyuan Chen <cyyever@outlook.com>

Merge branch 'main' into drop_onnx_object

700e6e6

justinchuby merged commit 1d2f1d3 into onnx:main Dec 13, 2025
40 of 41 checks passed

github-project-automation Bot moved this from Reviewer approved to Done in PR Tracker Dec 13, 2025

cyyever deleted the drop_onnx_object branch December 13, 2025 01:33

justinchuby mentioned this pull request Jan 8, 2026

Cherry pick into 1.20.1 #7560

Merged

franramirez688 mentioned this pull request Jan 28, 2026

onnx and onnxruntime bump versions (fixed onnx dependency with shared=True) conan-io/conan-center-index#29231

Closed

BrewTestBot mentioned this pull request Mar 27, 2026

onnx 1.21.0 Homebrew/homebrew-core#274669

Merged

Conversation

cyyever commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation and Context

Uh oh!

codecov Bot commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

justinchuby left a comment

Choose a reason for hiding this comment

Uh oh!

cyyever commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

justinchuby commented Dec 12, 2025

Uh oh!

cyyever commented Dec 12, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

justinchuby commented Dec 12, 2025

Uh oh!

cyyever commented Dec 12, 2025

Uh oh!

Uh oh!

justinchuby commented Dec 13, 2025

Uh oh!

franramirez688 commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cyyever commented Jan 22, 2026

Uh oh!

franramirez688 commented Jan 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cyyever commented Dec 11, 2025 •

edited

Loading

codecov Bot commented Dec 11, 2025 •

edited

Loading

cyyever commented Dec 11, 2025 •

edited

Loading

franramirez688 commented Jan 21, 2026 •

edited

Loading