Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Conversation

westonpace
Copy link
Member

This change modifies the destination directory of the clone so it is unique.

…n't clobber a source directory when doing an 'inline' cmake build
@westonpace westonpace changed the title ARROW-15700: [C++] Compilation of ARROW_ENGINE fails if doing an "inline" build ARROW-15709: [C++] Compilation of ARROW_ENGINE fails if doing an "inline" build Feb 17, 2022
@github-actions
Copy link

@github-actions
Copy link

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

@westonpace
Copy link
Member Author

This is blocking conbench and is a minor change so I'll push on green CI.

@github-actions
Copy link

@westonpace
Copy link
Member Author

Failure is a deadlock in the threading utility test. I've reported it but it's unrelated to this change.

@ursabot
Copy link

ursabot commented Feb 17, 2022

Benchmark runs are scheduled for baseline = 3ce4f81 and contender = afaa92e. afaa92e is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Failed] ec2-t3-xlarge-us-east-2
[Failed] test-mac-arm
[Failed] ursa-i9-9960x
[Failed] 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

@pitrou
Copy link
Member

pitrou commented Feb 17, 2022

Can we also stop doing a git clone and simply fetch a download URL from Github?

@pitrou
Copy link
Member

pitrou commented Feb 17, 2022

It seems this change wiped any non-tracked files from my entire Arrow checkout, for some reason. I will do an emergency revert and ask you to use a plain download URL rather than a git clone.

[266/553] Performing download step (git clone) for 'substrait_ep'
-- Avoiding repeated git clone, stamp file is up to date: '/home/antoine/arrow/dev/cpp/build-test/src/arrow/engine/substrait_ep-prefix/src/substrait_ep-stamp/substrait_ep-gitclone-lastrun.txt'
[507/553] Performing update step for 'substrait_ep'
FAILED: src/arrow/engine/substrait_ep-prefix/src/substrait_ep-stamp/substrait_ep-update /home/antoine/arrow/dev/cpp/build-test/src/arrow/engine/substrait_ep-prefix/src/substrait_ep-stamp/substrait_ep-update 
cd /home/antoine/arrow/dev/cpp/build-test/src/arrow/engine/substrait_clone && /home/antoine/miniconda3/envs/pyarrow/bin/cmake -P /home/antoine/arrow/dev/cpp/build-test/src/arrow/engine/substrait_ep-prefix/tmp/substrait_ep-gitupdate.cmake
From https://github.com/apache/arrow
 t [tag update]          apache-arrow-6.0.1 -> apache-arrow-6.0.1
 t [tag update]          apache-arrow-7.0.0 -> apache-arrow-7.0.0
Ignoring path cpp/build-test/src/arrow/engine/substrait/
warning: failed to remove python/repaired_wheels/pyarrow-7.0.0.dev19+gf2c9d20d9-cp36-cp36m-manylinux_2_17_x86_64.manylinux2014_x86_64.whl: Permission denied
CMake Error at /home/antoine/arrow/dev/cpp/build-test/src/arrow/engine/substrait_ep-prefix/tmp/substrait_ep-gitupdate.cmake:167 (execute_process):
  execute_process failed command indexes:

    1: "Child return code: 1"



[508/553] Building CXX object src/parquet/CMakeFiles/parquet-arrow-test.dir/arrow/arrow_reader_writer_test.cc.o
ninja: build stopped: subcommand failed.

kou pushed a commit that referenced this pull request Feb 25, 2022
…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](#12444) for the above [caused](#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 #12457 from jvanstraten/ARROW-15709-Compilation-of-ARROW-ENGINE-fails-if-doi

Authored-by: Jeroen van Straten <jeroen.van.straten@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
marin-ma pushed a commit to marin-ma/arrow-1 that referenced this pull request Mar 28, 2022
…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 <jeroen.van.straten@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
(cherry picked from commit 89cc6b3)
zhouyuan pushed a commit to oap-project/arrow that referenced this pull request Apr 18, 2022
* 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 <bengilgit@gmail.com>
Co-authored-by: Jeroen van Straten <jeroen.van.straten@gmail.com>
Co-authored-by: Weston Pace <weston.pace@gmail.com>
Signed-off-by: Weston Pace <weston.pace@gmail.com>

* 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 <jeroen.van.straten@gmail.com>
Signed-off-by: Weston Pace <weston.pace@gmail.com>
(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 <vibhatha@gmail.com>
Signed-off-by: Weston Pace <weston.pace@gmail.com>
(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 <jeroen.van.straten@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
(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 <antoine@python.org>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
(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 <weston.pace@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
(cherry picked from commit 8fce593)

* Enable TPCH Q6 & Q1

Co-authored-by: Benjamin Kietzman <bengilgit@gmail.com>
Co-authored-by: Jeroen van Straten <jeroen.van.straten@gmail.com>
Co-authored-by: Weston Pace <weston.pace@gmail.com>
Co-authored-by: Vibhatha Abeykoon <vibhatha@gmail.com>
Co-authored-by: Antoine Pitrou <antoine@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants