From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paul Garlick Subject: Re: [PATCH] gnu: Add openfoam Date: Fri, 28 Jul 2017 17:01:12 +0100 Message-ID: <1501257672.10251.51.camel@tourbillion-technology.com> References: <20170723201138.18131-1-pgarlick@tourbillion-technology.com> <871sp4nb2d.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="=-SyfVYLbKPMFdOzCS/582" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:58858) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1db7hc-00034z-S8 for guix-devel@gnu.org; Fri, 28 Jul 2017 12:01:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1db7hb-00058G-Q9 for guix-devel@gnu.org; Fri, 28 Jul 2017 12:01:24 -0400 In-Reply-To: <871sp4nb2d.fsf@gnu.org> List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: Ludovic =?ISO-8859-1?Q?Court=E8s?= Cc: guix-devel@gnu.org --=-SyfVYLbKPMFdOzCS/582 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit Hello Ludo, > I think it might make sense to create a new “simulation” module, and > eventually move OpenCascade there as well, WDYT? > Sure.  I will create a new module and put the OpenFOAM package definition in there as the first one. > 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? > I did consider both options, removing and modifying stages in the gnu- build-system or starting from scratch with the trivial-build-system.  I can give the gnu-build-system option a go, reverting to the trivial- build-system if the new attempt proves too troublesome.   One aspect I will need to investigate in the gnu-build-system is using the "./Allwmake" command to trigger the build process.  In OpenFOAM, wmake is short for "wrapped-make" and the package has its own configuration step.  In brief, it has its own sequence and does not follow the standard "./configure && make && make install" steps. Using the trivial-build-system the patch-shebang section is indeed a little extended.  I found this was necessary to avoid a failure of the patch-shebang command using a simple 'find-files "."' from top-level directory.  There is a scenario where sub-directories named '0', in the 'tutorials' directory, identified by the find-files command, being passed to 'patch-shebang', which should only receive files, not directories.  This causes 'patch-shebang' to stop and the build fails. > > > > + ) > > + ) > > + ) > > + ) > > > > > Please listen to what ‘guix lint’ has to say about these. :-) > Interestingly, 'guix lint' let me get away with this without making comment.  However, I shall condense things accordingly in the new patch. > What about wrapping the resulting binaries so that they have > ‘FOAM_INST_DIR’ set to %output? > > In fact, FOAM_INST_DIR is used in multiple places to navigate around the installed files (not just the binaries), so I think this variable may need to remain available as is. > > You could make it “if true”, thereby avoiding the need to define > $READLINE_ROOT. > > Could you elaborate on this idea a little?  At the moment the test is "if file exists". > Could you send an updated patch to guix-patches@gnu.org, where it will > , where it will > be visible in the patch tracker? > > > Aha, a new system! Best, Paul. --=-SyfVYLbKPMFdOzCS/582 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: quoted-printable
Hello Ludo,


I think it might make sense to create a new =E2=80=9Csimulation=E2=80=9D mo=
dule, and
eventually move OpenCascade there as well, WDYT?

Sure.  I will create a new modu= le and put the OpenFOAM package definition in there as the first one.
= 

Some comments:



+    (build-system trivial-build-system)



Did you consider using =E2=80=98gnu-build-system=E2=80=99?  I think this wo=
uld 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=E2=80=99t ne=
ed to
list all the usual build inputs (GCC, Coreutils, Make, etc.).  WDYT?

I did consider both options, removin= g and modifying stages in the gnu-build-system or starting from scratch wit= h the trivial-build-system.  I can give the gnu-build-system option a = go, reverting to the trivial-build-system if the new attempt proves too tro= ublesome.  

One aspect I will need to investi= gate in the gnu-build-system is using the "./Allwmake" command to trigger t= he build process.  In OpenFOAM, wmake is short for "wrapped-make" and = the package has its own configuration step.  In brief, it has its own = sequence and does not follow the standard "./configure && make &= ;& make install" steps.

Using the trivial-buil= d-system the patch-shebang section is indeed a little extended.  I fou= nd this was necessary to avoid a failure of the patch-shebang command using= a simple 'find-files "."' from top-level directory.  There is a scena= rio where sub-directories named '0', in the 'tutorials' directory, identifi= ed by the find-files command, being passed to 'patch-shebang', which should= only receive files, not directories.  This causes 'patch-shebang' to = stop and the build fails.



+		   )
+		 )
+       )
+     )



Please listen to what =E2=80=98guix lint=E2=80=99 has to say about these.  =
:-)

Interestingly, 'guix lint' let me ge= t away with this without making comment.  However, I shall condense th= ings accordingly in the new patch.


What about wrapping the resulting binaries so tha=
t they have
=E2=80=98FOAM_INST_DIR=E2=80=99 set to %output?

In fact, FOAM_INST_DIR is used in mu= ltiple places to navigate around the installed files (not just the binaries= ), so I think this variable may need to remain available as is.
<= br>


You could make it =E2=80=9Cif true=E2=80=9D, thereby avoiding the need to d=
efine
$READLINE_ROOT.

Could you elaborate on this idea a l= ittle?  At the moment the test is "if file exists".


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

Aha, a new system!

Best,

Paul.
--=-SyfVYLbKPMFdOzCS/582--