Expose build information in dynamic lib #15643

guoyuhong · 2023-04-22T15:57:07Z

Description

Add Build Info API to onnx.
Fix compile error while building onnxruntime_benchmark in MacOs.

Motivation and Context

When Onnxruntime lib is serving online, we need a way to detect how this lib is built. This PR helps the developer to get the build information using strings such as git branch, git commit id, build type and cmake cxx flags, which is showed as follows.

If the build env has no git, there will be no git related infor:

Fix the following compile error while building benchmark in MacOs.

guoyuhong · 2023-04-22T15:58:59Z

@microsoft-github-policy-service agree

snnn · 2023-04-24T00:41:33Z

cmake/CMakeLists.txt

@@ -1229,6 +1229,15 @@ if (onnxruntime_USE_VITISAI)
  endif()
 endif()

+execute_process(COMMAND git log -1 --format=%h


Some of our build environments do not have git. So this would not work.

@snnn It does not matter whether the build envs have git or not. The cmake command won't fail. Instead, it will output to the variable ORT_GIT_COMMIT & ORT_GIT_BRANCH with an empty string. To make this clearer, I added a comment here and explicitly change the empty string to "Not-Available". An example is showed below:

But it will be very confusing that users find our APIs has a function "GetBuildInfoString" but it always returns empty? Most users do not build onnxruntime from source. They use our prebuilt packages from nuget.org or the github release page.

@snnn I have reverted the change related to git in cmake/onnxruntime_config.h.in. For GetBuildInfoString in envs that has no git, it will returns the build type and cxx flags as follows. If the env has git, it will contain more information. Is it better?
I have also reverted the change in tuning_context_impl.h since the commit id requires git.

@snnn Do you have more comments on this PR?

snnn

No git.

guoyuhong · 2023-04-24T05:40:51Z

onnxruntime/core/framework/tuning_context_impl.h

 }

 Status TuningResultsValidator::ValidateOrtGitCommit(const std::string& value) const {
-  // TODO:
-  ORT_UNUSED_PARAMETER(value);
+  ORT_RETURN_IF(value != ORT_GIT_COMMIT, "onnxruntime git commit mismatch");


@snnn As you said there could be build envs that have no git, is it better to revert this change for ValidateOrtGitCommit ?

Never mind, reverted.

guoyuhong · 2023-04-24T05:42:03Z

cmake/CMakeLists.txt

@@ -1229,6 +1229,15 @@ if (onnxruntime_USE_VITISAI)
  endif()
 endif()

+execute_process(COMMAND git log -1 --format=%h


@snnn It does not matter whether the build envs have git or not. The cmake command won't fail. Instead, it will output to the variable ORT_GIT_COMMIT & ORT_GIT_BRANCH with an empty string. To make this clearer, I added a comment here and explicitly change the empty string to "Not-Available". An example is showed below:

xadupre · 2023-04-25T09:52:07Z

include/onnxruntime/core/session/onnxruntime_cxx_api.h

+/// git commit id, build type(Debug/Release/RelWithDebInfo) and cmake cpp flags.
+/// </summary>
+/// <returns>string</returns>
+std::string GetBuildInfoString();


It would be helpful to make it available in the python bindings as well. Any test to check its value?

That's a good point. I will take a look at how to add the python binding and the test to cover.

@xadupre I have added the build info to python bindings. The figure below shows the effect by some temporary code.

onnxruntime/test/python/onnxruntime_test_python.py

snnn · 2023-04-26T15:03:15Z

@pranavsharma

All API changes need to be reviewed by him.

reset

snnn · 2023-04-27T02:36:57Z

cmake/CMakeLists.txt

@@ -1229,6 +1229,19 @@ if (onnxruntime_USE_VITISAI)
  endif()
 endif()

+set(ORT_BUILD_INFO "ORT Build Info: ")
+find_package(Git)


All ONNX Runtime's build pipelines are hosted on Azure DevOps, which already expose this information through environment variables. https://learn.microsoft.com/en-us/azure/devops/pipelines/build/variables?view=azure-devops&tabs=yaml . The name of the env var is: "BUILD_SOURCEVERSION".
cmake can read env vars:
https://cmake.org/cmake/help/latest/variable/ENV.html

So we can add an if there to test the variable exists or not first. Then only run "find_package(Git)" when the env doesn't exist.

You don't have to make this enhancement for us. I just want to speak it out.

If from the beginning we were very strict on this, we won't see such an issue: #15678

If from the beginning we were very strict on this, we won't see such an issue: #15678

By this, you mean the ORTCHAR_T* problem, right? Yes, I have changed const char * to const ORTCHAR_T *

snnn · 2023-04-27T02:38:25Z

include/onnxruntime/core/session/onnxruntime_c_api.h

@@ -619,7 +619,8 @@ struct OrtApiBase {
   *   older than the version created with this header file.
   */
  const OrtApi*(ORT_API_CALL* GetApi)(uint32_t version)NO_EXCEPTION;
-  const char*(ORT_API_CALL* GetVersionString)(void)NO_EXCEPTION;  ///< Returns a null terminated string of the version of the Onnxruntime library (eg: "1.8.1")
+  const char*(ORT_API_CALL* GetVersionString)(void)NO_EXCEPTION;    ///< Returns a null terminated string of the version of the Onnxruntime library (eg: "1.8.1")


In general we should use const ORTCHAR_T* instead of const char*, because on Windows usually we prefer to use wide char.

Good point. GetVersionString is originally defined using char *. I will change both.

snnn · 2023-04-27T02:41:21Z

LGTM.

I hope the "const char*" could be changed to "const ORTCHAR_T*" to avoid confusion.

HectorSVC · 2023-04-27T02:45:06Z

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux OpenVINO CI Pipeline, Linux QNN CI Pipeline, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, Windows ARM64 QNN CI Pipeline

azure-pipelines · 2023-04-27T02:45:44Z

Azure Pipelines successfully started running 9 pipeline(s).

HectorSVC · 2023-04-27T02:48:38Z

/azp run Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed

azure-pipelines · 2023-04-27T02:49:06Z

Azure Pipelines successfully started running 7 pipeline(s).

linkerzhang · 2023-04-27T07:58:12Z

Ahahaha, a good try to communicae with Microsoft ORT team. Antgroup will try to contribute more back to ORT based on what we did during last years. ORT rocks!

HectorSVC · 2023-04-27T15:53:19Z

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux OpenVINO CI Pipeline, Linux QNN CI Pipeline, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, Windows ARM64 QNN CI Pipeline

azure-pipelines · 2023-04-28T04:24:00Z

Azure Pipelines successfully started running 7 pipeline(s).

snnn · 2023-04-28T14:27:05Z

/azp run Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed

snnn · 2023-04-28T14:27:16Z

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux OpenVINO CI Pipeline, Linux QNN CI Pipeline, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, Windows ARM64 QNN CI Pipeline

azure-pipelines · 2023-04-28T14:27:38Z

Azure Pipelines successfully started running 7 pipeline(s).

azure-pipelines · 2023-04-28T14:27:54Z

Azure Pipelines successfully started running 9 pipeline(s).

snnn · 2023-04-28T14:29:57Z

java/src/main/native/ai_onnxruntime_OnnxRuntime.c

  assert(version != NULL);
+#ifdef _WIN32
+  size_t len = wcslen(version);
+  jstring versionStr = (*jniEnv)->NewString(jniEnv, (const jchar*)version, len);


If I remember correctly, Java use UTF-16-BE while most Windows use UTF-16-LE. @Craigacp, is it true?

We don't transform the output of jstring->GetStringChars to change UTF-16 endianness on Windows, and it can load & save things to those paths correctly. So I would expect it to be the same on platforms that we currently test in the CI. If it's the same on all platforms is beyond my knowledge of Windows. It should be straightforward to modify https://github.com/microsoft/onnxruntime/blob/main/java/src/test/java/ai/onnxruntime/InferenceTest.java#L74 to test that the output is correct. I haven't modified that test to check the string because it'll need updating every time the version number is bumped so it seems like it would fail a bunch of pipelines.

snnn · 2023-04-28T18:52:55Z

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux OpenVINO CI Pipeline, Linux QNN CI Pipeline, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, Windows ARM64 QNN CI Pipeline

snnn · 2023-04-28T18:53:04Z

/azp run Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed

azure-pipelines · 2023-04-28T18:53:29Z

Azure Pipelines successfully started running 9 pipeline(s).

azure-pipelines · 2023-04-28T18:53:31Z

Azure Pipelines successfully started running 7 pipeline(s).

snnn · 2023-04-29T01:20:57Z

/azp run Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed

snnn · 2023-04-29T01:21:06Z

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux OpenVINO CI Pipeline, Linux QNN CI Pipeline, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, Windows ARM64 QNN CI Pipeline

azure-pipelines · 2023-04-29T01:21:25Z

Azure Pipelines successfully started running 7 pipeline(s).

azure-pipelines · 2023-04-29T01:21:39Z

Azure Pipelines successfully started running 9 pipeline(s).

guoyuhong · 2023-05-01T03:59:58Z

Thanks to @snnn @HectorSVC for your patience. :-)

snnn · 2023-05-01T19:46:20Z

Happy holidays!

### Description  1. Add Build Info API to onnx. 2. Fix compile error while building onnxruntime_benchmark in MacOs. ### Motivation and Context  1. When Onnxruntime lib is serving online, we need a way to detect how this lib is built. This PR helps the developer to get the build information using `strings` such as git branch, git commit id, build type and cmake cxx flags, which is showed as follows. ![image](https://user-images.githubusercontent.com/19584326/233794371-b2f95a2c-27fb-4709-a6dd-bf4bb12b0b5b.png) ![image](https://user-images.githubusercontent.com/19584326/233794360-f96f5d2e-332c-405c-83f1-370ccc2b86f8.png) If the build env has no git, there will be no git related infor: ![image](https://user-images.githubusercontent.com/19584326/234558596-298c1b01-9a90-41bf-9372-7259a8f8e5be.png) 3. Fix the following compile error while building benchmark in MacOs. ![image](https://user-images.githubusercontent.com/19584326/233793571-c261ac1f-47b2-434d-a293-7e9edc6c8a66.png) --------- Co-authored-by: Yuhong Guo <yuhong.gyh@antgroup.com>

yuslepukhin · 2023-05-11T22:38:05Z

What is the reason why simple version information is being returned as ORTTCHAR instead of UTF-8 as all other strings inside ORT? That typedef was introduced for paths, otherwise all strings are expected to contain UTF-8. It is just makes coding so much more complicated for what essentially is a stupid version string.

…m to OrtApi (#15921) ### Description This PR partially reverts changes introduced in #15643 We make two API return std::string always in UTF-8. We also move the entry points from OrtApiBase to OrtApi to make them versioned. ### Motivation and Context `GetVersionString` always returns x.y.z numbers that are not subject to internationalization. `GetBuildInfoString` can hold international chars, but UTF-8 should be fine to contain those. We prefix them with u8"" in case the compiler default charset is not UTF-8. Furthermore, creating platform dependent APIs is discouraged. `ORTCHAR_T` is platform dependent and was created for paths only. On non-unix platforms would still produce `std::string` that can only contain UTF-8 The API was introduced after the latest release, and can still be adjusted.

guoyuhong · 2023-05-15T07:15:50Z

@yuslepukhin Sorry for the inconvenience. I just followed the comment from @snnn , and not expected that would bring so much trouble. I learned that "We define all strings as UTF-8 internally, except Windows paths.". Thanks.

…m to OrtApi (#15921) ### Description This PR partially reverts changes introduced in #15643 We make two API return std::string always in UTF-8. We also move the entry points from OrtApiBase to OrtApi to make them versioned. ### Motivation and Context `GetVersionString` always returns x.y.z numbers that are not subject to internationalization. `GetBuildInfoString` can hold international chars, but UTF-8 should be fine to contain those. We prefix them with u8"" in case the compiler default charset is not UTF-8. Furthermore, creating platform dependent APIs is discouraged. `ORTCHAR_T` is platform dependent and was created for paths only. On non-unix platforms would still produce `std::string` that can only contain UTF-8 The API was introduced after the latest release, and can still be adjusted.

…m to OrtApi (#15921) This PR partially reverts changes introduced in #15643 We make two API return std::string always in UTF-8. We also move the entry points from OrtApiBase to OrtApi to make them versioned. `GetVersionString` always returns x.y.z numbers that are not subject to internationalization. `GetBuildInfoString` can hold international chars, but UTF-8 should be fine to contain those. We prefix them with u8"" in case the compiler default charset is not UTF-8. Furthermore, creating platform dependent APIs is discouraged. `ORTCHAR_T` is platform dependent and was created for paths only. On non-unix platforms would still produce `std::string` that can only contain UTF-8 The API was introduced after the latest release, and can still be adjusted.

Yuhong Guo added 2 commits April 22, 2023 22:15

Add Build Info to a api

07c202d

Refine code

a926eed

snnn reviewed Apr 24, 2023

View reviewed changes

snnn previously requested changes Apr 24, 2023

View reviewed changes

Special treatment for envs that have no git

c174abd

guoyuhong commented Apr 24, 2023

View reviewed changes

Remove git related defines

efc6c3a

xadupre reviewed Apr 25, 2023

View reviewed changes

Add get_build_info to pybind

374cbfe

github-advanced-security bot found potential problems Apr 26, 2023

View reviewed changes

onnxruntime/test/python/onnxruntime_test_python.py Fixed Show fixed Hide fixed

Fix lint problems

ca7cedf

guoyuhong force-pushed the gyh_build_info branch from 5d1ffb3 to ca7cedf Compare April 26, 2023 11:58

snnn requested a review from pranavsharma April 26, 2023 15:02

pranavsharma previously approved these changes Apr 26, 2023

View reviewed changes

guoyuhong requested review from snnn and xadupre April 27, 2023 01:55

snnn reviewed Apr 27, 2023

View reviewed changes

Change char to ORTCHAR_T

665fc35

guoyuhong dismissed pranavsharma’s stale review via 665fc35 April 27, 2023 05:16

Try to fix windows compile errors

e8f54e9

snnn reviewed Apr 28, 2023

View reviewed changes

Fix JNI build error in windows

5fb5217

guoyuhong force-pushed the gyh_build_info branch from 55d910d to 5fb5217 Compare April 29, 2023 01:10

snnn approved these changes Apr 29, 2023

View reviewed changes

HectorSVC merged commit 41dcf0d into microsoft:main Apr 29, 2023

guoyuhong deleted the gyh_build_info branch April 29, 2023 12:20

yuslepukhin mentioned this pull request May 12, 2023

Adust GetVersionString() GetBuildInfoString() signatures and move them to OrtApi #15921

Merged

Expose build information in dynamic lib #15643

Expose build information in dynamic lib #15643

Conversation

guoyuhong commented Apr 22, 2023 • edited Loading

Description

Motivation and Context

guoyuhong commented Apr 22, 2023

Choose a reason for hiding this comment

guoyuhong Apr 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snnn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guoyuhong Apr 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snnn commented Apr 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guoyuhong Apr 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snnn commented Apr 27, 2023

HectorSVC commented Apr 27, 2023

azure-pipelines bot commented Apr 27, 2023

HectorSVC commented Apr 27, 2023

azure-pipelines bot commented Apr 27, 2023

linkerzhang commented Apr 27, 2023

HectorSVC commented Apr 27, 2023

azure-pipelines bot commented Apr 28, 2023

snnn commented Apr 28, 2023

snnn commented Apr 28, 2023

azure-pipelines bot commented Apr 28, 2023

azure-pipelines bot commented Apr 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snnn commented Apr 28, 2023

snnn commented Apr 28, 2023

azure-pipelines bot commented Apr 28, 2023

azure-pipelines bot commented Apr 28, 2023

snnn commented Apr 29, 2023

snnn commented Apr 29, 2023

azure-pipelines bot commented Apr 29, 2023

azure-pipelines bot commented Apr 29, 2023

guoyuhong commented May 1, 2023

snnn commented May 1, 2023

yuslepukhin commented May 11, 2023

guoyuhong commented May 15, 2023

guoyuhong commented Apr 22, 2023 •

edited

Loading

guoyuhong Apr 24, 2023 •

edited

Loading

guoyuhong Apr 24, 2023 •

edited

Loading

guoyuhong Apr 27, 2023 •

edited

Loading