Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

upgrade protobuf to 3.20.2 and onnx to 1.13 #14279

Merged
merged 33 commits into from
Jan 31, 2023
Merged

upgrade protobuf to 3.20.2 and onnx to 1.13 #14279

merged 33 commits into from
Jan 31, 2023

Conversation

mszhanyi
Copy link
Contributor

@mszhanyi mszhanyi commented Jan 13, 2023

Description

upgrade protobuf to 3.20.2, same as onnx 1.13.0

Motivation and Context

Per component governance requirement and Fixes #14060

unused-parameter error occurs in 2 conditions.

  1. compile protolbuf
    onnxruntime_src/cmake/external/protobuf/src/google/protobuf/repeated_ptr_field.h:752:66: error: unused parameter ‘prototype’ [-Werror=unused-parameter]
  2. include onnx_pb.h
2023-01-28T10:20:15.0410853Z FAILED: CMakeFiles/onnxruntime_pybind11_state.dir/onnxruntime_src/onnxruntime/python/onnxruntime_pybind_iobinding.cc.o 
......
2023-01-28T10:20:15.0466024Z                  from /build/Debug/_deps/onnx-src/onnx/onnx_pb.h:51,
2023-01-28T10:20:15.0466958Z                  from /onnxruntime_src/include/onnxruntime/core/framework/to_tensor_proto_element_type.h:10,
....
2023-01-28T10:20:15.0609678Z /build/Debug/_deps/onnx-build/onnx/onnx-operators-ml.pb.h:1178:25:   required from here
2023-01-28T10:20:15.0610895Z /onnxruntime_src/cmake/external/protobuf/src/google/protobuf/repeated_ptr_field.h:752:66: error: unused parameter ‘prototype’ [-Werror=unused-parameter]
2023-01-28T10:20:15.0611707Z cc1plus: all warnings being treated as errors

https://dev.azure.com/onnxruntime/2a773b67-e88b-4c7f-9fc0-87d31fea8ef2/_apis/build/builds/874605/logs/22

@mszhanyi
Copy link
Contributor Author

mszhanyi commented Jan 13, 2023

@snnn, how did you get the sha1. I calculated locally.

@mszhanyi mszhanyi requested review from snnn and edgchen1 January 13, 2023 04:06
@mszhanyi mszhanyi force-pushed the zhanyi/updateprotobuf branch from 0f2d1b3 to 79c6d37 Compare January 13, 2023 04:09
@mszhanyi mszhanyi requested a review from a team as a code owner January 13, 2023 04:30
@mszhanyi mszhanyi requested a review from a team as a code owner January 13, 2023 07:56
@mszhanyi mszhanyi changed the title upgrade protobuf to 3.19.6 upgrade protobuf to 3.20.2 Jan 13, 2023
@mszhanyi mszhanyi marked this pull request as draft January 13, 2023 09:48
@mszhanyi mszhanyi force-pushed the zhanyi/updateprotobuf branch from fc494fa to b5a7f28 Compare January 13, 2023 10:02
protobuf==3.19.6
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this one different?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thx. fixed.

cmake/deps.txt Show resolved Hide resolved
@snnn
Copy link
Member

snnn commented Jan 15, 2023

@snnn, how did you get the sha1. I calculated locally.

Use "C:\Program Files\git\usr\bin\sha1sum.exe" .

@mszhanyi mszhanyi force-pushed the zhanyi/updateprotobuf branch from 3a4c5ad to 0dd4358 Compare January 18, 2023 10:29
@mszhanyi mszhanyi force-pushed the zhanyi/updateprotobuf branch from 508d94d to 5f11e9f Compare January 19, 2023 07:20
@mszhanyi mszhanyi requested a review from pranavsharma January 27, 2023 15:27
endif()
# external/protobuf/src/google/protobuf/arena.h:445:18: error: unused parameter 'p'
# onnxruntime_src/cmake/external/protobuf/src/google/protobuf/repeated_ptr_field.h:752:66: error: unused parameter ‘prototype’ [-Werror=unused-parameter]
target_compile_options(${target_name} PRIVATE "-Wno-unused-parameter")
Copy link
Member

Choose a reason for hiding this comment

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

I think it is better to only disable the warning for protobuf related code. If you add the compile flag here, our code will start to have such warnings(though they are suppressed).

Copy link
Contributor Author

@mszhanyi mszhanyi Jan 28, 2023

Choose a reason for hiding this comment

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

There're 2 types of errors caused by this _unused-parameter . More exception info is in the description of this PR.

  1. compiling protobuf source code. The scope could be easily narrowed by updating the patch or other solution.
  2. source code which include onnx/onnx_pb.h.
    So far, there're 44 c++ and 4 Objective-c files include the header.

Copy link
Contributor Author

@mszhanyi mszhanyi Jan 29, 2023

Choose a reason for hiding this comment

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

So, I've patched ONNX directly. @snnn

@mszhanyi mszhanyi force-pushed the zhanyi/updateprotobuf branch 6 times, most recently from f860de7 to 39efc46 Compare January 28, 2023 11:57
@mszhanyi mszhanyi requested a review from snnn January 29, 2023 00:57
@mszhanyi
Copy link
Contributor Author

mszhanyi commented Jan 31, 2023

@pranavsharma @yuslepukhin do you have any more comments?

@snnn snnn merged commit 80f807c into main Jan 31, 2023
@snnn snnn deleted the zhanyi/updateprotobuf branch January 31, 2023 20:55
@faxu faxu added the triage:approved Approved for cherrypicks for release label Feb 1, 2023
mszhanyi added a commit that referenced this pull request Feb 2, 2023
rui-ren pushed a commit that referenced this pull request Feb 3, 2023
### Description
upgrade protobuf to 3.20.2, same as onnx 1.13.0

### Motivation and Context
Per component governance requirement and Fixes #14060

unused-parameter error occurs in 2 conditions.
1. compile protolbuf

`onnxruntime_src/cmake/external/protobuf/src/google/protobuf/repeated_ptr_field.h:752:66:
error: unused parameter ‘prototype’ [-Werror=unused-parameter]`
2. include onnx_pb.h
```
2023-01-28T10:20:15.0410853Z FAILED: CMakeFiles/onnxruntime_pybind11_state.dir/onnxruntime_src/onnxruntime/python/onnxruntime_pybind_iobinding.cc.o 
......
2023-01-28T10:20:15.0466024Z                  from /build/Debug/_deps/onnx-src/onnx/onnx_pb.h:51,
2023-01-28T10:20:15.0466958Z                  from /onnxruntime_src/include/onnxruntime/core/framework/to_tensor_proto_element_type.h:10,
....
2023-01-28T10:20:15.0609678Z /build/Debug/_deps/onnx-build/onnx/onnx-operators-ml.pb.h:1178:25:   required from here
2023-01-28T10:20:15.0610895Z /onnxruntime_src/cmake/external/protobuf/src/google/protobuf/repeated_ptr_field.h:752:66: error: unused parameter ‘prototype’ [-Werror=unused-parameter]
2023-01-28T10:20:15.0611707Z cc1plus: all warnings being treated as errors

```

https://dev.azure.com/onnxruntime/2a773b67-e88b-4c7f-9fc0-87d31fea8ef2/_apis/build/builds/874605/logs/22
rui-ren pushed a commit that referenced this pull request Feb 3, 2023
### Description
upgrade protobuf to 3.20.2, same as onnx 1.13.0

### Motivation and Context
Per component governance requirement and Fixes #14060

unused-parameter error occurs in 2 conditions.
1. compile protolbuf

`onnxruntime_src/cmake/external/protobuf/src/google/protobuf/repeated_ptr_field.h:752:66:
error: unused parameter ‘prototype’ [-Werror=unused-parameter]`
2. include onnx_pb.h
```
2023-01-28T10:20:15.0410853Z FAILED: CMakeFiles/onnxruntime_pybind11_state.dir/onnxruntime_src/onnxruntime/python/onnxruntime_pybind_iobinding.cc.o 
......
2023-01-28T10:20:15.0466024Z                  from /build/Debug/_deps/onnx-src/onnx/onnx_pb.h:51,
2023-01-28T10:20:15.0466958Z                  from /onnxruntime_src/include/onnxruntime/core/framework/to_tensor_proto_element_type.h:10,
....
2023-01-28T10:20:15.0609678Z /build/Debug/_deps/onnx-build/onnx/onnx-operators-ml.pb.h:1178:25:   required from here
2023-01-28T10:20:15.0610895Z /onnxruntime_src/cmake/external/protobuf/src/google/protobuf/repeated_ptr_field.h:752:66: error: unused parameter ‘prototype’ [-Werror=unused-parameter]
2023-01-28T10:20:15.0611707Z cc1plus: all warnings being treated as errors

```

https://dev.azure.com/onnxruntime/2a773b67-e88b-4c7f-9fc0-87d31fea8ef2/_apis/build/builds/874605/logs/22
rui-ren pushed a commit that referenced this pull request Feb 3, 2023
### Description
upgrade protobuf to 3.20.2, same as onnx 1.13.0

### Motivation and Context
Per component governance requirement and Fixes #14060

unused-parameter error occurs in 2 conditions.
1. compile protolbuf

`onnxruntime_src/cmake/external/protobuf/src/google/protobuf/repeated_ptr_field.h:752:66:
error: unused parameter ‘prototype’ [-Werror=unused-parameter]`
2. include onnx_pb.h
```
2023-01-28T10:20:15.0410853Z FAILED: CMakeFiles/onnxruntime_pybind11_state.dir/onnxruntime_src/onnxruntime/python/onnxruntime_pybind_iobinding.cc.o 
......
2023-01-28T10:20:15.0466024Z                  from /build/Debug/_deps/onnx-src/onnx/onnx_pb.h:51,
2023-01-28T10:20:15.0466958Z                  from /onnxruntime_src/include/onnxruntime/core/framework/to_tensor_proto_element_type.h:10,
....
2023-01-28T10:20:15.0609678Z /build/Debug/_deps/onnx-build/onnx/onnx-operators-ml.pb.h:1178:25:   required from here
2023-01-28T10:20:15.0610895Z /onnxruntime_src/cmake/external/protobuf/src/google/protobuf/repeated_ptr_field.h:752:66: error: unused parameter ‘prototype’ [-Werror=unused-parameter]
2023-01-28T10:20:15.0611707Z cc1plus: all warnings being treated as errors

```

https://dev.azure.com/onnxruntime/2a773b67-e88b-4c7f-9fc0-87d31fea8ef2/_apis/build/builds/874605/logs/22
rui-ren pushed a commit that referenced this pull request Feb 3, 2023
### Description
upgrade protobuf to 3.20.2, same as onnx 1.13.0

### Motivation and Context
Per component governance requirement and Fixes #14060

unused-parameter error occurs in 2 conditions.
1. compile protolbuf

`onnxruntime_src/cmake/external/protobuf/src/google/protobuf/repeated_ptr_field.h:752:66:
error: unused parameter ‘prototype’ [-Werror=unused-parameter]`
2. include onnx_pb.h
```
2023-01-28T10:20:15.0410853Z FAILED: CMakeFiles/onnxruntime_pybind11_state.dir/onnxruntime_src/onnxruntime/python/onnxruntime_pybind_iobinding.cc.o 
......
2023-01-28T10:20:15.0466024Z                  from /build/Debug/_deps/onnx-src/onnx/onnx_pb.h:51,
2023-01-28T10:20:15.0466958Z                  from /onnxruntime_src/include/onnxruntime/core/framework/to_tensor_proto_element_type.h:10,
....
2023-01-28T10:20:15.0609678Z /build/Debug/_deps/onnx-build/onnx/onnx-operators-ml.pb.h:1178:25:   required from here
2023-01-28T10:20:15.0610895Z /onnxruntime_src/cmake/external/protobuf/src/google/protobuf/repeated_ptr_field.h:752:66: error: unused parameter ‘prototype’ [-Werror=unused-parameter]
2023-01-28T10:20:15.0611707Z cc1plus: all warnings being treated as errors

```

https://dev.azure.com/onnxruntime/2a773b67-e88b-4c7f-9fc0-87d31fea8ef2/_apis/build/builds/874605/logs/22
@faxu faxu removed the release:1.14 label Feb 7, 2023
@pacowong
Copy link

pacowong commented Feb 9, 2023

It would be great if the library can upgrade to the protobuf version of 3.21.3+ earlier as I encounter a bug in this version while compiling ORT.

protocolbuffers/protobuf#9947
https://stackoverflow.com/questions/73047153/protobuf-ld-undefined-symbol-internalmetadata

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage:approved Approved for cherrypicks for release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

There is a vulnerability in protobuf:3.18.1,upgrade recommended
8 participants