Bug 72626 – [GTK] Integrate build-gtkdoc into build-webkit and make

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Martin Robinson]

build-gtkdoc is currently for WebKit1 only and does not integrate into build-webkit and "make docs".

[Martin Robinson]

Created attachment 115609 [details]
Patch

[Martin Robinson]

Created attachment 115729 [details]
Patch

[WebKit Review Bot]

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

[Gustavo Noronha (kov)]

Comment on attachment 115729 [details]
Patch

Attachment 115729 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10508494

[Martin Robinson]

This patch fails to build because the bots need a clean build.

[Carlos Garcia Campos]

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.

[Martin Robinson]

(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.

[Martin Robinson]

Created attachment 115835 [details]
Properly handle --enable-gtk-doc

[Gustavo Noronha (kov)]

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

[Martin Robinson]

Created attachment 116444 [details]
Patch

[Martin Robinson]

Created attachment 116493 [details]
Updated patch based on Carlos' comments

[Martin Robinson]

Created attachment 116498 [details]
Fix a couple bugs

[Philippe Normand]

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 :(

[Martin Robinson]

Created attachment 116516 [details]
No longer use webkit-build-directory, work with _build, move utilities to common.py

[Martin Robinson]

Created attachment 116519 [details]
Patch

[Martin Robinson]

Created attachment 116520 [details]
Correct patch

[Philippe Normand]

Comment on attachment 116519 [details]
Patch

Thanks!

[Philippe Normand]

Comment on attachment 116519 [details]
Patch

Oops

[Philippe Normand]

Comment on attachment 116520 [details]
Correct patch

Thanks :)

[Carlos Garcia Campos]

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.

[Martin Robinson]

Committed r101174: <http://trac.webkit.org/changeset/101174>

[Ryosuke Niwa]

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