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-36969: [R] Disable GCS by default when doing a bundled build on gcc-13 #37147

Merged
merged 8 commits into from
Aug 16, 2023

Conversation

paleolimbot
Copy link
Member

@paleolimbot paleolimbot commented Aug 14, 2023

Rationale for this change

Currently a naive install.packages("arrow") will result in a failed build if gcc-13 is the compiler. This is because we include GCS by default on this type of build (bundled). CRAN's check farm includes at least one system where gcc-13 is the compiler and so we can't error or suggest a user workaround.

What changes are included in this PR?

This PR explicitly sets the relevant environment variable if the compiler version string contains "g++" and "13.XX.XX". This is admittedly crude; however, the alternative of updating Abseil results in a cascading set of changes that may break other parts of Arrow. Few if any actual users will build the Arrow R package from source using gcc-13, so this has a much lower footprint (and a workaround: you can just set the ARROW_GCS environment variable + custom abseil location yourself before building if you do, in fact, want to attempt this).

Are these changes tested?

Tested via crossbow (see below).

Are there any user-facing changes?

No.

@paleolimbot
Copy link
Member Author

@github-actions crossbow submit test-r-library-r-base-latest test-r-depsource-bundled

@github-actions github-actions bot added the awaiting committer review Awaiting committer review label Aug 14, 2023
@github-actions
Copy link

Revision: b59798b

Submitted crossbow builds: ursacomputing/crossbow @ actions-2b1f8c7ba7

Task Status
test-r-depsource-bundled Azure
test-r-library-r-base-latest Azure

@paleolimbot
Copy link
Member Author

@github-actions crossbow submit test-r-library-r-base-latest test-r-depsource-bundled

@github-actions
Copy link

Revision: 095621b

Submitted crossbow builds: ursacomputing/crossbow @ actions-b319a1c901

Task Status
test-r-depsource-bundled Azure
test-r-library-r-base-latest Azure

@paleolimbot
Copy link
Member Author

@github-actions crossbow submit test-r-library-r-base-latest test-r-depsource-bundled

@assignUser
Copy link
Member

Another workaround would be to explicitly provide the absl dependency instead of building from source. That way a build on gcc 13 would be possible with gcs

@github-actions
Copy link

Revision: 34657f2

Submitted crossbow builds: ursacomputing/crossbow @ actions-dca0c8e764

Task Status
test-r-depsource-bundled Azure
test-r-library-r-base-latest Azure

@paleolimbot
Copy link
Member Author

@github-actions crossbow submit test-r-library-r-base-latest test-r-depsource-bundled

@github-actions
Copy link

Revision: 30cbe9e

Submitted crossbow builds: ursacomputing/crossbow @ actions-c4616885b4

Task Status
test-r-depsource-bundled Azure
test-r-library-r-base-latest Azure

Copy link
Member

@assignUser assignUser left a comment

Choose a reason for hiding this comment

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

Thanks, looks good I will merge later today.

@assignUser assignUser merged commit 3e7b35b into apache:main Aug 16, 2023
12 of 13 checks passed
@assignUser assignUser removed the awaiting committer review Awaiting committer review label Aug 16, 2023
raulcd pushed a commit that referenced this pull request Aug 16, 2023
…c-13 (#37147)

### Rationale for this change

Currently a naive `install.packages("arrow")` will result in a failed build if gcc-13 is the compiler. This is because we include GCS by default on this type of build (bundled). CRAN's check farm includes at least one system where gcc-13 is the compiler and so we can't error or suggest a user workaround.

### What changes are included in this PR?

This PR explicitly sets the relevant environment variable if the compiler version string contains "g++" and "13.XX.XX". This is admittedly crude; however, the alternative of updating Abseil results in a cascading set of changes that may break other parts of Arrow. Few if any actual users will build the Arrow R package from source using gcc-13, so this has a much lower footprint (and a workaround: you can just set the ARROW_GCS environment variable + custom abseil location yourself before building if you do, in fact, want to attempt this).

### Are these changes tested?

Tested via crossbow (see below).

### Are there any user-facing changes?

No.
* Closes: #36969

Authored-by: Dewey Dunnington <dewey@voltrondata.com>
Signed-off-by: Jacob Wujciak-Jens <jacob@wujciak.de>
@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 3e7b35b.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about possible false positives for unstable benchmarks that are known to sometimes produce them.

@paleolimbot paleolimbot deleted the r-gcc13-absl-patch branch August 21, 2023 13:05
loicalleyne pushed a commit to loicalleyne/arrow that referenced this pull request Nov 13, 2023
… on gcc-13 (apache#37147)

### Rationale for this change

Currently a naive `install.packages("arrow")` will result in a failed build if gcc-13 is the compiler. This is because we include GCS by default on this type of build (bundled). CRAN's check farm includes at least one system where gcc-13 is the compiler and so we can't error or suggest a user workaround.

### What changes are included in this PR?

This PR explicitly sets the relevant environment variable if the compiler version string contains "g++" and "13.XX.XX". This is admittedly crude; however, the alternative of updating Abseil results in a cascading set of changes that may break other parts of Arrow. Few if any actual users will build the Arrow R package from source using gcc-13, so this has a much lower footprint (and a workaround: you can just set the ARROW_GCS environment variable + custom abseil location yourself before building if you do, in fact, want to attempt this).

### Are these changes tested?

Tested via crossbow (see below).

### Are there any user-facing changes?

No.
* Closes: apache#36969

Authored-by: Dewey Dunnington <dewey@voltrondata.com>
Signed-off-by: Jacob Wujciak-Jens <jacob@wujciak.de>
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++][R] abseil fails to build with gcc-13
2 participants