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.

Sign up for GitHub

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

Jump to bottom

dev-libs/re2: add 0.2023.06.02 #31388

Closed
wants to merge 3 commits into from
Closed

dev-libs/re2: add 0.2023.06.02 #31388

wants to merge 3 commits into from

Conversation

stha09
Copy link
Contributor

@stha09 stha09 commented Jun 11, 2023

No description provided.

@gentoo-bot
Copy link

Pull Request assignment

Submitter: @stha09
Areas affected: ebuilds
Packages affected: dev-cpp/abseil-cpp, dev-libs/re2, dev-qt/qtwebengine

dev-cpp/abseil-cpp: @gyakovlev
dev-libs/re2: @gentoo/chromium, @Arfrever
dev-qt/qtwebengine: @gentoo/qt, @gyakovlev

Linked bugs

Bugs linked: 908372

In order to force reassignment and/or bug reference scan, please append [please reassign] to the pull request title.

Docs: Code of Conduct ● Copyright policy (expl.) ● Devmanual ● GitHub PRs ● Proxy-maint guide

@gentoo-bot gentoo-bot added assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR. labels Jun 11, 2023
thesamesam
thesamesam reviewed Jun 11, 2023
View reviewed changes
Copy link
Member

@thesamesam thesamesam left a comment

Choose a reason for hiding this comment

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

Thanks for looking at this!

dev-cpp/abseil-cpp/abseil-cpp-20230125.2.ebuild Outdated Show resolved Hide resolved
dev-qt/qtwebengine/qtwebengine-5.15.9_p20230505.ebuild Outdated Show resolved Hide resolved
@stha09
dev-cpp/abseil-cpp: add multilib support
8f9b16c 
Closes: https://bugs.gentoo.org/908372
Signed-off-by: Stephan Hartmann <sultan@gentoo.org>
@stha09
dev-libs/re2: add 0.2023.06.02
15bda4e 
Signed-off-by: Stephan Hartmann <sultan@gentoo.org>
@stha09
dev-qt/qtwebengine: pin dev-libs/re2 version
fb2540e 
dev-libs/re2 is compiled in C++17 mode, but qtwebengine in C++14
mode. absl::string_view has different ABI in C++14 and C++17 mode.

Signed-off-by: Stephan Hartmann <sultan@gentoo.org>
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2023-06-11 17:09 UTC
Newest commit scanned: fb2540e
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/d14aeb1b20/output.html

@stha09
Copy link
Contributor Author

stha09 commented Jun 11, 2023

I need to double check if it helps to compile re2 in C++14 mode. With cmake re2 is compiled with gnu++14, but with make it uses compiler default.

@stha09 stha09 added the do not merge Please DO NOT MERGE this PR. It will not be assigned but it will be scanned by CI. label Jun 11, 2023
@stha09
Copy link
Contributor Author

stha09 commented Jun 11, 2023

I need to double check if it helps to compile re2 in C++14 mode. With cmake re2 is compiled with gnu++14, but with make it uses compiler default.

Doesn't help, because chromium is compiled in C++20 mode.

@stha09 stha09 removed the do not merge Please DO NOT MERGE this PR. It will not be assigned but it will be scanned by CI. label Jun 11, 2023
@Pesa
Copy link
Contributor

Pesa commented Jun 11, 2023

Is something forcing c++14 for qtwebengine? The rest of Qt5 should be using the highest standard version supported by the current compiler (up to c++17 or 20, don't remember), but qtwebengine is a bit special...
/cc @Chiitoo

@stha09
Copy link
Contributor Author

stha09 commented Jun 11, 2023

Is something forcing c++14 for qtwebengine? The rest of Qt5 should be using the highest standard version supported by the current compiler (up to c++17 or 20, don't remember), but qtwebengine is a bit special... /cc @Chiitoo

I haven't tried, but Chromium enabled C++17 beginning 2022 (around M98-M100). qtwebengine is based on M87. I give it a try.

@ionenwks
Copy link
Contributor

Is something forcing c++14 for qtwebengine? The rest of Qt5 should be using the highest standard version supported by the current compiler (up to c++17 or 20, don't remember), but qtwebengine is a bit special...

If needed, on IRC we were talking about how it wouldn't be that bad to use bundled re2 on old slot 5 -- much like we ended up doing for ffmpeg given there's not really anyone to update old code for new libraries unlike slot 6.

Not great but it'll be hard to keep around for any extended period (before eventual removal) if issues like that keep coming up.

@stha09
Copy link
Contributor Author

stha09 commented Jun 12, 2023

I tried to compile with C++17. It works with few modifications. I only don't get it managed that the qmake generated makefiles use C++17. Those always use C++1y (C++14). Someone has an idea?
On the other hand, I don't know if it really works at runtime. There are few strange warnings.

@stha09 stha09 requested review from gyakovlev and a17r June 22, 2023 14:22
@themobiusproject themobiusproject mentioned this pull request Jun 26, 2023
Closed
@thesamesam thesamesam requested a review from Chiitoo June 26, 2023 01:21
Chiitoo
Chiitoo approved these changes Jun 26, 2023
View reviewed changes
Copy link
Contributor

@Chiitoo Chiitoo left a comment

Choose a reason for hiding this comment

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

It seems C++20 is used/should be possible to be used looking at these commits:

https://code.qt.io/cgit/qt/qtwebengine.git/commit/?h=5.15&id=08053c0a8a2cfc01c4d92155342a98d610e12473

https://code.qt.io/cgit/qt/qtwebengine.git/commit/?h=5.15&id=6369c52cebd276f03856dd333af727fd8427ac63

Don't really have anything else to add, not being familiar with re2 and all.

@a17r
Copy link
Member

a17r commented Jun 28, 2023

I tried to compile with C++17. It works with few modifications. I only don't get it managed that the qmake generated makefiles use C++17. Those always use C++1y (C++14). Someone has an idea?

I haven't had time to look at it yet. Do you have a link with your modifications?

@thesamesam thesamesam mentioned this pull request Jul 15, 2023
Closed
@stha09 stha09 closed this Jul 30, 2023
@stha09 stha09 deleted the re2-bump branch July 30, 2023 12:56
@hhoffstaette hhoffstaette mentioned this pull request Aug 13, 2023
Closed
gentoo-bot pushed a commit that referenced this pull request Aug 21, 2023
@thesamesam
net-libs/grpc: bundle abseil-cpp
61d0b43 
abseil isn't *really* intended to be a system library and having a year-old
pinned dep like this causes havoc for other packages and leads to uninstallable
combinations (e.g. libreoffice-bin + anything needing grpc).

Please see also the extensive discussion in the linked PRs wrt abseil now
depending on re2 and the ABI implications of the used C++ std. versions with
abseil. It's not worth it and there's more downsides to unbundling than not.

Note that we can't rebundle re2 here because the build system unconditionally
then tries to install it (no toggle unlike for abseil-cpp).

Bug: https://bugs.gentoo.org/906811
Bug: #31388
Bug: #31609
Bug: #31893
Bug: #32281
Closes: https://bugs.gentoo.org/908373
Signed-off-by: Sam James <sam@gentoo.org>
gentoo-bot pushed a commit that referenced this pull request Aug 21, 2023
@thesamesam
app-office/libreoffice: bundle abseil-cpp
ca082bd 
abseil isn't *really* intended to be a system library and having a year-old
pinned dep like this causes havoc for other packages and leads to uninstallable
combinations (e.g. libreoffice-bin + anything needing grpc).

Please see also the extensive discussion in the linked PRs wrt abseil now
depending on re2 and the ABI implications of the used C++ std. versions with
abseil. It's not worth it and there's more downsides to unbundling than not.

Bug: #31388
Bug: #31609
Bug: #31893
Bug: #32281
Bug: https://bugs.gentoo.org/908373
Closes: https://bugs.gentoo.org/912686
Signed-off-by: Sam James <sam@gentoo.org>
MocioF pushed a commit to MocioF/gentoo that referenced this pull request Aug 23, 2023
@thesamesam @MocioF
net-libs/grpc: bundle abseil-cpp
5a3e83d 
abseil isn't *really* intended to be a system library and having a year-old
pinned dep like this causes havoc for other packages and leads to uninstallable
combinations (e.g. libreoffice-bin + anything needing grpc).

Please see also the extensive discussion in the linked PRs wrt abseil now
depending on re2 and the ABI implications of the used C++ std. versions with
abseil. It's not worth it and there's more downsides to unbundling than not.

Note that we can't rebundle re2 here because the build system unconditionally
then tries to install it (no toggle unlike for abseil-cpp).

Bug: https://bugs.gentoo.org/906811
Bug: gentoo#31388
Bug: gentoo#31609
Bug: gentoo#31893
Bug: gentoo#32281
Closes: https://bugs.gentoo.org/908373
Signed-off-by: Sam James <sam@gentoo.org>
MocioF pushed a commit to MocioF/gentoo that referenced this pull request Aug 23, 2023
@thesamesam @MocioF
app-office/libreoffice: bundle abseil-cpp
e3c3f3d 
abseil isn't *really* intended to be a system library and having a year-old
pinned dep like this causes havoc for other packages and leads to uninstallable
combinations (e.g. libreoffice-bin + anything needing grpc).

Please see also the extensive discussion in the linked PRs wrt abseil now
depending on re2 and the ABI implications of the used C++ std. versions with
abseil. It's not worth it and there's more downsides to unbundling than not.

Bug: gentoo#31388
Bug: gentoo#31609
Bug: gentoo#31893
Bug: gentoo#32281
Bug: https://bugs.gentoo.org/908373
Closes: https://bugs.gentoo.org/912686
Signed-off-by: Sam James <sam@gentoo.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thesamesam thesamesam thesamesam left review comments

@Chiitoo Chiitoo Chiitoo approved these changes

@gyakovlev gyakovlev Awaiting requested review from gyakovlev

@a17r a17r Awaiting requested review from a17r

Assignees
No one assigned
Labels
assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR.
Projects
None yet
Milestone
No milestone
8 participants
@stha09 @gentoo-bot @gentoo-repo-qa-bot @Pesa @ionenwks @a17r @Chiitoo @thesamesam