@snnn I have a question are this compile definitions needs to be public?

also, in some headers, headers are not included relative to root dir, like:

also is this file: eigen_common_wrapper.h is intended to be used by user or this private header as this header file include onnxruntime_config.h which is cmake generate file and is not exported. If it is public then we should export onnxruntime_config.h or else not export it.

"onnxruntime_config.h" is a tricky thing. The idea was borrowed from autoconf, which is quite old. And it assumes that you won't do "build-once, run everywhere" such things. So the content of config.h is based on your current system's configuration. If you won't copy the generated file to another file, then it's totally fine to install it to /usr/include .

ok, and what about other headers, like

• "core/framework/allocatormgr.h", "core/framework/data_transfer.h" used by "core/framework/execution_provider.h"
• "core/framework/prepacked_weights_container.h" used by "core/framework/op_kernel.h"
• "core/framework/TensorSeq.h" used by "core/framework/data_types.h", "core/framework/ort_value.h"
• "core/framework/config_options.h" used by "core\framework\run_options.h"
• "core/platform/ort_mutex.h" used by "schema_registry.h"
• "core/platform/threadpool.h" used by environment.h
• "thread_utils.h" used by ""core/framework/session_options.h" used by graph_viewer.h
• path_string.h used by "core/common/path.h" used by graph.h

If they need be in the right place, please help us move them.

Why is this closed? I was looking for a proper CMake export. I was going to write it myself but realized this PR already does it. Now this one is closed. Does that mean the project does not want to merge it?

@ongun-kanat i got distracted to some other project, but now I am back and cleaning commits

@snnn Hey I saw now we have a NuGet package with tensorrt support, I wanted to ask right now Cmake install a lot of header files but in the NuGet package, there are only a few, do we want the same number of header in cmake to install as in NuGet or other headers are also useful.

Do we want the same number of header

Is this for static link to onnxruntime or dynamic link?
If it is for libonnxruntime.so, then you will only need one file: onnxruntime_c_api.h (and the files it includes, not much).
Otherwise, it will be a lot and I don't know how to handle it.

Do we want the same number of header

Is this for static link to onnxruntime or dynamic link? If it is for libonnxruntime.so, then you will only need one file: onnxruntime_c_api.h (and the files it includes, not much). Otherwise, it will be a lot and I don't know how to handle it.

in Nuget we have only these headers:

location: Microsoft.ML.OnnxRuntime.Gpu.1.9.0\build\native\include
but in cmake, we are installing a lot of other headers

@snnn Hey, this pr is ready for shared libs. Can you review it or ask someone else to review it if you dont have time.

/azp run Python packaging pipeline

/azp run Zip-Nuget-Java-Nodejs Packaging Pipeline

/azp run Python packaging pipeline

/azp run Zip-Nuget-Java-Nodejs Packaging Pipeline

I don't have enough React Native knowledge to fix these 2 remaining issues. Can anyone help with this?

I don't have enough React Native knowledge to fix these 2 remaining issues. Can anyone help with this?

It is fine since other jobs have same failure in the React pipeline. I think it is not related to this change.

@snnn, are there other pipelines need to verify for this PR?
@mszhanyi, I think some release pipelines (like packaging pipeline) cannot run in this PR since those require code in main/release branch.

I don't have enough React Native knowledge to fix these 2 remaining issues. Can anyone help with this?

Yeah, the react native pipeline is currently failing due to some other checked-in changes. We are working on a fix for that. It's not a required CI, won't affect this pr merge.

It seems that the target onnxruntime does not contain providers, which causes onnxruntime_providers.dll and onnxruntime_provider_cuda.dll to be lost when copying the target to the root directory.

@QiuYilin Libs of providers should be installed if the "install" target is built in "CMAKE_INSTALL_LIBDIR". Just that the provider's cmake targets are not exported, I initially tried to do it but some of the providers had PUBLIC dependencies which were creating issues.

@prateek9623 can you or someone else please explain, why the header structure had to be changed?..

In 1.15.1 and before, no matter if building ORT as shared or static, we always had include/onnxruntime/core/session/onnxruntime_cxx_api.h for example, same as it was in the ORT repo source tree.

But now, since in cmake/onnxruntime_session.cmake you have moved install(DIRECTORY ${PROJECT_SOURCE_DIR}/../include/onnxruntime/core/session ... under if (NOT onnxruntime_BUILD_SHARED_LIB), the aforementioned header may move around if ORT was built as shared. So in 1.16.3 it may become include/onnxruntime/onnxruntime_cxx_api.h instead.

Do we now have to use something like this in our code:

#ifdef ORT_SHARED
#  include <onnxruntime/onnxruntime_cxx_api.h>
#else
#  include <onnxruntime/core/session/onnxruntime_cxx_api.h>
#endif

, or am I missing something?

@Artalus With this PR I tried to replicate prebuilt shared lib releases. And have not disturbed static build.
@snnn do you have any comments on this?

@prateek9623 @snn could the cmake files be added to the tar.gz file so that people can download ort and link to it without having to clone the repo?

I agree with @thiagocrepaldi