Bug 257516

Summary: [GLIB] GResources being re-regenerated due to bad dependency file
Product: WebKit Reporter: Lauro Moura <lmoura>
Component: CMakeAssignee: Alicia Boya García <aboya>
Status: RESOLVED FIXED    
Severity: Normal CC: aboya, aperez, bugs-noreply, mcatanzaro, philn
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=258478

Description Lauro Moura 2023-05-30 17:31:15 PDT
Currently, the ".deps" files used to track the GResourceBundle dependencies are making ninja re-generate the bundle source even though there was nothing changed on the file. While these are quite minor files, it's triggering the relink of the main webkit library, which usually makes my laptop to become unusable.

Running ninja with "-d explain", it shows messages like these:

ninja explain: expected depfile '/app/webkit/WebKitBuild/Release/WebKitGTK/DerivedSources/PdfJSGResourceBundle.deps' to mention 'WebKitGTK/DerivedSources/PdfJSGResourceBundle.c', got '/app/webkit/WebKitBuild/Release/WebKitGTK/DerivedSources/PdfJSGResourceBundle.xml'
ninja explain: depfile '/app/webkit/WebKitBuild/Release/WebKitGTK/DerivedSources/PdfJSGResourceBundleExtras.deps' is missing
ninja explain: expected depfile '/app/webkit/WebKitBuild/Release/WebKitGTK/DerivedSources/WebKitResourcesGResourceBundle.deps' to mention 'WebKitGTK/DerivedSources/WebKitResourcesGResourceBundle.c', got '/app/webkit/WebKitBuild/Release/WebKitGTK/DerivedSources/WebKitResourcesGResourceBundle.xml' 

This led me to https://gitlab.gnome.org/GNOME/glib/-/issues/2829

glib-compile-resources is generating the depfile like this:

bundle.xml: dep1 dep2

instead of

bundle.c: bundle.xml dep1 dep2

Another issue seems to be related to ninja, which seems expect the target name in the deps file to be a relative path (maybe because it's the target name?) while the generated deps has an absolute path.

Tentative patch incoming.
Comment 1 Lauro Moura 2023-05-30 21:40:02 PDT
Pull request: https://github.com/WebKit/WebKit/pull/14523
Comment 2 Michael Catanzaro 2023-05-31 12:58:54 PDT
I submitted a fix for the first issue in https://gitlab.gnome.org/GNOME/glib/-/merge_requests/3460

(In reply to Lauro Moura from comment #0) 
> Another issue seems to be related to ninja, which seems expect the target
> name in the deps file to be a relative path (maybe because it's the target
> name?) while the generated deps has an absolute path.

Hm, tricky. Perhaps can fix this by passing relative paths to glib-compile-resources? That's quite a footgun though.
Comment 3 Michael Catanzaro 2023-07-03 12:26:22 PDT
Current status:

 * glib-compile-resources is fixed in https://gitlab.gnome.org/GNOME/glib/-/merge_requests/3460
 * Other misc issues fixed via bug #258478
 * The problem with relative vs. absolute paths is not fixed

Ninja bug report:

https://github.com/ninja-build/ninja/issues/1251

CMake workarounds:

https://gitlab.kitware.com/cmake/cmake/-/merge_requests/6143
https://gitlab.kitware.com/cmake/cmake/-/merge_requests/6148

Sadly, the CMake changes don't seem to be sufficient for me. We might need to change the paths that we pass to glib-compile-resources. Rewriting the dependency file works too, but that seems extreme.
Comment 4 Philippe Normand 2024-03-24 03:14:09 PDT
I've checked here, that making the target name in the deps file refer to relative paths fixes the issue, incremental build time went from ~20 seconds to 1 second...

Could this be another glib-compile-resources bug? Do we know why it uses the absolute path instead of relative path?
Comment 5 Michael Catanzaro 2024-03-24 12:38:40 PDT
It's a ninja bug (see previous comment)

(In reply to Philippe Normand from comment #4)
> Do we know why it uses the absolute path instead of relative path?

I think it just uses whatever we pass to it, and we pass absolute paths.
Comment 6 Alicia Boya García 2024-04-02 02:30:41 PDT
(In reply to Michael Catanzaro from comment #5)
> It's a ninja bug (see previous comment)
> 
> (In reply to Philippe Normand from comment #4)
> > Do we know why it uses the absolute path instead of relative path?
> 
> I think it just uses whatever we pass to it, and we pass absolute paths.

Can confirm this. I'm working on a patch to make it use relative paths, and also refactor all those `add_custom_command()` into a custom cmake function.
Comment 7 Adrian Perez 2024-04-02 04:08:43 PDT
(In reply to Alicia Boya García from comment #6)
> (In reply to Michael Catanzaro from comment #5)
> > It's a ninja bug (see previous comment)
> > 
> > (In reply to Philippe Normand from comment #4)
> > > Do we know why it uses the absolute path instead of relative path?
> > 
> > I think it just uses whatever we pass to it, and we pass absolute paths.
> 
> Can confirm this. I'm working on a patch to make it use relative paths, and
> also refactor all those `add_custom_command()` into a custom cmake function.

Nice, thanks! I had this in my list of things to do when having spare cycles
to improve the build; feel free to chat with me if you want some help with
the CMake build system 👍
Comment 8 Alicia Boya García 2024-04-03 06:33:18 PDT
Pull request: https://github.com/WebKit/WebKit/pull/26786
Comment 9 Alicia Boya García 2024-04-04 05:22:32 PDT
*** Bug 272090 has been marked as a duplicate of this bug. ***
Comment 10 EWS 2024-04-04 09:32:47 PDT
Committed 277059@main (18c724980da2): <https://commits.webkit.org/277059@main>

Reviewed commits have been landed. Closing PR #26786 and removing active labels.