ARROW-15066: [C++] Enable use of non-bundled OpenTelemetry #11963
Conversation
|
Reminder to enable this: ad470f6 |
|
@lidavidm I'm a bit confused about how this PR is related to the packages in conda-forge. Can you enlighten me? |
|
@xhochy sorry, this is more like "test with the conda-forge packages and make any changes necessary to get them to work" |
|
In any case, there are conflicts that need to be resolved. |
lidavidm
force-pushed
the
arrow-15066
branch
from
8573ba8 to
c08c699
Compare
lidavidm
changed the title
|
Sorry, I've rebased + changed the title to hopefully be less confusing. |
Quickly bump the version since it changes a few APIs we'll use (most notably for #11920). #11963 will also need updating, but the conda-forge packages need to be updated first. This does not include the fix needed for #12408, that will require another version bump. Closes #12516 from lidavidm/arrow-15789 Authored-by: David Li <[email protected]> Signed-off-by: David Li <[email protected]>
|
@github-actions crossbow submit -g cpp |
|
Revision: 15a8b017507eb91ed19be900b264193c5fff6327 Submitted crossbow builds: ursacomputing/crossbow @ actions-1722
|
|
@lidavidm Perhaps you can rebase to get the latest build fixes? |
|
Thanks, rebased. |
|
@github-actions crossbow submit -g cpp |
|
Revision: 6b3444f Submitted crossbow builds: ursacomputing/crossbow @ actions-1724
|
| add_dependencies(nlohmann_json::nlohmann_json nlohmann_json_ep) | ||
| endmacro() | ||
| if(ARROW_WITH_NLOHMANN_JSON) | ||
| set(nlohmann_json_SOURCE "AUTO") |
There was a problem hiding this comment.
We don't need it, you're right - removed
There was a problem hiding this comment.
Hmm, no, it is necessary after all. I suppose when some things are bundled and others aren't, this is needed to make it look for nlohmann_json, and fall back on building it.
There was a problem hiding this comment.
I think that it's a problem of our CI configuration not the CMake code.
Some of our CI jobs uses -DARROW_DEPENDENCY_SOURCE=SYSTEM explicitly to use system libraries. For example: https://github.com/apache/arrow/runs/5465413563?check_suite_focus=true#step:6:1843
cmake ... -DARROW_DEPENDENCY_SOURCE=SYSTEM ...
In the CI jobs, we must install nlohmann-json explicitly. e.g.:
- MSYS2: https://packages.msys2.org/base/mingw-w64-nlohmann-json
- Debian: https://packages.debian.org/search?keywords=nlohmann-json
- Ubuntu: https://packages.ubuntu.com/search?keywords=nlohmann-json
Here is a sample change for MSYS2:
diff --git a/ci/scripts/msys2_setup.sh b/ci/scripts/msys2_setup.sh
index 01cd5fa9ee..be6a947c6f 100755
--- a/ci/scripts/msys2_setup.sh
+++ b/ci/scripts/msys2_setup.sh
@@ -41,6 +41,7 @@ case "${target}" in
packages+=(${MINGW_PACKAGE_PREFIX}-make)
packages+=(${MINGW_PACKAGE_PREFIX}-mlir)
packages+=(${MINGW_PACKAGE_PREFIX}-ninja)
+ packages+=(${MINGW_PACKAGE_PREFIX}-nlohmann-json)
packages+=(${MINGW_PACKAGE_PREFIX}-polly)
packages+=(${MINGW_PACKAGE_PREFIX}-protobuf)
packages+=(${MINGW_PACKAGE_PREFIX}-python3-numpy)If there isn't nlohmann-json package in the system, we need to specify -Dnlohmann_json_SOURCE=BUNDLED explicitly for the CI jobs.
There was a problem hiding this comment.
Thanks for the explanation, I've updated the Fedora/MSYS/Debian/Ubuntu dockerfiles.
|
@github-actions crossbow submit -g cpp |
|
Revision: 13b7a23 Submitted crossbow builds: ursacomputing/crossbow @ actions-1728
|
|
@github-actions crossbow submit -g cpp |
|
Revision: 93883c5 Submitted crossbow builds: ursacomputing/crossbow @ actions-1731
|
|
@github-actions crossbow submit test-fedora-33-cpp |
|
Revision: 1b5183e Submitted crossbow builds: ursacomputing/crossbow @ actions-1736
|
|
@github-actions crossbow -g nightly |
|
|
@github-actions crossbow submit -g nightly |
|
Revision: 6095fdd Submitted crossbow builds: ursacomputing/crossbow @ actions-1751
|
|
Looks like nightly failures are generally unrelated. |
|
Benchmark runs are scheduled for baseline = dc2e0b2 and contender = 5592cf7. 5592cf7 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
6 participants
This is intended to make usage a little easier by enabling you to use a pre-built package instead of having to compile it from source.
Changes: