Header files are currently installed under $prefix/include/webkit-<api-version> directory, but directiory name should be webkitgtk-<api-version> since we renamed the library.
Created attachment 102777 [details] Patch
Attachment 102777 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'GNUmakefile.am', u'Source/Ja..." exit_code: 1 WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testwindow.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testatk.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testwebdatasource.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testdownload.c" Source/WebKit/gtk/webkitgtk.h:21: #ifndef header guard has wrong style, please use: webkitgtk_h [build/header_guard] [5] Source/WebKit/gtk/webkitgtk.h:25: Alphabetical sorting problem. [build/include_order] [4] WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testdomnode.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testnetworkrequest.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testatkroles.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testapplicationcache.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testwebbackforwardlist.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testcopyandpaste.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testwebsettings.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testdomdomwindow.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testhittestresult.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testloading.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testwebhistoryitem.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testwebresource.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testnetworkresponse.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testwebview.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testwebframe.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testwebplugindatabase.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testglobals.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testdomdocument.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testkeyevents.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testhttpbackend.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testmimehandling.c" Total errors found: 2 in 37 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 102777 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=102777&action=review This patch should probably also add webkitgtk.h to the list of headers skipped by the style bot. > GNUmakefile.am:44 > +libwebkitgtkincludedir := $(prefix)/include/webkitgtk-@WEBKITGTK_API_VERSION@ libwebkitgtk_include_dir? > Source/WebKit/gtk/GNUmakefile.am:34 > +# Global header > +libwebkitgtkinclude_HEADERS += \ > + $(srcdir)/Source/WebKit/gtk/webkitgtk.h > + Is 32libwebkitgtkinclude_HEADERS a primary? I don't see it referenced anywhere else. What is the significance of the string "libwebkitgtkinclude". I think this should be next to the other list of headers. > Source/WebKit/gtk/webkit/webkit.h:21 > +#warning "<webkit/webkit.h> is deprecated, use <webkitgtk.h> instead." Maybe say "Including <webkit/webkit.h> is deprecated, include <webkitgtk.h> directly instead."
Just want to say that I support this change. It does two things mainly: * Headers are installed to $PREFIX/include/webkitgtk-3.0. * WebKitGTK+ embedders now include "webkitgtk.h" in the same style as GLib. This change should be backward compatible, since -I flags are abstracted with pkg-config and it preserves webkitgtk.h.
(In reply to comment #3) > (From update of attachment 102777 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=102777&action=review > > This patch should probably also add webkitgtk.h to the list of headers skipped by the style bot. > > > GNUmakefile.am:44 > > +libwebkitgtkincludedir := $(prefix)/include/webkitgtk-@WEBKITGTK_API_VERSION@ > > libwebkitgtk_include_dir? I'm not sure you can use _ before dir, I think the pattern is foodir, foot_DATA, foo_HEADER, etc. like pkgconfigdir, typelibsdir, ... > > Source/WebKit/gtk/GNUmakefile.am:34 > > +# Global header > > +libwebkitgtkinclude_HEADERS += \ > > + $(srcdir)/Source/WebKit/gtk/webkitgtk.h > > + > > Is 32libwebkitgtkinclude_HEADERS a primary? I don't see it referenced anywhere else. What is the significance of the string "libwebkitgtkinclude". I think this should be next to the other list of headers. Yes, it's initialized in /GNUmakefile.am. For the name I followed the same pattern used by glib and gtk (glibincludedir, libgtkincludedir) > > Source/WebKit/gtk/webkit/webkit.h:21 > > +#warning "<webkit/webkit.h> is deprecated, use <webkitgtk.h> instead." > > Maybe say "Including <webkit/webkit.h> is deprecated, include <webkitgtk.h> directly instead." Ok.
(In reply to comment #5) > I'm not sure you can use _ before dir, I think the pattern is foodir, foot_DATA, foo_HEADER, etc. like pkgconfigdir, typelibsdir, ... Ah, I see. > Yes, it's initialized in /GNUmakefile.am. For the name I followed the same pattern used by glib and gtk (glibincludedir, libgtkincludedir) This works together with libwebkitincludedir. We just need to get some buy-in from Xan and Gustavo for me to r+ this patch.
Created attachment 102786 [details] Updated patch
Comment on attachment 102786 [details] Updated patch The change looks good to me, we should try to add this for the 1.6 this week.
Xan or Gustavo, any thoughts on this? I think we should get some more buy-in from the reviwers before doing this.
I have nothing against it, not sure about using #include <webkitgtk.h> only, though. I never understood why all other libraries use a directory but maybe there's a good reason? In any case, if Xan does not oppose it, I'm pro.
Comment on attachment 102786 [details] Updated patch LGTM but I'd rather wait on Xan :) This change should probably mentionned in the next release NEWS.
(In reply to comment #10) > I have nothing against it, not sure about using #include <webkitgtk.h> only, though. I never understood why all other libraries use a directory but maybe there's a good reason? In any case, if Xan does not oppose it, I'm pro. AFAIK <foo/foo.h> vs. <foo.h> is just a matter of style for the most part. IMHO <foo/foo.h> indicates better the namespacing, but that's about it. The GNOME platform strongly prefers <foo/foo.h>, I can only think of glib-object.h and cairo.h being "different", and the glib case is really old, with all the new libraries following the new pattern (like gio/gio.h). So my preference would be to try to do something like <webkit/webkitgtk.h> and only if it's really troublesome go for <webkitgtk.h>
Created attachment 108467 [details] New patch to use <webkit/webkitgtk.h> I also prefer the pattern foo/foo.h, so I've changed this patch and the webkit2 one to use webkit/webkitgtk.h and webkit2/webkit2gtk.h.
Attachment 108467 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'GNUmakefile.am', u'Source/Ja..." exit_code: 1 WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testwindow.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testatk.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testwebdatasource.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testdownload.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testdomnode.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testnetworkrequest.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testatkroles.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testapplicationcache.c" Source/WebKit/gtk/webkit/webkitgtk.h:21: #ifndef header guard has wrong style, please use: webkitgtk_h [build/header_guard] [5] Source/WebKit/gtk/webkit/webkitgtk.h:25: Alphabetical sorting problem. [build/include_order] [4] WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testwebbackforwardlist.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testcopyandpaste.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testwebsettings.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testdomdomwindow.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testhittestresult.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testloading.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testwebhistoryitem.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testwebresource.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testnetworkresponse.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testwebview.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testwebframe.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testwebplugindatabase.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testglobals.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testdomdocument.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testkeyevents.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testhttpbackend.c" WARNING: File exempt from style guide. Skipping: "Source/WebKit/gtk/tests/testmimehandling.c" Total errors found: 2 in 49 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 108803 [details] New patch After discussing this again yesterday we decided to try to use webkit2/webkit2.h for webkit2, so that we don't need to change wk1 header. This patch just installs the headers under $prefix/webkigtk-<api-version>.
Comment on attachment 108803 [details] New patch View in context: https://bugs.webkit.org/attachment.cgi?id=108803&action=review Seems reasonable to me! > GNUmakefile.am:44 > +libwebkitgtkincludedir := $(prefix)/include/webkitgtk-@WEBKITGTK_API_VERSION@ Please call this libwebkitgtk_include_dir if possible
Committed r96087: <http://trac.webkit.org/changeset/96087>