From mboxrd@z Thu Jan 1 00:00:00 1970 From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=) Subject: Re: [PATCH] gnu: Add openfoam Date: Tue, 01 Aug 2017 14:18:07 +0200 Message-ID: <87efsvpjls.fsf@gnu.org> References: <20170723201138.18131-1-pgarlick@tourbillion-technology.com> <871sp4nb2d.fsf@gnu.org> <1501257672.10251.51.camel@tourbillion-technology.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:58750) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dcW7q-0003X6-SK for guix-devel@gnu.org; Tue, 01 Aug 2017 08:18:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dcW7l-000164-SA for guix-devel@gnu.org; Tue, 01 Aug 2017 08:18:14 -0400 In-Reply-To: <1501257672.10251.51.camel@tourbillion-technology.com> (Paul Garlick's message of "Fri, 28 Jul 2017 17:01:12 +0100") 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: Paul Garlick Cc: guix-devel@gnu.org Hi Paul, Paul Garlick skribis: >> I think it might make sense to create a new =E2=80=9Csimulation=E2=80=9D= module, and >> eventually move OpenCascade there as well, WDYT? >>=20 > Sure. =C2=A0I will create a new module and put the OpenFOAM package > definition in there as the first one. Sounds good. >> Some comments: >>=20 >>=20 >> >=20 >> > + (build-system trivial-build-system) >> >=20 > >>=20 >>=20 >> Did you consider using =E2=80=98gnu-build-system=E2=80=99? 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=E2=80=99t= need to >> list all the usual build inputs (GCC, Coreutils, Make, etc.). WDYT? >>=20 > > I did consider both options, removing and modifying stages in the gnu- > build-system or starting from scratch with the trivial-build-system. =C2= =A0I > can give the gnu-build-system option a go, reverting to the trivial- > build-system if the new attempt proves too troublesome. =C2=A0 With =E2=80=98gnu-build-system=E2=80=99 you=E2=80=99ll have to replace/remo= ve some of the phases, but hopefully it=E2=80=99ll be less troublesome nonetheless. > One aspect I will need to investigate in the gnu-build-system is using > the "./Allwmake" command to trigger the build process. =C2=A0In OpenFOAM, > wmake is short for "wrapped-make" and the package has its own > configuration step. =C2=A0In 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. =C2=A0I found this was necessary to avoid a failure of t= he > patch-shebang command using a simple 'find-files "."' from top-level > directory. =C2=A0There 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. =C2=A0This causes 'patch-shebang' to stop and the build fail= s. =E2=80=98find-files=E2=80=99 excludes directories by default, so it=E2=80= =99s a little surprising. Anyway, we can surely find a way to work around the problem. >> >=20 >> > + ) >> > + ) >> > + ) >> > + ) >> >=20 > >>=20 >>=20 >> Please listen to what =E2=80=98guix lint=E2=80=99 has to say about these= . :-) >>=20 > > Interestingly, 'guix lint' let me get away with this without making > comment. =C2=A0However, I shall condense things accordingly in the new > patch. Oh indeed, until now it would only scan 60 lines from the start of the definition. Fixed! >> What about wrapping the resulting binaries so that they have >> =E2=80=98FOAM_INST_DIR=E2=80=99 set to %output? >>=20 >>=20 > > 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. OK, though it would be good to make it work out-of-the-box when FOAM_INST_DIR is undefined. >> You could make it =E2=80=9Cif true=E2=80=9D, thereby avoiding the need t= o define >> $READLINE_ROOT. >>=20 >>=20 > > Could you elaborate on this idea a little? =C2=A0At the moment the test is > "if file exists". I was thinking about doing something like this: -if [ -f /usr/include/readline/readline.h ] +if true and thus we wouldn=E2=80=99t need to define READLINE_ROOT, IIUC. HTH, Ludo=E2=80=99.