Bug 135544 – [GTK] Install all unstable webkitdom headers

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Michael Catanzaro]

We're accidentally failing to install unstable webkitdom headers for classes that have both stable and unstable functions.  In Source/WebCore/PlatformGTK.cmake we loop over all such headers that have been generated and add an install rule for each one, but do so before they've been generated so they all get missed.

(We're also going to need this patched in GNOME jhbuild; Epiphany is broken right now.)

[Michael Catanzaro]

Created attachment 235937 [details]
Patch

[Martin Robinson]

Comment on attachment 235937 [details]
Patch

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

> Source/WebCore/PlatformGTK.cmake:751
> +    if (EXISTS "${DERIVED_SOURCES_GOBJECT_DOM_BINDINGS_DIR}/WebKitDOM${classname}Unstable.h")

This still runs at configuration time, so won't this fail before the files are generated?

[Carlos Garcia Campos]

Oh, I see the problem, I wonder why it works for me though. I think checking those files exist before adding them to the list was a bad idea in any case. Looking a the CMake docs, it seems we have other options, for example:

 a) Assume all stable classes have an Unstable.h header and make them optional.
    OPTIONAL: Specify that it is not an error if the file to be installed does not exist.

 b) Use a DIRECTORY install with a FILES_MATCHING option to install only the files matching the pattern *Unstable.h.

The problem I see with option b) is that we need the list of headers to generate the introspection files, so I guess we have to go with option a)

[Carlos Garcia Campos]

(In reply to comment #3)
> Oh, I see the problem, I wonder why it works for me though. I think checking those files exist before adding them to the list was a bad idea in any case. Looking a the CMake docs, it seems we have other options, for example:
> 
>  a) Assume all stable classes have an Unstable.h header and make them optional.
>     OPTIONAL: Specify that it is not an error if the file to be installed does not exist.
> 
>  b) Use a DIRECTORY install with a FILES_MATCHING option to install only the files matching the pattern *Unstable.h.
> 
> The problem I see with option b) is that we need the list of headers to generate the introspection files, so I guess we have to go with option a)

Actually, I think we should probably not expose unstable symbols to the gir file, since there's not way to hide them by default as we do in C (or at least I don't know how to do it). The symbols are marked as unstable in the docs, but I think bindings don't use that info at all. So for now, I would only expose stable symbols to bindings.

[Carlos Garcia Campos]

Created attachment 235944 [details]
Another approach

[Michael Catanzaro]

(In reply to comment #2)
> This still runs at configuration time, so won't this fail before the files are generated?

Yes, my patch in bogus. I failed to removed my DerivedSources directory (maybe Carlos's problem too?) so any change at all to CMakeLists would have "fixed" this issue for me.

Carlos's patch actually works; I think that's the best approach.

[Carlos Garcia Campos]

Committed r171991: <http://trac.webkit.org/changeset/171991>