Skip to content

Add a flag to allow for linking against shared system protobuf#7577

Merged
snnn merged 2 commits intomicrosoft:masterfrom
xhochy:add-flag-for-shared-system-protobuf
May 19, 2021
Merged

Add a flag to allow for linking against shared system protobuf#7577
snnn merged 2 commits intomicrosoft:masterfrom
xhochy:add-flag-for-shared-system-protobuf

Conversation

@xhochy
Copy link
Contributor

@xhochy xhochy commented May 5, 2021

Description:

In some environments you would like to link to the shared libprotobuf from the system. This is fine to use if you have full control over all protobuf users in your environment.

This need for the existence of this option is already mentioned as a comment in CMakeLists.txt, this adds it now.

Describe your changes.

Motivation and Context

We have packaged onnxruntime for conda-forge. There we prefer shared linkage and have a good control (and automation) to link everything to the same protobuf library and also rebuild everything on a protobuf update. This adds an option to CMakeLists.txt to allow us to use the shared library without us patching the source.

In some environments you would like to link to the shared libprotobuf from the system. This is fine to use if you have full control over all protobuf users in your environment.

This need for the existence of this option is already mentioned as a comment in CMakeLists.txt, this adds it now.
@xhochy xhochy requested a review from a team as a code owner May 5, 2021 08:28
@snnn
Copy link
Contributor

snnn commented May 5, 2021

You can use the option: onnxruntime_PREFER_SYSTEM_LIB .

@xhochy
Copy link
Contributor Author

xhochy commented May 6, 2021

I'm using onnxruntime_PREFER_SYSTEM_LIB, that's why I mainly run into this code path. Currently, this enforces static protobuf linkage but in certain situations, I would prefer shared linkage.

@snnn
Copy link
Contributor

snnn commented May 7, 2021

Got it. How about this: instead of introducing a new build option, can you help me change:

      message(FATAL_ERROR "Please enable Protobuf_USE_STATIC_LIBS")

to a normal warning. Then please adjust the text message accordingly.

@snnn
Copy link
Contributor

snnn commented May 18, 2021

/azp run Linux CPU CI Pipeline,Linux CPU Minimal Build E2E CI Pipeline,Linux CPU x64 NoContribops CI Pipeline,Linux GPU CI Pipeline,Linux GPU TensorRT CI Pipeline,Linux Nuphar CI Pipeline,Linux OpenVINO CI Pipeline,MacOS CI Pipeline,MacOS NoContribops CI Pipeline

@snnn
Copy link
Contributor

snnn commented May 18, 2021

/azp run Windows CPU CI Pipeline,Windows GPU CI Pipeline,Windows GPU TensorRT CI Pipeline,Windows WebAssembly CI Pipeline,orttraining-amd-gpu-ci-pipeline,orttraining-linux-ci-pipeline,orttraining-linux-gpu-ci-pipeline,orttraining-ortmodule,orttraining-ortmodule-distributed

@snnn snnn self-assigned this May 18, 2021
@azure-pipelines
Copy link

Azure Pipelines successfully started running 9 pipeline(s).

1 similar comment
@azure-pipelines
Copy link

Azure Pipelines successfully started running 9 pipeline(s).

@snnn
Copy link
Contributor

snnn commented May 18, 2021

/azp run orttraining-amd-gpu-ci-pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@snnn snnn merged commit 203efef into microsoft:master May 19, 2021
@snnn
Copy link
Contributor

snnn commented May 19, 2021

Thank you! The change will be in our next release, which will be published in a few weeks. Then you can update your conda package to take it.

xkszltl added a commit to xkszltl/Roaster that referenced this pull request Apr 6, 2022
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.

2 participants