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

Test lower protoc version in Linux CI #4365

Merged
merged 28 commits into from
Jul 26, 2022
Merged

Conversation

jcwchen
Copy link
Member

@jcwchen jcwchen commented Jul 18, 2022

Description

  • Test lower protoc version in Linux CI
  • Modify two protobuf usage to make ONNX be compatible with lower protoc version
  • Improve script style for manylinux/entrypoint.sh
  • Make Linux CI of Azp test ONNX_USE_LITE_PROTO=OFF and Linux CI of GitHub Action test ONNX_USE_LITE_PROTO=ON, because lower protoc version from apt-get cannot work with ONNX_USE_LITE_PROTO=ON.
  • Sync option settings for all released wheels: same as Windows release CI, Linux and Mac release CI will use ONNX_USE_LITE_PROTO=ON as well. It can shrink binary size a little bit.

Motivation and Context
For now there is no CI in ONNX testing lower protoc version. For instance, using libprotobuf-dev and protobuf-compiler from apt-get. ONNX mention this usage in the document and ideally the CI should cover this case.

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
@jcwchen jcwchen added the run release CIs Use this label to trigger release tests in CI label Jul 18, 2022
Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
…o jcw/mini-protoc

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

# Conflicts:
#	.github/workflows/manylinux/entrypoint.sh
Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
@jcwchen
Copy link
Member Author

jcwchen commented Jul 22, 2022

One important change I would like to highlight is that this PR makes released ONNX wheel for Linux and Mac start to use ONNX_USE_LITE_PROTO=ON. The released wheel for Windows has already used it for quite a while (since ONNX 1.7.0) and so I think it's good to sync the option settings for every released wheel to prevent confusions. Also it can save some binary size (~0.5 MB).

It's possible that Linux users use older protoc versions (e.g., 3.0.0) from apt-get. Although using prebuilt wheel should be a fine case, to be safe, I tested the ONNX Linux wheel, which was built with ONNX_USE_LITE_PROTO=ON from this PR, on an Ubuntu 18.04 with protoc 3.0.0 and it did work well without any issue.

@jcwchen jcwchen merged commit ae3c87b into onnx:main Jul 26, 2022
@jcwchen jcwchen deleted the jcw/mini-protoc branch July 26, 2022 15:08
@jcwchen jcwchen mentioned this pull request Feb 3, 2023
broune pushed a commit to broune/onnx that referenced this pull request May 6, 2023
* use apt-get's libprotobuf-dev

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* change order

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* use Git instead of opeartor[]

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* use index

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* -DONNX_USE_PROTOBUF_SHARED_LIBS=ON

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* shape_inference test as well

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* test onnx_lite in GitHub Action instead

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* remove duplicate pip install

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* fix warnings in script

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* fix the rest of warnings

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* update docment

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* use $4 instead

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* use sh instead of shell

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* use $1

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* failed_wheels

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* add doc

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* missing $1

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* use $4

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* use original

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* onnx_lite does not work with old protoc version

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* make Mac use ONNX_USE_LITE_PROTO to sync options for all envs

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* update CI doc to reflect recent changes

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI pipelines Issues related to the CI pipeline run release CIs Use this label to trigger release tests in CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants