Bug 77551 – [GTK] Fix xrefs after installing API documentation

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Carlos Garcia Campos]

gtk-doc does the right thing when you have the api documentation of the dependencies (glib, soup, gtk, etc.) installed in the same prefix you are installing webkit, but we need to provide the additional directories to scan for api docs installed in a different prefix. We can get the prefix of every package from the pkg-config file, pass the directories where the documentation might be installed for every dependency to gtkdoc-fixxref. That's enough to fix gtkdoc-fixxref warnings (assuming you have the docs intalled), but links are created with path relative to the current one, assuming all deps will be in the same prefix. So, after install the documentation we need to run gtkdoc-rebase to fix all possible invalid links (valid links are just kept of course).

[Carlos Garcia Campos]

Created attachment 124947 [details]
Patch

[Martin Robinson]

I believe this is the patch for bug 77094.

[Carlos Garcia Campos]

Created attachment 124957 [details]
The correct patch

Sorry, I uploaded the wrong patch :-P

[Hayato Ito]

It seems I wrongly canceled the review flag for the patch in 77551. I intended 77511.
Could you set review flag again if it was canceled? I am sorry.

[Carlos Garcia Campos]

Comment on attachment 124957 [details]
The correct patch

No problem.

[Carlos Garcia Campos]

Btw, Martin suggested to get the deps automatically from the pkg-config files, starting with the packages required by webkitgtk and walking recursively. That might work to find the packages, but every package can have more than one module and it can be installed with a name it's not possible to get automatically. For example: glib-2.0 installs gio, glib and gobject, gtk+2-0 installs gdk and gtk, but gtk-3.0 instal gtk3 and gdk3, libsoup-2.4 installs libsoup-2.4 with the same name than the package. The deps are just a few, so I think it's not a problem to add them manually. If we introduce a new one, gtkdoc-fixxref will show new warnings.

[Martin Robinson]

Comment on attachment 124957 [details]
The correct patch

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

Nice stuff! I have just a few suggestions for moving things around.

> Tools/GNUmakefile.am:328
> +	$(srcdir)/Tools/gtk/generate-gtkdoc --rebase

I think this should happen automatically when --skip-html is not passed to generate-gtkdoc. Either that or you should add --skip-rebase to generate-gtkdoc and make sure to pass it in the build scripts.

> Tools/gtk/common.py:90
> +def xref_deps(xref_dep_packages):
> +    def package_prefix(package):
> +        process = subprocess.Popen(['pkg-config', '--variable=prefix', package],
> +                                   stdout=subprocess.PIPE)
> +        stdout = process.communicate()[0]
> +        if process.returncode != 0:
> +            return None
> +        return stdout.strip()
> +
> +    deps = []

I think odds are low that we are ever going to use xref_deps in any other file. I would keep package_prefix here and just call it something like prefix_of_pkg_config_file. I'd move xref_deps to generate-gtkdoc and call it get_gtkdoc_module_paths.

> Tools/gtk/common.py:103
> +def gtk_version(pkg_config_path):

Might want to name this to gtk_version_of_pkg_config_file.

> Tools/gtk/generate-gtkdoc:106
> +        'xref_deps' : common.xref_deps(xref_deps),

Do you mind changing this argument name to cross_reference_deps? I know it's a bit obtuse, but cross_reference seems a little clearer than xref here.

> Tools/gtk/generate-gtkdoc:140
> +    if '--rebase' not in sys.argv:
> +        print "Generating WebKit1 documentation..."
> +        saw_webkit1_warnings = generate_doc(pkg_config_path, get_webkit1_options(common.gtk_version(pkg_config_path)))
> +    else:
> +        print "Rebasing WebKit1 documentation..."
> +        rebase_doc(pkg_config_path, get_webkit1_options(common.gtk_version(pkg_config_path)))

I think it makes sense to just get the options once here.

[Carlos Garcia Campos]

(In reply to comment #7)
> (From update of attachment 124957 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=124957&action=review
> 
> Nice stuff! I have just a few suggestions for moving things around.
> 
> > Tools/GNUmakefile.am:328
> > +	$(srcdir)/Tools/gtk/generate-gtkdoc --rebase
> 
> I think this should happen automatically when --skip-html is not passed to generate-gtkdoc. Either that or you should add --skip-rebase to generate-gtkdoc and make sure to pass it in the build scripts.

I'm not sure I get what you mean, you only want to rebase installed docs, so rebase has to be called once the docs has been installed.

> > Tools/gtk/common.py:90
> > +def xref_deps(xref_dep_packages):
> > +    def package_prefix(package):
> > +        process = subprocess.Popen(['pkg-config', '--variable=prefix', package],
> > +                                   stdout=subprocess.PIPE)
> > +        stdout = process.communicate()[0]
> > +        if process.returncode != 0:
> > +            return None
> > +        return stdout.strip()
> > +
> > +    deps = []
> 
> I think odds are low that we are ever going to use xref_deps in any other file. I would keep package_prefix here and just call it something like prefix_of_pkg_config_file. I'd move xref_deps to generate-gtkdoc and call it get_gtkdoc_module_paths.

Ok

> > Tools/gtk/common.py:103
> > +def gtk_version(pkg_config_path):
> 
> Might want to name this to gtk_version_of_pkg_config_file.
> 
> > Tools/gtk/generate-gtkdoc:106
> > +        'xref_deps' : common.xref_deps(xref_deps),
> 
> Do you mind changing this argument name to cross_reference_deps? I know it's a bit obtuse, but cross_reference seems a little clearer than xref here.

xref is pretty standard name, but I don't mind to rename it

> > Tools/gtk/generate-gtkdoc:140
> > +    if '--rebase' not in sys.argv:
> > +        print "Generating WebKit1 documentation..."
> > +        saw_webkit1_warnings = generate_doc(pkg_config_path, get_webkit1_options(common.gtk_version(pkg_config_path)))
> > +    else:
> > +        print "Rebasing WebKit1 documentation..."
> > +        rebase_doc(pkg_config_path, get_webkit1_options(common.gtk_version(pkg_config_path)))
> 
> I think it makes sense to just get the options once here.

it happens once

[Martin Robinson]

Comment on attachment 124957 [details]
The correct patch

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

>>> Tools/GNUmakefile.am:328
>>> +	$(srcdir)/Tools/gtk/generate-gtkdoc --rebase
>> 
>> I think this should happen automatically when --skip-html is not passed to generate-gtkdoc. Either that or you should add --skip-rebase to generate-gtkdoc and make sure to pass it in the build scripts.
> 
> I'm not sure I get what you mean, you only want to rebase installed docs, so rebase has to be called once the docs has been installed.

Ah, I understand now. I suggested a small rename below then.

>>> Tools/gtk/generate-gtkdoc:140
>>> +        rebase_doc(pkg_config_path, get_webkit1_options(common.gtk_version(pkg_config_path)))
>> 
>> I think it makes sense to just get the options once here.
> 
> it happens once

Sorry, I meant just to factor out the code that gets the options to one place.
options = get_webkit1_options(common.gtk_version(pkg_config_path)
if '--rebase' not in sys.argv:
    print "Generating WebKit1 documentation..."
    saw_webkit1_warnings = generate_doc(pkg_config_path, options)
else:
    print "Rebasing WebKit1 documentation..."
    rebase_doc(pkg_config_path, options)

> Tools/gtk/gtkdoc.py:352
> +    def rebase(self):

Perhaps call this rebase_installed_docs, just to avoid confusing dumb people like me. :)

[Carlos Garcia Campos]

(In reply to comment #9)
> (From update of attachment 124957 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=124957&action=review
> 
> >>> Tools/GNUmakefile.am:328
> >>> +	$(srcdir)/Tools/gtk/generate-gtkdoc --rebase
> >> 
> >> I think this should happen automatically when --skip-html is not passed to generate-gtkdoc. Either that or you should add --skip-rebase to generate-gtkdoc and make sure to pass it in the build scripts.
> > 
> > I'm not sure I get what you mean, you only want to rebase installed docs, so rebase has to be called once the docs has been installed.
> 
> Ah, I understand now. I suggested a small rename below then.
> 
> >>> Tools/gtk/generate-gtkdoc:140
> >>> +        rebase_doc(pkg_config_path, get_webkit1_options(common.gtk_version(pkg_config_path)))
> >> 
> >> I think it makes sense to just get the options once here.
> > 
> > it happens once
> 
> Sorry, I meant just to factor out the code that gets the options to one place.
> options = get_webkit1_options(common.gtk_version(pkg_config_path)
> if '--rebase' not in sys.argv:
>     print "Generating WebKit1 documentation..."
>     saw_webkit1_warnings = generate_doc(pkg_config_path, options)
> else:
>     print "Rebasing WebKit1 documentation..."
>     rebase_doc(pkg_config_path, options)

Ok

> > Tools/gtk/gtkdoc.py:352
> > +    def rebase(self):
> 
> Perhaps call this rebase_installed_docs, just to avoid confusing dumb people like me. :)

Ok

[Carlos Garcia Campos]

Created attachment 125593 [details]
Updated patch according to review

[Martin Robinson]

Comment on attachment 125593 [details]
Updated patch according to review

Thanks!

[Carlos Garcia Campos]

Committed r106786: <http://trac.webkit.org/changeset/106786>