Summary: | [GTK] Fix xrefs after installing API documentation | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Carlos Garcia Campos <cgarcia> | ||||||||
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | gustavo, hayato, mrobinson, pnormand, xan.lopez | ||||||||
Priority: | P2 | Keywords: | Gtk | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | Linux | ||||||||||
Bug Depends on: | 77094, 77542 | ||||||||||
Bug Blocks: | |||||||||||
Attachments: |
|
Description
Carlos Garcia Campos
2012-02-01 06:49:05 PST
Created attachment 124947 [details]
Patch
Created attachment 124957 [details]
The correct patch
Sorry, I uploaded the wrong patch :-P
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. Comment on attachment 124957 [details]
The correct patch
No problem.
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. 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. (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 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. :) (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 Created attachment 125593 [details]
Updated patch according to review
Comment on attachment 125593 [details]
Updated patch according to review
Thanks!
Committed r106786: <http://trac.webkit.org/changeset/106786> |