oven-sh / bun

Incredibly fast JavaScript runtime, bundler, test runner, and package manager – all in one
https://bun.sh
Other
73.18k stars 2.68k forks source link

On RHEL-derivates bun install script should use `.bashrc.d` instead of `.bashrc` #10326

Open Coachonko opened 5 months ago

Coachonko commented 5 months ago

What version of Bun is running?

1.1.4+fbe2fe0c3

What platform is your computer?

Linux 5.14.0-362.24.1.el9_3.x86_64 x86_64 x86_64

What steps can reproduce the bug?

On RHEL derivates such as Rocky, Alma, Euro, Scientific, etc. there exists a .bashrc.d directory. The Bun install script should create a file in this directory instead of modifying the .bashrc file.

What is the expected behavior?

No response

What do you see instead?

No response

Additional information

No response

Woovie commented 5 months ago

Can you show where you derive this feature as being native to any of these distributions? My RHEL 9.2 instance does not have it.

Is it a particular version of bash? The man page for bash doesn't mention it. Everything I see using this solution online is using an import in their .bashrc like for example:

for file in ~/.bashrc.d/* ; do
    if [ -f "$file" ]; then
        source "$file"
    fi
done

I think it's bad to assume things that are not a standard to bash. If you find a .bashrc.d folder and the user doesn't have an import loop like this, it is not going to be included. Unless bash is natively handling this, it just seems like a bad idea that will lead to user confusion to me.

Coachonko commented 5 months ago

Can you show where you derive this feature as being native to any of these distributions? My RHEL 9.2 instance does not have it.

Is it a particular version of bash? The man page for bash doesn't mention it. Everything I see using this solution online is using an import in their .bashrc like for example:

for file in ~/.bashrc.d/* ; do
    if [ -f "$file" ]; then
        source "$file"
    fi
done

I think it's bad to assume things that are not a standard to bash. If you find a .bashrc.d folder and the user doesn't have an import loop like this, it is not going to be included. Unless bash is natively handling this, it just seems like a bad idea that will lead to user confusion to me.

I think you're misunderstanding something here: Bash does not do this automatically. Fedora introduced the .bashrc.d in 2020. In RHEL it was introduced with version 9. It is not good practice in Fedora/RHEL-based distros to dump anything in the .bashrc file.

While it works, in these distributions the .bashrc file is meant to be left untouched and user customizations to be located in the .bashrc.d directory. If you have ever packaged rpms, when the bash package is updated, changes to the .bashrc file may be lost according to the behavior described here

From the bash.spec file log

* Fri Dec  4 14:44:06 CET 2020 Siteshwar Vashisht <svashisht@redhat.com> - 5.0.17-3
- Enable sourcing files from ~/.bashrc.d
  Resolves: #1726397

And the default .bashrc file contained in the source file named dot-bashrc that comes with the first release of RHEL 9.0

# .bashrc

# Source global definitions
if [ -f /etc/bashrc ]; then
    . /etc/bashrc
fi

# User specific environment
if ! [[ "$PATH" =~ "$HOME/.local/bin:$HOME/bin:" ]]
then
    PATH="$HOME/.local/bin:$HOME/bin:$PATH"
fi
export PATH

# Uncomment the following line if you don't like systemctl's auto-paging feature:
# export SYSTEMD_PAGER=

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

unset rc
Woovie commented 5 months ago

Thanks, I was trying to understand who introduced the standard and couldn't find clear information. I am unsure why my hosts do not also have this, but perhaps I only checked the root user.

skull-squadron commented 1 week ago

First, I'm all for componentized and modular rc scripts. My home person dotfiles are modularized with POSIXly-common and few shell-specific bits for interactive and non-interactive scenarios where applicable per shell while sharing as much as possible. (I don't use C- and Korn-family shells where possible.)

This isn't a valid invariant assumption. While it maybe in /etc/skel/.bashrc for most users, it's not used for root, which is the default user in most Docker containers. (Knee-jerk PSA about running things as root, but it's really a simulated one. And while non-root isolation is possible, it generally requires some/much additional, nonstandard effort.)

Plus, dotfiles customization can and will change this.

And, globbing (~/.bashrc.d/*) doesn't ensure explicit ordering and the order of includes is important. (While zsh has glob qualifiers, the way to ensure determinism is to stable sort files prior to sourcing.)

In case you were wondering, deterministic sourcing logic in bash ```bash # Files should named with the format ~/.bashrc.d/NNNN-whatever # 0xxx - early user customizations # 1xxx - standard, provided boilerplate # 2000+ - later user customizations if [[ -d ~/.bashrc.d ]]; then while read -d '' -r rc; do if [[ -f "$filename" ]]; then . "$filename" fi done < <(printf '%s\0' ~/.bashrc.d/* | LC_ALL=C sort -nz) fi unset rc ```

Trying to get too clever and then fiddling without user confirmation across the multitude of environment possibilities will lead to breakage and angry users.

The safest option is to offer make the most compatible preferred as a default. Otherwise, offer a choice not to modify things, and then either a snippet (or snippet generator) or a script that can be symlinked (per shell).

~/.bashrc on CentOS 9 Stream Repro: `docker run --rm quay.io/centos/centos:stream9 bash -c 'cat ~/.bashrc'` ```bash # .bashrc # Source global definitions if [ -f /etc/bashrc ]; then . /etc/bashrc fi # User specific environment if ! [[ "$PATH" =~ "$HOME/.local/bin:$HOME/bin:" ]] then PATH="$HOME/.local/bin:$HOME/bin:$PATH" fi export PATH # Uncomment the following line if you don't like systemctl's auto-paging feature: # export SYSTEMD_PAGER= # User specific aliases and functions alias rm='rm -i' alias cp='cp -i' alias mv='mv -i' ```

It appears to be the same on the Fedora 40 (fedora), Alma 9.4 (almalinux), and RHEL 9.4 (redhat/ubi9) containers as one would expect. (Rocky lacks maintained (secure) EL-compatible 9.4+ containers.)