diff mbox

[contrib] download_prerequisites: check for existing symlinks before making new ones

Message ID CAMfHzOtL-nbQe5OUxyFyS2RfLia_PDyY4ZOokV81iGn8Km5iHA@mail.gmail.com
State New
Headers show

Commit Message

Eric Gallager June 28, 2016, 2:10 a.m. UTC
The last time I ran ./contrib/download_prerequisites, I already had
previous symlinks set up from a previous run of the script, so `ln`
followed the existing symlinks and created the new ones in the
directories to which the symlinks pointed. This patch should fix that
by removing the old symlinks before creating new ones. (For some
reason the `-f` flag to `ln` that was already there wasn't enough for
me.) Tested by running the script and ensuring that the new isl
symlink pointed to the correct directory, and that there were no bad
symlinks in the old isl directory. Could someone commit this trivial
patch for me, or something like it? I don't have write access.

Thanks,
Eric Gallager
contrib/download_prerequisites | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Jeff Law July 13, 2016, 9:36 p.m. UTC | #1
On 06/27/2016 08:10 PM, Eric Gallager wrote:
> The last time I ran ./contrib/download_prerequisites, I already had
> previous symlinks set up from a previous run of the script, so `ln`
> followed the existing symlinks and created the new ones in the
> directories to which the symlinks pointed. This patch should fix that
> by removing the old symlinks before creating new ones. (For some
> reason the `-f` flag to `ln` that was already there wasn't enough for
> me.) Tested by running the script and ensuring that the new isl
> symlink pointed to the correct directory, and that there were no bad
> symlinks in the old isl directory. Could someone commit this trivial
> patch for me, or something like it? I don't have write access.
I'd really rather know why the "-f" flag didn't work for you.  The whole 
point of -f is to remove the destination file first.

Jeff
Eric Gallager July 14, 2016, 10:57 a.m. UTC | #2
On 7/13/16, Jeff Law <law@redhat.com> wrote:
> On 06/27/2016 08:10 PM, Eric Gallager wrote:
>> The last time I ran ./contrib/download_prerequisites, I already had
>> previous symlinks set up from a previous run of the script, so `ln`
>> followed the existing symlinks and created the new ones in the
>> directories to which the symlinks pointed. This patch should fix that
>> by removing the old symlinks before creating new ones. (For some
>> reason the `-f` flag to `ln` that was already there wasn't enough for
>> me.) Tested by running the script and ensuring that the new isl
>> symlink pointed to the correct directory, and that there were no bad
>> symlinks in the old isl directory. Could someone commit this trivial
>> patch for me, or something like it? I don't have write access.
> I'd really rather know why the "-f" flag didn't work for you.  The whole
> point of -f is to remove the destination file first.
>
> Jeff
>

Reading my ln manpage, it describes the "-f" flag like this:


     -f    If the target file already exists, then unlink it so that the
           link may occur.  (The -f option overrides any previous -i
           options.)

Okay, so that seems like it should do what you say, but the manpage
also describes a separate uppercase "-F" option:

     -F    If the target file already exists and is a directory, then
           remove it so that the link may occur.  The -F option should be
           used with either -f or -i options.  If none is specified, -f is
           implied.  The -F option is a no-op unless -s option is speci-
           fied.

So it seems to imply that "-f" will only remove the destination file
if it's a regular file, while "-F" is needed if the destination file
is a directory. The page also has this to say about "-F" later:

     The -F option is FreeBSD extention and should not be used in portable
     scripts.

So this could be a BSD vs. GNU thing.
NightStrike July 14, 2016, 1:52 p.m. UTC | #3
On Wed, Jul 13, 2016 at 5:36 PM, Jeff Law <law@redhat.com> wrote:
> On 06/27/2016 08:10 PM, Eric Gallager wrote:
>>
>> The last time I ran ./contrib/download_prerequisites, I already had
>> previous symlinks set up from a previous run of the script, so `ln`
>> followed the existing symlinks and created the new ones in the
>> directories to which the symlinks pointed. This patch should fix that
>> by removing the old symlinks before creating new ones. (For some
>> reason the `-f` flag to `ln` that was already there wasn't enough for
>> me.) Tested by running the script and ensuring that the new isl
>> symlink pointed to the correct directory, and that there were no bad
>> symlinks in the old isl directory. Could someone commit this trivial
>> patch for me, or something like it? I don't have write access.
>
> I'd really rather know why the "-f" flag didn't work for you.  The whole
> point of -f is to remove the destination file first.
>
> Jeff
>

FWIW, it doesn't work for me, either:

$ mkdir d
$ ln -s d a
$ ls -la a
lrwxrwxrwx 1 owner owner 1 Jul 14 09:48 a -> d
$ mkdir e
$ ln -sf e a
$ ls -la a
lrwxrwxrwx 1 owner owner 1 Jul 14 09:48 a -> d

But adding the -n option does:

$ ln -sfn e a
$ ls -la a
lrwxrwxrwx 1 owner owner 1 Jul 14 09:51 a -> e


So perhaps add -n?
NightStrike July 14, 2016, 1:53 p.m. UTC | #4
On Thu, Jul 14, 2016 at 6:57 AM, Eric Gallager <egall@gwmail.gwu.edu> wrote:
> On 7/13/16, Jeff Law <law@redhat.com> wrote:
>> On 06/27/2016 08:10 PM, Eric Gallager wrote:
>>> The last time I ran ./contrib/download_prerequisites, I already had
>>> previous symlinks set up from a previous run of the script, so `ln`
>>> followed the existing symlinks and created the new ones in the
>>> directories to which the symlinks pointed. This patch should fix that
>>> by removing the old symlinks before creating new ones. (For some
>>> reason the `-f` flag to `ln` that was already there wasn't enough for
>>> me.) Tested by running the script and ensuring that the new isl
>>> symlink pointed to the correct directory, and that there were no bad
>>> symlinks in the old isl directory. Could someone commit this trivial
>>> patch for me, or something like it? I don't have write access.
>> I'd really rather know why the "-f" flag didn't work for you.  The whole
>> point of -f is to remove the destination file first.
>>
>> Jeff
>>
>
> Reading my ln manpage, it describes the "-f" flag like this:
>
>
>      -f    If the target file already exists, then unlink it so that the
>            link may occur.  (The -f option overrides any previous -i
>            options.)
>
> Okay, so that seems like it should do what you say, but the manpage
> also describes a separate uppercase "-F" option:
>
>      -F    If the target file already exists and is a directory, then
>            remove it so that the link may occur.  The -F option should be
>            used with either -f or -i options.  If none is specified, -f is
>            implied.  The -F option is a no-op unless -s option is speci-
>            fied.
>
> So it seems to imply that "-f" will only remove the destination file
> if it's a regular file, while "-F" is needed if the destination file
> is a directory. The page also has this to say about "-F" later:
>
>      The -F option is FreeBSD extention and should not be used in portable
>      scripts.
>
> So this could be a BSD vs. GNU thing.

On GNU, -F means:

       -d, -F, --directory
              allow the superuser to attempt to hard link directories
(note: will probably fail due to system restrictions, even for the
superuser)
Jeff Law July 14, 2016, 5:24 p.m. UTC | #5
On 07/14/2016 04:57 AM, Eric Gallager wrote:
> On 7/13/16, Jeff Law <law@redhat.com> wrote:
>> On 06/27/2016 08:10 PM, Eric Gallager wrote:
>>> The last time I ran ./contrib/download_prerequisites, I already had
>>> previous symlinks set up from a previous run of the script, so `ln`
>>> followed the existing symlinks and created the new ones in the
>>> directories to which the symlinks pointed. This patch should fix that
>>> by removing the old symlinks before creating new ones. (For some
>>> reason the `-f` flag to `ln` that was already there wasn't enough for
>>> me.) Tested by running the script and ensuring that the new isl
>>> symlink pointed to the correct directory, and that there were no bad
>>> symlinks in the old isl directory. Could someone commit this trivial
>>> patch for me, or something like it? I don't have write access.
>> I'd really rather know why the "-f" flag didn't work for you.  The whole
>> point of -f is to remove the destination file first.
>>
>> Jeff
>>
>
> Reading my ln manpage, it describes the "-f" flag like this:
>
>
>      -f    If the target file already exists, then unlink it so that the
>            link may occur.  (The -f option overrides any previous -i
>            options.)
>
> Okay, so that seems like it should do what you say, but the manpage
> also describes a separate uppercase "-F" option:
>
>      -F    If the target file already exists and is a directory, then
>            remove it so that the link may occur.  The -F option should be
>            used with either -f or -i options.  If none is specified, -f is
>            implied.  The -F option is a no-op unless -s option is speci-
>            fied.
>
> So it seems to imply that "-f" will only remove the destination file
> if it's a regular file, while "-F" is needed if the destination file
> is a directory. The page also has this to say about "-F" later:
>
>      The -F option is FreeBSD extention and should not be used in portable
>      scripts.
>
> So this could be a BSD vs. GNU thing.
I don't have any BSD systems running.  I can confirm that while "-f" 
refers to files in the man page, it will happy delete the old symlink as 
well.

-bash-4.3$ ln -s /bin/ls jj
-bash-4.3$ ln -s -f /bin/bash jj
-bash-4.3$ ls -l jj
lrwxrwxrwx. 1 law law 9 Jul 14 13:22 jj -> /bin/bash
-bash-4.3$ which ln
/usr/bin/ln
-bash-4.3$ rpm -q --whatprovides /usr/bin/ln
coreutils-8.24-6.fc23.x86_64

Could you test this on your system?

Jeff
Eric Gallager July 14, 2016, 7:57 p.m. UTC | #6
On 7/14/16, Jeff Law <law@redhat.com> wrote:
> On 07/14/2016 04:57 AM, Eric Gallager wrote:
>> On 7/13/16, Jeff Law <law@redhat.com> wrote:
>>> On 06/27/2016 08:10 PM, Eric Gallager wrote:
>>>> The last time I ran ./contrib/download_prerequisites, I already had
>>>> previous symlinks set up from a previous run of the script, so `ln`
>>>> followed the existing symlinks and created the new ones in the
>>>> directories to which the symlinks pointed. This patch should fix that
>>>> by removing the old symlinks before creating new ones. (For some
>>>> reason the `-f` flag to `ln` that was already there wasn't enough for
>>>> me.) Tested by running the script and ensuring that the new isl
>>>> symlink pointed to the correct directory, and that there were no bad
>>>> symlinks in the old isl directory. Could someone commit this trivial
>>>> patch for me, or something like it? I don't have write access.
>>> I'd really rather know why the "-f" flag didn't work for you.  The whole
>>> point of -f is to remove the destination file first.
>>>
>>> Jeff
>>>
>>
>> Reading my ln manpage, it describes the "-f" flag like this:
>>
>>
>>      -f    If the target file already exists, then unlink it so that the
>>            link may occur.  (The -f option overrides any previous -i
>>            options.)
>>
>> Okay, so that seems like it should do what you say, but the manpage
>> also describes a separate uppercase "-F" option:
>>
>>      -F    If the target file already exists and is a directory, then
>>            remove it so that the link may occur.  The -F option should be
>>            used with either -f or -i options.  If none is specified, -f
>> is
>>            implied.  The -F option is a no-op unless -s option is speci-
>>            fied.
>>
>> So it seems to imply that "-f" will only remove the destination file
>> if it's a regular file, while "-F" is needed if the destination file
>> is a directory. The page also has this to say about "-F" later:
>>
>>      The -F option is FreeBSD extention and should not be used in
>> portable
>>      scripts.
>>
>> So this could be a BSD vs. GNU thing.
> I don't have any BSD systems running.  I can confirm that while "-f"
> refers to files in the man page, it will happy delete the old symlink as
> well.
>
> -bash-4.3$ ln -s /bin/ls jj
> -bash-4.3$ ln -s -f /bin/bash jj
> -bash-4.3$ ls -l jj
> lrwxrwxrwx. 1 law law 9 Jul 14 13:22 jj -> /bin/bash
> -bash-4.3$ which ln
> /usr/bin/ln
> -bash-4.3$ rpm -q --whatprovides /usr/bin/ln
> coreutils-8.24-6.fc23.x86_64
>
> Could you test this on your system?
>
> Jeff
>

$ ln -s /bin/ls jj
$ ln -s -f /bin/bash jj
$ ls -l jj
lrwxr-xr-x  1 root  wheel  9 Jul 14 15:45 jj -> /bin/bash
$ which ln
/bin/ln
$ rpm -q --whatprovides /bin/ln
file /bin/ln is not owned by any package
$ which rpm
/sw/bin/rpm

So apparently the "-f" flag properly overwrites symlinks that point to
regular files, but I also did this in my gcc builddir:

$ mkdir isl-0.1.2.3
$ ln -s isl-0.1.2.3 isl-s
$ ln -sfv isl isl-s
isl-s/isl -> isl
$ ln -sfFv isl isl-s
isl-s/isl -> isl
$ ls -l isl-s
lrwxr-xr-x  1 root  wheel  11 Jul 14 07:03 isl-s -> isl-0.1.2.3
$ unlink isl-s
$ ln -sfFv isl isl-s
isl-s -> isl
$ ls -l isl-s
lrwxr-xr-x  1 root  wheel  3 Jul 14 15:51 isl-s -> isl

...it just doesn't overwrite symlinks that point to a directory.
Jeff Law July 21, 2016, 5:10 p.m. UTC | #7
On 07/14/2016 01:57 PM, Eric Gallager wrote:

>
> So apparently the "-f" flag properly overwrites symlinks that point to
> regular files, but I also did this in my gcc builddir:
>
> $ mkdir isl-0.1.2.3
> $ ln -s isl-0.1.2.3 isl-s
> $ ln -sfv isl isl-s
> isl-s/isl -> isl
> $ ln -sfFv isl isl-s
> isl-s/isl -> isl
> $ ls -l isl-s
> lrwxr-xr-x  1 root  wheel  11 Jul 14 07:03 isl-s -> isl-0.1.2.3
> $ unlink isl-s
> $ ln -sfFv isl isl-s
> isl-s -> isl
> $ ls -l isl-s
> lrwxr-xr-x  1 root  wheel  3 Jul 14 15:51 isl-s -> isl
>
> ...it just doesn't overwrite symlinks that point to a directory.
Joys :(

AFAIK unlink may not necessarily be available on the various host 
systems GCC supports (solaris, aix, hpux, etc etc).

So rather than relying on ln to remove the link, why don't we just 
explicitly remove it with rm -f?

Jeff
diff mbox

Patch

diff --git a/contrib/download_prerequisites b/contrib/download_prerequisites
index 917ee23..6c6e02f 100755
--- a/contrib/download_prerequisites
+++ b/contrib/download_prerequisites
@@ -36,14 +36,17 @@  MPC=mpc-1.0.3
 
 wget ftp://gcc.gnu.org/pub/gcc/infrastructure/$MPFR.tar.bz2 || exit 1
 tar xjf $MPFR.tar.bz2 || exit 1
+if test -L mpfr; then unlink mpfr; fi
 ln -sf $MPFR mpfr || exit 1
 
 wget ftp://gcc.gnu.org/pub/gcc/infrastructure/$GMP.tar.bz2 || exit 1
 tar xjf $GMP.tar.bz2  || exit 1
+if test -L gmp; then unlink gmp; fi
 ln -sf $GMP gmp || exit 1
 
 wget ftp://gcc.gnu.org/pub/gcc/infrastructure/$MPC.tar.gz || exit 1
 tar xzf $MPC.tar.gz || exit 1
+if test -L mpc; then unlink mpc; fi
 ln -sf $MPC mpc || exit 1
 
 # Necessary to build GCC with the Graphite loop optimizations.
@@ -52,5 +55,6 @@  if [ "$GRAPHITE_LOOP_OPT" = "yes" ] ; then
 
   wget ftp://gcc.gnu.org/pub/gcc/infrastructure/$ISL.tar.bz2 || exit 1
   tar xjf $ISL.tar.bz2  || exit 1
+  if test -L isl; then unlink isl; fi
   ln -sf $ISL isl || exit 1
 fi