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

GH-39930: [C++] Use Requires instead of Libs for system RE2 in arrow.pc #39932

Merged
merged 1 commit into from
Feb 5, 2024

Conversation

kou
Copy link
Member

@kou kou commented Feb 4, 2024

Rationale for this change

We chose Libs{,.private} with libre2.a for system RE2 in GH-10626. Because "Require{,.private} re2" may add "-std=c++11". If "-std=c++11" was added, users can't build Apache Arrow C++ because Apache Arrow C++ requires C++17 or later.

But this approach doesn't work with RE2 2024-06-01 or later because it at least requires Abseil. If we keep the Libs{,.private} approach, we also need to add Abseil libraries to Libs{,.private}. But it's unmaintainable.

What changes are included in this PR?

Let's use "Requires{,.private} re2" instead of Libs{,.private}. I hope recent re2.pc doesn't add "-std=c++11".

Are these changes tested?

Yes.

Are there any user-facing changes?

Yes.

…arrow.pc

We chose Libs{,.private} with libre2.a for system RE2 in
apacheGH-10626. Because "Require{,.private} re2" may add "-std=c++11". If
"-std=c++11" was added, users can't build Apache Arrow C++ because
Apache Arrow C++ requires C++17 or later.

But this approach doesn't work with RE2 2024-06-01 or later because it
at least requires Abseil. If we keep the Libs{,.private} approach, we
also need to add Abseil libraries to Libs{,.private}. But it's
unmaintainable.

Let's use "Requires{,.private} re2" instead of Libs{,.private}. I hope
recent re2.pc doesn't add "-std=c++11".
@kou
Copy link
Member Author

kou commented Feb 4, 2024

@github-actions crossbow submit -g cpp -g r -g linux

Copy link

github-actions bot commented Feb 4, 2024

⚠️ GitHub issue #39930 has been automatically assigned in GitHub to PR creator.

Copy link

github-actions bot commented Feb 4, 2024

Revision: 943d927

Submitted crossbow builds: ursacomputing/crossbow @ actions-d1674cc5d7

Task Status
almalinux-8-amd64 GitHub Actions
almalinux-8-arm64 GitHub Actions
almalinux-9-amd64 GitHub Actions
almalinux-9-arm64 GitHub Actions
amazon-linux-2023-amd64 GitHub Actions
amazon-linux-2023-arm64 GitHub Actions
centos-7-amd64 GitHub Actions
centos-8-stream-amd64 GitHub Actions
centos-8-stream-arm64 GitHub Actions
centos-9-stream-amd64 GitHub Actions
centos-9-stream-arm64 GitHub Actions
debian-bookworm-amd64 GitHub Actions
debian-bookworm-arm64 GitHub Actions
debian-bullseye-amd64 GitHub Actions
debian-bullseye-arm64 GitHub Actions
debian-trixie-amd64 GitHub Actions
debian-trixie-arm64 GitHub Actions
r-binary-packages GitHub Actions
test-alpine-linux-cpp GitHub Actions
test-build-cpp-fuzz GitHub Actions
test-conda-cpp GitHub Actions
test-conda-cpp-valgrind Azure
test-cuda-cpp GitHub Actions
test-debian-11-cpp-amd64 GitHub Actions
test-debian-11-cpp-i386 GitHub Actions
test-fedora-38-cpp GitHub Actions
test-fedora-r-clang-sanitizer Azure
test-r-arrow-backwards-compatibility GitHub Actions
test-r-depsource-bundled Azure
test-r-depsource-system GitHub Actions
test-r-dev-duckdb GitHub Actions
test-r-devdocs GitHub Actions
test-r-gcc-11 GitHub Actions
test-r-gcc-12 GitHub Actions
test-r-install-local GitHub Actions
test-r-install-local-minsizerel GitHub Actions
test-r-library-r-base-latest Azure
test-r-linux-as-cran GitHub Actions
test-r-linux-rchk GitHub Actions
test-r-linux-valgrind Azure
test-r-minimal-build Azure
test-r-offline-maximal GitHub Actions
test-r-offline-minimal Azure
test-r-rhub-debian-gcc-devel-lto-latest Azure
test-r-rhub-debian-gcc-release-custom-ccache Azure
test-r-rhub-ubuntu-gcc-release-latest Azure
test-r-rstudio-r-base-4.1-opensuse153 Azure
test-r-rstudio-r-base-4.2-centos7-devtoolset-8 Azure
test-r-rstudio-r-base-4.2-focal Azure
test-r-ubuntu-22.04 GitHub Actions
test-r-versions GitHub Actions
test-ubuntu-20.04-cpp GitHub Actions
test-ubuntu-20.04-cpp-bundled GitHub Actions
test-ubuntu-20.04-cpp-minimal-with-formats GitHub Actions
test-ubuntu-20.04-cpp-thread-sanitizer GitHub Actions
test-ubuntu-22.04-cpp GitHub Actions
test-ubuntu-22.04-cpp-20 GitHub Actions
test-ubuntu-22.04-cpp-no-threading GitHub Actions
test-ubuntu-r-sanitizer Azure
ubuntu-focal-amd64 GitHub Actions
ubuntu-focal-arm64 GitHub Actions
ubuntu-jammy-amd64 GitHub Actions
ubuntu-jammy-arm64 GitHub Actions
ubuntu-mantic-amd64 GitHub Actions
ubuntu-mantic-arm64 GitHub Actions
ubuntu-noble-amd64 GitHub Actions
ubuntu-noble-arm64 GitHub Actions

@kou
Copy link
Member Author

kou commented Feb 5, 2024

+1

No new CI failures.

@kou kou merged commit 9db823b into apache:main Feb 5, 2024
26 of 29 checks passed
@kou kou deleted the cpp-re2 branch February 5, 2024 23:50
@kou kou removed the awaiting committer review Awaiting committer review label Feb 5, 2024
Copy link

After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 9db823b.

There were 2 benchmark results with an error:

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

dgreiss pushed a commit to dgreiss/arrow that referenced this pull request Feb 19, 2024
…arrow.pc (apache#39932)

### Rationale for this change

We chose Libs{,.private} with libre2.a for system RE2 in apacheGH-10626. Because "Require{,.private} re2" may add "-std=c++11". If "-std=c++11" was added, users can't build Apache Arrow C++ because Apache Arrow C++ requires C++17 or later.

But this approach doesn't work with RE2 2024-06-01 or later because it at least requires Abseil. If we keep the Libs{,.private} approach, we also need to add Abseil libraries to Libs{,.private}. But it's unmaintainable.

### What changes are included in this PR?

Let's use "Requires{,.private} re2" instead of Libs{,.private}. I hope recent re2.pc doesn't add "-std=c++11".

### Are these changes tested?

Yes.

### Are there any user-facing changes?

Yes.
* Closes: apache#39930

Authored-by: Sutou Kouhei <kou@clear-code.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
zanmato1984 pushed a commit to zanmato1984/arrow that referenced this pull request Feb 28, 2024
…arrow.pc (apache#39932)

### Rationale for this change

We chose Libs{,.private} with libre2.a for system RE2 in apacheGH-10626. Because "Require{,.private} re2" may add "-std=c++11". If "-std=c++11" was added, users can't build Apache Arrow C++ because Apache Arrow C++ requires C++17 or later.

But this approach doesn't work with RE2 2024-06-01 or later because it at least requires Abseil. If we keep the Libs{,.private} approach, we also need to add Abseil libraries to Libs{,.private}. But it's unmaintainable.

### What changes are included in this PR?

Let's use "Requires{,.private} re2" instead of Libs{,.private}. I hope recent re2.pc doesn't add "-std=c++11".

### Are these changes tested?

Yes.

### Are there any user-facing changes?

Yes.
* Closes: apache#39930

Authored-by: Sutou Kouhei <kou@clear-code.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
thisisnic pushed a commit to thisisnic/arrow that referenced this pull request Mar 8, 2024
…arrow.pc (apache#39932)

### Rationale for this change

We chose Libs{,.private} with libre2.a for system RE2 in apacheGH-10626. Because "Require{,.private} re2" may add "-std=c++11". If "-std=c++11" was added, users can't build Apache Arrow C++ because Apache Arrow C++ requires C++17 or later.

But this approach doesn't work with RE2 2024-06-01 or later because it at least requires Abseil. If we keep the Libs{,.private} approach, we also need to add Abseil libraries to Libs{,.private}. But it's unmaintainable.

### What changes are included in this PR?

Let's use "Requires{,.private} re2" instead of Libs{,.private}. I hope recent re2.pc doesn't add "-std=c++11".

### Are these changes tested?

Yes.

### Are there any user-facing changes?

Yes.
* Closes: apache#39930

Authored-by: Sutou Kouhei <kou@clear-code.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
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.

[C++] arrow.pc with system RE2 2023-06-01 or later has missing dependencies
1 participant