Re: [PATCH] gnu: Add openfoam

[ludo@gnu.org (Ludovic Courtès)]

Hello Paul,

Thanks for this patch!

Paul Garlick <pgarlick@tourbillion-technology.com> skribis:

> * gnu/packages/maths.scm(openfoam): Add it
> * gnu/packages/patches/openfoam-cleanup.patch: New file
> * gnu/local.mk(dist_patch_DATA): Add patch

I think it might make sense to create a new “simulation” module, and
eventually move OpenCascade there as well, WDYT?

Some comments:

> +    (build-system trivial-build-system)

Did you consider using ‘gnu-build-system’?  I think this would save
quite a few lines corresponding to the initial setup (set-paths, unpack,
patch-shebang, etc.), and also ensure that the final phases (strip,
compress-documentation, etc.) are run.  In addition you wouldn’t need to
list all the usual build inputs (GCC, Coreutils, Make, etc.).  WDYT?

> +		   )
> +		 )
> +       )
> +     )

Please listen to what ‘guix lint’ has to say about these.  :-)

> +    (native-search-paths
> +     ;; define the FOAM_INST_DIR variable
> +     (list (search-path-specification
> +            (variable "FOAM_INST_DIR")
> +            (files '(".")))))

My guess is that ‘FOAM_INST_DIR’ is not a search path (in the sense of a
colon-separated list of directories like ‘PATH’.)

What about wrapping the resulting binaries so that they have
‘FOAM_INST_DIR’ set to %output?

> diff --git a/gnu/packages/patches/openfoam-cleanup.patch b/gnu/packages/patches/openfoam-cleanup.patch
> new file mode 100644
> index 000000000..e7760b7a5
> --- /dev/null
> +++ b/gnu/packages/patches/openfoam-cleanup.patch
> @@ -0,0 +1,267 @@
> +# This patch removes all need for the ThirdParty files of OpenFOAM

Nice!

> +# Derived from easybuild patch:  Ward Poelmans <wpoely86@gmail.com>
> +# Modified for GNU Guix:  Paul Garlick <pgarlick@tourbillion-technology.com>
> +
> +diff -ur OpenFOAM-4.x-version-4.1.org/applications/utilities/mesh/manipulation/setSet/Allwmake OpenFOAM-4.x-version-4.1/applications/utilities/mesh/manipulation/setSet/Allwmake
> +--- OpenFOAM-4.x-version-4.1.org/applications/utilities/mesh/manipulation/setSet/Allwmake
> ++++ OpenFOAM-4.x-version-4.1/applications/utilities/mesh/manipulation/setSet/Allwmake
> +@@ -9,7 +9,7 @@
> + #
> + # use readline if available
> + #
> +-if [ -f /usr/include/readline/readline.h ]
> ++if [ -f $READLINE_ROOT/include/readline/readline.h ]

You could make it “if true”, thereby avoiding the need to define
$READLINE_ROOT.

> +--- OpenFOAM-4.x-version-4.1.org/applications/utilities/mesh/manipulation/setSet/Make/options
> ++++ OpenFOAM-4.x-version-4.1/applications/utilities/mesh/manipulation/setSet/Make/options
> +@@ -1,7 +1,9 @@
> + EXE_INC = \
> +     -I$(LIB_SRC)/meshTools/lnInclude \
> ++    -I$(READLINE_ROOT)/include \

Likewise, ‘gnu-build-system’ will set C_INCLUDE_PATH appropriately so
this -I flag won’t be needed.

Could you send an updated patch to guix-patches@gnu.org, where it will
be visible in the patch tracker?

Thank you!

Ludo’.