ARROW-15709: [C++] Compilation of ARROW_ENGINE fails if doing an "inline" build #12457

jvanstraten · 2022-02-17T21:35:05Z

This should fix:

inline builds in general (ARROW-15709);
weird stuff with inline builds causing non-tracked files to be deleted that the previous fix for the above caused
dependencies on git for downloading dependencies (ARROW-15760);
the build process for Substrait previously being treated as something too special to use Arrow's normal method for dealing with third-party dependencies (i.e. ThirdpartyToolchain.cmake)

Initial attempt at making something functional to solve this issue properly.

The use of add_arrow_lib in ThirdpartyToolchain.cmake is certainly odd, and I'm sure I'm not following best practices in that file in general. I could use some advice on what the proper way to do this would be. Some of the issues:

The CMake property specifying that a path refers to a generated file is scoped only to the current CMake file, so only moving the externalproject_add over to ThirdpartyToolchain.cmake resulted in the add_arrow_lib in src/arrow/engine failing due to missing source files. An object library didn't seem to resolve it either, and there are probably portability issues with that anyway, so that's why I ended up just using add_arrow_lib.
Unlike all the other third-party dependencies (AFAICT), Substrait can't currently be installed, so Substrait_SOURCE=SYSTEM makes no sense. In the end I just decided to override it, but that's probably not ideal.
Substrait doesn't have releases yet, so I had to resort to a git hash instead.

github-actions · 2022-02-17T21:35:24Z

https://issues.apache.org/jira/browse/ARROW-15709

westonpace · 2022-02-17T21:47:45Z

@kou @pitrou We could use input on this approach. The Substrait integration is a little unique because the .proto files themselves are not owned by us. We could fork a copy of the proto files but that would make keeping up with updated versions a bit tricky.

Protobuf is also a bit tricky because the version of the code generation tool (protoc) must match the version of the linked protobuf library exactly (this is different from flatbuffers which doesn't require a runtime dependency).

Previously we were downloading the proto files using git and running protoc within the engine directory. This had some negative side effects (#12454) mainly due to our use of git. Even beyond git however it was rather strange that this was resulting in a call to externalproject_add outside of ThirdPartyToolchain.cmake so @jvanstraten and I agreed it might be clear to treat Substrait as a "build-from-source only" third party dependency. This PR is @jvanstraten 's attempt to make that work but it is also odd as it requires a call to add_arrow_lib inside of ThirdPartyToolchain.cmake.

Any suggestions on a better approach or feedback on our current approach would be very much welcomed.

kou · 2022-02-20T23:47:26Z

How about using just an OBJECT library https://cmake.org/cmake/help/latest/command/add_library.html#object-lib instead of arrow_add_lib()?

diff --git a/cpp/cmake_modules/BuildUtils.cmake b/cpp/cmake_modules/BuildUtils.cmake
index 391c43e0ac..3afecfc562 100644
--- a/cpp/cmake_modules/BuildUtils.cmake
+++ b/cpp/cmake_modules/BuildUtils.cmake
@@ -297,6 +297,14 @@ function(ADD_ARROW_LIB LIB_NAME)
       target_precompile_headers(${LIB_NAME}_objlib PRIVATE ${ARG_PRECOMPILED_HEADERS})
     endif()
     set(LIB_DEPS $<TARGET_OBJECTS:${LIB_NAME}_objlib>)
+    # We need to specify $<TARGET_OBJECTS:XXX> explicitly to SHARED
+    # library and STATIC library because $<TARGET_OBJECTS:XXX> in
+    # OBJECT isn't propagated to SHARED library and STATIC library.
+    foreach(ARG_SOURCE ${ARG_SOURCES})
+      if(ARG_SOURCE MATCHES "^\\$<TARGET_OBJECTS:")
+        list(APPEND LIB_DEPS ${ARG_SOURCE})
+      endif()
+    endforeach()
     set(LIB_INCLUDES)
     set(EXTRA_DEPS)
 
diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake
index 8b53272452..9143550df9 100644
--- a/cpp/cmake_modules/ThirdpartyToolchain.cmake
+++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake
@@ -1687,22 +1687,13 @@ macro(build_substrait)
 
   add_custom_target(substrait_gen ALL DEPENDS ${SUBSTRAIT_PROTO_GEN_ALL})
 
-  add_arrow_lib(arrow_substrait
-                SOURCES
-                ${SUBSTRAIT_SOURCES}
-                DEPENDENCIES
-                substrait_gen
-                SHARED_LINK_LIBS
-                ${ARROW_PROTOBUF_LIBPROTOBUF}
-                STATIC_LINK_LIBS
-                ${ARROW_PROTOBUF_LIBPROTOBUF}
-                PRIVATE_INCLUDES
-                ${SUBSTRAIT_CPP_DIR})
-
-  set(SUBSTRAIT_DEPENDENCIES substrait_gen)
-  set(SUBSTRAIT_SHARED arrow_substrait_shared)
-  set(SUBSTRAIT_STATIC arrow_substrait_static)
-  set(SUBSTRAIT_INCLUDES ${SUBSTRAIT_CPP_DIR})
+  add_library(substrait OBJECT ${SUBSTRAIT_SOURCES})
+  set_target_properties(substrait
+    PROPERTIES
+    INCLUDE_DIRECTORIES ${SUBSTRAIT_CPP_DIR}
+    POSITION_INDEPENDENT_CODE ON)
+  add_dependencies(substrait substrait_gen)
+  get_target_property(SUBSTRAIT_INCLUDES substrait INCLUDE_DIRECTORIES)
 endmacro()
 
 if(ARROW_WITH_SUBSTRAIT)
diff --git a/cpp/src/arrow/engine/CMakeLists.txt b/cpp/src/arrow/engine/CMakeLists.txt
index 50a84464c0..ff88f47e4a 100644
--- a/cpp/src/arrow/engine/CMakeLists.txt
+++ b/cpp/src/arrow/engine/CMakeLists.txt
@@ -36,9 +36,10 @@ add_arrow_lib(arrow_engine
               OUTPUTS
               ARROW_ENGINE_LIBRARIES
               DEPENDENCIES
-              ${SUBSTRAIT_DEPENDENCIES}
+              substrait
               SOURCES
               ${ARROW_ENGINE_SRCS}
+              $<TARGET_OBJECTS:substrait>
               PRECOMPILED_HEADERS
               "$<$<COMPILE_LANGUAGE:CXX>:arrow/engine/pch.h>"
               SHARED_LINK_FLAGS
@@ -46,11 +47,9 @@ add_arrow_lib(arrow_engine
               SHARED_LINK_LIBS
               arrow_shared
               arrow_dataset_shared
-              ${SUBSTRAIT_SHARED}
               STATIC_LINK_LIBS
               arrow_static
               arrow_dataset_static
-              ${SUBSTRAIT_STATIC}
               PRIVATE_INCLUDES
               ${SUBSTRAIT_INCLUDES})

jvanstraten · 2022-02-21T11:52:54Z

@kou Thanks for the input! It's certainly a lot nicer to do it that way, but unfortunately, we're not quite there yet... I got it working in most build environments using your diff with some minor changes (had to add protobuf to the include list for the object library, and add its runtime library to arrow_engine), but it fails here:

CMake Error at cmake_modules/BuildUtils.cmake:287 (add_library):
  OBJECT library "arrow_engine_objlib" contains:

    capabilities.pb.cc.o
    expression.pb.cc.o
    extensions.pb.cc.o
    function.pb.cc.o
    parameterized_types.pb.cc.o
    plan.pb.cc.o
    relations.pb.cc.o
    type.pb.cc.o
    type_expressions.pb.cc.o

  but may contain only sources that compile, header files, and other files
  that would not affect linking of a normal library.

I feel like the addition to BuildUtils.cmake is either causing this or supposed to resolve this (and failing to do so in this particular case), but other than that, my skill level for CMake is insufficient to see what exactly is going on here.

pitrou · 2022-02-21T17:58:53Z

cpp/cmake_modules/ThirdpartyToolchain.cmake

+  set(SUBSTRAIT_SOURCE_URL "$ENV{ARROW_SUBSTRAIT_URL}")
+else()
+  set_urls(SUBSTRAIT_SOURCE_URL
+           "https://github.com/substrait-io/substrait/archive/${ARROW_SUBSTRAIT_BUILD_VERSION}.tar.gz"


+1, this will be much better than involving some git commands.

pitrou · 2022-02-21T18:00:08Z

cpp/src/arrow/engine/CMakeLists.txt

+              substrait
              SOURCES
              ${ARROW_ENGINE_SRCS}
+              $<TARGET_OBJECTS:substrait>


Hmm... is there a particular reason for this? Should we instead create a static library containing these objects? Or perhaps I'm entirely misunderstanding this?

We can use a static library instead of an object library without using add_arrow_lib(). We can just use raw add_library().

Looks like that's working, 577979b. I wasn't sure how generally-applicable linking static libraries with -fPIC into shared libraries would be. I'm also not sure why I still need to specify PRIVATE_INCLUDES ${SUBSTRAIT_INCLUDES} for the add_arrow_lib call (shouldn't CMake be propagating public include directories to dependent libraries?), but I couldn't get it to work without it.

kou · 2022-02-22T21:18:34Z

cpp/cmake_modules/FindSubstrait.cmake

+# under the License.
+
+# Currently, we can only build Substrait from source.
+set(SUBSTRAIT_FOUND NO)


Can we remove this file?
It seems that this file is never used.

kou · 2022-02-22T21:20:29Z

cpp/src/arrow/engine/CMakeLists.txt

+              DEPENDENCIES
+              substrait


Can we remove this?
This will be automatically added by using substrait target.

It doesn't build (multithreaded) without it. I think it ensures that substrait is built completely prior to compilation of arrow_engine, rather than just prior to linking, which matters because arrow_engine uses generated code. Making it depend on substrait_gen would also work, if you prefer that option (IMO, the less arrow_engine depends on implementation details of ThirdpartyToolchain.cmake the better, so I set it to just substrait).

It seems that this is a problem of our add_arrow_lib():

diff --git a/cpp/cmake_modules/BuildUtils.cmake b/cpp/cmake_modules/BuildUtils.cmake index 391c43e0ac..7517e4e806 100644 --- a/cpp/cmake_modules/BuildUtils.cmake +++ b/cpp/cmake_modules/BuildUtils.cmake @@ -374,6 +374,13 @@ function(ADD_ARROW_LIB LIB_NAME) "$<INSTALL_INTERFACE:${ARG_SHARED_INSTALL_INTERFACE_LIBS}>" LINK_PRIVATE ${ARG_SHARED_PRIVATE_LINK_LIBS}) + if(USE_OBJLIB) + foreach(ARG_SHARED_LINK_LIB ${ARG_SHARED_LINK_LIBS}) + if(TARGET ${ARG_SHARED_LINK_LIB}) + add_dependencies(${LIB_NAME}_objlib ${ARG_SHARED_LINK_LIB}) + endif() + endforeach() + endif() if(ARROW_RPATH_ORIGIN) if(APPLE) @@ -449,6 +456,13 @@ function(ADD_ARROW_LIB LIB_NAME) if(ARG_STATIC_LINK_LIBS) target_link_libraries(${LIB_NAME}_static LINK_PRIVATE "$<BUILD_INTERFACE:${ARG_STATIC_LINK_LIBS}>") + if(USE_OBJLIB) + foreach(ARG_STATIC_LINK_LIB ${ARG_STATIC_LINK_LIBS}) + if(TARGET ${ARG_STATIC_LINK_LIB}) + add_dependencies(${LIB_NAME}_objlib ${ARG_STATIC_LINK_LIB}) + endif() + endforeach() + endif() endif() install(TARGETS ${LIB_NAME}_static ${INSTALL_IS_OPTIONAL} diff --git a/cpp/src/arrow/engine/CMakeLists.txt b/cpp/src/arrow/engine/CMakeLists.txt index 13d50b4244..edb878939f 100644 --- a/cpp/src/arrow/engine/CMakeLists.txt +++ b/cpp/src/arrow/engine/CMakeLists.txt @@ -35,8 +35,6 @@ add_arrow_lib(arrow_engine arrow-engine OUTPUTS ARROW_ENGINE_LIBRARIES - DEPENDENCIES - substrait SOURCES ${ARROW_ENGINE_SRCS} PRECOMPILED_HEADERS

Okay, that fixes it on my end, too. fffa65c

kou · 2022-02-22T21:26:07Z

cpp/cmake_modules/ThirdpartyToolchain.cmake

+  set_target_properties(substrait PROPERTIES POSITION_INDEPENDENT_CODE ON)
+  target_include_directories(substrait PUBLIC ${SUBSTRAIT_INCLUDES})
+  target_link_libraries(substrait INTERFACE ${ARROW_PROTOBUF_LIBPROTOBUF})
+  add_dependencies(substrait substrait_gen)


Could you add substrait to ARROW_BUNDLED_STATIC_LIBS?

Suggested change

add_dependencies(substrait substrait_gen)

list(APPEND ARROW_BUNDLED_STATIC_LIBS substrait)

If we don't add this, users can't link to libarrow_engine.a statically with libarrow_bundled_dependencies.a.

Added in 3d7bac9

The removal of add_dependencies(substrait substrait_gen) seems unrelated. I suppose it's technically redundant, since CMake will generate the .cc files by way of needing them as input for compiling substrait, and the .h files just so happen to be generated by the same command. If for whatever reason only the generated .h files are removed though, CMake won't know to recreate them without that rule.

Ah, sorry. It's my fault.

…ED anyway

…ions

jvanstraten · 2022-02-23T09:20:20Z

Failure of AMD64 Ubuntu 20.04 GLib & Ruby for
fffa65c seems to be unrelated (zlib download failure, I think). It builds correctly when I run it with archery locally.

kou · 2022-02-23T21:04:20Z

@github-actions crossbow submit python-sdist

github-actions · 2022-02-23T21:05:37Z

Revision: fffa65c

Submitted crossbow builds: ursacomputing/crossbow @ actions-1676

Task	Status
python-sdist

jvanstraten · 2022-02-24T16:53:11Z

@kou @pitrou @westonpace I believe this is ready to be merged, or am I still missing something?

pitrou · 2022-02-24T17:33:43Z

cpp/cmake_modules/BuildUtils.cmake

                          LINK_PRIVATE
                          ${ARG_SHARED_PRIVATE_LINK_LIBS})

+    if(USE_OBJLIB)


Can you add a comment explaining this snippet?

Added something in c2380ee. @kou, please verify if what I wrote for it is correct and complete.

pitrou · 2022-02-24T17:34:04Z

cpp/cmake_modules/BuildUtils.cmake

                          ${ARG_SHARED_PRIVATE_LINK_LIBS})

+    if(USE_OBJLIB)
+      foreach(ARG_SHARED_LINK_LIB ${ARG_SHARED_LINK_LIBS})


Minor, but naming ARG_SOMETHING a variable that is not one of the original function arguments is confusing.

pitrou · 2022-02-24T17:36:28Z

cpp/cmake_modules/ThirdpartyToolchain.cmake

+      add_custom_command(OUTPUT "${SUBSTRAIT_PROTO_GEN}.${EXT}"
+                         COMMAND ${ARROW_PROTOBUF_PROTOC} "-I${SUBSTRAIT_LOCAL_DIR}/proto"
+                                 "--cpp_out=${SUBSTRAIT_CPP_DIR}"
+                                 "${SUBSTRAIT_LOCAL_DIR}/proto/substrait/${SUBSTRAIT_PROTO}.proto"
+                         DEPENDS ${PROTO_DEPENDS} substrait_ep)


It seems like it compiles each proto file twice (once per extension)? Is that desired?

No, probably not. I noticed it too when I inherited this code, but incorrectly assumed it was written like that because add_custom_command only supported a single output file. 7ad2e1f

pitrou · 2022-02-24T17:37:40Z

Perhaps @kou can also double-check the CMake changes.

westonpace

I have nothing further to add. I think this looks much nicer than what we had before. We might try and take some snippets from here and push them upstream into the Substrait project's documentation in some kind of "getting started with C++" section.

kou

+1

kou · 2022-02-25T07:48:03Z

cpp/cmake_modules/BuildUtils.cmake

                          LINK_PRIVATE
                          ${ARG_SHARED_PRIVATE_LINK_LIBS})

+    if(USE_OBJLIB)


ursabot · 2022-02-25T08:01:40Z

Benchmark runs are scheduled for baseline = acfd1d2 and contender = 89cc6b3. 89cc6b3 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Finished ⬇️0.21% ⬆️0.0%] test-mac-arm
[Finished ⬇️0.71% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.3% ⬆️0.0%] ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

…ine" build This should fix: - inline builds in general (ARROW-15709); - [weird stuff with inline builds causing non-tracked files to be deleted](https://github.com/apache/arrow;/pull/12444#issuecomment-1043143303) that the [previous fix](apache#12444) for the above [caused](apache#12454) - dependencies on git for downloading dependencies (ARROW-15760); - the build process for Substrait previously being treated as something too special to use Arrow's normal method for dealing with third-party dependencies (i.e. `ThirdpartyToolchain.cmake`) --- Initial attempt at making something functional to solve this issue properly. The use of `add_arrow_lib` in `ThirdpartyToolchain.cmake` is certainly odd, and I'm sure I'm not following best practices in that file in general. I could use some advice on what the proper way to do this would be. Some of the issues: - The CMake property specifying that a path refers to a generated file is scoped only to the current CMake file, so only moving the `externalproject_add` over to `ThirdpartyToolchain.cmake` resulted in the `add_arrow_lib` in `src/arrow/engine` failing due to missing source files. An object library didn't seem to resolve it either, and there are probably portability issues with that anyway, so that's why I ended up just using `add_arrow_lib`. - Unlike all the other third-party dependencies (AFAICT), Substrait can't currently be installed, so `Substrait_SOURCE=SYSTEM` makes no sense. In the end I just decided to override it, but that's probably not ideal. - Substrait doesn't have releases yet, so I had to resort to a git hash instead. </details> Closes apache#12457 from jvanstraten/ARROW-15709-Compilation-of-ARROW-ENGINE-fails-if-doi Authored-by: Jeroen van Straten <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]> (cherry picked from commit 89cc6b3)

@bkietz

* ARROW-15238: [C++] ARROW_ENGINE module with substrait consumer Continuation of apache#11707. I'm taking over from @bkietz for now because he's unavailable right now for personal reasons. Closes apache#12279 from jvanstraten/substrait-consumer Lead-authored-by: Benjamin Kietzman <[email protected]> Co-authored-by: Jeroen van Straten <[email protected]> Co-authored-by: Weston Pace <[email protected]> Signed-off-by: Weston Pace <[email protected]> * ARROW-15700: [C++] Compilation error on Ubuntu 18.04 Modified Substrait consumer interaction with libprotobuf to support versions down to 3.0.0, which is the minimum required due to Substrait's usage of proto3 syntax. Tested locally with: ``` export ARROW_PROTOBUF_URL=https://github.com/protocolbuffers/protobuf/releases/download/v3.0.0/protobuf-cpp-3.0.0.tar.gz cmake \ --preset ninja-debug \ -DProtobuf_SOURCE=BUNDLED \ -DARROW_PROTOBUF_BUILD_VERSION=v3.0.0 \ -DARROW_PROTOBUF_BUILD_SHA256_CHECKSUM=318e8f375fb4e5333975a40e0d1215e855b4a8c581d692eb0eb7df70db1a8d4e ``` (Is there an easier way to do this without modifying versions.txt or 751fb9d? Also, the env var is needed only because Google isn't at all consistent with their release file naming that far back.) It'd also be nice to add this to CI, but it's probably excessive to always run for a PR, unless combined with some other run. Closes apache#12448 from jvanstraten/ARROW-15700-Compilation-error-on-Ubuntu-18-04 Authored-by: Jeroen van Straten <[email protected]> Signed-off-by: Weston Pace <[email protected]> (cherry picked from commit 7d16a78) * ARROW-15258: [C++] Easy options to create a source node from a table This PR includes the addition of `TableSourceNode` to create a `ExecNode` easily using a table as the data source. ### TODO - [x] Fix test case for chunk_size Closes apache#12267 from vibhatha/arrow-15258-rb Authored-by: Vibhatha Abeykoon <[email protected]> Signed-off-by: Weston Pace <[email protected]> (cherry picked from commit fffdca2) * ARROW-15709: [C++] Compilation of ARROW_ENGINE fails if doing an "inline" build This should fix: - inline builds in general (ARROW-15709); - [weird stuff with inline builds causing non-tracked files to be deleted](https://github.com/apache/arrow;/pull/12444#issuecomment-1043143303) that the [previous fix](apache#12444) for the above [caused](apache#12454) - dependencies on git for downloading dependencies (ARROW-15760); - the build process for Substrait previously being treated as something too special to use Arrow's normal method for dealing with third-party dependencies (i.e. `ThirdpartyToolchain.cmake`) --- Initial attempt at making something functional to solve this issue properly. The use of `add_arrow_lib` in `ThirdpartyToolchain.cmake` is certainly odd, and I'm sure I'm not following best practices in that file in general. I could use some advice on what the proper way to do this would be. Some of the issues: - The CMake property specifying that a path refers to a generated file is scoped only to the current CMake file, so only moving the `externalproject_add` over to `ThirdpartyToolchain.cmake` resulted in the `add_arrow_lib` in `src/arrow/engine` failing due to missing source files. An object library didn't seem to resolve it either, and there are probably portability issues with that anyway, so that's why I ended up just using `add_arrow_lib`. - Unlike all the other third-party dependencies (AFAICT), Substrait can't currently be installed, so `Substrait_SOURCE=SYSTEM` makes no sense. In the end I just decided to override it, but that's probably not ideal. - Substrait doesn't have releases yet, so I had to resort to a git hash instead. </details> Closes apache#12457 from jvanstraten/ARROW-15709-Compilation-of-ARROW-ENGINE-fails-if-doi Authored-by: Jeroen van Straten <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]> (cherry picked from commit 89cc6b3) * ARROW-15830: [C++] Ensure target directory exists before running Substrait generation Closes apache#12548 from pitrou/ARROW-15830-ubuntu-cpp-bundled Authored-by: Antoine Pitrou <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]> (cherry picked from commit e989fb3) * ARROW-15850: [C++] Engine substrait headers missing from install Closes apache#12569 from westonpace/feature/ARROW-15850--substrait-headers-missing Authored-by: Weston Pace <[email protected]> Signed-off-by: David Li <[email protected]> (cherry picked from commit 8fce593) * Enable TPCH Q6 & Q1 Co-authored-by: Benjamin Kietzman <[email protected]> Co-authored-by: Jeroen van Straten <[email protected]> Co-authored-by: Weston Pace <[email protected]> Co-authored-by: Vibhatha Abeykoon <[email protected]> Co-authored-by: Antoine Pitrou <[email protected]>

github-actions bot added the Component: C++ label Feb 17, 2022

jvanstraten added 3 commits February 18, 2022 11:40

First attempt at building Substrait via ThirdpartyToolchain.cmake

b683d0b

Force Substrait build from source

b40ca72

Remove defunct CMake options

fd6e5d4

jvanstraten force-pushed the ARROW-15709-Compilation-of-ARROW-ENGINE-fails-if-doi branch from a049bdd to fd6e5d4 Compare February 18, 2022 10:48

Use object library to compile Substrait

66c8cc2

pitrou reviewed Feb 21, 2022

View reviewed changes

Try using static library instead

577979b

jvanstraten marked this pull request as ready for review February 22, 2022 18:27

kou reviewed Feb 22, 2022

View reviewed changes

jvanstraten added 3 commits February 22, 2022 22:26

Remove FindSubstrait.cmake, since Substrait_SOURCE is forced to BUNDL…

b108e3e

…ED anyway

Add substrait to ARROW_BUNDLED_STATIC_LIBS if compiled

3d7bac9

Update dependency management in add_arrow_lib() as per @kou's suggest…

fffa65c

…ions

jvanstraten mentioned this pull request Feb 23, 2022

ARROW-15760: [C++] Avoid hard dependency on git in cmake (download tarballs from github instead) #12494

Closed

pitrou mentioned this pull request Feb 23, 2022

MINOR: [CI][Python] Disable Arrow Engine (substrait) in the ubuntu-python-sdist-test build #12493

Closed

pitrou reviewed Feb 24, 2022

View reviewed changes

jvanstraten added 3 commits February 24, 2022 23:11

Use single add_custom_commands for both protoc outputs

7ad2e1f

Add comments to additions in BuildUtils.cmake

c2380ee

Remove ARG_ prefix for loop variables

eb88cfd

westonpace reviewed Feb 24, 2022

View reviewed changes

kou approved these changes Feb 25, 2022

View reviewed changes

cpp/cmake_modules/BuildUtils.cmake

LINK_PRIVATE

${ARG_SHARED_PRIVATE_LINK_LIBS})

if(USE_OBJLIB)

Copy link

Member

kou Feb 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right.

kou closed this in 89cc6b3 Feb 25, 2022

	add_dependencies(substrait substrait_gen)

	list(APPEND ARROW_BUNDLED_STATIC_LIBS substrait)

ARROW-15709: [C++] Compilation of ARROW_ENGINE fails if doing an "inline" build #12457

ARROW-15709: [C++] Compilation of ARROW_ENGINE fails if doing an "inline" build #12457

Uh oh!

Conversation

jvanstraten commented Feb 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Feb 17, 2022

Uh oh!

westonpace commented Feb 17, 2022

Uh oh!

kou commented Feb 20, 2022

Uh oh!

jvanstraten commented Feb 21, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kou Feb 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jvanstraten commented Feb 23, 2022

Uh oh!

kou commented Feb 23, 2022

Uh oh!

github-actions bot commented Feb 23, 2022

Uh oh!

jvanstraten commented Feb 24, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pitrou commented Feb 24, 2022

Uh oh!

westonpace left a comment

Choose a reason for hiding this comment

Uh oh!

kou left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ursabot commented Feb 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

jvanstraten commented Feb 17, 2022 •

edited

Loading

kou Feb 21, 2022 •

edited

Loading

ursabot commented Feb 25, 2022 •

edited

Loading