Created attachment 230092 [details] patch Hello Sunpoet, Besides the update, the attached patch: * changes to cmake and makes the package install cmake files * adds the 'test' target databases/arrow finds re2 through cmake, this was the motivation. Thank you, Yuri
Created attachment 230093 [details] patch
We still need the pc file afaik? See: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=254697
Created attachment 233454 [details] patch Update the patch. Some projects (or-tools) now depend on cmake files, and some other projects (bloaty) depend on .pc files. The linked upstream bug report needs to be fixed first.
Created attachment 233482 [details] patch Now .pc file is installed along with cmake files.
Created attachment 233484 [details] patch
Is there a specific reason to replace `USES=gmake` ? Is it incompatible with the CMake build? Why is gmake still involved, anyway, since the CMake use on FreeBSD defaults to using ninja as the actual build system? (Answer: to invoke the common-install target; it would probably be better to generate and install the PC files from CMake as well, even if that needs a downstream patch) There's a duplicate CMAKE_*_ON setting for RE2_BUILD_TESTING; you only need CMAKE_TESTING_ON for that one (because then TESTING off means that the CMake option will be explicitly turned off, too).
(In reply to Adriaan de Groot from comment #6) > Is there a specific reason to replace `USES=gmake` ? USES=gmake doesn't install .cmake files needed for some dependencies. > Why is gmake still involved [...] gmake is used to install the .pc file in a post- step, which is still needed for some dependencies. This was recommended by the upstream. > There's a duplicate CMAKE_*_ON setting for RE2_BUILD_TESTING [...] It isn't duplicate. If you would remove CMAKE_OFF=RE2_BUILD_TESTING it would build tests during normal build. This line prevents that. Hope this answers your questions. Best, Yuri
@sunpoet If the reason for holding this back is because you don't feel comfortable with CMake please consider letting someone else adopt the port.
(In reply to Yuri Victorovich from comment #0) > databases/arrow finds re2 through cmake, this was the motivation. arrow builds fine right now. Why do we need this patch? (In reply to Yuri Victorovich from comment #5) Is it possible to simply run cmake to generate and install .cmake files? Otherwise, please install libre2.a.
(In reply to Po-Chuan Hsieh from comment #9) > arrow builds fine right now. Why do we need this patch? Because some of Facebook ports (devel/folly or its dependencies) require cmake files. > Is it possible to simply run cmake to generate and install .cmake files? > Otherwise, please install libre2.a. That's what is done here - cmake installs cmake files. I don't think cmake can be run as an add-on to gnake build, though. Yuri
sunpoet, What's your objection for this patch? All it does is adding *.cmake files for some dependencies that expect them. I have ~8 Facebook ports held up by this issue. Yuri
(In reply to Yuri Victorovich from comment #11) As I said in comment #9, please install libre2.a.
Created attachment 234102 [details] patch The updated patch installs the static library.
(In reply to Yuri Victorovich from comment #13) It does not install libre2.a. I do not like combining cmake and gmake. I'm working on the cmake patch to install re2.pc.
BTW, I thought CXXFLAGS+=-I${WRKSRC} can be removed by patching CMakeLists.txt. Do you have any idea why "target_include_directories(re2 PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>)" does not work?
(In reply to Po-Chuan Hsieh from comment #15) I am not sure why doesn't it work. I'd personally keep CXXFLAGS+=-I${WRKSRC} since it looks simpler.
I still does not find a way to remove CXXFLAGS+=-I${WRKSRC}. Here's my work to install all required file via cmake. https://people.freebsd.org/~sunpoet/patch/devel-re2.txt
Created attachment 238046 [details] Patch for CMake-ification I have updated Yuri's patch so it works better: - When doing testing, you also need pkgconf and pcre installed (because the tests compare against pcre behavior, and we need pkgconf to find pcre). I have added that to the CMakeLists.txt, not to the port Makefile (because I don't know if TESTING_BUILD_DEPENDS is even a thing -- strikes me as something Yuri should chase). - Running `make test` does take about 10 minutes, the exhaustive tests are exhaustive. - I have ignored the BUILD_INTERFACE issue and just added the source directory to the includes. Bear in mind that the generator (make vs ninja) might influence things here. - The tests need to link to pcre, but also need the include path (because we're not Linux, and pcre lives in ${LOCALBASE}). I don't have any tests -- e.g. "can you find re2 with pkgconf after this" and "can you find re2 with CMake" -- and consider that's something Yuri should figure out.
Er .. I mean "Po-Chuan Hsieh's patch" in that last comment, I used the last patch attached to this issue, which was from them.
@yuri: ping? I believe the test-this-patch part is in your court now.
Please also submit this patch upstream, https://github.com/google/re2
Created attachment 239890 [details] Refresh Patch for CMake-ification Update to latest upstream version Adjust port for follow Porters Handbook more closely Add TEST option Compile and runtime tested on FreeBSD 13.1-STABLE (amd64) (make, make check-plist, make test) Poudriere testport OK 12.3-RELEASE (amd64) Poudriere testport OK 13.1-RELEASE (i386)
Created attachment 242859 [details] Patch for re2
Compile and runtime tested on FreeBSD 13.2-RELEASE (aarch64) (make, make check-plist, make test) Compile and runtime tested on FreeBSD 13.2-RELEASE (amd64) (make, make check-plist, make test) Poudriere testport OK 12.4-RELEASE (amd64) Poudriere testport OK 13.2-RELEASE (amd64) Tested with following users in Poudriere (13.2-RELEASE, amd64): cad/camotics databases/arrow - Fails (also fails due to Googletest on build cluster) error: no member named 'as_string' in 'absl::string_view' devel/bear devel/bloaty -Fails error: no member named 'LogPrefixHook' in namespace 'absl::raw_logging_internal' devel/cbang devel/electron19 devel/electron21 devel/electron22 devel/electron23 devel/electron24 - Fails unrelated (unable to fetch) devel/google-cloud-cpp devel/google-cloud-cpp117 devel/grpc devel/grpc142 devel/libddwaf devel/py-google-re2 devel/rubygem-re2 dns/dnsdist editors/vscode math/or-tools - Fails error: no member named 'as_string' in 'std::string_view' math/py-or-tools - Fails, same as above net-im/mtxclient science/py-tensorflow textproc/kibana7 - Fails error: no member named 'as_string' in 'absl::string_view' textproc/kibana8 - Fails, same as above www/chromium www/iridium www/qt5-webengine www/qt6-webengine - Fails, error: no member named 'set' in 'absl::string_view' www/ungoogled-chromium
*** Bug 272832 has been marked as a duplicate of this bug. ***
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=9d23691afbbc151dbc63933dc8ea370afc391eed commit 9d23691afbbc151dbc63933dc8ea370afc391eed Author: Kai Knoblich <kai@FreeBSD.org> AuthorDate: 2023-08-28 13:23:32 +0000 Commit: Kai Knoblich <kai@FreeBSD.org> CommitDate: 2023-08-28 13:40:40 +0000 www/qt6-webengine: Switch to bundled re2 * Prepare the port for newer versions of devel/re2, which also require devel/abseil. This combination however causes build errors [1] due to missing symbols: [...] In file included from gen/extensions/browser/browser_sources_jumbo_8.cc:6: ./../../../../../qtwebengine-everywhere-src-6.5.2/src/3rdparty/chromium/extensions/browser/api/web_request/form_data_parser.cc:429:11: error: no member named 'set' in 'absl::string_view' source_.set(source.data(), source.size()); ~~~~~~~ ^ ./../../../../../qtwebengine-everywhere-src-6.5.2/src/3rdparty/chromium/extensions/browser/api/web_request/form_data_parser.cc:573:11: error: no member named 'set' in 'absl::string_view' source_.set(source.data(), source.size()); ~~~~~~~ ^ [...] Unbundling abseil and its-subcomponents via "replace_gn_files.py" didn't help hence switch to the bundled re2 for now to have a consistent combination of re2 and abseil. * Bump PORTREVISION due dependency change. PR: 260402 [1] Reported by: diizzy [1] Reviewed by: diizzy, tcberner Differential Revision: https://reviews.freebsd.org/D41571 www/qt6-webengine/Makefile | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)