public inbox for elfutils@sourceware.org
 help / color / mirror / Atom feed
From: "Dmitry V. Levin" <ldv@altlinux.org>
To: "Frank Ch. Eigler" <fche@redhat.com>
Cc: elfutils-devel@sourceware.org
Subject: Re: [patch] PR27783: default debuginfod-urls profile rework
Date: Thu, 11 Nov 2021 01:20:25 +0300	[thread overview]
Message-ID: <20211110222025.GD3988@altlinux.org> (raw)
In-Reply-To: <20211110214247.GA3988@altlinux.org>

On Thu, Nov 11, 2021 at 12:42:47AM +0300, Dmitry V. Levin wrote:
> Hi Frank,
> 
> On Sun, Oct 03, 2021 at 05:33:33PM -0400, Frank Ch. Eigler via Elfutils-devel wrote:
> > commit 0c634f243d266ce8841fd311433d5d79555fabf9
> > Author: Frank Ch. Eigler <fche@redhat.com>
> > Date:   Sun Oct 3 17:04:24 2021 -0400
> > 
> >     PR27783: switch default debuginfod-urls to drop-in style files
> >     
> >     Rewrote and commented the /etc/profile.d csh and sh script fragments
> >     to take the default $DEBUGINFOD_URLS from the union of drop-in files:
> >     /etc/debuginfod/*.urls.  Hand-tested with csh and bash, with
> >     conditions including no prior $DEBUGINFOD_URLS, nonexistent .urls
> >     files, multiple entries in .urls files.
> [...]
> > diff --git a/config/profile.csh.in b/config/profile.csh.in
> > index 0a2d6d162019..29e59a709450 100644
> > --- a/config/profile.csh.in
> > +++ b/config/profile.csh.in
> > @@ -1,11 +1,16 @@
> > -if ("@DEBUGINFOD_URLS@" != "") then
> > -	if ($?DEBUGINFOD_URLS) then
> > -		if ($%DEBUGINFOD_URLS) then
> > -			setenv DEBUGINFOD_URLS "$DEBUGINFOD_URLS @DEBUGINFOD_URLS@"
> > -		else
> > -			setenv DEBUGINFOD_URLS "@DEBUGINFOD_URLS@"
> > -		endif
> > -	else
> > -		setenv DEBUGINFOD_URLS "@DEBUGINFOD_URLS@"
> > -	endif
> > +
> > +# $HOME/.login* or similar files may first set $DEBUGINFOD_URLS.
> > +# If $DEBUGINFOD_URLS is not set there, we set it from system *.url files.
> > +# $HOME/.*rc or similar files may then amend $DEBUGINFOD_URLS.
> > +# See also [man debuginfod-client-config] for other environment variables
> > +# such as $DEBUGINFOD_MAXSIZE, $DEBUGINFOD_MAXTIME, $DEBUGINFOD_PROGRESS.
> > +
> > +if (! $?DEBUGINFOD_URLS) then
> > +    set prefix="@prefix@"
> > +    set debuginfod_urls=`find "@sysconfdir@/debuginfod/" -name '*.urls' | xargs cat | tr '\n' ' '`
> > +    if ( "$debuginfod_urls" != "" ) then
> > +        setenv DEBUGINFOD_URLS "$debuginfod_urls"
> > +    endif
> > +    unset debuginfod_urls
> > +    unset prefix
> >  endif
> > diff --git a/config/profile.sh.in b/config/profile.sh.in
> > index aa228a0dcd16..94b2983b9f90 100644
> > --- a/config/profile.sh.in
> > +++ b/config/profile.sh.in
> > @@ -1,4 +1,17 @@
> > -if [ -n "@DEBUGINFOD_URLS@" ]; then
> > -	DEBUGINFOD_URLS="${DEBUGINFOD_URLS-}${DEBUGINFOD_URLS:+ }@DEBUGINFOD_URLS@"
> > -	export DEBUGINFOD_URLS
> > +
> > +# $HOME/.profile* or similar files may first set $DEBUGINFOD_URLS.
> > +# If $DEBUGINFOD_URLS is not set there, we set it from system *.url files.
> > +# $HOME/.*rc or similar files may then amend $DEBUGINFOD_URLS.
> > +# See also [man debuginfod-client-config] for other environment variables
> > +# such as $DEBUGINFOD_MAXSIZE, $DEBUGINFOD_MAXTIME, $DEBUGINFOD_PROGRESS.
> > +
> > +if [ -z "$DEBUGINFOD_URLS" ]; then

By the way, this test has undesired consequences in "set -eu" mode:

$ sh -c 'set -eu; if [ -z "$DEBUGINFOD_URLS" ]; then echo bingo; fi; echo ok'
sh: DEBUGINFOD_URLS: unbound variable

It has to be "${DEBUGINFOD_URLS-}" to avoid this side effect.

Also, this method cannot distinguish an empty DEBUGINFOD_URLS from unset
DEBUGINFOD_URLS.  As result, there is no easy way for the user to turn
this feature off.  Consider a test like [ -z "${DEBUGINFOD_URLS+1}" ]
that would enable /etc/debuginfod/*.urls scanning only if DEBUGINFOD_URLS
is unset.


-- 
ldv

  reply	other threads:[~2021-11-10 22:20 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-03 21:33 Frank Ch. Eigler
2021-10-06 21:23 ` Mark Wielaard
2021-10-06 23:25   ` Frank Ch. Eigler
2021-10-07  9:10     ` Mark Wielaard
2021-11-10 21:42 ` Dmitry V. Levin
2021-11-10 22:20   ` Dmitry V. Levin [this message]
2021-11-10 22:41   ` Dmitry V. Levin
2021-11-10 22:42   ` Vitaly Chikunov
2021-11-10 23:04   ` Frank Ch. Eigler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211110222025.GD3988@altlinux.org \
    --to=ldv@altlinux.org \
    --cc=elfutils-devel@sourceware.org \
    --cc=fche@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).