Upgrade abseil to 20230802.1 #95

mudge · 2023-09-09T11:43:53Z

RE2 2023-09-01 was released a week ago and there is a more recent LTS version of Abseil for us to use. However, when I initially pushed this to our v2.0.x branch, it looks like there are quite a few build failures to investigate.

mudge · 2023-09-09T11:56:53Z

If it’s a distraction right now, we could skip the abseil upgrade but it’d be good to know why it doesn’t work when the January release does.

stanhu · 2023-09-09T19:05:51Z

It looks like there are two main issues:

macOS (https://github.com/mudge/re2/actions/runs/6130653443/job/16639942206): absel::optional is aliased to C++17 std::optional, but the call to value() (introduced in abseil-cpp via abseil/abseil-cpp@751ade0) is only available starting in macOS 10.14. Even though the macOS SDK 11.1 is used, I'd have to presume the compiler tries to target a lower minimum target version:

 [ 97%] Building CXX object absl/status/CMakeFiles/status.dir/status.cc.o
/tmp/d20230909-93-us36r9/tmp/arm64-apple-darwin/ports/abseil/20230802.0/abseil-cpp-20230802.0/absl/status/status.cc:126:51: error: 'value' is unavailable: introduced in macOS 10.14
  if (index.has_value()) return (*payloads)[index.value()].payload;
                                                  ^

I think we might be able to set ABSL_OPTION_USE_STD_OPTIONAL to 0 to disable this aliasing, or consider raising the macOS minimum version. clang --help shows this option:

-mmacosx-version-min=<value>
                      Set Mac OS X deployment target

Windows (https://github.com/mudge/re2/actions/runs/6130653443/job/16639942231):

[ 37%] Building CXX object absl/synchronization/CMakeFiles/synchronization.dir/internal/create_thread_identity.cc.obj
In file included from /tmp/d20230909-93-l6kcab/tmp/x86_64-w64-mingw32/ports/abseil/20230802.0/abseil-cpp-20230802.0/absl/synchronization/internal/create_thread_identity.cc:21:
/tmp/d20230909-93-l6kcab/tmp/x86_64-w64-mingw32/ports/abseil/20230802.0/abseil-cpp-20230802.0/absl/synchronization/internal/waiter.h:44:2: error: #error ABSL_WAITER_MODE is undefined
   44 | #error ABSL_WAITER_MODE is undefined
      |  ^~~~~
/tmp/d20230909-93-l6kcab/tmp/x86_64-w64-mingw32/ports/abseil/20230802.0/abseil-cpp-20230802.0/absl/synchronization/internal/waiter.h:51:5: warning: "ABSL_WAITER_MODE" is not defined, evaluates to 0 [-Wundef]
   51 | #if ABSL_WAITER_MODE == ABSL_WAITER_MODE_FUTEX
      |     ^~~~~~~~~~~~~~~~
In file included from /tmp/d20230909-93-l6kcab/tmp/x86_64-w64-mingw32/ports/abseil/20230802.0/abseil-cpp-20230802.0/absl/synchronization/internal/create_thread_identity.cc:21:
/tmp/d20230909-93-l6kcab/tmp/x86_64-w64-mingw32/ports/abseil/20230802.0/abseil-cpp-20230802.0/absl/synchronization/internal/waiter.h:52:16: error: 'FutexWaiter' does not name a type
   52 | using Waiter = FutexWaiter;
      |                ^~~~~~~~~~~
make[2]: *** [absl/synchronization/CMakeFiles/synchronization.dir/build.make:92: absl/synchronization/CMakeFiles/synchronization.dir/internal/create_thread_identity.cc.obj] Error 1
make[1]: *** [CMakeFiles/Makefile2:4540: absl/synchronization/CMakeFiles/synchronization.dir/all] Error 2

There appears to be a workaround: abseil/abseil-cpp#1510 (comment).

stanhu · 2023-09-10T07:09:04Z

scripts/test-gem-build

@@ -23,6 +23,7 @@ if [[ "${BUILD_NATIVE_GEM}" == "ruby" ]] ; then
  bundle exec rake clean
  bundle exec rake gem
 else
+  export MACOSX_DEPLOYMENT_TARGET=10.14


I tried hard to avoid this duplication in the Rakefile. I originally tried to pass cmake -DCMAKE_CXX_FLAGS=-mmacosx-version-min=10.14 (https://github.com/tpoechtrager/osxcross#deployment-target), but the compiler flags had redundant values, with the latter one overriding the one I set:

-mmacosx-version-min=10.14 -mmacosx-version-min=10.13 ...

Does this mean the native gems for macOS will no longer run on High Sierra (10.13) and instead require Mojave (10.14)? If so, perhaps we should add that to the README along with the glibc requirements.

Yes. The alternative is to file an upstream fix to abseil to be able control the behavior of ABSL_OPTION_USE_STD_OPTIONAL independently of MACOSX_DEPLOYMENT_TARGET, but I suspect it's not worth the effort.

stanhu · 2023-09-10T07:10:28Z

ext/re2/extconf.rb

  -labsl_hashtablez_sampler
  -labsl_exponential_biased
  -labsl_bad_optional_access
  -labsl_str_format_internal
  -labsl_synchronization
  -labsl_graphcycles_internal
+  -labsl_kernel_timeout_internal


I was hoping we could drop this workaround in #96, but the runners are still using pkgconf 1.9.3. 2.0.0 will fix this: pkgconf/pkgconf#268

stanhu · 2023-09-10T07:10:38Z

@mudge I think this is ready to go.

mudge · 2023-09-23T19:18:01Z

Updated for the recently released 20230802.1 (~~note I removed c284a7e just to see if that regression has been fixed but we may need to restore it~~ restored as it looks like it is still a problem).

mudge · 2023-09-23T19:41:25Z

@stanhu any chance you could take a look at this? The Windows failures seem different to with patch level 0.

undefined reference to `absl::lts_20230802::synchronization_internal::Win32Waiter::Win32Waiter()

mudge · 2023-09-23T20:13:09Z

Out of interest, is MacPorts providing a version of Abseil that works with versions of macOS older than 10.14? See https://github.com/judaew/macports-ports/blob/master/devel/abseil/Portfile

stanhu · 2023-09-23T20:34:21Z

std::optional was added in C++17, and I see that they have set C++14 in https://github.com/judaew/macports-ports/blob/c536e357838df685f32fdedc7b74dd5d4f0325ce/devel/abseil/Portfile#L68. This might avoid the need to upgrade macOS. Maybe for macOS we can use C++14.

mudge · 2023-09-24T06:56:11Z

std::optional was added in C++17, and I see that they have set C++14 in https://github.com/judaew/macports-ports/blob/c536e357838df685f32fdedc7b74dd5d4f0325ce/devel/abseil/Portfile#L68. This might avoid the need to upgrade macOS. Maybe for macOS we can use C++14.

Good spot. Given Abseil only needs C++14 (and RE2 follows suit), I’ve set our MiniPortile recipes to only target that standard and it seems to fix the macOS build without dropping support for older versions.

stanhu · 2023-09-24T07:15:28Z

Did this branch drop 0b57520?

mudge · 2023-09-24T07:20:33Z

Yes, I've restored it now (I was trying to see what was and wasn't required with patch level 1).

mudge · 2023-09-24T07:36:36Z

The build is now green and (aside from the necessary workaround for abseil/abseil-cpp#1510) I'm happier with the diff especially as we're not dropping support for any existing users.

mudge · 2023-09-24T07:39:13Z

If you're happy, I'd be tempted to merge this but not include it in a release until RE2 2023-10-01 is out (assuming it has a monthly release cadence now).

mudge · 2023-09-24T12:26:38Z

I've raised #111 to partially automate this sort of thing in future: it's set to run on the first of the month and raise a PR with an updated dependencies.yml (if there are any releases). I'm tempted to merge that, trigger it manually and then cherry pick these commits onto the PR that is automatically raised.

So we can stay on top of updates to both RE2 and Abseil now they are vendored with the gem, run a monthly job to check for any new releases and update `dependencies.yml` accordingly, raising a PR if there are any. Closes #107

stanhu · 2023-09-24T14:57:08Z

If you're happy, I'd be tempted to merge this but not include it in a release until RE2 2023-10-01 is out (assuming it has a monthly release cadence now).

Sounds good!

#112 didn't run CI. Does the user need to be approved?

mudge · 2023-09-24T14:58:22Z

I think it’s due to https://github.com/peter-evans/create-pull-request/blob/main/docs/concepts-guidelines.md#triggering-further-workflow-runs I’ll try one of the workarounds and try triggering it again to make sure it runs CI too.

Due to a bug in pkgconf v1.9.3 (pkgconf/pkgconf#268) that has since been fixed in pkgconf v2.0, we need to maintain the correct order for linking abseil-cpp libraries. abseil 20230802 added a few new libraries and re-arranged the link order.

For compatibility with macOS < 10.14, only try to compile Abseil with C++14 rather than C++17 (which would require macOS 14 or later).

Previously the build would fail with this error message: ``` [ 37%] Building CXX object absl/synchronization/CMakeFiles/synchronization.dir/internal/create_thread_identity.cc.obj In file included from /tmp/d20230909-93-l6kcab/tmp/x86_64-w64-mingw32/ports/abseil/20230802.0/abseil-cpp-20230802.0/absl/synchronization/internal/create_thread_identity.cc:21: /tmp/d20230909-93-l6kcab/tmp/x86_64-w64-mingw32/ports/abseil/20230802.0/abseil-cpp-20230802.0/absl/synchronization/internal/waiter.h:44:2: error: #error ABSL_WAITER_MODE is undefined 44 | #error ABSL_WAITER_MODE is undefined | ^~~~~ /tmp/d20230909-93-l6kcab/tmp/x86_64-w64-mingw32/ports/abseil/20230802.0/abseil-cpp-20230802.0/absl/synchronization/internal/waiter.h:51:5: warning: "ABSL_WAITER_MODE" is not defined, evaluates to 0 [-Wundef] 51 | #if ABSL_WAITER_MODE == ABSL_WAITER_MODE_FUTEX | ^~~~~~~~~~~~~~~~ In file included from /tmp/d20230909-93-l6kcab/tmp/x86_64-w64-mingw32/ports/abseil/20230802.0/abseil-cpp-20230802.0/absl/synchronization/internal/create_thread_identity.cc:21: /tmp/d20230909-93-l6kcab/tmp/x86_64-w64-mingw32/ports/abseil/20230802.0/abseil-cpp-20230802.0/absl/synchronization/internal/waiter.h:52:16: error: 'FutexWaiter' does not name a type 52 | using Waiter = FutexWaiter; | ^~~~~~~~~~~ make[2]: *** [absl/synchronization/CMakeFiles/synchronization.dir/build.make:92: absl/synchronization/CMakeFiles/synchronization.dir/internal/create_thread_identity.cc.obj] Error 1 ``` As mentioned in abseil/abseil-cpp#1510 (comment), work around the issue by explicitly defining `-DABSL_FORCE_WAITER_MODE=4` (4 = `ABSL_WAITER_MODE_STDCPP`).

mudge · 2023-09-24T18:33:45Z

Closing in favour of #113 as that PR was raised automatically.

mudge requested a review from stanhu September 9, 2023 11:44

stanhu force-pushed the upgrade-re2-to-2023-09-01 branch from d28c745 to 3e28c66 Compare September 10, 2023 06:03

stanhu reviewed Sep 10, 2023

View reviewed changes

mudge mentioned this pull request Sep 10, 2023

Upgrade RE2 to 2023-09-01 #97

Merged

mudge force-pushed the upgrade-re2-to-2023-09-01 branch from 357c626 to 9a9070c Compare September 10, 2023 12:42

mudge changed the title ~~Upgrade re2 to 2023-09-01 and abseil to 20230802~~ Upgrade abseil to 20230802 Sep 10, 2023

mudge mentioned this pull request Sep 10, 2023

Drop abseil-cpp build workarounds on Windows #96

Closed

mudge force-pushed the v2.0.x branch from 0841b6c to 2660642 Compare September 13, 2023 14:59

Base automatically changed from v2.0.x to main September 13, 2023 17:34

mudge force-pushed the main branch 2 times, most recently from 073ee08 to 09b9b90 Compare September 23, 2023 18:58

mudge force-pushed the upgrade-re2-to-2023-09-01 branch from 9a9070c to c66733a Compare September 23, 2023 19:17

mudge changed the title ~~Upgrade abseil to 20230802~~ Upgrade abseil to 20230802.1 Sep 23, 2023

mudge assigned stanhu Sep 23, 2023

mudge force-pushed the upgrade-re2-to-2023-09-01 branch from 8f32657 to 1147c0b Compare September 24, 2023 06:41

mudge closed this Sep 24, 2023

mudge reopened this Sep 24, 2023

mudge force-pushed the upgrade-re2-to-2023-09-01 branch from 1147c0b to 0c23084 Compare September 24, 2023 07:18

Add workflow to upgrade vendored dependencies

7bb2b06

So we can stay on top of updates to both RE2 and Abseil now they are vendored with the gem, run a monthly job to check for any new releases and update `dependencies.yml` accordingly, raising a PR if there are any. Closes #107

mudge and others added 4 commits September 24, 2023 07:59

Upgrade Abseil to 20230802.1

f3180e2

Update abseil library list for Windows

a227b5f

Due to a bug in pkgconf v1.9.3 (pkgconf/pkgconf#268) that has since been fixed in pkgconf v2.0, we need to maintain the correct order for linking abseil-cpp libraries. abseil 20230802 added a few new libraries and re-arranged the link order.

Compile against C++14, not C++17

6de8bca

For compatibility with macOS < 10.14, only try to compile Abseil with C++14 rather than C++17 (which would require macOS 14 or later).

stanhu force-pushed the upgrade-re2-to-2023-09-01 branch from 0c23084 to 5f742e5 Compare September 24, 2023 15:00

mudge force-pushed the main branch from 7bb2b06 to 662dbb1 Compare September 24, 2023 18:23

mudge closed this Sep 24, 2023

mudge deleted the upgrade-re2-to-2023-09-01 branch September 24, 2023 18:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade abseil to 20230802.1 #95

Upgrade abseil to 20230802.1 #95

mudge commented Sep 9, 2023

mudge commented Sep 9, 2023

stanhu commented Sep 9, 2023

stanhu Sep 10, 2023

mudge Sep 10, 2023

stanhu Sep 11, 2023

stanhu Sep 10, 2023

stanhu commented Sep 10, 2023

mudge commented Sep 23, 2023 •

edited

Loading

mudge commented Sep 23, 2023

mudge commented Sep 23, 2023

stanhu commented Sep 23, 2023

mudge commented Sep 24, 2023 •

edited

Loading

stanhu commented Sep 24, 2023

mudge commented Sep 24, 2023

mudge commented Sep 24, 2023

mudge commented Sep 24, 2023

mudge commented Sep 24, 2023

stanhu commented Sep 24, 2023

mudge commented Sep 24, 2023

mudge commented Sep 24, 2023

Upgrade abseil to 20230802.1 #95

Upgrade abseil to 20230802.1 #95

Conversation

mudge commented Sep 9, 2023

mudge commented Sep 9, 2023

stanhu commented Sep 9, 2023

stanhu Sep 10, 2023

Choose a reason for hiding this comment

mudge Sep 10, 2023

Choose a reason for hiding this comment

stanhu Sep 11, 2023

Choose a reason for hiding this comment

stanhu Sep 10, 2023

Choose a reason for hiding this comment

stanhu commented Sep 10, 2023

mudge commented Sep 23, 2023 • edited Loading

mudge commented Sep 23, 2023

mudge commented Sep 23, 2023

stanhu commented Sep 23, 2023

mudge commented Sep 24, 2023 • edited Loading

stanhu commented Sep 24, 2023

mudge commented Sep 24, 2023

mudge commented Sep 24, 2023

mudge commented Sep 24, 2023

mudge commented Sep 24, 2023

stanhu commented Sep 24, 2023

mudge commented Sep 24, 2023

mudge commented Sep 24, 2023

mudge commented Sep 23, 2023 •

edited

Loading

mudge commented Sep 24, 2023 •

edited

Loading