diff mbox series

[bug#77525,v4,2/6] system: /etc/profile: Rearrange to avoid search path duplication.

Message ID 94cf181828817e485cb92cf80365881fe8a8cf47.1743736516.git.hako@ultrarare.space
State New
Headers
Series system: /etc/profile: Rearrange to avoid search path duplication. |

Commit Message

Hilton Chain April 4, 2025, 3:22 a.m. UTC 
* gnu/system.scm (operating-system-etc-service)[profile]: Set umask first.
Group environment variables setup with profile sourcing to avoid duplication.
Export $GUILE_LOAD_PATH and $GUILE_LOAD_COMPILED_PATH for ‘guix pull’ profile.
* gnu/system/shadow.scm (%default-bash-profile): Remove duplicated exports.

Change-Id: I42ae153b9cd47ca24448fa18ce7f80a5e5c06621
---
 gnu/system.scm        | 96 +++++++++++++++++++++++++++++--------------
 gnu/system/shadow.scm | 10 -----
 2 files changed, 65 insertions(+), 41 deletions(-)

Comments

Ludovic Courtès April 14, 2025, 3:12 p.m. UTC | #1 
Hi Hilton,

(Cc: 宋文武.)

Hilton Chain <hako@ultrarare.space> writes:

> * gnu/system.scm (operating-system-etc-service)[profile]: Set umask first.
> Group environment variables setup with profile sourcing to avoid duplication.
> Export $GUILE_LOAD_PATH and $GUILE_LOAD_COMPILED_PATH for ‘guix pull’ profile.
> * gnu/system/shadow.scm (%default-bash-profile): Remove duplicated exports.

The last item does not say the whole story:

> +++ b/gnu/system/shadow.scm
> @@ -179,16 +179,6 @@ (define %default-bash-profile
>  
>  # Honor per-interactive-shell startup file
>  if [ -f ~/.bashrc ]; then . ~/.bashrc; fi
> -
> -# Merge search-paths from multiple profiles, the order matters.
> -eval \"$(guix package --search-paths \\
> --p $HOME/.config/guix/current \\
> --p $HOME/.guix-home/profile \\
> --p $HOME/.guix-profile \\
> --p /run/current-system/profile)\"
> -
> -# Prepend setuid programs.
> -export PATH=/run/setuid-programs:$PATH
>  "))

I am the one who suggested to not invoke ‘guix package’ from shell
startup files:

  https://issues.guix.gnu.org/77035#8-lineno17

However, I hadn’t realized that this had previously been introduced by
宋文武 in commit 40310efde9b4a4f2cf98081d6cd10f843685ebb6.

Since this patch essentially reverts this commit, could you share your
thoughts on this, 宋文武?

Thanks,
Ludo’.
宋文武 April 15, 2025, 2:05 p.m. UTC | #2 
Ludovic Courtès <ludo@gnu.org> writes:

> [...]
>
> The last item does not say the whole story:
>
>> +++ b/gnu/system/shadow.scm
>> @@ -179,16 +179,6 @@ (define %default-bash-profile
>>  
>>  # Honor per-interactive-shell startup file
>>  if [ -f ~/.bashrc ]; then . ~/.bashrc; fi
>> -
>> -# Merge search-paths from multiple profiles, the order matters.
>> -eval \"$(guix package --search-paths \\
>> --p $HOME/.config/guix/current \\
>> --p $HOME/.guix-home/profile \\
>> --p $HOME/.guix-profile \\
>> --p /run/current-system/profile)\"
>> -
>> -# Prepend setuid programs.
>> -export PATH=/run/setuid-programs:$PATH
>>  "))
>
> I am the one who suggested to not invoke ‘guix package’ from shell
> startup files:
>
>   https://issues.guix.gnu.org/77035#8-lineno17
>
> However, I hadn’t realized that this had previously been introduced by
> 宋文武 in commit 40310efde9b4a4f2cf98081d6cd10f843685ebb6.
>
> Since this patch essentially reverts this commit, could you share your
> thoughts on this, 宋文武?

Hi, Sorry for that, it seems I had leave it partly commited in, without
a news entry and documentation.  The change about
/etc/skel/.bash_profile in commit 40310e is to provide a sensible
default to merge multiple profiles, which will fix
<https://issues.guix.gnu.org/20255>.  Users can easily change his own
~/.bash_profile to not invoke 'guix package', to avoid merge multiple
profiles.

Now I think it's better to remove it as the patch did, so we default to
not merge search paths from multiple profiles, and document how to
custom your ~/.bash_profile with 'guix package --search-paths` to merge
them.  Also mention the drawback when you choose to merge profiles for
search paths, mainly ABI incompatible problems from plugins (GIO
modules, Qt, etc.).

Thanks.
Ludovic Courtès April 15, 2025, 3:05 p.m. UTC | #3 
宋文武 <iyzsong@envs.net> writes:

> Hi, Sorry for that, it seems I had leave it partly commited in, without
> a news entry and documentation.  The change about
> /etc/skel/.bash_profile in commit 40310e is to provide a sensible
> default to merge multiple profiles, which will fix
> <https://issues.guix.gnu.org/20255>.  Users can easily change his own
> ~/.bash_profile to not invoke 'guix package', to avoid merge multiple
> profiles.
>
> Now I think it's better to remove it as the patch did, so we default to
> not merge search paths from multiple profiles,

OK, I see.

Any thoughts on the patch series Hilton submitted?

  https://issues.guix.gnu.org/77035

Thanks for your feedback,
Ludo’.
宋文武 April 19, 2025, 2:13 a.m. UTC | #4 
Hilton Chain <hako@ultrarare.space> writes:

> * gnu/system.scm (operating-system-etc-service)[profile]: Set umask first.
OK.

> Group environment variables setup with profile sourcing to avoid duplication.
It was already grouped in previous commit, maybe "Setup crucial
variables from all the default profiles."

> Export $GUILE_LOAD_PATH and $GUILE_LOAD_COMPILED_PATH for ‘guix pull’ profile.
Could be a seperated commit, but maybe not needed?  Since 'guix repl'
would ensure it.

> * gnu/system/shadow.scm (%default-bash-profile): Remove duplicated exports.
I think this better go into a seperated commit, and with comment or
cookbook doc for the merge search paths from multiple profiles usage.

> [...]
>  do
> -  if [ -f \"$GUIX_PROFILE/etc/profile\" ]
> -  then
> +  if [ -f \"$GUIX_PROFILE/etc/profile\" ]; then
>      . \"$GUIX_PROFILE/etc/profile\"
> -  else
> -    # At least define this one so that basic things just work
> -    # when the user installs their first package.
> -    export PATH=\"$GUIX_PROFILE/bin:$PATH\"
> +    if [ ! \"$GUIX_PROFILE\" = \"$HOME/.config/guix/current\" ]; then
> +      # Crucial variables that could be missing in the profiles' 'etc/profile'
> +      # because they would require combining both profiles.
> +      # FIXME: See <http://bugs.gnu.org/20255>.
> +      case $XDG_DATA_DIRS in
> +        *$GUIX_PROFILE/share*) ;;
> +        *) export XDG_DATA_DIRS=\"$GUIX_PROFILE/share${XDG_DATA_DIRS:+:}$XDG_DATA_DIRS\" ;;
> +      esac
> +      case $XDG_CONFIG_DIRS in
> +        *$GUIX_PROFILE/etc/xdg*) ;;
> +        *) export XDG_CONFIG_DIRS=\"$GUIX_PROFILE/etc/xdg${XDG_CONFIG_DIRS:+:}$XDG_CONFIG_DIRS\" ;;
> +      esac
> +      # Make sure libXcursor finds cursors installed into user or system profiles.
> +      # See <http://bugs.gnu.org/24445>

Commit 2acfafff set XCURSORPATH contains user and system profiles, but
not home profile as this patch did, this comment about 24445 can be
removed.

Otherwise, look good to me!
diff mbox series

Patch

diff --git a/gnu/system.scm b/gnu/system.scm
index c166222854..46e6729e74 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -1060,16 +1060,9 @@  (define* (operating-system-etc-service os)
         ;; Startup file for POSIX-compliant login shells, which set system-wide
         ;; environment variables.
         (profile    (mixed-text-file "profile"  "\
-# Crucial variables that could be missing in the profiles' 'etc/profile'
-# because they would require combining both profiles.
-# FIXME: See <http://bugs.gnu.org/20255>.
-export MANPATH=$HOME/.guix-profile/share/man:/run/current-system/profile/share/man
-export INFOPATH=$HOME/.guix-profile/share/info:/run/current-system/profile/share/info
-export XDG_DATA_DIRS=$HOME/.guix-profile/share:/run/current-system/profile/share
-export XDG_CONFIG_DIRS=$HOME/.guix-profile/etc/xdg:/run/current-system/profile/etc/xdg
-
-# Make sure libXcursor finds cursors installed into user or system profiles.  See <http://bugs.gnu.org/24445>
-export XCURSOR_PATH=$HOME/.icons:$HOME/.guix-profile/share/icons:/run/current-system/profile/share/icons
+# Set the umask, notably for users logging in via 'lsh'.
+# See <http://bugs.gnu.org/22650>.
+umask 022
 
 # Ignore the default value of 'PATH'.
 unset PATH
@@ -1091,32 +1084,73 @@  (define* (operating-system-etc-service os)
                     \"$HOME/.guix-profile\"        \\
                     \"$HOME/.config/guix/current\"
 do
-  if [ -f \"$GUIX_PROFILE/etc/profile\" ]
-  then
+  if [ -f \"$GUIX_PROFILE/etc/profile\" ]; then
     . \"$GUIX_PROFILE/etc/profile\"
-  else
-    # At least define this one so that basic things just work
-    # when the user installs their first package.
-    export PATH=\"$GUIX_PROFILE/bin:$PATH\"
+    if [ ! \"$GUIX_PROFILE\" = \"$HOME/.config/guix/current\" ]; then
+      # Crucial variables that could be missing in the profiles' 'etc/profile'
+      # because they would require combining both profiles.
+      # FIXME: See <http://bugs.gnu.org/20255>.
+      case $XDG_DATA_DIRS in
+        *$GUIX_PROFILE/share*) ;;
+        *) export XDG_DATA_DIRS=\"$GUIX_PROFILE/share${XDG_DATA_DIRS:+:}$XDG_DATA_DIRS\" ;;
+      esac
+      case $XDG_CONFIG_DIRS in
+        *$GUIX_PROFILE/etc/xdg*) ;;
+        *) export XDG_CONFIG_DIRS=\"$GUIX_PROFILE/etc/xdg${XDG_CONFIG_DIRS:+:}$XDG_CONFIG_DIRS\" ;;
+      esac
+      # Make sure libXcursor finds cursors installed into user or system profiles.
+      # See <http://bugs.gnu.org/24445>
+      case $XCURSOR_PATH in
+        *$GUIX_PROFILE/share/icons*) ;;
+        *) export XCURSOR_PATH=\"$GUIX_PROFILE/share/icons${XCURSOR_PATH:+:}$XCURSOR_PATH\" ;;
+      esac
+      # Allow Hunspell-based applications (IceCat, LibreOffice, etc.) to find
+      # dictionaries.
+      case $DICPATH in
+        *$GUIX_PROFILE/share/hunspell*) ;;
+        *) export DICPATH=\"$GUIX_PROFILE/share/hunspell${DICPATH:+:}$DICPATH\" ;;
+      esac
+      # Allow GStreamer-based applications to find plugins.
+      case $GST_PLUGIN_PATH in
+        *$GUIX_PROFILE/lib/gstreamer-1.0*) ;;
+        *) export GST_PLUGIN_PATH=\"$GUIX_PROFILE/lib/gstreamer-1.0${GST_PLUGIN_PATH:+:}$GST_PLUGIN_PATH\" ;;
+      esac
+    fi
+  fi
+  if [ \"$GUIX_PROFILE\" = \"$HOME/.config/guix/current\" ]; then
+    # Expose the latest Guix modules to Guile so guix shell and repls spawned by
+    # e.g. Geiser work out of the box.
+    case $GUILE_LOAD_PATH in
+      *$GUIX_PROFILE/share/guile/site/3.0*) ;;
+      *) export GUILE_LOAD_PATH=\"$GUIX_PROFILE/share/guile/site/3.0${GUILE_LOAD_PATH:+:}$GUILE_LOAD_PATH\" ;;
+    esac
+    case $GUILE_LOAD_COMPILED_PATH in
+      *$GUIX_PROFILE/lib/guile/3.0/site-ccache*) ;;
+      *) export GUILE_LOAD_COMPILED_PATH=\"$GUIX_PROFILE/lib/guile/3.0/site-ccache${GUILE_LOAD_COMPILED_PATH:+:}$GUILE_LOAD_COMPILED_PATH\" ;;
+    esac
   fi
+  # Make basic things just work when the user installs their first package.
+  case $PATH in
+    *$GUIX_PROFILE/bin*) ;;
+    *) export PATH=\"$GUIX_PROFILE/bin${PATH:+:}$PATH\" ;;
+  esac
+  # When INFOPATH is unset, add a trailing colon so Emacs searches
+  # 'Info-default-directory-list'.
+  case $INFOPATH in
+    *$GUIX_PROFILE/share/info*) ;;
+    *) export INFOPATH=\"$GUIX_PROFILE/share/info:$INFOPATH\" ;;
+  esac
+  # When MANPATH is unset, add a trailing colon so the system default search
+  # path is used.
+  case $MANPATH in
+    *$GUIX_PROFILE/share/man*) ;;
+    *) export MANPATH=\"$GUIX_PROFILE/share/man:$MANPATH\" ;;
+  esac
 done
 
-# Prepend privileged programs.
+# Prepend search paths not in a profile.
 export PATH=/run/privileged/bin:$PATH
-
-# Arrange so that ~/.config/guix/current/share/info comes first.
-export INFOPATH=\"$HOME/.config/guix/current/share/info:$INFOPATH\"
-
-# Set the umask, notably for users logging in via 'lsh'.
-# See <http://bugs.gnu.org/22650>.
-umask 022
-
-# Allow Hunspell-based applications (IceCat, LibreOffice, etc.) to
-# find dictionaries.
-export DICPATH=\"$HOME/.guix-profile/share/hunspell:/run/current-system/profile/share/hunspell\"
-
-# Allow GStreamer-based applications to find plugins.
-export GST_PLUGIN_PATH=\"$HOME/.guix-profile/lib/gstreamer-1.0\"
+export XCURSOR_PATH=\"$HOME/.icons:$XCURSOR_PATH\"
 
 if [ -n \"$BASH_VERSION\" -a -f /etc/bashrc ]
 then
diff --git a/gnu/system/shadow.scm b/gnu/system/shadow.scm
index b68a818871..cdb804da18 100644
--- a/gnu/system/shadow.scm
+++ b/gnu/system/shadow.scm
@@ -179,16 +179,6 @@  (define %default-bash-profile
 
 # Honor per-interactive-shell startup file
 if [ -f ~/.bashrc ]; then . ~/.bashrc; fi
-
-# Merge search-paths from multiple profiles, the order matters.
-eval \"$(guix package --search-paths \\
--p $HOME/.config/guix/current \\
--p $HOME/.guix-home/profile \\
--p $HOME/.guix-profile \\
--p /run/current-system/profile)\"
-
-# Prepend setuid programs.
-export PATH=/run/setuid-programs:$PATH
 "))
 
 (define %default-zprofile