[pacman-dev,5/5] meson-make-symlink.sh: Make compatible with non-GNU ln

Message ID 20210417034524.204755-6-mark.weiman@markzz.com
State New
Headers show
Series Various fixes for FreeBSD (and perhaps others) | expand

Commit Message

Mark Weiman April 17, 2021, 3:45 a.m. UTC
The "-T" flag is something available with GNU ln, but on other
implementations of ln, "-T" is not an option. So, to make this script
more portable, a strings check of ln for "GNU coreutils" is done to
determine if "-T" should be used.

Signed-off-by: Mark Weiman <mark.weiman@markzz.com>
---
 build-aux/meson-make-symlink.sh | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Bjoern Bidar April 17, 2021, 12:44 p.m. UTC | #1
> The "-T" flag is something available with GNU ln, but on other
> implementations of ln, "-T" is not an option. So, to make this script
> more portable, a strings check of ln for "GNU coreutils" is done to
> determine if "-T" should be used.
> 
> Signed-off-by: Mark Weiman <mark.weiman@markzz.com>
> ---
>  build-aux/meson-make-symlink.sh | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/build-aux/meson-make-symlink.sh
> b/build-aux/meson-make-symlink.sh index 501cd43d..24a0dd2c 100644
> --- a/build-aux/meson-make-symlink.sh
> +++ b/build-aux/meson-make-symlink.sh
> @@ -4,9 +4,13 @@ set -eu
>  # this is needed mostly because $DESTDIR is provided as a variable,
>  # and we need to create the target directory...
> 
> +# test if ln is GNU or not
> +LN_FLAG="-T"
> +strings /bin/ln | grep -q 'GNU coreutils' || LN_FLAG=""
No strings needed just do:
ln --version|grep GNU

Or skip -T even with GNU ln.
Mark Weiman April 17, 2021, 6:52 p.m. UTC | #2
On Sat, 2021-04-17 at 15:44 +0300, Bjoern Bidar wrote:
> > The "-T" flag is something available with GNU ln, but on other
> > implementations of ln, "-T" is not an option. So, to make this script
> > more portable, a strings check of ln for "GNU coreutils" is done to
> > determine if "-T" should be used.
> > 
> > Signed-off-by: Mark Weiman <mark.weiman@markzz.com>
> > ---
> >  build-aux/meson-make-symlink.sh | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/build-aux/meson-make-symlink.sh
> > b/build-aux/meson-make-symlink.sh index 501cd43d..24a0dd2c 100644
> > --- a/build-aux/meson-make-symlink.sh
> > +++ b/build-aux/meson-make-symlink.sh
> > @@ -4,9 +4,13 @@ set -eu
> >  # this is needed mostly because $DESTDIR is provided as a variable,
> >  # and we need to create the target directory...
> > 
> > +# test if ln is GNU or not
> > +LN_FLAG="-T"
> > +strings /bin/ln | grep -q 'GNU coreutils' || LN_FLAG=""
> No strings needed just do:
> ln --version|grep GNU
> 
> Or skip -T even with GNU ln.

ln --version is not valid on all implementations, but I guess that could be a
check...

I can just remove -T altogether as well, also --relative may be an issue.

Mark
Eli Schwartz April 18, 2021, 2:14 a.m. UTC | #3
On 4/17/21 2:52 PM, Mark Weiman wrote:
> On Sat, 2021-04-17 at 15:44 +0300, Bjoern Bidar wrote:
>>> The "-T" flag is something available with GNU ln, but on other
>>> implementations of ln, "-T" is not an option. So, to make this script
>>> more portable, a strings check of ln for "GNU coreutils" is done to
>>> determine if "-T" should be used.
>>>
>>> Signed-off-by: Mark Weiman <mark.weiman@markzz.com>
>>> ---
>>>  build-aux/meson-make-symlink.sh | 8 ++++++--
>>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/build-aux/meson-make-symlink.sh
>>> b/build-aux/meson-make-symlink.sh index 501cd43d..24a0dd2c 100644
>>> --- a/build-aux/meson-make-symlink.sh
>>> +++ b/build-aux/meson-make-symlink.sh
>>> @@ -4,9 +4,13 @@ set -eu
>>>  # this is needed mostly because $DESTDIR is provided as a variable,
>>>  # and we need to create the target directory...
>>>
>>> +# test if ln is GNU or not
>>> +LN_FLAG="-T"
>>> +strings /bin/ln | grep -q 'GNU coreutils' || LN_FLAG=""
>> No strings needed just do:
>> ln --version|grep GNU
>>
>> Or skip -T even with GNU ln.
> 
> ln --version is not valid on all implementations, but I guess that could be a
> check...
> 
> I can just remove -T altogether as well, also --relative may be an issue.


You are warmly encouraged to rewrite this into something that supports
FreeBSD ln, or any other simple POSIX ln... however, once you have done
so that implementation will work fine even for GNU ln, so you'd better
use it rather than some weird string search of the ln binary (which may
not even be in /bin).

While you're at it, --relative is hardly portable either... but I don't
think we use this side of the logic fork? So that can simply be removed...

A replacement for -T I guess (rather than simply removing it, which is
pointless) would be

if [ -d "${DESTDIR:-}$2" ]; then
    echo "notln: "${DESTDIR:-}$2": cannot overwrite directory"
    exit 1
fi

but on the other hand we could try something really clever by rmdir'ing
it so that the symlink succeeds...

The FreeBSD ln -F "If the target file already exists and is a directory,
then remove it so that the link may occur." seems ever so much more
useful than GNU ln -F "allow root to try to hardlink directories, note
that it will probably fail due to filesystem semantics".

But, this is especially not portable so this is not a suitable
replacement for -T either, even though -T is essentially just "FreeBSD's
-F flag, but raises an error instead".

ln -nf covers the other case where the destination is neither a file
(-f) nor a directory (-fT or -F), but a symlink to a directory (-fT
works here too). The -n option is implemented on a variety of ln
programs, including GNU, busybox, FreeBSD, OpenBSD, NetBSD, macOS, etc.
Eli Schwartz April 18, 2021, 2:44 a.m. UTC | #4
On 4/17/21 10:14 PM, Eli Schwartz wrote:
> On 4/17/21 2:52 PM, Mark Weiman wrote:
>> On Sat, 2021-04-17 at 15:44 +0300, Bjoern Bidar wrote:
>>>> The "-T" flag is something available with GNU ln, but on other
>>>> implementations of ln, "-T" is not an option. So, to make this script
>>>> more portable, a strings check of ln for "GNU coreutils" is done to
>>>> determine if "-T" should be used.
>>>>
>>>> Signed-off-by: Mark Weiman <mark.weiman@markzz.com>
>>>> ---
>>>>  build-aux/meson-make-symlink.sh | 8 ++++++--
>>>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/build-aux/meson-make-symlink.sh
>>>> b/build-aux/meson-make-symlink.sh index 501cd43d..24a0dd2c 100644
>>>> --- a/build-aux/meson-make-symlink.sh
>>>> +++ b/build-aux/meson-make-symlink.sh
>>>> @@ -4,9 +4,13 @@ set -eu
>>>>  # this is needed mostly because $DESTDIR is provided as a variable,
>>>>  # and we need to create the target directory...
>>>>
>>>> +# test if ln is GNU or not
>>>> +LN_FLAG="-T"
>>>> +strings /bin/ln | grep -q 'GNU coreutils' || LN_FLAG=""
>>> No strings needed just do:
>>> ln --version|grep GNU
>>>
>>> Or skip -T even with GNU ln.
>>
>> ln --version is not valid on all implementations, but I guess that could be a
>> check...
>>
>> I can just remove -T altogether as well, also --relative may be an issue.
> 
> 
> You are warmly encouraged to rewrite this into something that supports
> FreeBSD ln, or any other simple POSIX ln... however, once you have done
> so that implementation will work fine even for GNU ln, so you'd better
> use it rather than some weird string search of the ln binary (which may
> not even be in /bin).
> 
> While you're at it, --relative is hardly portable either... but I don't
> think we use this side of the logic fork? So that can simply be removed...
> 
> A replacement for -T I guess (rather than simply removing it, which is
> pointless) would be
> 
> if [ -d "${DESTDIR:-}$2" ]; then
>     echo "notln: "${DESTDIR:-}$2": cannot overwrite directory"
>     exit 1
> fi
> 
> but on the other hand we could try something really clever by rmdir'ing
> it so that the symlink succeeds...
> 
> The FreeBSD ln -F "If the target file already exists and is a directory,
> then remove it so that the link may occur." seems ever so much more
> useful than GNU ln -F "allow root to try to hardlink directories, note
> that it will probably fail due to filesystem semantics".
> 
> But, this is especially not portable so this is not a suitable
> replacement for -T either, even though -T is essentially just "FreeBSD's
> -F flag, but raises an error instead".
> 
> ln -nf covers the other case where the destination is neither a file
> (-f) nor a directory (-fT or -F), but a symlink to a directory (-fT
> works here too). The -n option is implemented on a variety of ln
> programs, including GNU, busybox, FreeBSD, OpenBSD, NetBSD, macOS, etc.

And I've implemented this all in the patch I just sent.

Patch

diff --git a/build-aux/meson-make-symlink.sh b/build-aux/meson-make-symlink.sh
index 501cd43d..24a0dd2c 100644
--- a/build-aux/meson-make-symlink.sh
+++ b/build-aux/meson-make-symlink.sh
@@ -4,9 +4,13 @@  set -eu
 # this is needed mostly because $DESTDIR is provided as a variable,
 # and we need to create the target directory...
 
+# test if ln is GNU or not
+LN_FLAG="-T"
+strings /bin/ln | grep -q 'GNU coreutils' || LN_FLAG=""
+
 mkdir -vp "$(dirname "${DESTDIR:-}$2")"
 if [ "$(dirname $1)" = . ]; then
-        ln -vfs -T "$1" "${DESTDIR:-}$2"
+        ln -vfs ${LN_FLAG} "$1" "${DESTDIR:-}$2"
 else
-        ln -vfs -T --relative "${DESTDIR:-}$1" "${DESTDIR:-}$2"
+        ln -vfs ${LN_FLAG} --relative "${DESTDIR:-}$1" "${DESTDIR:-}$2"
 fi