libtool-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH, take 3][cygwin|mingw] Control where win32 DLLs get installed


From: Dave Korn
Subject: Re: [PATCH, take 3][cygwin|mingw] Control where win32 DLLs get installed.
Date: Fri, 14 Aug 2009 14:11:37 +0100
User-agent: Thunderbird 2.0.0.17 (Windows/20080914)

Eric Blake wrote:

> Cancelling out .. is wrong for 'symlink/..', when symlink does not
> necessarily point to a working directory exactly one level down.  (True,
> cygwin does not implement POSIX semantics here, and cancels out foo/..
> behind your back rather than properly resolving foo, but this function
> will need to work on all other systems that do it correctly).

  This is abspath, not realpath; it doesn't follow symlinks.  It has to work
before install time, there's no guarantee that any of the paths even exist.

>>   func_normal_abspath_tpath="$1"
> 
> Double-quoting not necessary here - variable assignment is already in a
> quoted context, and quotes are only needed to protect metacharacters.

>>   case "$func_normal_abspath_tpath" in
> 
> Likewise - the case argument is already in a double-quoted context.

  Ok, but is it harmful?  I just prefer to quote always for simplicity and
AFAIK it can't hurt (in an ordinary direct execution context like this, it
might be different if I was assigning these commands as strings to variables
and trying to exec them later.)

>>     "")
>>       # Empty path, that just means $cwd.
>>       func_stripname '' '/' "`pwd`"
> 
> Can we rely on $PWD instead of forking a process?

  I don't know if it's supported by all the shells we have to support.  I'm
not too concerned with optimising the hell out of this, I want robustness
first, and it's not something that I think should need to be used more than a
couple of times during any given libtool invocation so I hope it's not a
significant overhead.

>>       func_normal_abspath_result="$func_stripname_result"
>>       return
>>     ;;
>>     /*)
>>       # Absolute path, do nothing.
> 
> Do we want to cater to DOS style absolute paths?  [a-z]:\\

  I don't, personally.  Does anything else in libtool handle them?  Does
libtool run on djgpp?  Eurgh, that's going to make life horribly complicated.
 I'm tempted to just bang a wrapper around the current function that spots a
dos path, strips off the driveletter-colon, converts to forward slashes, lets
the current code handle it as if it were a posix path, then converts the
slashes back and stitches the driveletter back on.

>>     ;;
>>   esac
>>   # Cancel out all the simple stuff to save iterations.
>>   func_normal_abspath_tpath="`$ECHO "$func_normal_abspath_tpath" | $SED \
>>         -e "$removedotparts" -e "$collapseslashes"`"
> 
> I thought we were trying to move away from $ECHO and instead use the shell
> functions.

  I don't know, I copied func_dirname_and_basename.  I thought there's a
problem about backslashes being interpreted by some shell echos even in the
absence of -e?

>  And you have a portability no-no, documented in the autoconf
> manual:
> 
> a="`""`"
> 
> is non-portable, but can always be replaced by the portable:
> 
> a=`""`
> 
> See why I don't like spurious "" around variable assignments?

  Ah, now I do.  Ok, but is some nice clear list of where you should and
shouldn't quote anywhere?  As far as I can tell you have to memorize the
entire bash manual and then infer what to do from all the overlapping sets of
expansions and contexts and rules.  Then you have to to the same for all the
other shells we're supposed to support.  Then you have to infer a set of
lowest-common-denominator rules from all of that.

>>   # We want the path to end with a slash for ease of parsing, and
>>   # doubled-up slashes won't hurt us here, so just add one on.
>>   func_normal_abspath_tpath="$func_normal_abspath_tpath/"
>>   while :; do
>>     func_normal_abspath_tcomponent="`$ECHO "$func_normal_abspath_tpath" | 
>> $SED \
>>         -e "$pathcar"`"
>>     func_normal_abspath_tpath="`$ECHO "$func_normal_abspath_tpath" | $SED \
>>         -e "$pathcdr"`"
> 
> Nit - I'd break these long lines at |, not after $SED, so that the entire
> sed command is on a single line.

  I took my cue from func_quote_for_expand, which made me think it would be at
least acceptable to do it this way, but if you're looking for a convention,
the libtool convention appears by overwhelming consensus to be not to wrap
ECHO ... | SED at all, even if the line is really long.

> $ grep -R 'ECHO.*SED' libltdl/ | wc -l
> 591
> 
> $ grep -R '| \$SED \\' libltdl/ | wc -l
> 24
> 
> $

    cheers,
      DaveK






reply via email to

[Prev in Thread] Current Thread [Next in Thread]