After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 779566 - Replace shell executables with Python
Replace shell executables with Python
Status: RESOLVED FIXED
Product: gtk-doc
Classification: Platform
Component: general
unspecified
Other Linux
: Normal normal
: 1.26
Assigned To: gtk-doc maintainers
gtk-doc maintainers
Depends on:
Blocks:
 
 
Reported: 2017-03-04 11:26 UTC by Jussi Pakkanen
Modified: 2017-03-20 19:34 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Convert mkpdf (9.86 KB, patch)
2017-03-07 22:47 UTC, Jussi Pakkanen
none Details | Review
Convert mkman (5.76 KB, patch)
2017-03-07 22:47 UTC, Jussi Pakkanen
none Details | Review
Convert mkhtml (7.12 KB, patch)
2017-03-07 22:47 UTC, Jussi Pakkanen
none Details | Review
New version of mkpdf (9.87 KB, patch)
2017-03-18 08:32 UTC, Jussi Pakkanen
none Details | Review
Squashed and rebased (20.84 KB, patch)
2017-03-19 14:36 UTC, Jussi Pakkanen
none Details | Review
Path fixed (20.84 KB, patch)
2017-03-19 20:23 UTC, Jussi Pakkanen
none Details | Review
Hide pdf output unless verbose (20.84 KB, patch)
2017-03-20 17:09 UTC, Jussi Pakkanen
committed Details | Review
Converted mkpdf, mkman and mkhtml to Python. (20.93 KB, patch)
2017-03-20 19:32 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
committed Details | Review

Description Jussi Pakkanen 2017-03-04 11:26:53 UTC
Gtk-doc executables are currently writteh in Perl, Python and shell. This increases maintenance burden and makes portability harder. I have converted mkpdf, mkman and mkhtml from shell to Python. The code is here:

https://github.com/jpakkane/gtkdoc

After this only gtkdozice is in shell and since it it only used to parse and modify configure scripts so it requires the shell anyway. Runtime dependencies should only consist of Perl and Python now.

All tests pass except for sanity.sh, which fails for me in trunk even without these changes.
Comment 1 Jussi Pakkanen 2017-03-07 22:47:10 UTC
Created attachment 347429 [details] [review]
Convert mkpdf
Comment 2 Jussi Pakkanen 2017-03-07 22:47:34 UTC
Created attachment 347430 [details] [review]
Convert mkman
Comment 3 Jussi Pakkanen 2017-03-07 22:47:54 UTC
Created attachment 347431 [details] [review]
Convert mkhtml
Comment 4 Stefan Sauer (gstreamer, gtkdoc dev) 2017-03-16 22:24:42 UTC
Actually I was hoping to replace the python tool with a perl tool one day to reduce the deps (especially since the depscan tool is sort of experimental). Rewriting all perl code in python would unfortunately be quite a bit of work :/

Out of curiosity, how is the shell dependency a problem? windows?

In general I agree that it would be nice to harmonize this. There is a gtk-doc mailing list and irc channel. Especially the irc channel is good to sync on this before sending patches.
Comment 5 Nirbheek Chauhan 2017-03-16 23:47:29 UTC
Yeah, it's a bit annoying on Windows, especially if you're building with the Meson build system (written in Python) which means your project can build out of the box with MSVC with no extra dependencies.

Except that you still need MSYS/MinGW because gtk-doc needs shell, which sort of defeats the purpose of being able to build natively on Windows. MSYS/MinGW and MSYS2/MinGW are also not very well-maintained (even though MSYS2 is "updated" regularly).

We're trying to create a sane workflow around building the gtk+ stack on Windows that doesn't involve downloading and installing a house of cards in just the right way, and it would be really helpful to reduce build-tool dependencies where possible.
Comment 6 Jussi Pakkanen 2017-03-17 17:41:01 UTC
> Rewriting all perl code in python would unfortunately be quite a bit of work

Yes. I looked into this and it is some work, yes, but not a huge amount and it can be done one script at a time. I would not mind spending some time to convert these scripts from Perl to Python if there was some sort of guarantee that those changes would be merged.

Getting rid of Perl would be beneficial by the mere fact that a lot more people know Python than Perl (and are willing to work with it) so getting new contributors would become easier. Going even further if gtk-doc were converted into a proper Python package deploying it would become easy and people might start using it more in place of e.g. Doxygen.
Comment 7 Stefan Sauer (gstreamer, gtkdoc dev) 2017-03-17 20:55:19 UTC
Review of attachment 347429 [details] [review]:

::: gtkdoc-mkpdf.in
@@ +47,3 @@
+    # MAKE_SCRDIR=$(abs_srcdir) MAKE_BUILDDIR=$(abs_builddir) gtkdoc-mkpdf ...
+    gtkdocdir=os.path.split(sys.argv[0])
+    #echo "uninstalled, gtkdocdir=$gtkdocdir"

can we change this to logging.debug()?
Comment 8 Stefan Sauer (gstreamer, gtkdoc dev) 2017-03-17 21:02:42 UTC
The review tool throws an error right now. The other two patches are okay. If you have commit access go ahead ad push.

In the long run, I'd like to use classes to avoid the global vars. This would also make them easier to test.

+1 for making this a proper python package then and e.g. allowing to install via pip.
Comment 9 Jussi Pakkanen 2017-03-17 23:07:39 UTC
> can we change this to logging.debug()?

I can update this tomorrow if you want.

> f you have commit access go ahead ad push.

I don't have the rights, sorry.

> In the long run, I'd like to use classes to avoid the global vars.

Yes, definitely. For this conversion I specifically did not do that to keep the script as close as possible to the original so debugging will be easier in case of regressions.
Comment 10 Jussi Pakkanen 2017-03-18 08:32:01 UTC
Created attachment 348216 [details] [review]
New version of mkpdf

Mkpdf.in with logging.
Comment 11 Stefan Sauer (gstreamer, gtkdoc dev) 2017-03-19 13:01:25 UTC
Thanks could you squash the commits? git bz apply seems to mess up the ordering:
> git bz apply 779566
Bug 779566 - Replace shell executables with Python

347430 - Convert mkman
347431 - Convert mkhtml
348216 - New version of mkpdf

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Converted mkman to Python.
error: sha1 information is lacking or useless (tests/tools.sh.in).
error: could not build fake ancestor
Patch failed at 0001 Converted mkman to Python.
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Convert-mkman-kbXOpl.patch

I've also tried interactive and reorder the patches.
Comment 12 Jussi Pakkanen 2017-03-19 14:36:52 UTC
Created attachment 348259 [details] [review]
Squashed and rebased
Comment 13 Stefan Sauer (gstreamer, gtkdoc dev) 2017-03-19 19:18:18 UTC
Review of attachment 348259 [details] [review]:

::: gtkdoc-mkpdf.in
@@ +48,3 @@
+    # MAKE_SCRDIR=$(abs_srcdir) MAKE_BUILDDIR=$(abs_builddir) gtkdoc-mkpdf ...
+    gtkdocdir=os.path.split(sys.argv[0])
+    logging.debug("uninstalled, gtkdocdir=" + gtkdocdir)

Traceback (most recent call last):
  • File "/home/ensonic/projects/gnome/gtk-doc/gtkdoc-mkpdf", line 50 in <module>
    logging.debug("uninstalled, gtkdocdir=" + gtkdocdir)
TypeError: cannot concatenate 'str' and 'tuple' objects

please run the tests. Maybe ue os.path.dirname or gtkdocdir = os.path.split(sys.argv[0])[0] like on gtkdoc-mkhtml.
Comment 14 Jussi Pakkanen 2017-03-19 20:23:49 UTC
Created attachment 348274 [details] [review]
Path fixed

I did run tests. They reported all passing except for the above mentioned sanity.sh, which fails on trunk for me.
Comment 15 Stefan Sauer (gstreamer, gtkdoc dev) 2017-03-20 08:20:28 UTC
Review of attachment 348274 [details] [review]:

Almost there.

::: gtkdoc-mkpdf.in
@@ +95,3 @@
+        #echo "calling: @DBLATEX@ $dblatex_options"
+        if options.verbose:
+        # xsltproc is already called with --xinclude

typo: stdout

@@ +104,3 @@
+        for line in stde.split('\n'):
+            if 'programlisting or screen' not in line:
+            dblatex_options += ['-I', i]

The tests still fail for me. 

Without your patch:
cat /home/ensonic/projects/gnome/gtk-doc/tests/annotations/docs/gtkdoc-mkpdf.log
gtkdoc-mkpdf --uninstalled --path=/home/ensonic/projects/gnome/gtk-doc/tests/annotations/docs  tester tester-docs.xml 

With your patch:
cat /home/ensonic/projects/gnome/gtk-doc/tests/annotations/docs/gtkdoc-mkpdf.log
gtkdoc-mkpdf --uninstalled --path=/home/ensonic/projects/gnome/gtk-doc/tests/annotations/docs  tester tester-docs.xml 
Build the book set list...
Build the listings...
XSLT stylesheets DocBook - LaTeX 2e (0.3.4-3)
===================================================
Build tester-docs.pdf
processing index /tmp/tmplzPNWm/tester-docs.idx...
This is makeindex, version 2.15 [TeX Live 2013] (kpathsea + Thai support).
Scanning style file /usr/share/dblatex/latex/scripts/doc.ist....done (4 attributes redefined, 0 ignored).
Scanning input file /tmp/tmplzPNWm/tester-docs.idx....done (23 entries accepted, 0 rejected).
Sorting entries....done (111 comparisons).
Generating output file tester-docs.ind....done (36 lines written, 0 warnings).
Output written in tester-docs.ind.
Transcript written in tester-docs.ilg.
'tester.pdf' successfully built

The "grep -v 'programlisting or screen'" is a work around for the xslt-stylesheets that printed noise. The dblatex output is suppressed using "2>&1 >/dev/null".

So maybe you can use (stdout=subprocess.DEVNULL) if not verbose?
Comment 16 Stefan Sauer (gstreamer, gtkdoc dev) 2017-03-20 08:21:46 UTC
Hej, once this is in. I'll remove so cruft and make a release. If you could join the #gtkdoc channel on gimpnet irc, we could sync on a decent plan. Otherwise gtkdoc has a mailing list, desktop-devel could also work. Thanks.
Comment 17 Jussi Pakkanen 2017-03-20 17:09:06 UTC
Created attachment 348336 [details] [review]
Hide pdf output unless verbose

Updated. Did not touch the sanity.sh issue yet.
Comment 18 Stefan Sauer (gstreamer, gtkdoc dev) 2017-03-20 17:28:18 UTC
/home/ensonic/projects/gnome/gtk-doc/gtkdoc-scangobj syntax OK
  • File "/home/ensonic/projects/gnome/gtk-doc/gtkdoc-mkhtml", line 4
    usage() {
SyntaxError: invalid syntax
  File "/home/ensonic/projects/gnome/gtk-doc/gtkdoc-mkman", line 4
    usage() {
            ^
SyntaxError: invalid syntax
  File "/home/ensonic/projects/gnome/gtk-doc/gtkdoc-mkpdf", line 4
    usage() {
            ^
SyntaxError: invalid syntax
82 %: Checks 17, Failures: 3

I'll fix this on the and submit the patch tonight. Wonder why the tests are not catching this for you :/
Comment 19 Stefan Sauer (gstreamer, gtkdoc dev) 2017-03-20 17:50:19 UTC
Sorry, this was a state patch.
Comment 20 Stefan Sauer (gstreamer, gtkdoc dev) 2017-03-20 19:32:48 UTC
The following fix has been pushed:
91bc782 Converted mkpdf, mkman and mkhtml to Python.
Comment 21 Stefan Sauer (gstreamer, gtkdoc dev) 2017-03-20 19:32:57 UTC
Created attachment 348344 [details] [review]
Converted mkpdf, mkman and mkhtml to Python.
Comment 22 Stefan Sauer (gstreamer, gtkdoc dev) 2017-03-20 19:34:45 UTC
The output was still wrong (inverted option and a typo in that branch). Fixed now. Thanks.