On Tue, May 5, 2015 at 7:25 PM, Bruno Coutinho Mundim <[email protected]>
wrote:

> Hi Erik:
>
> On 05/05/2015 07:37 PM, Erik Schnetter wrote:
> > On Tue, May 5, 2015 at 12:32 PM, <[email protected]> wrote:
> >
> >> ]Directory: /trunk/src/
> >> ======================
> >>
> >> File [modified]: detect.sh
> >> Delta lines: +11 -2
> >> ===================================================================
> >> --- trunk/src/detect.sh 2015-02-23 17:24:40 UTC (rev 102)
> >> +++ trunk/src/detect.sh 2015-05-05 16:32:59 UTC (rev 103)
> >> @@ -158,10 +158,19 @@
> >>  # Check for additional libraries
> >>
> >>
> ################################################################################
> >>
> >> +# Set library directory name for machine architecture:
> >> +gcc -dumpversion > /dev/null 2>&1
> >> +
> >> +if [ $? -eq 0 ]; then
> >> +  MACHINE=${MACHINE:=`gcc -dumpmachine`}
> >> +else
> >> +  MACHINE=${MACHINE:=""}
> >> +fi
> >> +
> >>  # Set options
> >>  # Fortran modules may be located in the lib directory
> >> -HDF5_INC_DIRS="${HDF5_DIR}/include ${HDF5_DIR}/lib"
> >> -HDF5_LIB_DIRS="${HDF5_DIR}/lib"
> >> +HDF5_INC_DIRS="${HDF5_DIR}/include ${HDF5_DIR}/lib/${MACHINE}
> >> ${HDF5_DIR}/lib"
> >> +HDF5_LIB_DIRS="${HDF5_DIR}/lib/${MACHINE} ${HDF5_DIR}/lib"
> >>  HDF5_LIBS="${HDF5_CXX_LIBS} ${HDF5_FORTRAN_LIBS} ${HDF5_C_LIBS}"
> >>
> >
> > This patch won't work.
> >
>
> It works for me on ubuntu. I guess it won't work if we don't have gcc
> installed because it would abort on errors as you pointed out below.
> My apologies for missing the 'set -e' statement.
>
> > 1. The shell script detect.sh is set up such that any failing command
> will
> > immediately abort the script. Thus the check $? won't work. Instead, you
> > can run "gcc -dumpversion" directly in the if command.
> > 2. "gcc -dumpversion" is called twice; it suffices to call it once.
>
> I called gcc twice, each with a different flag. In any case your
> solution is superior.
>
> > 3. There is a convention to have externally visible variable upper case,
> > and internal variables lower case.
> > 4. The way in which MACHINE is set is overly complicated. Saying
> > "MACHINE=..." is sufficient, unless MACHINE can have a previous value.
>
> My rationale was for a fictitious user that doesn't have gcc installed
> but does have the new architecture directory layout present in her
> system to set the MACHINE as a an environment variable; or another
> user which simply wants to set it by hand could do so.
>

Relying on environment variables is problematic. In Cactus, we prefer to
use configuration variables that can explicitly be set in an option list.
Their default is still taken from the environment. We can make MACHINE such
a variable, but since that's currently not the case, I thought it would be
best to ignore the environment setting. The problem with environment
settings is that it often confuses people since they don't know what is in
the environment, or that Cactus is looking at it.

-erik

> 5. If MACHINE is empty, then HDF5_LIB_DIRS still contains the string
> > "${HDF5_DIR}/lib/". This should be avoided.
>
> ok, thanks! I thought that wouldn't be a big deal.
>
> > 6. A minor issue: They syntax $(...) is preferred over `...`.
>
> Agreed, it is more modern. Old habits are hard to kill...
>
> >
> > Here is an updated version:
> >
> > # Set library directory name for machine architecture
> > if machine=$(gcc -dumpversion 2>/dev/null); then
> >     machinedir="${HDF5_DIR}/lib/${machine}"
> > else
> >     machinedir=''
> > fi
> >
> > # Set options
> > # Fortran modules may be located in the lib directory
> > HDF5_INC_DIRS="${HDF5_DIR}/include ${machinedir} ${HDF5_DIR}/lib"
> > HDF5_LIB_DIRS="${machinedir} ${HDF5_DIR}/lib"
> > HDF5_LIBS="${HDF5_CXX_LIBS} ${HDF5_FORTRAN_LIBS} ${HDF5_C_LIBS}"
> >
> > I've updated thorn HDF5; can you update thorn Boost?
>
> Thanks for your updated version! I will update Boost shortly.
>
> Cheers,
> Bruno.
>
>
> >
> > -erik
> >
> >
> >
> > _______________________________________________
> > Users mailing list
> > [email protected]
> > http://cactuscode.org/mailman/listinfo/users
> >
>
> _______________________________________________
> Users mailing list
> [email protected]
> http://cactuscode.org/mailman/listinfo/users
>



-- 
Erik Schnetter <[email protected]>
http://www.perimeterinstitute.ca/personal/eschnetter/
_______________________________________________
Users mailing list
[email protected]
http://cactuscode.org/mailman/listinfo/users

Reply via email to