ARROW-17694: [C++] Remove std::optional backport by pitrou · Pull Request #14105 · apache/arrow

pitrou · 2022-09-13T07:38:07Z

Just use the C++17 standard library version.

github-actions · 2022-09-13T08:59:04Z

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

github-actions · 2022-09-13T08:59:06Z

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

pitrou · 2022-09-14T08:42:59Z

@assignUser Would you know how to change the macOS deployment target for autobrew? I tried to set it to 10.14 using the env var MACOSX_DEPLOYMENT_TARGET but it wasn't picked up:
https://github.com/ursacomputing/crossbow/actions/runs/3047525991/jobs/4911606506#step:8:6773

/Users/voltrondata/tmp/hbtmp/apache-arrow-20220913-7531-1f8l15l/cpp/src/arrow/memory_pool.cc:143:20: error: call to unavailable member function 'value': introduced in macOS 10.14
return backend.value();
~~~~~~~~^~~~~

assignUser · 2022-09-14T10:31:25Z

Not of the bat, I'll look into it. I hope it is possible otherwise that might be an issue due to CRAN building on 10.13.

assignUser · 2022-09-14T11:02:57Z

The envvar is correctly set in the wf and looking at our cmake it should get picked up. I wonder if the issue is that the deployment target can't be higher than the host? I haven't found anything documenting such behavior and the llvm/clang version installed should support c++17 (as we confirmed previously).

I also had a glance at autobrew and I don't think it is forcing the deployment target anywhere. Maybe passing it explicitly as a cmake arg in the formula works? https://github.com/pitrou/arrow/blob/ARROW-17694-std-optional/dev/tasks/homebrew-formulae/autobrew/apache-arrow.rb

pitrou · 2022-09-14T11:52:13Z

@assignUser Well, it got even worse as there's now an error about something being available only in 10.12+:
https://github.com/ursacomputing/crossbow/actions/runs/3052496381/jobs/4921948251#step:7:6790

Yet as you can see the correct compiler command-line options seem to be given... I can only assume that homebrew is doing something horrible under the hood :-(

pitrou · 2022-09-14T11:58:29Z

Revision: 93f4f3e1ed45c9c045c8e279ce6d6b95addabf68

Submitted crossbow builds: ursacomputing/crossbow @ optional-4

Task	Status
homebrew-cpp
homebrew-cpp-autobrew
homebrew-r-autobrew
homebrew-r-brew

assignUser · 2022-09-14T12:28:51Z

I looked at some other packages on CRAN using C++17 and found that httpgd uses std::optional IIUC https://github.com/nx10/httpgd/blob/ae03f478c0fa4cc09b729cc00738ad2c1ceba4ed/src/lib/belle.h#L353

I'll check their configure/autobrew script

pitrou · 2022-09-14T12:45:48Z

Revision: a18442593ac0a2fc3654247706740b8699ea9537

Submitted crossbow builds: ursacomputing/crossbow @ optional-5

Task	Status
homebrew-r-autobrew
homebrew-r-brew

assignUser · 2022-09-14T14:26:51Z

That looks promising 🤞

pitrou · 2022-09-14T15:54:43Z

@kou Would you mind giving this another look?

kou

+1

Thanks!

kou · 2022-09-15T00:20:58Z

Could you put this after -DARROW_CSV=ON to keep order?

Just use the C++17 standard library version.

pitrou · 2022-09-15T07:44:43Z

Revision: 3566dee

Submitted crossbow builds: ursacomputing/crossbow @ optional-7

Task	Status
almalinux-8-amd64
almalinux-8-arm64
almalinux-9-amd64
almalinux-9-arm64
amazon-linux-2-amd64
amazon-linux-2-arm64
centos-7-amd64
centos-8-stream-amd64
centos-8-stream-arm64
centos-9-stream-amd64
centos-9-stream-arm64
conan-maximum
conan-minimum
conda-clean
conda-linux-gcc-py310-arm64
conda-linux-gcc-py310-cpu
conda-linux-gcc-py310-cuda
conda-linux-gcc-py310-ppc64le
conda-linux-gcc-py37-arm64
conda-linux-gcc-py37-cpu-r40
conda-linux-gcc-py37-cpu-r41
conda-linux-gcc-py37-cuda
conda-linux-gcc-py37-ppc64le
conda-linux-gcc-py38-arm64
conda-linux-gcc-py38-cpu
conda-linux-gcc-py38-cuda
conda-linux-gcc-py38-ppc64le
conda-linux-gcc-py39-arm64
conda-linux-gcc-py39-cpu
conda-linux-gcc-py39-cuda
conda-linux-gcc-py39-ppc64le
conda-osx-arm64-clang-py310
conda-osx-arm64-clang-py38
conda-osx-arm64-clang-py39
conda-osx-clang-py310
conda-osx-clang-py37-r40
conda-osx-clang-py37-r41
conda-osx-clang-py38
conda-osx-clang-py39
conda-win-vs2019-py310
conda-win-vs2019-py37-r40
conda-win-vs2019-py37-r41
conda-win-vs2019-py38
conda-win-vs2019-py39
debian-bookworm-amd64
debian-bookworm-arm64
debian-bullseye-amd64
debian-bullseye-arm64
example-cpp-minimal-build-static
example-cpp-minimal-build-static-system-dependency
example-python-minimal-build-fedora-conda
example-python-minimal-build-ubuntu-venv
homebrew-cpp
homebrew-r-autobrew
homebrew-r-brew
java-jars
nuget
python-sdist
r-binary-packages
test-alpine-linux-cpp
test-build-cpp-fuzz
test-build-vcpkg-win
test-conda-cpp
test-conda-cpp-valgrind
test-conda-python-3.10
test-conda-python-3.7
test-conda-python-3.7-hdfs-2.9.2
test-conda-python-3.7-hdfs-3.2.1
test-conda-python-3.7-kartothek-latest
test-conda-python-3.7-kartothek-master
test-conda-python-3.7-pandas-0.24
test-conda-python-3.7-pandas-latest
test-conda-python-3.7-spark-v3.1.2
test-conda-python-3.8
test-conda-python-3.8-hypothesis
test-conda-python-3.8-pandas-latest
test-conda-python-3.8-pandas-nightly
test-conda-python-3.8-spark-v3.2.0
test-conda-python-3.9
test-conda-python-3.9-dask-latest
test-conda-python-3.9-dask-master
test-conda-python-3.9-pandas-master
test-conda-python-3.9-spark-master
test-debian-10-cpp-amd64
test-debian-10-cpp-i386
test-debian-11-cpp-amd64
test-debian-11-cpp-i386
test-debian-11-go-1.17
test-debian-11-python-3
test-debian-c-glib
test-debian-ruby
test-fedora-35-cpp
test-fedora-35-python-3
test-fedora-r-clang-sanitizer
test-r-arrow-backwards-compatibility
test-r-depsource-bundled
test-r-depsource-system
test-r-dev-duckdb
test-r-devdocs
test-r-gcc-11
test-r-gcc-12
test-r-install-local
test-r-linux-as-cran
test-r-linux-rchk
test-r-linux-valgrind
test-r-minimal-build
test-r-offline-maximal
test-r-offline-minimal
test-r-rhub-debian-gcc-devel-lto-latest
test-r-rhub-debian-gcc-release-custom-ccache
test-r-rhub-ubuntu-gcc-release-latest
test-r-rocker-r-base-latest
test-r-rstudio-r-base-4.1-opensuse153
test-r-rstudio-r-base-4.2-centos7-devtoolset-8
test-r-rstudio-r-base-4.2-focal
test-r-ubuntu-22.04
test-r-versions
test-skyhook-integration
test-ubuntu-18.04-cpp
test-ubuntu-18.04-cpp-release
test-ubuntu-18.04-cpp-static
test-ubuntu-18.04-r-sanitizer
test-ubuntu-20.04-cpp
test-ubuntu-20.04-cpp-17
test-ubuntu-20.04-cpp-bundled
test-ubuntu-20.04-cpp-thread-sanitizer
test-ubuntu-20.04-python-3
test-ubuntu-22.04-cpp
test-ubuntu-c-glib
test-ubuntu-default-docs
test-ubuntu-ruby
ubuntu-bionic-amd64
ubuntu-bionic-arm64
ubuntu-focal-amd64
ubuntu-focal-arm64
ubuntu-jammy-amd64
ubuntu-jammy-arm64
wheel-macos-big-sur-cp310-arm64
wheel-macos-big-sur-cp310-universal2
wheel-macos-big-sur-cp38-arm64
wheel-macos-big-sur-cp39-arm64
wheel-macos-big-sur-cp39-universal2
wheel-macos-mojave-cp310-amd64
wheel-macos-mojave-cp37-amd64
wheel-macos-mojave-cp38-amd64
wheel-macos-mojave-cp39-amd64
wheel-manylinux2014-cp310-amd64
wheel-manylinux2014-cp310-arm64
wheel-manylinux2014-cp37-amd64
wheel-manylinux2014-cp37-arm64
wheel-manylinux2014-cp38-amd64
wheel-manylinux2014-cp38-arm64
wheel-manylinux2014-cp39-amd64
wheel-manylinux2014-cp39-arm64
wheel-windows-cp310-amd64
wheel-windows-cp37-amd64
wheel-windows-cp38-amd64
wheel-windows-cp39-amd64

pitrou · 2022-09-15T10:34:42Z

The test failures seem the same as usual, so I'm going to merge.

ursabot · 2022-09-15T19:52:53Z

Benchmark runs are scheduled for baseline = 2749fef and contender = 5e49174. 5e49174 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
[Failed ⬇️0.51% ⬆️0.03%] test-mac-arm
[Failed ⬇️0.56% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.6% ⬆️0.39%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] 5e49174d ec2-t3-xlarge-us-east-2
[Finished] 5e49174d test-mac-arm
[Failed] 5e49174d ursa-i9-9960x
[Finished] 5e49174d ursa-thinkcentre-m75q
[Finished] 2749fef9 ec2-t3-xlarge-us-east-2
[Failed] 2749fef9 test-mac-arm
[Failed] 2749fef9 ursa-i9-9960x
[Finished] 2749fef9 ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. 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

Just use the C++17 standard library version. Authored-by: Antoine Pitrou <antoine@python.org> Signed-off-by: Antoine Pitrou <antoine@python.org>

pitrou force-pushed the ARROW-17694-std-optional branch from d0bdb0b to 9168b04Compare September 13, 2022 08:11

This comment was marked as outdated.
Sign in to view

github-actionsBot added Component: Documentation Component: FlightRPC Component: Gandiva Component: C++ Component: GLib Component: Python Component: R Component: Parquet labels Sep 13, 2022

pitrou commented Sep 13, 2022
View reviewed changes

Comment thread.github/workflows/cpp.yml Outdated

pitrou force-pushed the ARROW-17694-std-optional branch from 0809adb to 5bd1967Compare September 13, 2022 14:27

This comment was marked as outdated.
Sign in to view

pitrou force-pushed the ARROW-17694-std-optional branch from 93f4f3e to a184425Compare September 14, 2022 12:45

This comment was marked as outdated.
Sign in to view

pitrou requested a review from kou September 14, 2022 15:54

kou approved these changes Sep 15, 2022
View reviewed changes

Comment threaddev/tasks/homebrew-formulae/autobrew/apache-arrow.rb Outdated
kouSep 15, 2022
Copy link
Copy Markdown
Member
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you put this after -DARROW_CSV=ON to keep order?

pitrou added 10 commits September 15, 2022 09:31

ARROW-17694: [C++] Remove std::optional backport
3a4e573
Just use the C++17 standard library version.

Remove remaining references to util::optional
cbca6e0

Dump contents of src/Makevars.win
6674815

Bump ccache version and avoid reusing old caches
c92378a

Suggestions for R and ccache
1507976

Assorted fixes
78c26f2

Try to set deployment target in formula
1cd2b2a

Try setting _LIBCPP_DISABLE_AVAILABILITY instead
4a3faca

Remove test_hdfs workaround
96dbda9

Nit
3566dee

pitrou force-pushed the ARROW-17694-std-optional branch from 4941806 to 3566deeCompare September 15, 2022 07:35

pitrou merged commit 5e49174 into apache:masterSep 15, 2022

pitrou deleted the ARROW-17694-std-optional branch September 15, 2022 10:37

jeroen mentioned this pull request Oct 24, 2022
apache-arrow 10.0.0 autobrew/homebrew-core#119
Merged

asfimport mentioned this pull request Sep 15, 2022
[C++] Remove optional backport #32933
Closed

jorisvandenbossche mentioned this pull request Jan 24, 2023
[Python] Unable to build pyarrow wheel for macOS 10.13 when installing from PyPI #33826
Open

Uh oh!

Conversation

pitrou commented Sep 13, 2022

Uh oh!

This comment was marked as outdated.

github-actionsBot commented Sep 13, 2022

Uh oh!

github-actionsBot commented Sep 13, 2022

Uh oh!

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

pitrou commented Sep 14, 2022

Uh oh!

assignUser commented Sep 14, 2022

Uh oh!

assignUser commented Sep 14, 2022

Uh oh!

This comment was marked as outdated.

pitrou commented Sep 14, 2022

Uh oh!

pitrou commented Sep 14, 2022

Uh oh!

assignUser commented Sep 14, 2022

Uh oh!

pitrou commented Sep 14, 2022

Uh oh!

assignUser commented Sep 14, 2022

Uh oh!

This comment was marked as outdated.

pitrou commented Sep 14, 2022

Uh oh!

kou left a comment

Choose a reason for hiding this comment

Uh oh!

kouSep 15, 2022

Choose a reason for hiding this comment

Uh oh!

pitrou commented Sep 15, 2022

Uh oh!

pitrou commented Sep 15, 2022

Uh oh!

ursabot commented Sep 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants