GNU bug report logs - #28045
[PATCH] gnu: Add openfoam

Previous Next

Package: guix-patches;

Reported by: Paul Garlick <pgarlick <at> tourbillion-technology.com>

Date: Fri, 11 Aug 2017 11:08:01 UTC

Severity: normal

Tags: moreinfo, patch

Done: ludo <at> gnu.org (Ludovic Courtès)

Bug is archived. No further changes may be made.

Full log


Message #55 received at 28045 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Paul Garlick <pgarlick <at> tourbillion-technology.com>
Cc: 28045 <at> debbugs.gnu.org
Subject: Re: [PATCH] gnu: Add openfoam
Date: Fri, 08 Sep 2017 17:39:08 +0200
Hi Paul,

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

>> Does it address the use case you have in mind?
>
> Yes, I think that both the multiple-profile solution and the 'ad-hoc'
> environment will work for Guix/OpenFOAM.  

Good!

> So, continuing the 'middle road' line of thought, the 'install-dir'
> variable would be modified to add a '/lib' element:
>
> -                                %output "/OpenFOAM-" ,version)))
> +                                %output "/OpenFOAM-" ,version
> "/lib")))

Sounds good.

> You suggest adding a link between bin and lib/OpenFOAM-
> 4.1/platforms/linux64GccDPInt32Opt/bin.  What would be the best way to
> add this to the package definition?  

Perhaps adding an extra phase at the end that simply calls ‘symlink’?

> There could also be a link between lib and lib/OpenFOAM-
> 4.1/platforms/linux64GccDPInt32Opt/lib.

Yes.

> The links would allow the runpaths to be validated.  So; 
>
> -       #:validate-runpath? #f ; '#:elf-directories' is not recognised
> here

That’d be great.  If that phase errors out, it probably means that the
binaries won’t work out of the box, so it’s good to fix it.

(BTW, please note that executables should go to bin/, libraries and
other architecture-dependent files to lib/, and share/ is for
architecture-independent stuff.  I suppose we’ll only have bin/ and lib/
for a start, that’s OK.)

> The FOAM_INST_DIR variable would need to be updated:
>
> -            (files '(".")))))
>  +          (files '("./lib")))))

I really dislike this FOAM_INST_DIR variable (usually packages “know”
where they are installed and don’t need an extra variable for that), but
if it has to be there, then so be it.  :-)

I think we should be all set?  I’ll wait for your hopefully last patch
revision!

Besides, for the future, if you have an opportunity to discuss these
matters with upstream, I’d recommend suggesting the addition of a proper
installation phase (“make install”), and also support at least for an
installation prefix, and ideally for more directory categories (see
<https://www.gnu.org/prep/standards/html_node/Directory-Variables.html>).

Thanks for your patience!

Ludo’.




This bug report was last modified 7 years and 253 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.