Bug 249476 - [GTK][WPE] Do not include deprecated APIs when building 2022 API
Summary: [GTK][WPE] Do not include deprecated APIs when building 2022 API
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Carlos Garcia Campos
URL:
Keywords: Gtk
: 235151 (view as bug list)
Depends on:
Blocks: GTK4
  Show dependency treegraph
 
Reported: 2022-12-16 07:57 PST by Carlos Garcia Campos
Modified: 2022-12-28 13:40 PST (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2022-12-16 07:57:51 PST
.
Comment 1 Carlos Garcia Campos 2022-12-16 07:59:21 PST
Pull request: https://github.com/WebKit/WebKit/pull/7752
Comment 2 Michael Catanzaro 2022-12-22 12:15:42 PST
*** Bug 235151 has been marked as a duplicate of this bug. ***
Comment 3 EWS 2022-12-23 02:53:29 PST
Committed 258290@main (bd6aa66739fa): <https://commits.webkit.org/258290@main>

Reviewed commits have been landed. Closing PR #7752 and removing active labels.
Comment 4 Zan Dobersek 2022-12-26 23:30:41 PST
> In file included from /build/webkit/build-unstable/build-webkit/DerivedSources/WebKit/wpe/WebKitEnumTypes.cpp:26:
> /home/zan/Work/webkit/git/Source/WebKit/UIProcess/API/wpe/webkit.h:58:10: fatal error: 'wpe/WebKitMimeInfo.h' file not found
> #include <wpe/WebKitMimeInfo.h>
>          ^~~~~~~~~~~~~~~~~~~~~~
> 1 error generated.

This header got deprecated and isn't generated anymore when ENABLE_2022_GLIB_API is turned on, but it's still included unconditionally in WPE's webkit.h and GTK's webkit2.h headers.
Comment 5 Michael Catanzaro 2022-12-27 06:13:48 PST
(In reply to Zan Dobersek from comment #4)
> > In file included from /build/webkit/build-unstable/build-webkit/DerivedSources/WebKit/wpe/WebKitEnumTypes.cpp:26:
> > /home/zan/Work/webkit/git/Source/WebKit/UIProcess/API/wpe/webkit.h:58:10: fatal error: 'wpe/WebKitMimeInfo.h' file not found
> > #include <wpe/WebKitMimeInfo.h>
> >          ^~~~~~~~~~~~~~~~~~~~~~
> > 1 error generated.
> 
> This header got deprecated and isn't generated anymore when
> ENABLE_2022_GLIB_API is turned on, but it's still included unconditionally
> in WPE's webkit.h

Uh-oh. This header is not generated, so we'll need either generate it, or have two separate copies of WPE's webkit.h now.

(We don't have any EWS coverage for the WPE 1.1 API version anymore, so problems like these are likely to reoccur.)

> and GTK's webkit2.h headers.

That's OK since this is the legacy API header. (With GTK, our EWS coverage is the opposite of what we have for WPE: there is no EWS for the modern API, but the legacy API is still covered.)
Comment 6 Carlos Alberto Lopez Perez 2022-12-27 21:17:26 PST
(In reply to Michael Catanzaro from comment #5)
> (In reply to Zan Dobersek from comment #4)
> > > In file included from /build/webkit/build-unstable/build-webkit/DerivedSources/WebKit/wpe/WebKitEnumTypes.cpp:26:
> > > /home/zan/Work/webkit/git/Source/WebKit/UIProcess/API/wpe/webkit.h:58:10: fatal error: 'wpe/WebKitMimeInfo.h' file not found
> > > #include <wpe/WebKitMimeInfo.h>
> > >          ^~~~~~~~~~~~~~~~~~~~~~
> > > 1 error generated.
> > 
> > This header got deprecated and isn't generated anymore when
> > ENABLE_2022_GLIB_API is turned on, but it's still included unconditionally
> > in WPE's webkit.h
> 
> Uh-oh. This header is not generated, so we'll need either generate it, or
> have two separate copies of WPE's webkit.h now.
> 
> (We don't have any EWS coverage for the WPE 1.1 API version anymore, so
> problems like these are likely to reoccur.)
> 
> > and GTK's webkit2.h headers.
> 
> That's OK since this is the legacy API header. (With GTK, our EWS coverage
> is the opposite of what we have for WPE: there is no EWS for the modern API,
> but the legacy API is still covered.)

We have coverage. The problem here is a continuous build issue. The problem only appears if you do a clean build on 258290@main or after it.

It works only if you come from a build previous to 258290@main instead of doing a clean build (which happens to all our bots: either EWS or post-commits, all bots are doing continuous builds)

I just tested:

- If I build WPE on 258289@main and then I build it 258290@main or after it, then everything works as expected
- However, If I do a clean build on 258290@main (or a commit after it) then the build error happens:

[...] -o Source/WebKit/CMakeFiles/WebKit.dir/__/__/DerivedSources/WebKit/wpe/WebKitEnumTypes.cpp.o -c /home/clopez/webkit/webkit/WebKitBuild/WPE/Release_rpi3-32bits-mesa/DerivedSources/WebKit/wpe/WebKitEnumTypes.cpp
In file included from /home/clopez/webkit/webkit/WebKitBuild/WPE/Release_rpi3-32bits-mesa/DerivedSources/WebKit/wpe/WebKitEnumTypes.cpp:26:
/home/clopez/webkit/webkit/Source/WebKit/UIProcess/API/wpe/webkit.h:58:10: fatal error: wpe/WebKitMimeInfo.h: No such file or directory
   58 | #include <wpe/WebKitMimeInfo.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

So as soon as anyone does a clean build the issue will surface.

The catch this kind of errors I think we should improve our CMake build system to wipe the whole directory DerivedSources before generating DerivedSources, otherwise it can happen that leftovers from previous builds can be there making problems like this go unnoticed.
Comment 7 Michael Catanzaro 2022-12-28 10:07:24 PST
(In reply to Carlos Alberto Lopez Perez from comment #6)
> The catch this kind of errors I think we should improve our CMake build
> system to wipe the whole directory DerivedSources before generating
> DerivedSources, otherwise it can happen that leftovers from previous builds
> can be there making problems like this go unnoticed.

Hmm, well it would solve this problem, but could cause very substantial extra compilation after modifying any file that's used to generate DerivedSources. That seems worse than cleaning up after an occasional build failure.
Comment 8 Carlos Alberto Lopez Perez 2022-12-28 13:40:51 PST
(In reply to Michael Catanzaro from comment #7)
> (In reply to Carlos Alberto Lopez Perez from comment #6)
> > The catch this kind of errors I think we should improve our CMake build
> > system to wipe the whole directory DerivedSources before generating
> > DerivedSources, otherwise it can happen that leftovers from previous builds
> > can be there making problems like this go unnoticed.
> 
> Hmm, well it would solve this problem, but could cause very substantial
> extra compilation after modifying any file that's used to generate
> DerivedSources. That seems worse than cleaning up after an occasional build
> failure.

I don't think it should be much more time if you are using ccache (which is the default). In any case I guess is worth doing a benchmark if this gets implemented.