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

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

[Bernd Edlinger]

Hi,

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

sounds good, I ran into similar issues already.

ln -nfs does not follow the target if it is a symlink

        -n, --no-dereference
               treat  LINK_NAME  as a normal file if it is a symbolic 
link to a
               directory


but I think a simple rm -f will do as well, and avoid potential
interoperability issues.

However wget has a similar issue, if the $MPFR.tar.gz file is already
there, maybe incomplete, the wget chooses a new name, so I'd suggest
to rm -f that file as well, and the whole $MPFR subtree while you are
already there.


Bernd.

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

[Jeff Law]

On 07/21/2016 12:15 PM, Bernd Edlinger wrote:
> Hi,
>
>  > So rather than relying on ln to remove the link, why don't we just
>  > explicitly remove it with rm -f?
>
> sounds good, I ran into similar issues already.
>
> ln -nfs does not follow the target if it is a symlink
>
>         -n, --no-dereference
>                treat  LINK_NAME  as a normal file if it is a symbolic
> link to a
>                directory
>
>
> but I think a simple rm -f will do as well, and avoid potential
> interoperability issues.
>
> However wget has a similar issue, if the $MPFR.tar.gz file is already
> there, maybe incomplete, the wget chooses a new name, so I'd suggest
> to rm -f that file as well, and the whole $MPFR subtree while you are
> already there.
Agreed.  And naturally the question is do we bother to check the return 
code from the rm -f?  I think we should and exit with an error if it fails.

jeff

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

[Eric Gallager]

[-- Attachment #1: Type: text/plain, Size: 692 bytes --]

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

[-- Attachment #2: patch_contrib_download_prerequisites.diff --]
[-- Type: text/plain, Size: 1107 bytes --]

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

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

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

[Jeff Law]

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

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

[Eric Gallager]

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.

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

[NightStrike]

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)

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

[Jeff Law]

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

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

[Eric Gallager]

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.

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

[Jeff Law]

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

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

[Eric Gallager]

[-- Attachment #1: Type: text/plain, Size: 1177 bytes --]

On 7/21/16, Jeff Law <law@redhat.com> wrote:
> 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
>

Sure, rm -f works, too; I just went with "unlink" in my patch because
it more clearly expresses programmer intent. But I guess portability
is more important. Updated patch attached, although someone else would
have to commit it, as I don't have commit access.

Eric

[-- Attachment #2: patch_contrib_download_prerequisites.diff --]
[-- Type: text/plain, Size: 1103 bytes --]

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

diff --git a/contrib/download_prerequisites b/contrib/download_prerequisites
index 917ee23..2c963f2 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 rm -f 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 rm -f 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 rm -f 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 rm -f isl; fi
   ln -sf $ISL isl || exit 1
 fi

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

[Jeff Law]

On 07/21/2016 01:39 PM, Eric Gallager wrote:
> On 7/21/16, Jeff Law <law@redhat.com> wrote:
>> 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
>>
>
> Sure, rm -f works, too; I just went with "unlink" in my patch because
> it more clearly expresses programmer intent. But I guess portability
> is more important. Updated patch attached, although someone else would
> have to commit it, as I don't have commit access.
Thanks for your patience.  I've installed your patch.

Jeff

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

[NightStrike]

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?