Bug 72626 - [GTK] Integrate build-gtkdoc into build-webkit and make
Summary: [GTK] Integrate build-gtkdoc into build-webkit and make
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Martin Robinson
URL:
Keywords:
Depends on: 72627
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-17 09:50 PST by Martin Robinson
Modified: 2011-11-25 12:28 PST (History)
6 users (show)

See Also:


Attachments
Patch (7.41 KB, patch)
2011-11-17 09:55 PST, Martin Robinson
no flags Details | Formatted Diff | Diff
Patch (52.90 KB, patch)
2011-11-17 20:12 PST, Martin Robinson
no flags Details | Formatted Diff | Diff
Properly handle --enable-gtk-doc (61.59 KB, patch)
2011-11-18 10:49 PST, Martin Robinson
no flags Details | Formatted Diff | Diff
Patch (62.65 KB, patch)
2011-11-23 15:33 PST, Martin Robinson
no flags Details | Formatted Diff | Diff
Updated patch based on Carlos' comments (63.07 KB, patch)
2011-11-24 03:09 PST, Martin Robinson
no flags Details | Formatted Diff | Diff
Fix a couple bugs (63.30 KB, patch)
2011-11-24 04:45 PST, Martin Robinson
no flags Details | Formatted Diff | Diff
No longer use webkit-build-directory, work with _build, move utilities to common.py (62.41 KB, patch)
2011-11-24 08:14 PST, Martin Robinson
no flags Details | Formatted Diff | Diff
Patch (64.79 KB, patch)
2011-11-24 09:03 PST, Martin Robinson
no flags Details | Formatted Diff | Diff
Correct patch (64.81 KB, patch)
2011-11-24 09:18 PST, Martin Robinson
pnormand: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Robinson 2011-11-17 09:50:17 PST
build-gtkdoc is currently for WebKit1 only and does not integrate into build-webkit and "make docs".
Comment 1 Martin Robinson 2011-11-17 09:55:21 PST
Created attachment 115609 [details]
Patch
Comment 2 Martin Robinson 2011-11-17 20:12:49 PST
Created attachment 115729 [details]
Patch
Comment 3 WebKit Review Bot 2011-11-17 20:16:24 PST
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 4 Gustavo Noronha (kov) 2011-11-17 20:24:00 PST
Comment on attachment 115729 [details]
Patch

Attachment 115729 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10508494
Comment 5 Martin Robinson 2011-11-17 20:31:37 PST
This patch fails to build because the bots need a clean build.
Comment 6 Carlos Garcia Campos 2011-11-18 00:24:54 PST
Comment on attachment 115729 [details]
Patch

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

Does this unconditionally run gtk-doc without generating the html for every build?

> Source/WebCore/GNUmakefile.list.am:-2379
> -	Source/WebCore/page/MouseLockable.h \

This looks unrelated, is it a distcheck build fix?

> Source/WebCore/GNUmakefile.list.am:2404
> +	Source/WebCore/page/PointerLock.h \

Ditto.

> Source/WebKit/gtk/docs/GNUmakefile.am:-105
> -include $(top_srcdir)/Source/WebKit/gtk/GNUmakefile.gtk-doc.am

I guess you should remove GNUmakefile.gtk-doc.am too.

> Tools/GNUmakefile.am:235
> +docs: $(BUILT_SOURCES) \
> +	libwebkitgtk-@WEBKITGTK_API_MAJOR_VERSION@.@WEBKITGTK_API_MINOR_VERSION@.la \
> +	libwebkit2gtk-@WEBKITGTK_API_MAJOR_VERSION@.@WEBKITGTK_API_MINOR_VERSION@.la
> +	$(srcdir)/Tools/gtk/generate-gtkdoc
> +

Does this only happen when using make docs? We should generate the docs when running make distcheck to include the html docs in the tarball, so I guess we should add doc-dist-hook. And also rules to clean it up. We still have GTK_DOC_CHECK macro in configure, which adds --enable-gtk-doc, --enable-gtk-doc-html and --enable-gtk-doc-pdf, I think we should respect them, at least --enable-gtk-doc, because everybody expects it to work in a project using gtk-doc.
Comment 7 Martin Robinson 2011-11-18 01:05:47 PST
(In reply to comment #6)
> (From update of attachment 115729 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=115729&action=review
> 
> Does this unconditionally run gtk-doc without generating the html for every build?

This happens for builds run via build-webkit. This is so that the EWS will fail if a patch introduces documentation warnings.

> 
> > Source/WebCore/GNUmakefile.list.am:-2379
> > -	Source/WebCore/page/MouseLockable.h \
> 
> This looks unrelated, is it a distcheck build fix?
> 
> > Source/WebCore/GNUmakefile.list.am:2404
> > +	Source/WebCore/page/PointerLock.h \
> 
> Ditto.

Oops. webkit-patch snuck those in. These were make distcheck fixes I needed to test 'make dist'.
> 
> > Source/WebKit/gtk/docs/GNUmakefile.am:-105
> > -include $(top_srcdir)/Source/WebKit/gtk/GNUmakefile.gtk-doc.am
> 
> I guess you should remove GNUmakefile.gtk-doc.am too.

Yep!

> 
> > Tools/GNUmakefile.am:235
> > +docs: $(BUILT_SOURCES) \
> > +	libwebkitgtk-@WEBKITGTK_API_MAJOR_VERSION@.@WEBKITGTK_API_MINOR_VERSION@.la \
> > +	libwebkit2gtk-@WEBKITGTK_API_MAJOR_VERSION@.@WEBKITGTK_API_MINOR_VERSION@.la
> > +	$(srcdir)/Tools/gtk/generate-gtkdoc
> > +
> 
> Does this only happen when using make docs? We should generate the docs when running make distcheck to include the html docs in the tarball, so I guess we should add doc-dist-hook. And also rules to clean it up. We still have GTK_DOC_CHECK macro in configure, which adds --enable-gtk-doc, --enable-gtk-doc-html and --enable-gtk-doc-pdf, I think we should respect them, at least --enable-gtk-doc, because everybody expects it to work in a project using gtk-doc.

After talking with Carlos via IRC, it seems that adding --enable-gtk-doc is quite useful.
Comment 8 Martin Robinson 2011-11-18 10:49:21 PST
Created attachment 115835 [details]
Properly handle --enable-gtk-doc
Comment 9 Gustavo Noronha (kov) 2011-11-18 11:00:34 PST
Comment on attachment 115835 [details]
Properly handle --enable-gtk-doc

Attachment 115835 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10519167
Comment 10 Martin Robinson 2011-11-23 15:33:33 PST
Created attachment 116444 [details]
Patch
Comment 11 Martin Robinson 2011-11-24 03:09:18 PST
Created attachment 116493 [details]
Updated patch based on Carlos' comments
Comment 12 Martin Robinson 2011-11-24 04:45:59 PST
Created attachment 116498 [details]
Fix a couple bugs
Comment 13 Philippe Normand 2011-11-24 05:31:33 PST
Comment on attachment 116498 [details]
Fix a couple bugs

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

Nice patch! r- mostly because of the webkit-build-directory issue

> Tools/GNUmakefile.am:260
> +	@rm -rf Documentation/webkitgtk Documentation/webkit2gtk
> +	-@rmdir Documentation

Can that be replaced with rm -rf Documentation?

> Tools/gtk/generate-gtkdoc:58
> +    process = subprocess.Popen(['perl', script_path('webkit-build-directory'), '--configuration', 'gtk'],
> +                               stdout=subprocess.PIPE)

This will always return the path to a Release build because we never call set-webkit-configuration :(
Comment 14 Martin Robinson 2011-11-24 08:14:59 PST
Created attachment 116516 [details]
No longer use webkit-build-directory, work with _build, move utilities to common.py
Comment 15 Martin Robinson 2011-11-24 09:03:48 PST
Created attachment 116519 [details]
Patch
Comment 16 Martin Robinson 2011-11-24 09:18:13 PST
Created attachment 116520 [details]
Correct patch
Comment 17 Philippe Normand 2011-11-24 09:39:05 PST
Comment on attachment 116519 [details]
Patch

Thanks!
Comment 18 Philippe Normand 2011-11-24 09:39:46 PST
Comment on attachment 116519 [details]
Patch

Oops
Comment 19 Philippe Normand 2011-11-24 09:43:57 PST
Comment on attachment 116520 [details]
Correct patch

Thanks :)
Comment 20 Carlos Garcia Campos 2011-11-25 02:37:56 PST
Comment on attachment 116520 [details]
Correct patch

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

> Tools/gtk/common.py:57
> +    build_dir = top_level_path()

This should be build_dir = top_level_path('WebKitBuild') for the cases where webkit-build is not used and there isn't Release nor Debug dirs

> Tools/gtk/common.py:62
> +    sys.exit(1)

There's no import sys in this file, this will fail.
Comment 21 Martin Robinson 2011-11-25 08:19:58 PST
Committed r101174: <http://trac.webkit.org/changeset/101174>
Comment 22 Ryosuke Niwa 2011-11-25 12:28:30 PST
It appears that this patch broke GTK build:
http://build.webkit.org/builders/GTK%20Linux%2064-bit%20Debug/builds/27884

http://build.webkit.org/builders/GTK%20Linux%2064-bit%20Debug/builds/27884/steps/compile-webkit/logs/stdio
make: *** No rule to make target `../../Source/WebKit/gtk/docs/GNUmakefile.am', needed by `../../GNUmakefile.in'.  Stop.

Failed to build WebKit using 'make'!
program finished with exit code 2
elapsedTime=6.273611