Subject: Re: [bug#29392] [PATCH 02/11] build-system: Add scons-build-system.

From: Leo Famulari <[email-masked]>
Date: Sun, 26 Nov 2017 16:04:12 -0500

On Wed, Nov 22, 2017 at 03:04:58PM +0530, Arun Isaac wrote:
> * guix/build-system/scons.scm: New file.
> * guix/build/scons-build-system.scm: New file.
> * Makefile.am (MODULES): Register them.
> * doc/guix.texi (Build Systems): Add scons-build-system.

> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -42,6 +42,7 @@ Copyright @copyright{} 2017 Hartmut Goebel@*
>  Copyright @copyright{} 2017 Maxim Cournoyer@*
>  Copyright @copyright{} 2017 Tobias Geerinckx-Rice@*
>  Copyright @copyright{} 2017 George Clemmer
> +Copyright @copyright{} 2017 Arun Isaac

Make sure to add a line break (@*) after George's name.

https://www.gnu.org/software/texinfo/manual/texinfo/html_node/Line-Breaks.html

> +@defvr {Scheme Variable} scons-build-system
> +This variable is exported by @code{(guix build-system scons)}.  It
> +implements the build procedure used by the @code{scons} software

Here we are not referring to SCons itself, but not a code variable. So,
I think we can skip using @code here.

> +construction tool.  This build system runs @code{scons} to build the
> +package, @code{scons test} to run tests, and then @code{scons install}
> +to install the package.  Additional flags to be passed to @code{scons}
                                                             ^
Same here ---------------------------------------------------|

> diff --git a/guix/build-system/scons.scm b/guix/build-system/scons.scm

[...]

> +;; Standard build procedure for applications using 'scons'. This is

I would say SCons instead of 'scons', and anywhere we refer to SCons as
a project rather than a particular program. This is mostly in the
manual, comments, docstrings, etc.

> +         (build-inputs `(("scons-python2" ,(module-ref (resolve-interface '(gnu packages python)) 'scons-python2))
> +                         ,@native-inputs))

Is there a particular reason we use a Python 2 variant of SCons? If so,
can you explain it in a code comment?

> +(define* (scons-build store name inputs
> +                      #:key
> +                      (tests? #t)
> +                      (scons-flags ''())
> +                      (test-target "test")
> +                      (phases '(@ (guix build scons-build-system)
> +                                  %standard-phases))
> +                      (outputs '("out"))
> +                      (search-paths '())
> +                      (system (%current-system))
> +                      (guile #f)
> +                      (imported-modules %scons-build-system-modules)
> +                      (modules '((guix build scons-build-system)
> +                                 (guix build utils))))
> +  "Build SOURCE using SCONS, and with INPUTS.  This assumes that SOURCE

There isn't a variable named 'scons' here, so I think it would be
SCONS-PYTHON2.