2.33.3 does not build with -DENABLE_GTK_DOC=ON: jsc-glib-4.0-scan.c:34: error: undefined reference to 'jsc_class_get_type' jsc-glib-4.0-scan.c:35: error: undefined reference to 'jsc_context_get_type' jsc-glib-4.0-scan.c:36: error: undefined reference to 'jsc_exception_get_type' jsc-glib-4.0-scan.c:37: error: undefined reference to 'jsc_value_get_type' jsc-glib-4.0-scan.c:38: error: undefined reference to 'jsc_virtual_machine_get_type' jsc-glib-4.0-scan.c:39: error: undefined reference to 'jsc_weak_value_get_type' Example failures: 1. https://gnome.pages.gitlab.gnome.org/-/gnome-build-meta/-/jobs/1457787/artifacts/logs/gnome/sdk-webkitgtk/31ad2288-build.14200.log 2. https://kojipkgs.fedoraproject.org//work/tasks/3009/73963009/build.log I don't immediately see what's wrong, so I'll probably wind up bisecting it, I suppose. Note that -DENABLE_DEVELOPER_MODE=ON (used by build-webkit) would hide this error since that exports everything.
(In reply to Michael Catanzaro from comment #0) > 2.33.3 does not build with -DENABLE_GTK_DOC=ON: The right flag is -DENABLE_GTKDOC=ON. And for some reason my build from trunk did not fail, even though both GNOME and Fedora builds failed. I'll need to investigate harder....
And Berto reported the same error in debian.
I made some preliminary investigation, this is the command line that fails: /usr/bin/cc jsc-glib-4.0-scan.o -L/tmp/webkit2gtk-2.33.3/build/lib -L -ljavascriptcoregtk-4.0 -lgobject-2.0 -lglib-2.0 -Wl,-z,relro -Wl,-z,now -fuse-ld=gold -Wl,--disable-new-dtags -o jsc-glib-4.0-scan The problem seems to be the " -L " option with no value, if you remove that and run the command manually it builds. This comes from the .pc file: $ grep Libs /tmp/webkit2gtk-2.33.3/build/Source/JavaScriptCore/javascriptcoregtk-4.0.pc Libs: -L -ljavascriptcoregtk-4.0 This is -L${libdir} , so ${libdir} is empty
I haven't tested it yet, but I suspect it broke in r280382 "[WPE][GTK] SVN_REVISION drifting away if bots don't re-run cmake"
(In reply to Michael Catanzaro from comment #4) > I haven't tested it yet, but I suspect it broke in r280382 "[WPE][GTK] > SVN_REVISION drifting away if bots don't re-run cmake" That patch didn't change anything related to LIB_INSTALL_DIR though. OTOH LIB_INSTALL_DIR should be either lib or lib64 IIUC? Why do we have this in OptionsCommon? # GTK and WPE use the GNU installation directories as defaults. if (NOT PORT STREQUAL "GTK" AND NOT PORT STREQUAL "WPE") set(LIB_INSTALL_DIR "${CMAKE_INSTALL_PREFIX}/lib" CACHE PATH "Absolute path to library installation directory") set(EXEC_INSTALL_DIR "${CMAKE_INSTALL_PREFIX}/bin" CACHE PATH "Absolute path to executable installation directory") set(LIBEXEC_INSTALL_DIR "${CMAKE_INSTALL_PREFIX}/bin" CACHE PATH "Absolute path to install executables executed by the library") endif ()
(In reply to Michael Catanzaro from comment #4) > I haven't tested it yet, but I suspect it broke in r280382 "[WPE][GTK] > SVN_REVISION drifting away if bots don't re-run cmake" Squinting closer at this commit, I no longer think it's to blame. Hmmm
(In reply to Philippe Normand from comment #5) > Why do we have this in OptionsCommon? > > # GTK and WPE use the GNU installation directories as defaults. > if (NOT PORT STREQUAL "GTK" AND NOT PORT STREQUAL "WPE") > set(LIB_INSTALL_DIR "${CMAKE_INSTALL_PREFIX}/lib" CACHE PATH "Absolute > path to library installation directory") > set(EXEC_INSTALL_DIR "${CMAKE_INSTALL_PREFIX}/bin" CACHE PATH "Absolute > path to executable installation directory") > set(LIBEXEC_INSTALL_DIR "${CMAKE_INSTALL_PREFIX}/bin" CACHE PATH > "Absolute path to install executables executed by the library") > endif () GTK and WPE ports are a bit smarter, see e.g. OptionsGTK.cmake: # These are shared variables, but we special case their definition so that we can use the # CMAKE_INSTALL_* variables that are populated by the GNUInstallDirs macro. set(LIB_INSTALL_DIR "${CMAKE_INSTALL_FULL_LIBDIR}" CACHE PATH "Absolute path to library installation directory") set(EXEC_INSTALL_DIR "${CMAKE_INSTALL_FULL_BINDIR}" CACHE PATH "Absolute path to executable installation directory") set(LIBEXEC_INSTALL_DIR "${CMAKE_INSTALL_FULL_LIBEXECDIR}/webkit2gtk-${WEBKITGTK_API_VERSION}" CACHE PATH "Absolute path to install executables executed by the library")
(In reply to Michael Catanzaro from comment #6) > Squinting closer at this commit, I no longer think it's to blame. Hmmm Ah no, that's definitely it. Problem is you removed @ONLY from the configure_file() calls in order to allow CMake to replace ${BUILD_REVISION}, but now it also replaces the other placeholders like ${libdir} which we don't want. We'll need to switch back to using @ONLY.
(In reply to Michael Catanzaro from comment #8) > (In reply to Michael Catanzaro from comment #6) > > Squinting closer at this commit, I no longer think it's to blame. Hmmm > > Ah no, that's definitely it. Problem is you removed @ONLY from the > configure_file() calls in order to allow CMake to replace ${BUILD_REVISION}, > but now it also replaces the other placeholders like ${libdir} which we > don't want. We'll need to switch back to using @ONLY. Or use @..@ instead ${..} ?
Well we definitely need @ONLY because we do not want to expand the pkg-config variables like ${prefix} or ${libdir}. So yes, that means we'll need to switch from ${BUILD_REVISION} to @BUILD_REVISION@.
I think we'll need to run apply-build-revision-to-files.py first, since updating @BUILD_REVISION@ is the first step that needs to happen, *then* do CMake configure in a separate step. Yes? Because the way it is now, if we switch from ${BUILD_REVISION} to @BUILD_REVISION@, then CMake is going to replace @BUILD_REVISION@ before apply-build-revision-to-files.py runs, and then the python script will do nothing. So we'd need to switch from *.in suffix to *.in.in. Phil, do you agree?
(In reply to Michael Catanzaro from comment #11) > I think we'll need to run apply-build-revision-to-files.py first, since > updating @BUILD_REVISION@ is the first step that needs to happen, *then* do > CMake configure in a separate step. Yes? Because the way it is now, if we > switch from ${BUILD_REVISION} to @BUILD_REVISION@, then CMake is going to > replace @BUILD_REVISION@ before apply-build-revision-to-files.py runs, and > then the python script will do nothing. So we'd need to switch from *.in > suffix to *.in.in. Phil, do you agree? The python script actually has logic now to restore the revision=${BUILD_REVISION} template in already expanded .pc files.
Another issue is that the python script is not included in tarballs, initially it was meant only for developer builds.
(In reply to Philippe Normand from comment #12) > The python script actually has logic now to restore the > revision=${BUILD_REVISION} template in already expanded .pc files. So in theory, all we need to do is switch back to @@ and everything ought to work. Yes? I'm going to attach a patch that does this. Unfortunately, my testing indicates this patch reintroduces bug #228290, although I'm not sure why. At least it should fix the build failures, though. Distros: you'll need to remove the changes to apply-build-revision-to-files.py because this file is not included in release tarballs.
Created attachment 435702 [details] Patch
Comment on attachment 435702 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=435702&action=review > Source/JavaScriptCore/javascriptcoregtk.pc.in:5 > +revision=@BUILD_REVISION@ is this set in CMake now for tarballs? `set(BUILD_REVISION "tarball")` somewhere?
Created attachment 435705 [details] Patch for downstreams
Created attachment 435706 [details] Patch for downstreams (which actually works this time)
(In reply to Philippe Normand from comment #16) > > Source/JavaScriptCore/javascriptcoregtk.pc.in:5 > > +revision=@BUILD_REVISION@ > > is this set in CMake now for tarballs? `set(BUILD_REVISION "tarball")` > somewhere? Yes, that is handled in OptionsGTK.cmake and OptionsWPE.cmake.
(In reply to Michael Catanzaro from comment #14) > Unfortunately, my > testing indicates this patch reintroduces bug #228290, although I'm not sure > why. That's likely an orthogonal issue. Your patch shouldn't introduce change in behavior in dev builds, and actually fixes tarball builds, so let's get it in IMHO.
(In reply to Michael Catanzaro from comment #18) > Created attachment 435706 [details] > Patch for downstreams (which actually works this time) I just wanted to confirm that this fixed the build in Debian, thanks!
Committed r281192 (240636@main): <https://commits.webkit.org/240636@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 435702 [details].