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

[C++][R] abseil fails to build with gcc-13 #36969

Closed
thisisnic opened this issue Aug 1, 2023 · 9 comments · Fixed by #37147
Closed

[C++][R] abseil fails to build with gcc-13 #36969

thisisnic opened this issue Aug 1, 2023 · 9 comments · Fixed by #37147
Assignees
Labels
Component: R Priority: Blocker Marks a blocker for the release Type: bug
Milestone

Comments

@thisisnic
Copy link
Member

Describe the bug, including details regarding any error messages, version, and platform.

e.g. https://dev.azure.com/ursacomputing/crossbow/_build/results?buildId=52112&view=logs&j=0da5d1d9-276d-5173-c4c4-9d4d4ed14fdb&t=d9b15392-e4ce-5e4c-0c8c-b69645229181&l=1975

/tmp/RtmpL1MTjz/file4a454972d15/absl_ep-prefix/src/absl_ep/absl/strings/internal/str_format/extension.cc:45:1: note: in expansion of macro ‘ABSL_INTERNAL_CONVERSION_CHARS_EXPAND_’
   45 | ABSL_INTERNAL_CONVERSION_CHARS_EXPAND_(ABSL_INTERNAL_CHAR_SET_CASE, )
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/RtmpL1MTjz/file4a454972d15/absl_ep-prefix/src/absl_ep/absl/strings/internal/str_format/extension.cc:44:13: error: ‘FormatConversionCharSet’ does not name a type; did you mean ‘FormatConversionCharInternal’?
   44 |   constexpr FormatConversionCharSet FormatConversionCharSetInternal::c;
      |             ^~~~~~~~~~~~~~~~~~~~~~~

Component(s)

R

@thisisnic
Copy link
Member Author

@assignUser - I don't suppose you've got any idea why this might have started failing?

@assignUser
Copy link
Member

hm that reads like a version mismatch or something... I'll check it out

@assignUser
Copy link
Member

I don't think this is abseil related as the files in question haven't had changes in a few years ...it looks like there was a major docker upgrade on azure from 20.x to 23.x

@assignUser
Copy link
Member

looks like the r-base image bumped gcc from 12 to 13 that might be it?

@assignUser
Copy link
Member

assignUser commented Aug 8, 2023

can reproduce the error locally with gcc 13, will investigate but as cran has multiple machines running 13 this is likely a blocker for the next 13 rc

HEAD abseil build without a problem so we might just need to bump the abseil version (which iirc is potentially difficult?)
cc @raulcd

@assignUser assignUser added the Priority: Blocker Marks a blocker for the release label Aug 8, 2023
@assignUser
Copy link
Member

I think these are the relevant issues that are cause by gcc 13 https://gcc.gnu.org/gcc-13/porting_to.html#header-dep-changes

abseil-cpp/absl/strings/internal/str_format/extension.h:34:6: warning: elaborated-type-specifier for a scoped enum must not use the ‘class’ keyword
   34 | enum class FormatConversionChar : uint8_t;
      | ~~~~ ^~~~~
      |      -----
abseil-cpp/absl/strings/internal/str_format/extension.h:34:33: error: found ‘:’ in nested-name-specifier, expected ‘::’
   34 | enum class FormatConversionChar : uint8_t;
      |                                 ^
      |                                 ::
abseil-cpp/absl/strings/internal/str_format/extension.h:29:1: note: ‘uint8_t’ is defined in header ‘<cstdint>’; did you forget to ‘#include <cstdint>’?
   28 | #include "absl/strings/internal/str_format/output.h"
  +++ |+#include <cstdint>
   29 | #include "absl/strings/string_view.h"
abseil-cpp/absl/strings/internal/str_format/extension.h:187:29: error: default member initializer for unnamed bit-field
  187 |   enum class Enum : uint8_t {

@assignUser
Copy link
Member

assignUser commented Aug 8, 2023

We actually don't have any CI build with gcc 13 (other than test-r-library-r-base-latest and test-r-depsource-bundled) so this is an issue not only affecting R but was missed thus far.

@assignUser assignUser changed the title [R] test-r-library-r-base-latest and test-r-depsource-bundled nightly builds failing due to issues with building abseil [C++][R] abseil fails to build with gcc-13 Aug 8, 2023
assignUser pushed a commit that referenced this issue 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>
@assignUser assignUser added this to the 13.0.0 milestone Aug 16, 2023
raulcd pushed a commit that referenced this issue 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>
@h-vetinari
Copy link
Contributor

@assignUser, is there a replacement issue, now that #37066 was closed unmerged? #37147 is a work-around, not a solution. Abseil should be kept up-to-date like any other dependency.

@assignUser
Copy link
Member

With our move to cmake 3.16 as minumum version we can finally use FetchContent instead of ep, this will make updating dependencies much easier as we do not have to recreate the internal target tree of the dependency as we currently do.

It should also be possible to move singular dependencies to FC (caveat is that we have to move all of it's dependencies to FC as well afaik). I don't think there is a general issue for this or for abseil specifically.

loicalleyne pushed a commit to loicalleyne/arrow that referenced this issue 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
Labels
Component: R Priority: Blocker Marks a blocker for the release Type: bug
Projects
None yet
3 participants