Repository navigation
Conversation
…IC,TESTS}=ON on Windows
parquet_static isn't compiled from the shared object library on Windows. So it needs OpenSSL in its own link libraries to find OpenSSL headers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Member
Author
|
Confirmed: https://github.com/apache/arrow/actions/runs/40611094510/job/122562525836?pr=55294#step:11:6776 |
kou
force-pushed
the
ci-windows-build-static
branch
2 times, most recently
from
October 28, 2026 02:22
15d8bbc to
24e1a08
Compare
2 of 5 tasks
arrow_compute_core_testing, arrow_compute_testing, arrow_compute_kernels_testing, arrow_acero_testing and arrow_dataset_testing were object libraries. Object libraries are compiled with their own compile definitions, not with test's compile definitions. So we needed to set ARROW_STATIC and ARROW_COMPUTE_STATIC manually for arrow_compute_testing on Windows. But it was wrong with ARROW_BUILD_SHARED=ON, ARROW_BUILD_STATIC=ON and ARROW_TEST_LINKAGE=shared. Interface libraries with interface sources are compiled as sources of each test executable. So they use the same compile definitions as the test automatically. Testing interface libraries must be linked only by test and benchmark executables. If they are linked by other targets such as object libraries, their sources are also compiled into the targets. arrow_compute_kernels_testing and arrow_acero_testing now link arrow_compute_core_testing because they use arrow_compute_core_testing. Sources of testing interface libraries are compiled in unity build files with test sources. So we need to use a fully qualified name for MapVector in arrow/compute/test_util_internal.cc. internal:: refers to arrow::compute::internal:: in unity build files. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ARROW_BUILD_STATIC=ON doesn't mean that benchmarks are linked statically. Benchmarks use ARROW_TEST_LINK_LIBS that depends on ARROW_TEST_LINKAGE. If we use arrow_compute_static with ARROW_TEST_LINKAGE=shared, both arrow_static and arrow_shared are linked. It causes duplicated symbol errors on Windows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kou
force-pushed
the
ci-windows-build-static
branch
from
October 28, 2026 02:46
24e1a08 to
ee78b4d
Compare
Member
Author
|
@github-actions crossbow submit -g cpp |
|
Revision: ee78b4d Submitted crossbow builds: ursacomputing/crossbow @ actions-9252eadce2
|
kou
marked this pull request as ready for review
October 28, 2026 05:23
kou
requested review from
HuaHuaY,
pitrou,
wgtmac and
zanmato1984
as code owners
October 28, 2026 05:24
kou
changed the title
Member
Author
|
This is ready. I think that we should use We need to build the same source files multiple times with |
zanmato1984
approved these changes
Oct 28, 2026
zanmato1984
left a comment
zanmato1984
left a comment
Contributor
There was a problem hiding this comment.
+1
Thanks for working on this.
wgtmac
approved these changes
Oct 28, 2026
raulcd
approved these changes
Oct 28, 2026
raulcd
left a comment
raulcd
left a comment
Member
There was a problem hiding this comment.
LGTM, just a couple of minor things I have found trying to find other patterns that might not be covered by this specific build.
github-actions
Bot
added
awaiting merge
parquet/arrow/reader.cc uses OpenTelemetry headers via arrow/util/tracing_internal.h. parquet_static can't find them on Windows because we don't use a shared object library for shared and static libraries on Windows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kou
force-pushed
the
ci-windows-build-static
branch
from
October 29, 2026 03:12
1c48296 to
a958284
Compare
github-actions
Bot
added
awaiting changes
Parquet benchmarks used parquet_shared with static test libraries when ARROW_BUILD_SHARED=ON and ARROW_TEST_LINKAGE=static. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
test_util.cc is compiled as a source of each test executable so that it uses the same compile definitions as the test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kou
force-pushed
the
ci-windows-build-static
branch
from
October 29, 2026 08:02
a958284 to
5756a51
Compare
github-actions
Bot
added
awaiting change review
Member
That doesn't sound great. Not everyone uses ccache I suppose, and it can have limitations too (such as unsupported compile options). For example, "AMD64 Conda C++ AVX2" used to have 1312 build steps, now it has 1522 build steps... |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale for this change
We can't build Arrow C++ with
ARROW_BUILD_SHARED=ON,ARROW_BUILD_STATIC=ONandARROW_BUILD_TESTS=ONon Windows. Tests are linked with shared libraries by default (ARROW_TEST_LINKAGE=shared) but some targets useARROW_BUILD_STATICto choose static libraries/definitions:arrow_compute_testingis an object library and it setsARROW_STATIC/ARROW_COMPUTE_STATICas public compile definitions withARROW_BUILD_STATIC. So tests that usearrow_compute_testingare compiled withARROW_STATIC/ARROW_COMPUTE_STATICeven when they are linked with shared libraries.arrow-compute-grouper-benchmarklinksarrow_compute_staticwithARROW_BUILD_STATIC. So it's linked with both ofarrow_staticandarrow_shared.parquet_staticcan't be built with Parquet encryption on Windows too. It doesn't link OpenSSL. We don't use a shared object library for shared and static libraries on Windows. Soparquet_staticcan't find OpenSSL headers.parquet/arrow/reader.ccuses OpenTelemetry headers viaarrow/util/tracing_internal.hbutparquet_staticdoesn't link OpenTelemetry.Our CI doesn't detect them because we don't have any Windows job that uses
ARROW_BUILD_SHARED=ONandARROW_BUILD_STATIC=ON.There are similar problems that aren't detected by our CI:
parquet_sharedwith static test libraries whenARROW_BUILD_SHARED=ON,ARROW_BUILD_STATIC=ONandARROW_TEST_LINKAGE=static. So they're linked with both ofarrow_staticandarrow_shared.parquet_testingis an object library that doesn't haveARROW_STATIC/PARQUET_STATIC. Soparquet/test_util.ccis compiled without them even when tests are linked with static libraries on Windows.What changes are included in this PR?
ARROW_BUILD_STATIC=ONin the Windows CI job to detect this case.parquet_staticwhen Parquet encryption is enabled.parquet_staticwhen OpenTelemetry is enabled.arrow_compute_core_testing,arrow_compute_testing,arrow_compute_kernels_testing,arrow_acero_testingandarrow_dataset_testing. Sources of interface libraries are compiled as sources of each test executable. So they use the same compile definitions as the test. We don't need to setARROW_STATIC/ARROW_COMPUTE_STATICmanually.arrow_dataset_testingdoesn't link${ARROW_DATASET_TEST_LINK_LIBS}now.)arrow_compute_kernels_testingandarrow_acero_testinglinkarrow_compute_core_testingbecause they use it.MapVectorinarrow/compute/test_util_internal.ccbecauseinternal::refers toarrow::compute::internal::in unity build files.ARROW_TEST_LINKAGEinstead ofARROW_BUILD_STATICto choose a compute library forarrow-compute-grouper-benchmark.ARROW_TEST_LINKAGEinstead ofARROW_BUILD_SHAREDto choose a Parquet library for Parquet benchmarks.Note that the ORC adapter test still uses
ARROW_BUILD_STATICto choose libraries. It uses liborc directly and linksorc::orc. If it's linked witharrow_shared, ORC's protobuf messages are registered twice (bylibarrowand the test). We'll work on it separately if we need it.Are these changes tested?
Yes.
Are there any user-facing changes?
No.
Was AI used for this PR?
In accordance to the AI generation guidelines, please disclose below whether and how AI was used in this PR.
PR code and description written by:
Reviewed before submission by: