Re: [PATCH] Detect location of `ldconfig' before use

[Thiago Macieira]

On domingo, 5 de fevereiro de 2017 12:53:38 PST Cristian Ionescu-Idbohrn 
wrote:
> +   _ldconfig=$(PATH=/usr/local/sbin:/usr/sbin:/sbin:$PATH command -v
> ldconfig) || { +   echo "can't find ldconfig" >&2
> +   exit 1
> +   }
> +   LIBGIT=$($_ldconfig -p | grep libgit2\\.so\\. | awk -F. '{ print $NF
> }')

Instead of trying to find it, I'd just do:

ldconfig 2>/dev/null || true

-- 
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
   Software Architect - Intel Open Source Technology Center

___
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface

Re: [PATCH] Detect location of ldconfig before use

[Cristian Ionescu-Idbohrn]

On Sun, 5 Feb 2017, Robert Helling wrote:
> > On 05 Feb 2017, at 15:21, Cristian Ionescu-Idbohrn 
> >  wrote:
> > 
> > I have an idea ;)
> > 
> > Why not error handle those the code that produces those "warnings" (as
> > you call them) on that particular architecture, instead of
> > ignoring/hiding the errors (exit status failure) for everyone?  That
> > kind of thing can bite you later in various ways, leading to bug
> > reports saying "I tried that but it doesn't work :/  The build script
> > exits with success, but...  Could you fix it for me?  I can't tell you
> > where to start looking :(".
> > 
> > 'errexit' was not invented to punish people, but to help coding robust
> > scripts.  In my experience, error handling is one of the most
> > neglected areas.
> 
> ok, took up the challenge. My memory was wrong. It wasn’t Qt. It was 
> the construct
> 
> echo $NAME | grep / > /dev/null 2>&1
> if [ $? -eq 1 ] ; then
> install_name_tool -id "$INSTALL_ROOT/lib/$NAME" 
> "$INSTALL_ROOT/lib/$NAME"
> fi
> 
> (this appears twice, you find it by searching for $?). Problem is, 
> when I run it, grep returns non-zero and this is then where the 
> script silently stops.

So, I read that as:

* if there is a '/' in $NAME, do nothing
* otherwise install $NAME

> Fixing this is beyond my bash abilities. If you want -e, please 
> rewrite these two occurrences so that the script does not silently 
> fail.

To even avoid the `grep' fork and the pipe, I would write that as:

case $NAME in
*/*)
:
;;
*)
install_name_tool -id "$INSTALL_ROOT/lib/$NAME" 
"$INSTALL_ROOT/lib/$NAME"
;;
esac

If you insist on using `grep':

echo $NAME | grep -q / ||
install_name_tool -id "$INSTALL_ROOT/lib/$NAME" 
"$INSTALL_ROOT/lib/$NAME"

and if it _must_ be done using an if/fi block:

if ! echo $NAME | grep -q /; then
install_name_tool -id "$INSTALL_ROOT/lib/$NAME" 
"$INSTALL_ROOT/lib/$NAME"
fi


Cheers,

-- 
Cristian
___
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface

Re: [PATCH] Detect location of `ldconfig' before use

[Robert Helling]

Hi,

> On 05 Feb 2017, at 15:21, Cristian Ionescu-Idbohrn 
>  wrote:
> 
> I have an idea ;)
> 
> Why not error handle those the code that produces those "warnings" (as
> you call them) on that particular architecture, instead of
> ignoring/hiding the errors (exit status failure) for everyone?  That
> kind of thing can bite you later in various ways, leading to bug
> reports saying "I tried that but it doesn't work :/  The build script
> exits with success, but...  Could you fix it for me?  I can't tell you
> where to start looking :(".
> 
> 'errexit' was not invented to punish people, but to help coding robust
> scripts.  In my experience, error handling is one of the most
> neglected areas.

ok, took up the challenge. My memory was wrong. It wasn’t Qt. It was the 
construct

echo $NAME | grep / > /dev/null 2>&1
if [ $? -eq 1 ] ; then
install_name_tool -id "$INSTALL_ROOT/lib/$NAME" 
"$INSTALL_ROOT/lib/$NAME"
fi

(this appears twice, you find it by searching for $?). Problem is, when I run 
it, grep returns non-zero and this is then where the script silently stops.

Fixing this is beyond my bash abilities. If you want -e, please rewrite these 
two occurrences so that the script does not silently fail.

Thanks
Robert


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface

Re: [PATCH] Detect location of `ldconfig' before use

[Cristian Ionescu-Idbohrn]

On Sun, 5 Feb 2017, Robert C. Helling wrote:
> > Am 05.02.2017 um 12:53 schrieb Cristian Ionescu-Idbohrn 
> > :
> > 
> > That's the reason shell option '-e' (errexit) was introduced.
> 
> That option had been introduced before and then later been removed 
> again. The build on mac throws warnings about certain Qt things that 
> one should do (but which are not necessarily) and with -e that is 
> the end of the build.
> 
> So please don't. 

I have an idea ;)

Why not error handle those the code that produces those "warnings" (as 
you call them) on that particular architecture, instead of 
ignoring/hiding the errors (exit status failure) for everyone?  That 
kind of thing can bite you later in various ways, leading to bug 
reports saying "I tried that but it doesn't work :/  The build script 
exits with success, but...  Could you fix it for me?  I can't tell you 
where to start looking :(".

'errexit' was not invented to punish people, but to help coding robust 
scripts.  In my experience, error handling is one of the most 
neglected areas.


Cheers,

-- 
Cristian
___
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface

Re: [PATCH] Detect location of `ldconfig' before use

[Robert C. Helling]

Hi,

> Am 05.02.2017 um 12:53 schrieb Cristian Ionescu-Idbohrn 
> :
> 
> That's the reason shell option
> '-e' (errexit) was introduced.

That option had been introduced before and then later been removed again. The 
build on mac throws warnings about certain Qt things that one should do (but 
which are not necessarily) and with -e that is the end of the build. 

So please don't. 

Best 
Robert
___
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface

[PATCH] Detect location of `ldconfig' before use

[Cristian Ionescu-Idbohrn]

The unhandled error was:

./subsurface/scripts/build.sh: line 84: ldconfig: command not found

On my box, the path is /sbin/ldconfig, and /sbin is not in common users'
PATH.

The script just continued to execute after that error, which can result
in other odd bugs showing up later.  That's the reason shell option
'-e' (errexit) was introduced.

Signed-off-by: Cristian Ionescu-Idbohrn 
---
 scripts/build.sh | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/scripts/build.sh b/scripts/build.sh
index f60d7045..cf87f26b 100755
--- a/scripts/build.sh
+++ b/scripts/build.sh
@@ -16,6 +16,8 @@
 # of the current directory (except on Mac where the Subsurface.app ends up
 # in subsurface/build
 
+set -e
+
 # create a log file of the build
 exec 1> >(tee build.log) 2>&1
 
@@ -81,7 +83,11 @@ else
# check if we need to build libgit2 (and do so if necessary)
 
LIBGIT_ARGS=" -DLIBGIT2_DYNAMIC=ON "
-   LIBGIT=$(ldconfig -p | grep libgit2\\.so\\. | awk -F. '{ print $NF }')
+   _ldconfig=$(PATH=/usr/local/sbin:/usr/sbin:/sbin:$PATH command -v 
ldconfig) || {
+   echo "can't find ldconfig" >&2
+   exit 1
+   }
+   LIBGIT=$($_ldconfig -p | grep libgit2\\.so\\. | awk -F. '{ print $NF }')
 fi
 
 if [[ $PLATFORM = Darwin || "$LIBGIT" < "24" ]] ; then
-- 
2.11.0

___
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface