[patch] PR27783: default debuginfod-urls profile rework

[patch] PR27783: default debuginfod-urls profile rework

[Frank Ch. Eigler]

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.
    
    Signed-off-by: Frank Ch. Eigler <fche@redhat.com>

diff --git a/config/ChangeLog b/config/ChangeLog
index b2c0af8ac816..bd41654f5492 100644
--- a/config/ChangeLog
+++ b/config/ChangeLog
@@ -1,3 +1,11 @@
+2021-10-03  Frank Ch. Eigler  <fche@redhat.com>
+
+	PR27783
+	* profile.sh, profile.csh: Rewrite to document and set DEBUGINFOD_URLS
+	from /etc/debuginfod/*.urls files.
+	* Makefile.am: Install the configury-specified .urls file.
+	* elfutils.spec: Package it.
+
 2021-09-05  Dmitry V. Levin  <ldv@altlinux.org>
 
 	* eu.am (STACK_USAGE_NO_ERROR): New variable.
diff --git a/config/Makefile.am b/config/Makefile.am
index a66f54904991..0d3ba164ee3a 100644
--- a/config/Makefile.am
+++ b/config/Makefile.am
@@ -40,9 +40,16 @@ pkgconfig_DATA += libdebuginfod.pc
 install-data-local:
 	$(INSTALL_DATA) profile.sh -D $(DESTDIR)$(sysconfdir)/profile.d/debuginfod.sh
 	$(INSTALL_DATA) profile.csh -D $(DESTDIR)$(sysconfdir)/profile.d/debuginfod.csh
+	mkdir -p $(DESTDIR)$(sysconfdir)/debuginfod
+	if [ -n "@DEBUGINFOD_URLS@" ]; then \
+		echo "@DEBUGINFOD_URLS@" > $(DESTDIR)$(sysconfdir)/debuginfod/elfutils.urls; \
+	fi
 
 uninstall-local:
-	rm -f $(DESTDIR)$(sysconfdir)/profile.d/debuginfod.sh $(DESTDIR)$(sysconfdir)/profile.d/debuginfod.csh
+	rm -f $(DESTDIR)$(sysconfdir)/profile.d/debuginfod.sh
+	rm -f $(DESTDIR)$(sysconfdir)/profile.d/debuginfod.csh
+	rm -f $(DESTDIR)$(sysconfdir)/debuginfod/elfutils.urls
+	-rmdir $(DESTDIR)$(sysconfdir)/debuginfod
 endif
 
 if MAINTAINER_MODE
diff --git a/config/elfutils.spec.in b/config/elfutils.spec.in
index 043762653c90..8f6a8e03202f 100644
--- a/config/elfutils.spec.in
+++ b/config/elfutils.spec.in
@@ -301,6 +301,7 @@ fi
 %{_mandir}/man1/debuginfod-find.1*
 %{_mandir}/man7/debuginfod*.7*
 %config(noreplace) %{_sysconfdir}/profile.d/*
+%config(noreplace) %{_sysconfdir}/debuginfod/*
   
 %files debuginfod-client-devel
 %defattr(-,root,root)
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
+    prefix="@prefix@"
+    debuginfod_urls=`find "@sysconfdir@/debuginfod/" -name '*.urls' | xargs cat | tr '\n' ' '`
+    if [ -n "$debuginfod_urls" ]; then
+        DEBUGINFOD_URLS="$debuginfod_urls"
+        export DEBUGINFOD_URLS
+    fi
+    unset debuginfod_urls
+    unset prefix
 fi

Re: [patch] PR27783: default debuginfod-urls profile rework

[Mark Wielaard]

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.
>     
>     Signed-off-by: Frank Ch. Eigler <fche@redhat.com>

Could you add something about this to NEWS so packagers know how to
update to the new scheme?

> diff --git a/config/ChangeLog b/config/ChangeLog
> index b2c0af8ac816..bd41654f5492 100644
> --- a/config/ChangeLog
> +++ b/config/ChangeLog
> @@ -1,3 +1,11 @@
> +2021-10-03  Frank Ch. Eigler  <fche@redhat.com>
> +
> +	PR27783
> +	* profile.sh, profile.csh: Rewrite to document and set DEBUGINFOD_URLS
> +	from /etc/debuginfod/*.urls files.
> +	* Makefile.am: Install the configury-specified .urls file.
> +	* elfutils.spec: Package it.
> +
>  2021-09-05  Dmitry V. Levin  <ldv@altlinux.org>
>  
>  	* eu.am (STACK_USAGE_NO_ERROR): New variable.
> diff --git a/config/Makefile.am b/config/Makefile.am
> index a66f54904991..0d3ba164ee3a 100644
> --- a/config/Makefile.am
> +++ b/config/Makefile.am
> @@ -40,9 +40,16 @@ pkgconfig_DATA += libdebuginfod.pc
>  install-data-local:
>  	$(INSTALL_DATA) profile.sh -D $(DESTDIR)$(sysconfdir)/profile.d/debuginfod.sh
>  	$(INSTALL_DATA) profile.csh -D $(DESTDIR)$(sysconfdir)/profile.d/debuginfod.csh
> +	mkdir -p $(DESTDIR)$(sysconfdir)/debuginfod
> +	if [ -n "@DEBUGINFOD_URLS@" ]; then \
> +		echo "@DEBUGINFOD_URLS@" > $(DESTDIR)$(sysconfdir)/debuginfod/elfutils.urls; \
> +	fi
>  
>  uninstall-local:
> -	rm -f $(DESTDIR)$(sysconfdir)/profile.d/debuginfod.sh $(DESTDIR)$(sysconfdir)/profile.d/debuginfod.csh
> +	rm -f $(DESTDIR)$(sysconfdir)/profile.d/debuginfod.sh
> +	rm -f $(DESTDIR)$(sysconfdir)/profile.d/debuginfod.csh
> +	rm -f $(DESTDIR)$(sysconfdir)/debuginfod/elfutils.urls
> +	-rmdir $(DESTDIR)$(sysconfdir)/debuginfod
>  endif
>  
>  if MAINTAINER_MODE
> diff --git a/config/elfutils.spec.in b/config/elfutils.spec.in
> index 043762653c90..8f6a8e03202f 100644
> --- a/config/elfutils.spec.in
> +++ b/config/elfutils.spec.in
> @@ -301,6 +301,7 @@ fi
>  %{_mandir}/man1/debuginfod-find.1*
>  %{_mandir}/man7/debuginfod*.7*
>  %config(noreplace) %{_sysconfdir}/profile.d/*
> +%config(noreplace) %{_sysconfdir}/debuginfod/*
>    
>  %files debuginfod-client-devel
>  %defattr(-,root,root)
> 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' ' '`

Can we use cat "@sysconfdir@/debuginfod/*.urls" | tr '\n' ' ' instead
so we don't need to rely on findutils and xargs?

> +    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
> +    prefix="@prefix@"
> +    debuginfod_urls=`find "@sysconfdir@/debuginfod/" -name '*.urls' | xargs cat | tr '\n' ' '`

Likewise.

> +    if [ -n "$debuginfod_urls" ]; then
> +        DEBUGINFOD_URLS="$debuginfod_urls"
> +        export DEBUGINFOD_URLS
> +    fi
> +    unset debuginfod_urls
> +    unset prefix
>  fi

Thanks,

Mark

Re: [patch] PR27783: default debuginfod-urls profile rework

[Frank Ch. Eigler]

Hi -

> >     Signed-off-by: Frank Ch. Eigler <fche@redhat.com>
> 
> Could you add something about this to NEWS so packagers know how to
> update to the new scheme?

Sure.

> > +    set debuginfod_urls=`find "@sysconfdir@/debuginfod/" -name '*.urls' | xargs cat | tr '\n' ' '`
> 
> Can we use cat "@sysconfdir@/debuginfod/*.urls" | tr '\n' ' ' instead
> so we don't need to rely on findutils and xargs?

One problem with that is that several shells (csh, zsh) throw errors
when a glob expression has no matches.

- FChE

Re: [patch] PR27783: default debuginfod-urls profile rework

[Mark Wielaard]

Hi Frank,

On Wed, Oct 06, 2021 at 07:25:11PM -0400, Frank Ch. Eigler wrote:
> > > +    set debuginfod_urls=`find "@sysconfdir@/debuginfod/" -name '*.urls' | xargs cat | tr '\n' ' '`
> > 
> > Can we use cat "@sysconfdir@/debuginfod/*.urls" | tr '\n' ' ' instead
> > so we don't need to rely on findutils and xargs?
> 
> One problem with that is that several shells (csh, zsh) throw errors
> when a glob expression has no matches.

Groan. shells...  But there is always the default.url file in there
(elfutils.url). Maybe we could even drop an empty.url file in there
that is just, well, empty. That way the glob always globs?

But if the above is to cumbersome, leave it at find and xargs. I
assume they are normally always installed. And for those distros where
they aren't, they might not want to install a default DEBUGINFOD_URLS
set anyway.

Cheers,

Mark

Re: [patch] PR27783: default debuginfod-urls profile rework

[Dmitry V. Levin]

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
> +    prefix="@prefix@"
> +    debuginfod_urls=`find "@sysconfdir@/debuginfod/" -name '*.urls' | xargs cat | tr '\n' ' '`
> +    if [ -n "$debuginfod_urls" ]; then
> +        DEBUGINFOD_URLS="$debuginfod_urls"
> +        export DEBUGINFOD_URLS
> +    fi
> +    unset debuginfod_urls
> +    unset prefix
>  fi

Is there any particular reason to assign (and unset) prefix?


-- 
ldv

Re: [patch] PR27783: default debuginfod-urls profile rework

[Dmitry V. Levin]

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

Re: [patch] PR27783: default debuginfod-urls profile rework

[Dmitry V. Levin]

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
> > +    prefix="@prefix@"
> > +    debuginfod_urls=`find "@sysconfdir@/debuginfod/" -name '*.urls' | xargs cat | tr '\n' ' '`
> > +    if [ -n "$debuginfod_urls" ]; then
> > +        DEBUGINFOD_URLS="$debuginfod_urls"
> > +        export DEBUGINFOD_URLS
> > +    fi
> > +    unset debuginfod_urls
> > +    unset prefix
> >  fi
> 
> Is there any particular reason to assign (and unset) prefix?

Another potential issue with this "cat" approach is that strings
without trailing newlines are concatenated with other strings.
I suggest the following code snippet instead:

if [ -z "${DEBUGINFOD_URLS+1}" ]; then
    debuginfod_urls=$(sed -n 's/^[^#].*/&/p' @sysconfdir@/debuginfod/*.urls 2>/dev/null |tr '\n' ' ' ||:)
    if [ -n "$debuginfod_urls" ]; then
       	DEBUGINFOD_URLS="$debuginfod_urls"
       	export DEBUGINFOD_URLS
    fi
    unset debuginfod_urls
fi

As a bonus, it also handles comments. :)


-- 
ldv

Re: [patch] PR27783: default debuginfod-urls profile rework

[Vitaly Chikunov]

Frank,

On Thu, Nov 11, 2021 at 12:42:47AM +0300, Dmitry V. Levin wrote:
> 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.
> [...]
> > +# $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
> > +    prefix="@prefix@"
> > +    debuginfod_urls=`find "@sysconfdir@/debuginfod/" -name '*.urls' | xargs cat | tr '\n' ' '`

If *.urls file does not contain "\n" at the EOF its url will be
concatenated to another without separator making multiple urls invalid.

I'd suggest you don't use find (supporting subdirectories, why?) with
xargs cat ..., but just `for x in *.urls` cycle.

Also, running find, xargs, cat, tr, always (even if there is no urls,
you can optimize calling cat with xargs -r) Calling so much binaries on
login will slow down login to a loaded box, so 'for' cycle perhaps would
be faster.

If you still going to use so complicated processing would be beneficial
to support commentaries in .urls files too, so people can add some
description, or comment out some url (there can be many implied by
'urls' name).

Thanks,

Re: [patch] PR27783: default debuginfod-urls profile rework

[Frank Ch. Eigler]

Hi -

> Is there any particular reason to assign (and unset) prefix?

Yes.  It's because @sysconfdir@ as expanded by autoconf often turns
out to be a string like "$prefix/etc", which relies on the $prefix
variable being available there.

- FChE