Subject: Re: [PATCH 0/9] VTE integration support / Shell startup files refactor

From: Maxim Cournoyer <[email-masked]>
Date: Mon, 12 May 2025 22:12:26 +0900

Hi,

Rutherther <[email-masked]> writes:

> Hi Maxim,
>
> thank you for this. I think it's important for users to be able to
> extend their system profile and bashrc files, currently it was hard to
> do. Basically the only way was to modify etc-service-type in
> essential-services, removing the profile and bashrc completely, and
> adding your own contents.

Glad to know that'll be useful!

> I have a few comments that I am posting before I actually tested it just
> based on the changes:
>
> I think that the docs for etc-profile-d-service-type could be more
> specific about the files accepted - that only .sh files are going to
> have some effect.  Additionally the example plain-file is wrong,
> plain-file accepts two arguments, name and content. Here the name is
> quite important as it has to have the .sh extension for the file to be sourced.

Thanks for spotting this.  I'll fix it, and emphasize more that only
files of the @file{.sh} extension are considered.

> As for the bash modification, is is a good idea to go through grafts?
> Aren't grafts mainly meant for security fixes as to no changes are
> introduced? How can we make sure this change is not going to break any
> of the packages using bash? I understand we cannot rebuild the whole
> world via this patch series if it is to be applied to master, but this
> doesn't feel right, maybe those changes to bash should go on a team branch?

A full world rebuild seems very expensive for this change alone.  The
only thing this switch does is cause Bash to source /etc/bashrc when it
also sources also ~/.bashrc.  Since there is no /etc/bashrc in the build
container, we shoud be good in terms of not breaking anything build wise.

> It's not super important, but do you happen to know why bash-completion
> package provides profile.d directory for bash_completion.sh that should
> actually be bashrc? Maybe it should be in etc/bashrc.d output dir
> instead - that the package should be adapted?

That seems to come from Fedora, and maybe other distributions, that have
this in their /etc/bashrc file:

--8<---------------cut here---------------start------------->8---
    # Only display echos from profile.d scripts if we are no login shell
    # and interactive - otherwise just process them to set envvars
    for i in /etc/profile.d/*.sh; do
        if [ -r "$i" ]; then
            if [ "$PS1" ]; then
                . "$i"
            else
                . "$i" >/dev/null
            fi
        fi
    done

    unset i
--8<---------------cut here---------------end--------------->8---

So on these systems, /etc/profile.d/ is used to extend Bash too.  That
means these scripts are limited to be POSIX-compliant, as /etc/profile
also source these, which seems an odd requirement to me for configuring
Bash :-).  In practice, what vte.sh does for example is check if the
Shell is Bash or Zsh, and return early if it isn't.  Not as clean as
having a /etc/bashrc.d, I'd say.  Let's start some new trend ;-).

> I don't think removing the PS1 from guix home bashrc is appropriate. I think
> the PS1 is mostly for foreign distros, not for Guix System, it's even
> mentioned in the comment that it's for distros that don't provide a good PS1.

It's not removed, but moved, as the change log says for the "system:
Factorize bashrc default configuration." commit:

--8<---------------cut here---------------start------------->8---
* gnu/home/services/shells.scm (add-bash-configuration): Do not set PS1, now
part of %default-bashrc.
--8<---------------cut here---------------end--------------->8---

Guix home uses the %default-bashrc, so it's covered.  It's something
I've tried paying attention to, it's good you saw that too.

> Similar question goes to removing sourcing of /etc/bashrc. Was this
> because of Guix System or because of foreign distros? I don't know here.

Both, I think!  The home-shell-service-type uses the bash package from
Guix, so it's safe to assume /etc/bashrc will now be sourced anytime
~/.bashrc is.

> Note that users not using %base-services are going to get no bash
> completions after this update. Maybe there should be news entry about
> the bashrc-d and profile-d services?

Good idea too!  I'll write one and send in a v2.

-- 
Thanks,
Maxim