[PATCH] gnu: doxygen: Use sh from the store.

[PATCH] gnu: doxygen: Use sh from the store.

[Thomas Danckaert]

[-- Attachment #1: Type: Text/Plain, Size: 390 bytes --]

Hi Guix,

Doxygen uses a hardcoded "/bin/sh" to run "dot" (and maybe other 
processes as well) to generate documentation graphs, which fails in a 
build environment.  This patch adds the store item of bash-minimal as 
a prefix.

According to `guix refresh --list-dependent', this patch would cause 
689 packages to be rebuilt, so it probably shouldn't be applied on 
master.

best,

Thomas

[-- Attachment #2: 0001-gnu-doxygen-Use-sh-from-the-store.patch --]
[-- Type: Text/X-Patch, Size: 2233 bytes --]

From 306e4805734bdaf5d0ef2b26a30ff2c558aaf93c Mon Sep 17 00:00:00 2001
From: Thomas Danckaert <thomas.danckaert@gmail.com>
Date: Tue, 3 Jan 2017 12:58:28 +0100
Subject: [PATCH] gnu: doxygen: Use sh from the store.

* gnu/packages/documentation.scm (doxygen)[inputs]: Add bash-minimal.
  [arguments]: Add phase to add store prefix to "/bin/sh".
---
 gnu/packages/documentation.scm | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/documentation.scm b/gnu/packages/documentation.scm
index bbc25e8..6f648db 100644
--- a/gnu/packages/documentation.scm
+++ b/gnu/packages/documentation.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2014, 2016 Andreas Enge <andreas@enge.fr>
 ;;; Copyright © 2016 Eric Bavier <bavier@member.fsf.org>
 ;;; Copyright © 2016 Roel Janssen <roel@gnu.org>
+;;; Copyright © 2016 Thomas Danckaert <post@thomasdanckaert.be>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -26,6 +27,7 @@
   #:use-module (guix build-system gnu)
   #:use-module (guix build-system cmake)
   #:use-module (gnu packages)
+  #:use-module (gnu packages bash)
   #:use-module (gnu packages python)
   #:use-module (gnu packages bison)
   #:use-module (gnu packages docbook)
@@ -99,8 +101,17 @@ markup) can be customized and extended by the user.")
        ("flex" ,flex)
        ("libxml2" ,libxml2) ; provides xmllint for the tests
        ("python" ,python-2))) ; for creating the documentation
+    (inputs
+     `(("bash" ,bash-minimal)))
     (arguments
-     `(#:test-target "tests"))
+     `(#:test-target "tests"
+       #:phases (modify-phases %standard-phases
+                  (add-before 'configure 'patch-sh
+                              (lambda* (#:key inputs #:allow-other-keys)
+                                (substitute* "src/portable.cpp"
+                                  (("/bin/sh")
+                                   (string-append
+                                    (assoc-ref inputs "bash") "/bin/sh"))))))))
     (home-page "http://www.stack.nl/~dimitri/doxygen/")
     (synopsis "Generate documentation from annotated sources")
     (description "Doxygen is the de facto standard tool for generating
-- 
2.7.4

Re: [PATCH] gnu: doxygen: Use sh from the store.

[Danny Milosavljevic]

Thanks for the patch! Looks good to me.

In any case, if we search for "portable_system", we find that there are lots of other things that don't use in-store names:

./cite.cpp:  if ((exitCode=portable_system("perl","\""+bib2xhtmlFile+"\" "+bibOutputFiles+" \""+
./dia.cpp:  if ((exitCode=portable_system(diaExe,diaArgs,FALSE))!=0) // from config
./dia.cpp:    if (portable_system("epstopdf",epstopdfArgs)!=0)
./diagram.cpp:    if (portable_system("epstopdf",epstopdfArgs)!=0)
./docparser.cpp:      if (portable_system("epstopdf",epstopdfArgs)!=0)
./dot.cpp:    if ((exitCode=portable_system(m_dotExe.data(),dotArgs,FALSE))!=0) // from config
./dot.cpp:      if ((exitCode=portable_system(m_dotExe.data(),dotArgs,FALSE))!=0) // from config
./dot.cpp:  if (!m_postCmd.isEmpty() && portable_system(m_postCmd.data(),m_postArgs.data())!=0) // seems to be not set anywhere
./doxygen.cpp:  if (portable_system(Config_getString("HHC_LOCATION"), "index.hhp", Debug::isFlagSet(Debug::ExtCmd))) // from config
./doxygen.cpp:    if (portable_system(Config_getString("QHG_LOCATION"), args.data(), FALSE)) // from config
./formula.cpp:    if (portable_system(latexCmd,"_formulas.tex")!=0) // from config
./formula.cpp:      if (portable_system("dvips",dviArgs)!=0)
./formula.cpp:      if (portable_system(portable_ghostScriptCommand(),gsArgs)!=0) // portable_ghostScriptCommand body hardcodes executable name
./htags.cpp:  bool result=portable_system("htags",commandLine,FALSE)==0;
./msc.cpp:  if ((exitCode=portable_system(mscExe,mscArgs,FALSE))!=0) // from config
./msc.cpp:    if (portable_system("epstopdf",epstopdfArgs)!=0)
./msc.cpp:  if ((exitCode=portable_system(mscExe,mscArgs,FALSE))!=0) // from config
./plantuml.cpp:  if ((exitCode=portable_system(pumlExe,pumlArgs,TRUE))!=0) // java, hardcoded
./plantuml.cpp:    if ((exitCode=portable_system("epstopdf",epstopdfArgs))!=0)
./portable.cpp:int portable_system(const char *command,const char *args,bool commandHasConsole) // should have argument list, not huge string
./portable.h:int            portable_system(const char *command,const char *args,bool commandHasConsole=TRUE);
./vhdldocgen.cpp:    if (portable_system("dot",vlargs)!=0)
./vhdldocgen.cpp:  if (portable_system("dot",vlargs)!=0)

Aaaah found sprintf without max length specifier. 

Dear god is doxygen unsafe. I should refrain from reading the source code of some popular packages - it doesn't end well.

If someone is motivated, could also fix the above executable names later.

Re: [PATCH] gnu: doxygen: Use sh from the store.

[Thomas Danckaert]

From: Danny Milosavljevic <dannym@scratchpost.org>
Subject: Re: [PATCH] gnu: doxygen: Use sh from the store.
Date: Tue, 3 Jan 2017 13:28:37 +0100

> Thanks for the patch! Looks good to me.
>
> In any case, if we search for "portable_system", we find that there 
> are lots of other things that don't use in-store names:
>
> ./cite.cpp:  if 
> ((exitCode=portable_system("perl","\""+bib2xhtmlFile+"\" 
> "+bibOutputFiles+" \""+
> ./dia.cpp:  if 
> ((exitCode=portable_system(diaExe,diaArgs,FALSE))!=0) // from config
> [...]

AFAIU those could work, because "portable_system()" runs “sh -c” with
the specified command.  So if the executables can be found on the
current PATH, I suppose it will work.

> Aaaah found sprintf without max length specifier.
>
> Dear god is doxygen unsafe. I should refrain from reading the 
> source code of some popular packages - it doesn't end well.

:-) Perhaps this is not so critical, because users likely don't run 
doxygen on arbitrary unverified input data?

Thomas

Re: [PATCH] gnu: doxygen: Use sh from the store.

[Thomas Danckaert]

From: Danny Milosavljevic <dannym@scratchpost.org>
Subject: Re: [PATCH] gnu: doxygen: Use sh from the store.
Date: Tue, 3 Jan 2017 13:28:37 +0100

> Thanks for the patch! Looks good to me.

I think this patch wasn't applied yet (or I missed it?).  Is it OK 
for staging or core-updates? (There are a lot of dependent packages.)

Thomas

Re: [PATCH] gnu: doxygen: Use sh from the store.

[Leo Famulari]

On Mon, Jan 09, 2017 at 08:37:23PM +0100, Thomas Danckaert wrote:
> From: Danny Milosavljevic <dannym@scratchpost.org>
> Subject: Re: [PATCH] gnu: doxygen: Use sh from the store.
> Date: Tue, 3 Jan 2017 13:28:37 +0100
> 
> > Thanks for the patch! Looks good to me.
> 
> I think this patch wasn't applied yet (or I missed it?).  Is it OK for
> staging or core-updates? (There are a lot of dependent packages.)

I've put it on a local staging-next branch for the next staging cycle
(currently staging is frozen for building).

These are the current guidelines for which branch to use:

http://lists.gnu.org/archive/html/guix-devel/2016-10/msg00933.html