On Tue, Feb 04, 2014 at 07:55:51PM +0000, Dave Goodell (dgoodell) wrote:
> On Feb 4, 2014, at 1:44 PM, svn-commit-mai...@open-mpi.org wrote:
> 
> > Author: hjelmn (Nathan Hjelm)
> > Date: 2014-02-04 14:44:08 EST (Tue, 04 Feb 2014)
> > New Revision: 30555
> > URL: https://svn.open-mpi.org/trac/ompi/changeset/30555
> > 
> > Log:
> > Fix wrapper ldflags.
> > 
> > cmr=v1.7.4:reviewer=jsquyres
> > 
> > Text files modified: 
> >   trunk/config/opal_setup_wrappers.m4              |     9 ++++-----        
> >                        
> >   trunk/configure.ac                               |     2 ++               
> >                        
> >   trunk/contrib/platform/lanl/cray_xe6/cray-common |     4 ----             
> >                        
> >   3 files changed, 6 insertions(+), 9 deletions(-)
> > 
> > Modified: trunk/config/opal_setup_wrappers.m4
> > ==============================================================================
> > --- trunk/config/opal_setup_wrappers.m4     Tue Feb  4 09:47:04 2014        
> > (r30554)
> > +++ trunk/config/opal_setup_wrappers.m4     2014-02-04 14:44:08 EST (Tue, 
> > 04 Feb 2014)      (r30555)
> > @@ -150,7 +150,7 @@
> > # (because if script A sources script B, and B calls "exit", then both
> > # B and A will exit).  Instead, we have to send the output to a file
> > # and then source that.
> > -$OMPI_TOP_BUILDDIR/opal/libltdl/libtool --config > $rpath_outfile
> > +$OMPI_TOP_BUILDDIR/libtool --config > $rpath_outfile
> > 
> > chmod +x $rpath_outfile
> > . ./$rpath_outfile
> > @@ -214,9 +214,8 @@
> > # runtime), and the RUNPATH args, if we have them.
> > AC_DEFUN([RPATHIFY_LDFLAGS],[
> >     OPAL_VAR_SCOPE_PUSH([rpath_out rpath_dir rpath_tmp])
> > -    AS_IF([test "$enable_wrapper_rpath" = "no" -o "$WRAPPER_RPATH_SUPPORT" 
> > = "disabled"],
> > -          [:],
> > -          [rpath_out=
> > +    AS_IF([test "$enable_wrapper_rpath" = "yes" -a ! 
> > "$WRAPPER_RPATH_SUPPORT" = "disabled" -a ! "WRAPPER_RPATH_SUPPORT" = 
> > "unnecessary"], [
> 
> This "test" looks dangerous to me.  Both non-portable [1] and slightly 
> challenging to read at first glance.  It would be better as:
> 
> ----8<----
> test "$enable_wrapper_rpath" = "yes" &&
> test "$WRAPPER_RPATH_SUPPORT" != "disabled" &&
> test "$WRAPPER_RPATH_SUPPORT" != "unnecessary"
> ----8<----
> 
> In fact, if you look carefully at the third test, there is a missing "$" 
> before "WRAPPER_RPATH_SUPPORT" in the SVN version...

Good catch on the typo. Neither I nor the reviewers noticed that. As for
the test semantics, I wasn't aware that -a, -o, and () were
deprecated. I will update the test.

-Nathan

Attachment: pgpxbcaO6bXML.pgp
Description: PGP signature

Reply via email to