Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bash: Fix bashisms being applied to non-bash shells #4334

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

bash: Fix bashisms being applied to non-bash shells #4334

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ec04084
bash: Fix bashisms being applied to non-bash shells
Nov 12, 2024
9cb4757
zsh: Re-add sourcing of profile.d files
Nov 12, 2024
6405097
bash: Fix 50-prompt.sh to work with zsh as well as bash
TraceyC77 Jan 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion packages/b/bash/files/profile/50-history.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
# Begin /usr/share/defaults/etc/profile.d/50-history.sh

# Append to history file on exit instead of overwrite (parallel terminals)
shopt -s histappend
# shopt is bash only
if [ $SHELL = "/usr/bin/bash" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It turns out that

Suggested change
if [ $SHELL = "/usr/bin/bash" ]; then
if [ -n "$BASH_VERSION" ]; then

... is a very effective way to check if the running shell is bash.

shopt -s histappend
fi

export HISTSIZE=1500
export HISTIGNORE="&:[bf]g:exit"
Expand Down
42 changes: 36 additions & 6 deletions packages/b/bash/files/profile/50-prompt.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,53 @@
# Begin /usr/share/defaults/etc/profile.d/50-prompt.sh

# We cant use $SHELL since we need the *current* shell.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the prompt is only really relevant for interactive shells, maybe return early if the terminal is not interactive?

Suggested change
# We cant use $SHELL since we need the *current* shell.
# No point parsing this file for non-interactive shells that don't need a prompt
[ -t 0 ] || return 0
# We cant use $SHELL since we need the *current* shell.

# The user may switch to a different shell than their default
# e.g. $SHELL is zsh but user switches to bash temporarily

current_shell=$(readlink /proc/$$/exe)

endchar="\$"
if [ "$UID" = "0" ]; then
endchar="#"
elif [[ "$current_shell" =~ zsh ]]; then
Copy link
Contributor

@ermo ermo Jan 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure this logic is sound.

What I would do is to add a check like this:

# Default that doesn't look like anything a shell would normally ship
endchar="?" # pick a suitable default here
if [ -n "$BASH_VERSION" ]; then
    endchar="\$"
elif [ -n "$ZSH_VERSION" ]; then
    endchar="%#"
fi

... as those change in a shell-defined way, depending on whether a shell is running as euid 0 or not.

endchar='%'
fi

FG="\[\033[38;5;081m\]"
BG="\[\033[38;5;245m\]"
AT="\[\033[38;5;245m\]"
HCOLOR="\[\033[38;5;206m\]"
# Prefer tput to ANSI codes since they work with bash and zsh
# and are much easier to read
grey=$(tput setaf 245)
pink=$(tput setaf 206)
cyan=$(tput setaf 81)
reset_color=$(tput sgr0)

PS1="${FG}\u${AT}@${HCOLOR}\H ${BG}\w ${FG}$endchar \[\e[0m\]"
FG=$cyan
DIR=$grey
AT=$grey
HCOLOR=$pink

# bash and zsh handle prompt setup and character escaping differently

if [[ "$current_shell" =~ bash ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is another instance where the [ -n "$BASH_VERSION"] or [ -n "$ZSH_VERSION" ] idiom might be nice?

PS1="\[${FG}\]\u\[${AT}\]@\[${HCOLOR}\]\H \[${DIR}\]\w \[${FG}\]$endchar \[${reset_color}\]"
elif [[ "$current_shell" =~ zsh ]]; then
PS1="%{${FG}%}%n%{${AT}%}@%{${HCOLOR}%}%m %{${DIR}%}%1~ %{${FG}%}$endchar# ${reset_color}"
Copy link
Contributor

@ermo ermo Jan 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taken together with my suggestion above, the # after $endchar is no longer necessary.

Note also that this uses double brackets, which is not very POSIX-y.

fi

unset FG
unset BG
unset AT
unset HCOLOR
if [ $SHELL != "/bin/zsh" ]; then
unset grey
unset pink
unset cyan
unset reset_color
unset endchar

# shopt is bash only
if [[ "$current_shell" =~ bash ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more instance where [ -n "$BASH_VERSION" ] might be fruitful (and more POSIX-y).

shopt -s checkwinsize
fi

unset current_shell

# End /usr/share/defaults/etc/profile.d/50-prompt.sh
11 changes: 11 additions & 0 deletions packages/b/bash/files/profile/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
Files in this directory must have code that is POSIX compliant, since the file is loaded by all BOURNE compatible shells
If you must use a bash-only function like shopt it must be put in a block that will load it only for bash

e.g.

```
current_shell=$(readlink /proc/$$/exe)
if [ $current_shell = "/usr/bin/bash" ]; then
shopt -s checkwinsize
fi
```
10 changes: 0 additions & 10 deletions packages/b/bash/files/profile/profile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,4 @@ if [ -d /etc/profile.d ] ; then
unset script
fi

# User specific aliases and functions
if [ -d ~/.bashrc.d ]; then
for rc in ~/.bashrc.d/*; do
if [ -f "$rc" ]; then
. "$rc"
fi
done
unset rc
fi

# End /usr/share/defaults/etc/profile
10 changes: 10 additions & 0 deletions packages/b/bash/files/skel/.bashrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,11 @@
source /usr/share/defaults/etc/profile

# User specific bash aliases and functions
if [ -d ~/.bashrc.d ]; then
for rc in ~/.bashrc.d/*; do
if [ -f "$rc" ]; then
. "$rc"
fi
done
unset rc
fi
2 changes: 1 addition & 1 deletion packages/b/bash/package.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name : bash

Check notice on line 1 in packages/b/bash/package.yml

View workflow job for this annotation

GitHub Actions / Checks 

This package is included in the ISO. Consider validating the functionality in a newly built ISO.
version : 5.2.37
release : 84
release : 85
source :
- https://ftp.gnu.org/gnu/bash/bash-5.2.37.tar.gz : 9599b22ecd1d5787ad7d3b7bf0c59f312b3396d1e281175dd1f8a4014da621ff
license :
Expand Down
14 changes: 7 additions & 7 deletions packages/b/bash/pspec_x86_64.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
<Name>bash</Name>
<Homepage>https://www.gnu.org/software/bash</Homepage>
<Packager>
<Name>Reilly Brogan</Name>
<Email>[email protected]</Email>
<Name>Tracey Clark</Name>
<Email>[email protected]</Email>
</Packager>
<License>GPL-3.0-or-later</License>
<PartOf>system.base</PartOf>
Expand Down Expand Up @@ -135,7 +135,7 @@
</Description>
<PartOf>programming.devel</PartOf>
<RuntimeDependencies>
<Dependency release="84">bash</Dependency>
<Dependency release="85">bash</Dependency>
</RuntimeDependencies>
<Files>
<Path fileType="header">/usr/include/bash/alias.h</Path>
Expand Down Expand Up @@ -201,12 +201,12 @@
</Files>
</Package>
<History>
<Update release="84">
<Date>2024-10-18</Date>
<Update release="85">
<Date>2025-01-03</Date>
<Version>5.2.37</Version>
<Comment>Packaging update</Comment>
<Name>Reilly Brogan</Name>
<Email>[email protected]</Email>
<Name>Tracey Clark</Name>
<Email>[email protected]</Email>
</Update>
</History>
</PISI>
11 changes: 11 additions & 0 deletions packages/z/zsh/files/skel/.zshrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
source /usr/share/defaults/etc/zsh/zprofile

# User specific zsh scripts and functions
if [ -d ~/.zshrc.d ]; then
for rc in ~/.zshrc.d/*; do
if [ -f "$rc" ]; then
. "$rc"
fi
done
unset rc
fi
54 changes: 10 additions & 44 deletions packages/z/zsh/files/zprofile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,51 +1,17 @@
## DO NOT edit this file. Your changes will be overridden on package updates.
# Instead add your changes to a new profile script under /etc/profile.d/ (create this directory if it does not exist) for system wide use or
# ~/.zshrc.d for an individual user

# Begin /usr/share/defaults/etc/zprofile

if [ -f /etc/zprofile ]; then
source /etc/zprofile
elif [ -f /etc/zsh/zprofile ]; then
source /etc/zsh/zprofile
fi

# Source paths that may not be included by systemctl
if [ -f /usr/share/defaults/etc/profile.d/10-path.sh ]; then
emulate sh -c 'source /usr/share/defaults/etc/profile.d/10-path.sh'
fi

# Source paths not included by systemctl necessary for XDG_DATA_DIRS

if [ -f /usr/share/defaults/etc/profile.d/10-xdg.sh ]; then
emulate sh -c 'source /usr/share/defaults/etc/profile.d/10-xdg.sh'
fi

# Source paths not included by systemctl necessary for flatpaks to be found in the app menu and run

if [ -f /usr/share/defaults/etc/profile.d/70-flatpak.sh ]; then
emulate sh -c 'source /usr/share/defaults/etc/profile.d/70-flatpak.sh'
fi

# Source paths not included by systemctl necessary for snaps to be found in the app menu and run

if [ -f /usr/share/defaults/etc/profile.d/70-snapd.sh ]; then
emulate sh -c 'source /usr/share/defaults/etc/profile.d/70-snapd.sh'
fi

# Source gtk modules variable for menus

if [ -f /usr/share/defaults/etc/profile.d/appmenu-gtk2-module.sh ]; then
emulate sh -c 'source /usr/share/defaults/etc/profile.d/appmenu-gtk2-module.sh'
fi

# Paths for gawk

if [ -f /usr/share/defaults/etc/profile.d/gawk.sh ]; then
emulate sh -c 'source /usr/share/defaults/etc/profile.d/gawk.sh'
fi

# Environment for vte, zsh compatible

if [ -f /usr/share/defaults/etc/profile.d/vte.sh ]; then
source /usr/share/defaults/etc/profile.d/vte.sh
fi
# This should always be sourced regardless of the above files
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block should probably be at the top of the script, so the system zprofile in /etc can override vendor config settings...?

# Warning: Do not remove the line below, otherwise it will break the integrity of other packages which provide some scripts in /etc/profile.d/.
emulate sh -c 'source /usr/share/defaults/etc/profile'

# Do not source /etc/profile as Arch does
# This will source all other bash scripts in /etc/profile.d, some of which may not load properly.
# Some may also conflict with zsh frameworks or have other unintended consequences
# Additionally, Solus sources ~/.bashrc.d from /etc/profile which is not obvious to the user or expected in zsh
# End /usr/share/defaults/etc/zprofile
5 changes: 4 additions & 1 deletion packages/z/zsh/package.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name : zsh
version : '5.9'
release : 34
release : 35
source :
- https://sourceforge.net/projects/zsh/files/zsh/5.9/zsh-5.9.tar.xz : 9b8d1ecedd5b5e81fbf1918e876752a7dd948e05c1a0dba10ab863842d45acd5
homepage : https://www.zsh.org/
Expand Down Expand Up @@ -46,6 +46,9 @@ install : |
ln -srv $installdir/usr/bin/$FILE $installdir/bin/$FILE
done
popd

# Install defaults to /etc/skel
install -Dm0644 -t $installdir/etc/skel $pkgfiles/skel/.zshrc
check : |
unset LD_PRELOAD
%make check
5 changes: 3 additions & 2 deletions packages/z/zsh/pspec_x86_64.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
<Files>
<Path fileType="executable">/bin/zsh</Path>
<Path fileType="executable">/bin/zsh-5.9</Path>
<Path fileType="config">/etc/skel/.zshrc</Path>
<Path fileType="executable">/usr/bin/zsh</Path>
<Path fileType="executable">/usr/bin/zsh-5.9</Path>
<Path fileType="library">/usr/lib64/zsh/5.9/zsh/attr.so</Path>
Expand Down Expand Up @@ -1397,8 +1398,8 @@
</Files>
</Package>
<History>
<Update release="34">
<Date>2024-11-11</Date>
<Update release="35">
<Date>2025-01-01</Date>
<Version>5.9</Version>
<Comment>Packaging update</Comment>
<Name>Tracey Clark</Name>
Expand Down
Loading