Skip to content

Simplify onnx.patch - #25204

Merged
titaiwangms merged 2 commits into
microsoft:mainfrom
titaiwangms:titaiwang/onnx_patch
Jun 28, 2025
Merged

Simplify onnx.patch#25204
titaiwangms merged 2 commits into
microsoft:mainfrom
titaiwangms:titaiwang/onnx_patch

Conversation

@titaiwangms

@titaiwangms titaiwangms commented Jun 27, 2025

Copy link
Copy Markdown
Contributor

Delete the legacy patches related to protobuf, which was added from #14279 and #15878 to simplify the ONNX patches.

@titaiwangms
titaiwangms marked this pull request as ready for review June 27, 2025 22:35
@titaiwangms
titaiwangms requested review from edgchen1 and snnn June 27, 2025 22:36
@titaiwangms

Copy link
Copy Markdown
Contributor Author

cc @snnn and @edgchen1

I am experimenting on this to see whether it passes CI. It looks like it all works out. Is there other concerns that CI can't tell us?

@snnn snnn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM.

@snnn

snnn commented Jun 28, 2025

Copy link
Copy Markdown
Contributor

I think they are enough. If our nightly package pipelines were not happy with that, we can revert this PR.

@titaiwangms
titaiwangms merged commit 849eee8 into microsoft:main Jun 28, 2025
85 of 89 checks passed
@sztaylor

sztaylor commented Jul 4, 2025

Copy link
Copy Markdown

Hi, I think my macos builds are now failing on main due to this PR. Here is the error being reported:
error: implicit conversion loses integer precision: 'long' to 'int' [-Werror,-Wshorten-64-to-32]
Is it expected that this error should now be thrown? What would you recommend as a work around?
Thank you.

@titaiwangms

Copy link
Copy Markdown
Contributor Author

Hi, I think my macos builds are now failing on main due to this PR. Here is the error being reported: error: implicit conversion loses integer precision: 'long' to 'int' [-Werror,-Wshorten-64-to-32] Is it expected that this error should now be thrown? What would you recommend as a work around? Thank you.

Hi @sztaylor,
We also have MacOS build on CI, and it does not seem to break it. I guess maybe check your protobuf version? @snnn Do you have any suggestion?

@snnn

snnn commented Jul 7, 2025

Copy link
Copy Markdown
Contributor

We need more context. Which .cc/.cpp file generated the warning? @sztaylor

@sztaylor

sztaylor commented Jul 7, 2025

Copy link
Copy Markdown

Thanks for getting back to me on this. This is the location of the first reported issue:

[ 77%] Building CXX object CMakeFiles/onnxruntime_perf_test.dir/Users/Shared/jenkins/onnx/Builds/onnx-github-main-macos/ws/build/onnxruntime/onnxruntime/test/perftest/performance_runner.cc.o In file included from /Users/Shared/jenkins/onnx/Builds/onnx-github-main-macos/ws/build/onnxruntime/onnxruntime/test/optimizer/graph_transform_test.cc:12: In file included from /Users/Shared/jenkins/onnx/Builds/onnx-github-main-macos/ws/build/onnxruntime/build/MacOS/Release/_deps/onnx-src/onnx/defs/parser.h:15: In file included from /Users/Shared/jenkins/onnx/Builds/onnx-github-main-macos/ws/build/onnxruntime/build/MacOS/Release/_deps/onnx-src/onnx/onnx_pb.h:51: In file included from /Users/Shared/jenkins/onnx/Builds/onnx-github-main-macos/ws/build/onnxruntime/build/MacOS/Release/_deps/onnx-build/onnx/onnx-ml.pb.h:30: In file included from /Users/Shared/jenkins/onnx/Builds/onnx-github-main-macos/ws/build/onnxruntime/build/MacOS/Release/_deps/protobuf-src/src/google/protobuf/extension_set.h:53: /Users/Shared/jenkins/onnx/Builds/onnx-github-main-macos/ws/build/onnxruntime/build/MacOS/Release/_deps/protobuf-src/src/google/protobuf/parse_context.h:328:47: error: implicit conversion loses integer precision: 'long' to 'int' [-Werror,-Wshorten-64-to-32] 328 | int chunk_size = buffer_end_ + kSlopBytes - ptr; | ~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~ /Users/Shared/jenkins/onnx/Builds/onnx-github-main-macos/ws/build/onnxruntime/build/MacOS/Release/_deps/protobuf-src/src/google/protobuf/parse_context.h:342:45: error: implicit conversion loses integer precision: 'long' to 'int' [-Werror,-Wshorten-64-to-32] 342 | chunk_size = buffer_end_ + kSlopBytes - ptr; | ~ ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~ /Users/Shared/jenkins/onnx/Builds/onnx-github-main-macos/ws/build/onnxruntime/build/MacOS/Release/_deps/protobuf-src/src/google/protobuf/parse_context.h:786:41: error: implicit conversion loses integer precision: 'long' to 'int' [-Werror,-Wshorten-64-to-32] 786 | int nbytes = buffer_end_ + kSlopBytes - ptr; | ~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~ /Users/Shared/jenkins/onnx/Builds/onnx-github-main-macos/ws/build/onnxruntime/build/MacOS/Release/_deps/protobuf-src/src/google/protobuf/parse_context.h:804:39: error: implicit conversion loses integer precision: 'long' to 'int' [-Werror,-Wshorten-64-to-32] 804 | nbytes = buffer_end_ + kSlopBytes - ptr; | ~ ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~ /Users/Shared/jenkins/onnx/Builds/onnx-github-main-macos/ws/build/onnxruntime/build/MacOS/Release/_deps/protobuf-src/src/google/protobuf/parse_context.h:836:32: error: implicit conversion loses integer precision: 'long' to 'int' [-Werror,-Wshorten-64-to-32] 836 | int chunk_size = buffer_end_ - ptr; | ~~~~~~~~~~ ~~~~~~~~~~~~^~~~~ /Users/Shared/jenkins/onnx/Builds/onnx-github-main-macos/ws/build/onnxruntime/build/MacOS/Release/_deps/protobuf-src/src/google/protobuf/parse_context.h:840:23: error: implicit conversion loses integer precision: 'long' to 'int' [-Werror,-Wshorten-64-to-32] 840 | int overrun = ptr - buffer_end_; | ~~~~~~~ ~~~~^~~~~~~~~~~~~ /Users/Shared/jenkins/onnx/Builds/onnx-github-main-macos/ws/build/onnxruntime/build/MacOS/Release/_deps/protobuf-src/src/google/protobuf/parse_context.h:861:30: error: implicit conversion loses integer precision: 'long' to 'int' [-Werror,-Wshorten-64-to-32] 861 | chunk_size = buffer_end_ - ptr; | ~ ~~~~~~~~~~~~^~~~~ /Users/Shared/jenkins/onnx/Builds/onnx-github-main-macos/ws/build/onnxruntime/build/MacOS/Release/_deps/protobuf-src/src/google/protobuf/parse_context.h:884:25: error: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long long') to 'uint32_t' (aka 'unsigned int') [-Werror,-Wshorten-64-to-32] 884 | uint32_t number = tag >> 3; | ~~~~~~ ~~~~^~~~

ankus-qti pushed a commit to CodeLinaro/onnxruntime that referenced this pull request Nov 25, 2025
Delete the legacy patches related to protobuf, which was added from
microsoft#14279 and
microsoft#15878 to simplify the ONNX
patches.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants