Re: [PATCH] docs-rst: automatically convert Graphviz and SVG images

[Mauro Carvalho Chehab <mchehab@s-opensource.com>]

Em Thu, 2 Mar 2017 22:16:49 +0100
Markus Heiser <markus.heiser@darmarit.de> escreveu:

> > Am 02.03.2017 um 20:34 schrieb Mauro Carvalho Chehab <mchehab@s-opensource.com>:
> > 
> > Em Thu, 2 Mar 2017 20:06:39 +0100
> > Markus Heiser <markus.heiser@darmarit.de> escreveu:
> >   
> >> Hi Mauro,
> >>   
> >>> Tested here with the enclosed patch.    
> >> 
> >> great, big step forward making /media/Makefile smaller ...  thanks a lot!!!!
> >>   
> >>> It crashed:
> >>> Exception occurred:
> >>> File "/devel/v4l/patchwork/Documentation/sphinx/kfigure.py", line 222, in dot2format
> >>>   sys.stderr.write(err)
> >>> TypeError: write() argument must be str, not bytes
> >>> The full traceback has been saved in /tmp/sphinx-err-_1vahbmg.log, if you want to report the issue to the developers.
> >>> Please also report this if it was a user error, so that a better error message can be provided next time.
> >>> A bug report can be filed in the tracker at <https://github.com/sphinx-doc/sphinx/issues>. Thanks!
> >>> Documentation/Makefile.sphinx:69: recipe for target 'htmldocs' failed
> >>> make[1]: *** [htmldocs] Error 1
> >>> Makefile:1450: recipe for target 'htmldocs' failed
> >>> make: *** [htmldocs] Error 2
> >>> 
> >>> Weird enough, it produced a Documentation/output/media/uapi/v4l/pipeline.svg file.    
> >> 
> >> I guess that the dot command writes something to stderr. This is captured 
> >> by the extension and printed to stderr ...
> >> 
> >> +def dot2format(dot_fname, out_fname):
> >> ...
> >> +    exit_code = 42
> >> +    with open(out_fname, "w") as out:
> >> +        p = subprocess.Popen(
> >> +            cmd, stdout = out, stderr = subprocess.PIPE )
> >> +        nil, err = p.communicate()
> >> +
> >> +        sys.stderr.write(err)
> >> +
> >> +        exit_code = p.returncode
> >> +        out.flush()
> >> +    return bool(exit_code == 0)
> >>   
> >>> File "/devel/v4l/patchwork/Documentation/sphinx/kfigure.py", line 222, in dot2format
> >>>   sys.stderr.write(err)
> >>> TypeError: write() argument must be str, not bytes    
> >> 
> >> Do we need this stderr output? For a first test, uncomment the 
> >> "sys.stderr.write(err)“ in line 222. Or, if we really need the
> >> stderr, try:
> >> 
> >> -        sys.stderr.write(err)
> >> +        sys.stderr.write(str(err))  
> > 
> > Yes, this fixed. I actually did:
> > 
> > -        sys.stderr.write(err)
> > +        sys.stderr.write(str(err))
> > +        sys.stderr.write("\n")
> > 
> > It is now printing:
> > 	b''
> > 
> > I added the \n print to avoid it to be mixed with the "writing output"
> > prints.
> > No idea how to make sense from it - but clearly, the error report
> > logic require some care ;-)  
> 
> 
> Aargh, I’am a idiot ... I guess 'sys.stderr.write(err)‘ is a artefact
> of my development, simply drop it and the subprocess.PIPE of stderr
> also.
> 
> +    with open(out_fname, "w") as out:
> +        p = subprocess.Popen(
> -            cmd, stdout = out, stderr = subprocess.PIPE )
> +            cmd, stdout = out)
> +        nil, err = p.communicate()
> -
> -        sys.stderr.write(err)
> -
> +        exit_code = p.returncode
> +        out.flush()
> +    return bool(exit_code == 0)
> 
> I can’t test it ATM, but without redirect stderr, the stderr
> of the parent process is inherited.
> 
>   https://docs.python.org/3.6/library/subprocess.html#popen-constructor
> 
> The Popen.communicate() always returns a tuple (stdout_data, stderr_data)
> with above the tuple is always (None, None).
> 
>   https://docs.python.org/3.6/library/subprocess.html#subprocess.Popen.communicate
> 
>   """to get anything other than None in the result tuple,
>      you need to give stdout=PIPE and/or stderr=PIPE too."""
> 
> Sorry, that I made all this mistakes, but „here“ I have only mail
> and web, no dev-env and I miss my emacs  ;) 
> 
> If the suggestion above does not work, I have to investigate
> more time next weekend.

Hmm... I would be more verbose on output the error code, printing from
where the error came, e. g. printing a message like:

Error #0 when calling dot for '/devel/v4l/patchwork/Documentation/media/uapi/v4l/pipeline.dot': None                                                                                          

As on this patch:

diff --git a/Documentation/sphinx/kfigure.py b/Documentation/sphinx/kfigure.py
index 32eab0f4cfba..a366a89f4f98 100644
--- a/Documentation/sphinx/kfigure.py
+++ b/Documentation/sphinx/kfigure.py
@@ -217,11 +217,13 @@ def dot2format(dot_fname, out_fname):
     with open(out_fname, "w") as out:
         p = subprocess.Popen(
             cmd, stdout = out, stderr = subprocess.PIPE )
-        nil, err = p.communicate()
-
-        sys.stderr.write(err)
+        err = p.communicate()
 
         exit_code = p.returncode
+
+        if exit_code != 0:
+            sys.stderr.write("Error #%d when calling dot for '%s': %s\n" % (exit_code, dot_fname, repr(err[0])))
+
         out.flush()
     return bool(exit_code == 0)
 
@@ -239,11 +241,13 @@ def svg2pdf(svg_fname, pdf_fname):
     cmd = [convert_cmd, svg_fname, pdf_fname]
     p = subprocess.Popen(
         cmd, stdout = out, stderr = subprocess.PIPE )
-    nil, err = p.communicate()
-
-    sys.stderr.write(err)
+    err = p.communicate()
 
     exit_code = p.returncode
+
+    if exit_code != 0:
+        sys.stderr.write("Error #%d when calling convert for '%s': %s\n" % (exit_code, dot_fname, repr(err[0])))
+
     return bool(exit_code == 0)