[bug#78308,0/9] VTE integration support / Shell startup files refactor

The aim of this series was ultimately to add VTE integration support out of
the box, resolving <https://issues.guix.gnu.org/72172>.  In order to do so, I
introduce the new etc-profile-d-service-type and etc-bashrc-d-service-type
service types.

While at it, define SYS_BASHRC in our Bash package to treat /etc/bashrc as a
first class startup file, and get rid of /etc/skel/.bashrc, whose
functionality is moved to /etc/bashrc or /etc/bashrc.d/aliases.sh and
/etc/bashrc.d/bash_completion.sh.

To test, I've used:

--8<---------------cut here---------------start------------->8---
make check-system TESTS='basic openssh dropbear'
--8<---------------cut here---------------end--------------->8---

as well as live testing in a GNOME desktop VM to validate the VTE integration.

Thank you,

Maxim Cournoyer (9):
  system: Source scripts from the /etc/profile.d directory.
  services: Add etc-profile-d-service-type.
  gnu: bash: Define the SYS_BASHRC macro.
  system: Source scripts from the /etc/bashrc.d directory.
  services: Add etc-bashrc-d-service-type.
  system: Migrate sourcing bash_completion.sh to
    etc-bashrc-d-service-type.
  system: Factorize bashrc default configuration.
  services: Add vte-integration-service-type.
  services: Add vte-integration-service-type to %desktop-services.

 doc/guix.texi                |  57 ++++++++++++++++-
 gnu/home/services/shells.scm |  14 ++---
 gnu/packages/bash.scm        |  20 ++++++
 gnu/services.scm             | 116 +++++++++++++++++++++++++++++++++++
 gnu/services/base.scm        |   5 +-
 gnu/services/desktop.scm     |   4 ++
 gnu/system.scm               |  43 ++++++-------
 gnu/system/shadow.scm        |  21 +++----
 gnu/tests/base.scm           |  45 +++++++++++++-
 9 files changed, 278 insertions(+), 47 deletions(-)


base-commit: 9d9a6291c4e61f3af71e94e549926bd9905e99db
  

Hi Maxim

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> * etc/news.scm (channel-news): New entry.
[...]
> + (entry (commit "XXX")
> +        (title
> +         (en "New services for /etc/profile.d and /etc/bashrc.d"))
> +        (body
> +         (en "Two new Shepherd services, @code{etc-profile-d-service-type} and
> +@code{etc-bashrc-d-service-type}, can now be used to configure and extend your
>
these are not Shepherd services, right?

also, I wonder if `etc/profile` produced by `build-etc/profile` should
also source files in corresponing `etc/profile.d`. This would allow
packages install shell profile extensions and it would fix e.g.
https://issues.guix.gnu.org/44997
  

Hi Sergey,

Sergey Trofimov <sarg@sarg.org.ru> writes:

> Hi Maxim
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>
>> * etc/news.scm (channel-news): New entry.
> [...]
>> + (entry (commit "XXX")
>> +        (title
>> +         (en "New services for /etc/profile.d and /etc/bashrc.d"))
>> +        (body
>> +         (en "Two new Shepherd services, @code{etc-profile-d-service-type} and
>> +@code{etc-bashrc-d-service-type}, can now be used to configure and extend your
>>
> these are not Shepherd services, right?

At the core, they are, but they are wrapped with some sugar in Guix, so
perhaps I can say just 'services' or 'Guix services'.

> also, I wonder if `etc/profile` produced by `build-etc/profile` should
> also source files in corresponing `etc/profile.d`. This would allow
> packages install shell profile extensions and it would fix e.g.
> https://issues.guix.gnu.org/44997

That would be useful, but there's one issue I see, is that the Red
Hat/Fedora have standardized on /etc/profile.d/ as the place to put any
shell extension scripts, which are even sourced for example by
/etc/bashrc, which is a bit odd to me: /etc/profile is for interactive
login shells, and /etc/profile.d should logically follow, it seems.

Having /etc/profile.d instead of /etc/bashrc.d also means that scripts
placed there must be POSIX compliant or contain conditional guards for
the specific Shell they target.
  

Hi Maxim,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

>>> * etc/news.scm (channel-news): New entry.
>> [...]
>>> + (entry (commit "XXX")
>>> +        (title
>>> +         (en "New services for /etc/profile.d and /etc/bashrc.d"))
>>> +        (body
>>> +         (en "Two new Shepherd services, @code{etc-profile-d-service-type} and
>>> +@code{etc-bashrc-d-service-type}, can now be used to configure and extend your
>>>
>> these are not Shepherd services, right?
>
> At the core, they are, but they are wrapped with some sugar in Guix,
>

Could you please explain this bit? As I see these are just computed
files that produce `profile.d` and `bashrc.d` unions which are then
sourced by a piece of code placed in a well-known file. When I read
"shepherd", I expect that the service is managed with "herd".

> 
> so perhaps I can say just 'services' or 'Guix services'.

>> also, I wonder if `etc/profile` produced by `build-etc/profile` should
>> also source files in corresponing `etc/profile.d`. This would allow
>> packages install shell profile extensions and it would fix e.g.
>> https://issues.guix.gnu.org/44997
>
> That would be useful, but there's one issue I see, is that the Red
> Hat/Fedora have standardized on /etc/profile.d/ as the place to put any
> shell extension scripts, which are even sourced for example by
> /etc/bashrc, which is a bit odd to me: /etc/profile is for interactive
> login shells, and /etc/profile.d should logically follow, it seems.
>
> Having /etc/profile.d instead of /etc/bashrc.d also means that scripts
> placed there must be POSIX compliant or contain conditional guards for
> the specific Shell they target.

It seems I've phrased it a bit vague. I was writing about `profile.d`
directories in any guix profile (be it `guix home`, `guix shell` or
whatever). What I propose is that `<...>/etc/profile` (produced by
`build-etc/profile` procedure) would include the snippet to source
`profile.d`. The snippet could find the dir relative to its own location
so that the very same code could be placed as in the global file
(/etc/profile) as in per-profile files.
  

Hi Sergey,

Sergey Trofimov <sarg@sarg.org.ru> writes:

> Hi Maxim,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>
>>>> * etc/news.scm (channel-news): New entry.
>>> [...]
>>>> + (entry (commit "XXX")
>>>> +        (title
>>>> +         (en "New services for /etc/profile.d and /etc/bashrc.d"))
>>>> +        (body
>>>> +         (en "Two new Shepherd services, @code{etc-profile-d-service-type} and
>>>> +@code{etc-bashrc-d-service-type}, can now be used to configure and extend your
>>>>
>>> these are not Shepherd services, right?
>>
>> At the core, they are, but they are wrapped with some sugar in Guix,
>>
>
> Could you please explain this bit? As I see these are just computed
> files that produce `profile.d` and `bashrc.d` unions which are then
> sourced by a piece of code placed in a well-known file. When I read
> "shepherd", I expect that the service is managed with "herd".

Ah, you are right!  Re-reading (info "(guix) Service Composition") made
that clear to me.  I always assumed services had to be sequenced by
Shepherd no matter the type, but it seems it is Guix System that is in
charge during the early boot here, when running the activation script,
IIUC.

>>> also, I wonder if `etc/profile` produced by `build-etc/profile` should
>>> also source files in corresponing `etc/profile.d`. This would allow
>>> packages install shell profile extensions and it would fix e.g.
>>> https://issues.guix.gnu.org/44997
>>
>> That would be useful, but there's one issue I see, is that the Red
>> Hat/Fedora have standardized on /etc/profile.d/ as the place to put any
>> shell extension scripts, which are even sourced for example by
>> /etc/bashrc, which is a bit odd to me: /etc/profile is for interactive
>> login shells, and /etc/profile.d should logically follow, it seems.
>>
>> Having /etc/profile.d instead of /etc/bashrc.d also means that scripts
>> placed there must be POSIX compliant or contain conditional guards for
>> the specific Shell they target.
>
> It seems I've phrased it a bit vague. I was writing about `profile.d`
> directories in any guix profile (be it `guix home`, `guix shell` or
> whatever). What I propose is that `<...>/etc/profile` (produced by
> `build-etc/profile` procedure) would include the snippet to source
> `profile.d`. The snippet could find the dir relative to its own location
> so that the very same code could be placed as in the global file
> (/etc/profile) as in per-profile files.

Thank you for clarifying what you meant.  So just rephrasing to make
extra sure, what you would like to see is that every profile generated
would have in their etc/profile generated file something sourcing its
etc/profile.d/*.sh files, so that installing e.g. bash_completion in a
profile would automatically have its etc/profile.d/bash_completion.sh
file sourced, correct?

I'm not sure how useful that would be, because IIRC the
$profile/etc/profile of a generated profile is only ever sourced:

1. When it appears at /etc/profile, e.g. mapped there within a
container
2. The shell is started as a login shell, e.g. 'bash --login'

Note the --login requirement; which would not be in effect for example
when entering a 'guix shell' (which spawns 'sh') by default.  The
etc/profile could be sourced manually, of course, but that seems to
defeat the purpose of having it there in the first place.

Another idea would be to have /etc/bashrc source /etc/profile.d like
done in Fedora; that's a bit odd and I don't like it much for reasons
explained earlier, but it has the benefit of being compatible with the
packages like flatpak and vte that install things under etc/profile.d to
extend the non-login shells as well.  That could be made to work
automatically for the fixed location like
/run/current-system/profile/etc/profile.d for system-installed packages
as well as $HOME/.profile/etc/profile.d/, and even for non-containerized
environments via $GUIX_ENVIRONMENT/etc/profile.d/.  It wouldn't work in
containerized environment; this would need adding a /etc/bashrc inside
the container that would source $GUIX_ENVIROMNENT/etc/profile.d.
  

Hi Maxim,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

[...]
>>>> also, I wonder if `etc/profile` produced by `build-etc/profile` should
>>>> also source files in corresponing `etc/profile.d`. This would allow
>>>> packages install shell profile extensions and it would fix e.g.
>>>> https://issues.guix.gnu.org/44997
>>>
>>> That would be useful, but there's one issue I see, is that the Red
>>> Hat/Fedora have standardized on /etc/profile.d/ as the place to put any
>>> shell extension scripts, which are even sourced for example by
>>> /etc/bashrc, which is a bit odd to me: /etc/profile is for interactive
>>> login shells, and /etc/profile.d should logically follow, it seems.
>>>
>>> Having /etc/profile.d instead of /etc/bashrc.d also means that scripts
>>> placed there must be POSIX compliant or contain conditional guards for
>>> the specific Shell they target.
>>
>> It seems I've phrased it a bit vague. I was writing about `profile.d`
>> directories in any guix profile (be it `guix home`, `guix shell` or
>> whatever). What I propose is that `<...>/etc/profile` (produced by
>> `build-etc/profile` procedure) would include the snippet to source
>> `profile.d`. The snippet could find the dir relative to its own location
>> so that the very same code could be placed as in the global file
>> (/etc/profile) as in per-profile files.
>
> Thank you for clarifying what you meant.  So just rephrasing to make
> extra sure, what you would like to see is that every profile generated
> would have in their etc/profile generated file something sourcing its
> etc/profile.d/*.sh files, so that installing e.g. bash_completion in a
> profile would automatically have its etc/profile.d/bash_completion.sh
> file sourced, correct?

Exactly. Basically, install the "for i in /etc/profile.d/*.sh; do"
snippet adjusted to look for `profile.d` relative to current file.

>
> I'm not sure how useful that would be, because IIRC the
> $profile/etc/profile of a generated profile is only ever sourced:
>
> 1. When it appears at /etc/profile, e.g. mapped there within a
> container

... or is one of the default user profiles:

--8<---------------cut here---------------start------------->8---
# from /etc/profile

for profile in "$HOME/.guix-profile"        \
               "$HOME/.guix-home/profile"   \
               "$HOME/.config/guix/current"
do
  if [ -f "$profile/etc/profile" ]
  then
    # Load the user profile's settings.
    GUIX_PROFILE="$profile" ; \
    . "$profile/etc/profile"
  else
    # At least define this one so that basic things just work
    # when the user installs their first package.
    export PATH="$profile/bin:$PATH"
  fi
done
--8<---------------cut here---------------end--------------->8---

this alone is already helpful as it resolves the flatpak bug
granted, custom profiles are not sourced by `/etc/profile`, but then
it's a power-user territory and they'll sort it out themselves.

> 2. The shell is started as a login shell, e.g. 'bash --login'
> Note the --login requirement; which would not be in effect for example
> when entering a 'guix shell' (which spawns 'sh') by default.  The
> etc/profile could be sourced manually, of course, but that seems to
> defeat the purpose of having it there in the first place.
>

This is a tougher nut to crack, could be done separately. On a first
glance it seems logical that 'guix shell' would start login shells,
however this would lead to duplicated search paths as these would be set
when starting the container and then when sourcing /etc/profile inside.
It could be remedied with such steps:

- Extract mandatory search path variables (PATH=, MANPATH=, etc) to a
  separate file (e.g. profile.env).

- Prevent 'profile.env' files to be sourced twice:
  if [ "GUIX_PROFILE_<hash>_LOADED" -ne 1 ]; then
     export PATH=...
     ...
     GUIX_PROFILE_<hash>_LOADED=1
  fi

- 'guix/scripts/environment.scm' procedures should be adjusted to
  additionally set 'GUIX_PROFILE_<hash>_LOADED' env variable to avoid
  duplication of search paths

>
> Another idea would be to have /etc/bashrc source /etc/profile.d like
> done in Fedora; that's a bit odd and I don't like it much for reasons
> explained earlier, but it has the benefit of being compatible with the
> packages like flatpak and vte that install things under etc/profile.d to
> extend the non-login shells as well.

I also don't like this idea. Why would non-login shells run
`/etc/profile.d`? Aren't they launched in environment where login
happened and therefore `/etc/profile` got applied already?

> That could be made to work automatically for the fixed location like
> /run/current-system/profile/etc/profile.d for system-installed
> packages as well as $HOME/.profile/etc/profile.d/, and even for
> non-containerized environments via $GUIX_ENVIRONMENT/etc/profile.d/.
> It wouldn't work in containerized environment; this would need adding
> a /etc/bashrc inside the container that would source
> $GUIX_ENVIROMNENT/etc/profile.d.
  

Hi Sergey,

Thanks for the brainstorm / ideas, even patch!

[...]

> I also don't like this idea. Why would non-login shells run
> `/etc/profile.d`? Aren't they launched in environment where login
> happened and therefore `/etc/profile` got applied already?

That's what I initially thought, and I originally didn't see an
immediate need for etc-bashrc-d-service-type, but upon testing with the
/etc/profile.d/vte.sh file installed for example, at least in GNOME
Console it wouldn't take effect.  Perhaps the environment is cleaned up
by GNOME Console and it expects the shell, e.g. Bash, to source
/etc/profile.d/vte.sh from bashrc (GNOME Console starts the shell as an
interactive, non-login shell).  I had tested in an actual VM.

Are you sure that it would help for the flatpak cases (have you actually
tested it?).

Reading flatpak.sh, I assume it would work, as long as flatpak is
installed to the given profile, as it needs to be available on PATH when
/etc/profile is sourced, which should be the case if the snippet is
added below the existing variable exports in ~/.guix-profile/etc/profile
(which contain PATH=).

I'll try taking a look at your snippet.  The guard avoiding double
sourcing /etc/profile looks interesting, perhaps already useful to guard
against bug#23118 for example?
  

Hi again,

Sergey Trofimov <sarg@sarg.org.ru> writes:

I just had a look a vte.sh, and I think I now understand why that
particular one wouldn't work as /etc/profile.d/vte.sh:

--8<---------------cut here---------------start------------->8---
# Not bash or zsh?
[ -n "${BASH_VERSION:-}" -o -n "${ZSH_VERSION:-}" ] || return 0

# Not an interactive shell?
[[ $- == *i* ]] || return 0

# Not running under vte?
[ "${VTE_VERSION:-0}" -ge 3405 ] || return 0

# TERM not supported?
case "$TERM" in
    xterm*|vte*|gnome*) :;;
    *) return 0 ;;
esac
--8<---------------cut here---------------end--------------->8---

I guess at least the VTE_VERSION check would cause the script to return
early for the login shell, which is not run from a VTE-powered terminal.