Hi,

On Thu, Mar 21, 2024 at 04:04:11PM +0100, Frederik “Freso” S. Olesen wrote:
> Add support for setting $DEBUGINFOD_URLS automatically in the fish shell
> similar to the profile scripts for POSIX and csh shells.
> 
> Makefile is set to install this into fish’s $XDG_DATA_DIRS vendor
> directory instead of under /etc:
> https://fishshell.com/docs/current/language.html#configuration-files
> 
>       * config/profile.fish.in: Set $DEBUGINFOD_URLS in fish shells.
>       * config/Makefile.am: Include profile.fish in install and
>         uninstall targets.

Very nice. I don't know fish, but this looks reasonable.

Two small comments below.

> Signed-off-by: Frederik “Freso” S. Olesen <freso...@gmail.com>
> ---
>  config/ChangeLog       |  5 +++++
>  config/Makefile.am     |  5 ++++-
>  config/profile.fish.in | 14 ++++++++++++++
>  3 files changed, 23 insertions(+), 1 deletion(-)
>  create mode 100644 config/profile.fish.in
> 
> diff --git a/config/ChangeLog b/config/ChangeLog
> index ce1f74f6..7d88c071 100644
> --- a/config/ChangeLog
> +++ b/config/ChangeLog
> @@ -1,3 +1,8 @@
> +2024-03-21  Frederik “Freso” S. Olesen  <fr...@freso.dk>
> +
> +     * profile.fish.in: Set $DEBUGINFOD_URLS in fish shells.
> +     * Makefile.am: Include profile.fish in install and uninstall targets.

Since we have been putting the ChangeLog entry into the commit message
it doesn't need to also go into the actual Changelog file.

>  2023-02-21  Mark Wielaard  <m...@klomp.org>
>  
>       * eu.am (USE_AFTER_FREE3_WARNING): Define.
> diff --git a/config/Makefile.am b/config/Makefile.am
> index 0d3ba164..ae14e625 100644
> --- a/config/Makefile.am
> +++ b/config/Makefile.am
> @@ -30,7 +30,8 @@
>  ##
>  EXTRA_DIST = elfutils.spec.in known-dwarf.awk 10-default-yama-scope.conf \
>            libelf.pc.in libdw.pc.in libdebuginfod.pc.in \
> -          debuginfod.service debuginfod.sysconfig profile.sh.in 
> profile.csh.in
> +          debuginfod.service debuginfod.sysconfig \
> +          profile.sh.in profile.csh.in profile.fish.in
>  
>  pkgconfigdir = $(libdir)/pkgconfig
>  pkgconfig_DATA = libelf.pc libdw.pc
> @@ -40,6 +41,7 @@ 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
> +     $(INSTALL_DATA) profile.fish -D 
> $(DESTDIR)$(datadir)/fish/vendor_conf.d/debuginfod.fish
>       mkdir -p $(DESTDIR)$(sysconfdir)/debuginfod
>       if [ -n "@DEBUGINFOD_URLS@" ]; then \
>               echo "@DEBUGINFOD_URLS@" > 
> $(DESTDIR)$(sysconfdir)/debuginfod/elfutils.urls; \
> @@ -48,6 +50,7 @@ install-data-local:
>  uninstall-local:
>       rm -f $(DESTDIR)$(sysconfdir)/profile.d/debuginfod.sh
>       rm -f $(DESTDIR)$(sysconfdir)/profile.d/debuginfod.csh
> +     rm -f $(DESTDIR)$(datadir)/fish/vendor_conf.d/debuginfod.fish
>       rm -f $(DESTDIR)$(sysconfdir)/debuginfod/elfutils.urls
>       -rmdir $(DESTDIR)$(sysconfdir)/debuginfod
>  endif

Right, with --prefix=/usr that expands to
/usr/share/fish/vendor_conf.d which seems to match the default install
location you pointed out above. Good.

> diff --git a/config/profile.fish.in b/config/profile.fish.in
> new file mode 100644
> index 00000000..34b1ab85
> --- /dev/null
> +++ b/config/profile.fish.in
> @@ -0,0 +1,14 @@
> +# $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 not set --query DEBUGINFOD_URLS
> +    # Use local variables so we don't need to manually unset them
> +    set --local prefix="@prefix@"
> +    set --local DEBUGINFOD_URLS (cat /dev/null 
> "@sysconfdir@/debuginfod"/*.urls 2>/dev/null | string replace '\n' ' ')
> +    if test -n "$DEBUGINFOD_URLS"
> +        set --global --export DEBUGINFOD_URLS "$DEBUGINFOD_URLS"
> +    end
> +end

I don't know fish, but this looks OK.

Note that to turn this profile.fish.in into profile.fish you need to
mark it as a config file in configure.ac:

diff --git a/configure.ac b/configure.ac
index 098d13067ee6..a279bb5282c9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -881,7 +881,7 @@ AC_ARG_ENABLE(debuginfod-urls,
              fi],
             [default_debuginfod_urls=""])
 AC_SUBST(DEBUGINFOD_URLS, $default_debuginfod_urls)                
-AC_CONFIG_FILES([config/profile.sh config/profile.csh])
+AC_CONFIG_FILES([config/profile.sh config/profile.csh config/profile.fish])
 
 AC_OUTPUT
 
Could you sent a new patch with those two changes?

Thanks,

Mark

Reply via email to