Bug 192541 – Build failure due to missing include of APIWebsiteDataStore.h

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Adrian Perez]

Found this while making a WPE build with -DCMAKE_BUILD_TYPE=Debug
and it looks like a missing #include which went unnoticed due to
the usage of unified sources.

We really should have a way of disabling unified sources and have
at least one EWS do builds with them disabled to catch this kind
of issues ~_~

---

In file included from DerivedSources/WebKit/unified-sources/UnifiedSource47.cpp:1:
../Source/WebKit/UIProcess/WebsiteData/WebsiteDataStoreConfiguration.cpp: In constructor  WebKit::WebsiteDataStoreConfiguration::WebsiteDataStoreConfiguration()’:
../Source/WebKit/UIProcess/WebsiteData/WebsiteDataStoreConfiguration.cpp:33:46: error: ‘API::WebsiteDataStore’ has not been declared
     : m_resourceLoadStatisticsDirectory(API::WebsiteDataStore::defaultResourceLoadStatisticsDirectory())
                                              ^~~~~~~~~~~~~~~~

[Adrian Perez]

Created attachment 356922 [details]
Patch

[youenn fablet]

Comment on attachment 356922 [details]
Patch

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

> Source/WebKit/UIProcess/WebsiteData/WebsiteDataStoreConfiguration.cpp:27
> +

We usually do not put a line between config.h and the header of the corresponding cpp.

[Michael Catanzaro]

(In reply to Adrian Perez from comment #0)
> We really should have a way of disabling unified sources and have
> at least one EWS do builds with them disabled to catch this kind
> of issues ~_~

The answer will always be: if there's a way to disable unified sources, people will use it, and they will introduce build failures that only occur with unified sources enabled.

What I don't understand is why you hit this build failure when none of the bots did. I presume a different set of build flags somehow results in different unified source bundles, which is a worrisome prospect.

[Adrian Perez]

(In reply to Michael Catanzaro from comment #3)
> (In reply to Adrian Perez from comment #0)
> > We really should have a way of disabling unified sources and have
> > at least one EWS do builds with them disabled to catch this kind
> > of issues ~_~
> 
> The answer will always be: if there's a way to disable unified sources,
> people will use it, and they will introduce build failures that only occur
> with unified sources enabled.

Wrong: people will use whatever is the default. If we have EWS bots which
build both *with* and *without* unified sources, breaking any of them would
prevent patches from going in, which would ensure that both modes are always
working.

> What I don't understand is why you hit this build failure when none of the
> bots did. I presume a different set of build flags somehow results in
> different unified source bundles, which is a worrisome prospect.

Yes, I am working on having content extensions working for the WPE and GTK+
ports, so on top of having the option enabled there are additional files
added to the list of sources to build ;-)

[Adrian Perez]

(In reply to youenn fablet from comment #2)
> Comment on attachment 356922 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=356922&action=review
> 
> > Source/WebKit/UIProcess/WebsiteData/WebsiteDataStoreConfiguration.cpp:27
> > +
> 
> We usually do not put a line between config.h and the header of the
> corresponding cpp.

I think the style checker complained, but I'll double-check before
landing. Thanks for the review!

[Adrian Perez]

Created attachment 356927 [details]
Patch for landing

[Adrian Perez]

(In reply to Adrian Perez from comment #5)
> (In reply to youenn fablet from comment #2)
> > Comment on attachment 356922 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=356922&action=review
> > 
> > > Source/WebKit/UIProcess/WebsiteData/WebsiteDataStoreConfiguration.cpp:27
> > > +
> > 
> > We usually do not put a line between config.h and the header of the
> > corresponding cpp.
> 
> I think the style checker complained, but I'll double-check before
> landing. Thanks for the review!

You were right -- the style checker complained about a different thing
(I still had the message in the scrollback of my terminal emulator!).
I have removed the empty line as suggested and re-sent the patch for
landing.

:)

[WebKit Commit Bot]

Comment on attachment 356927 [details]
Patch for landing

Clearing flags on attachment: 356927

Committed r239022: <https://trac.webkit.org/changeset/239022>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/46581101>