Bug 220636

Summary: [GTK] Multilib conflicts in gir files
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: WebKitGTKAssignee: Michael Catanzaro <mcatanzaro>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, bugs-noreply, cgarcia, ews-watchlist, gyuyoung.kim, keith_miller, mark.lam, mcatanzaro, msaboff, ryuan.choi, saam, sergio, tzagallo
Priority: P2    
Version: WebKit Nightly Build   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Description Michael Catanzaro 2021-01-14 14:51:10 PST
Our generated gir files currently include absolute paths. This is a problem for multilib because the gir files need to be identical on all architectures, since they are installed under /usr/share, but distro build directory names often vary based on architecture. For example, the top of JavaScriptCore-4.0.gir:

<?xml version="1.0"?>
<!-- This file was automatically generated from C sources - DO NOT EDIT!
To affect the contents of this file, edit the original C definitions,
and/or use gtk-doc annotations.  -->
<repository version="1.2"
            xmlns="http://www.gtk.org/introspection/core/1.0"
            xmlns:c="http://www.gtk.org/introspection/c/1.0"
            xmlns:glib="http://www.gtk.org/introspection/glib/1.0">
  <include name="GObject" version="2.0"/>
  <package name="javascriptcoregtk-4.0"/>
  <c:include name="jsc/jsc.h"/>
  <namespace name="JavaScriptCore"
             version="4.0"
             shared-library="libjavascriptcoregtk-4.0.so.18"
             c:identifier-prefixes="JSC"
             c:symbol-prefixes="jsc">
    <function-macro name="CHECK_VERSION"
                    c:identifier="JSC_CHECK_VERSION"
                    introspectable="0">
      <source-position filename="x86_64-redhat-linux-gnu/DerivedSources/JavaScriptCore/javascriptcoregtk/jsc/JSCVersion.h"
                       line="67"/>

So the problem is clear: on i686, the <source-position filename="i686-redhat-linux-gnu" instead. This was not a problem in the past due to downstream build reasons: we had been creating our own build directory, but now the CMake RPM macro creates the build directory for us and includes the target arch in the name of the build directory.

Anyway, this is easy to fix pas passing --sources-top-dirs to g-ir-scanner. Then we get this instead:

      <source-position filename="DerivedSources/JavaScriptCore/javascriptcoregtk/jsc/JSCVersion.h"
                       line="67"/>

Much better!
Comment 1 Michael Catanzaro 2021-01-14 14:54:01 PST
(In reply to Michael Catanzaro from comment #0)
> Our generated gir files currently include absolute paths.

Er, well that's not true. Point remains: paths need to be relative to the builddir or we'll have multilib conflicts.
Comment 2 Michael Catanzaro 2021-01-14 15:04:40 PST
Created attachment 417660 [details]
Patch
Comment 3 Michael Catanzaro 2021-01-14 15:09:29 PST
Created attachment 417661 [details]
Patch
Comment 4 Carlos Garcia Campos 2021-01-18 00:26:53 PST
Comment on attachment 417661 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=417661&action=review

> Source/JavaScriptCore/PlatformGTK.cmake:83
> +            --sources-top-dirs=${CMAKE_BINARY_DIR}
> +            --sources-top-dirs=${CMAKE_SOURCE_DIR}

why do we need to pass both? do we want the toplevel soures dir or the toplevel build dir?
Comment 5 Michael Catanzaro 2021-01-18 07:26:41 PST
(In reply to Carlos Garcia Campos from comment #4)
> Comment on attachment 417661 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=417661&action=review
> 
> > Source/JavaScriptCore/PlatformGTK.cmake:83
> > +            --sources-top-dirs=${CMAKE_BINARY_DIR}
> > +            --sources-top-dirs=${CMAKE_SOURCE_DIR}
> 
> why do we need to pass both? do we want the toplevel soures dir or the
> toplevel build dir?

The first version of my patch only passed the builddir, which is sufficient to avoid the multilib conflicts.

Then I looked into what meson was doing and realized it was passing both. Setting the builddir shortens the paths for anything built from DerivedSources, while setting sourcedir just shortens the paths for things under Source/ when the introspection scanner is called from a subdirectory of the toplevel source directory. Except... cmake actually runs all commands from the toplevel source directory, so passing it does nothing (except make the command line longer). So yeah, it's not needed. Let's go back to the first version of my patch.
Comment 6 Michael Catanzaro 2021-01-18 07:30:12 PST
Er... also it seems I missed the WebKit2WebExtension-4.0.gir... oops. Will fix that in the patch that lands.
Comment 7 Michael Catanzaro 2021-01-18 07:30:40 PST
Created attachment 417831 [details]
Patch for landing
Comment 8 EWS 2021-01-18 07:56:53 PST
Committed r271580: <https://trac.webkit.org/changeset/271580>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 417831 [details].