cmd/shellenv.sh: make brew shellenv idempotent

[XuehaiPan]

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same change?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes? Here's an example.
• Have you successfully run brew style with your changes locally?
• Have you successfully run brew typecheck with your changes locally?
• Have you successfully run brew tests with your changes locally?

---

Make brew shellenv idempotent. This implementation simply a new environment variable HOMEBREW_SHELLENV_SET to avoid duplicate initialization.

$ /home/linuxbrew/.linuxbrew/bin/brew shellenv
export HOMEBREW_PREFIX="/home/linuxbrew/.linuxbrew";
export HOMEBREW_CELLAR="/home/linuxbrew/.linuxbrew/Cellar";
export HOMEBREW_REPOSITORY="/home/linuxbrew/.linuxbrew/Homebrew";
export PATH="/home/linuxbrew/.linuxbrew/bin:/home/linuxbrew/.linuxbrew/sbin${PATH+:$PATH}";
export MANPATH="/home/linuxbrew/.linuxbrew/share/man${MANPATH+:$MANPATH}:";
export INFOPATH="/home/linuxbrew/.linuxbrew/share/info:${INFOPATH:-}";
export HOMEBREW_SHELLENV_SET="/home/linuxbrew/.linuxbrew${HOMEBREW_SHELLENV_SET+:$HOMEBREW_SHELLENV_SET}";

$ eval "$(/home/linuxbrew/.linuxbrew/bin/brew shellenv)"

$ /home/linuxbrew/.linuxbrew/bin/brew shellenv
export HOMEBREW_PREFIX="/home/linuxbrew/.linuxbrew";
export HOMEBREW_CELLAR="/home/linuxbrew/.linuxbrew/Cellar";
export HOMEBREW_REPOSITORY="/home/linuxbrew/.linuxbrew/Homebrew";

I make HOMEBREW_SHELLENV_SET a colon separated list of brew prefixes rather than a constant to handle multi-instance of brew installation.

It has several limitations, but I think it doesn't matter. For example, it can not handle file redirection ([ ! -t 1 ] can not work):

$ eval "$(/home/linuxbrew/.linuxbrew/bin/brew shellenv)"

$ brew shellenv >> my_shell_profile

$ cat my_shell_profile
export HOMEBREW_PREFIX="/home/linuxbrew/.linuxbrew";
export HOMEBREW_CELLAR="/home/linuxbrew/.linuxbrew/Cellar";
export HOMEBREW_REPOSITORY="/home/linuxbrew/.linuxbrew/Homebrew";

$ HOMEBREW_SHELLENV_SET='' brew shellenv >> my_shell_profile

$ cat my_shell_profile
export HOMEBREW_PREFIX="/home/linuxbrew/.linuxbrew";
export HOMEBREW_CELLAR="/home/linuxbrew/.linuxbrew/Cellar";
export HOMEBREW_REPOSITORY="/home/linuxbrew/.linuxbrew/Homebrew";
export PATH="/home/linuxbrew/.linuxbrew/bin:/home/linuxbrew/.linuxbrew/sbin${PATH+:$PATH}";
export MANPATH="/home/linuxbrew/.linuxbrew/share/man${MANPATH+:$MANPATH}:";
export INFOPATH="/home/linuxbrew/.linuxbrew/share/info:${INFOPATH:-}";
HOMEBREW_SHELLENV_SET="/home/linuxbrew/.linuxbrew${HOMEBREW_SHELLENV_SET+:$HOMEBREW_SHELLENV_SET}";

Resolves Homebrew/install#559.

[carlocab]

I wonder if it's worth emitting a warning when brew shellenv is called with HOMEBREW_SHELLENV_SET set, since this likely happens when someone calls brew shellenv multiple times inadvertently (perhaps because one copied echo 'eval $(brew shellenv)' >> ~/.profile into their shell profile...)

[XuehaiPan]

I wonder if it's worth emitting a warning when brew shellenv is called with HOMEBREW_SHELLENV_SET set.

Then it will raise a warning when the user create a subshell or source the shell profile manually. Maybe variable SHLVL can help for the first situation. But for the second one, we cannot figure out whether brew shellenv is called by intentional or:

since this is likely happens when someone is calls brew shellenv multiple times inadvertently (perhaps because one copied echo 'eval $(brew shellenv)' >> ~/.profile into their shell profile...)

Besides, for Linux users with a desktop environment, the file ~/.profile maybe sourced at user desktop session startup. For example, the ~/.profile will be sourced at the beginning of a GNOME session. Then if you open a gnome-terminal window to start a bash session, the variable HOMEBREW_SHELLENV_SET will be inherited from the gnome-terminal, which will cause duplicate brew shell initialization. Using bash with the GNOME desktop environment (default setting for Ubuntu) will always source the ~/.profile at least twice.

[carlocab]

Ok, let's skip the warning then.

I have another question: what are the advantages of this approach over, say, making the output of brew shellenv itself idempotent? i.e. the logic for checking whether brew is in PATH is in the code emitted by brew shellenv, rather than brew shellenv itself checking that brew is in PATH, etc.

[XuehaiPan]

i.e. the logic for checking whether brew is in PATH is in the code emitted by brew shellenv, rather than brew shellenv itself checking that brew is in PATH, etc.

Only check whether brew is in PATH is not sufficient. MANPATH and INFOPATH should be checked as well.

Here is my first implementation (d0ff10b). It checks the occurrences of brew's entries in PATH, MANPATH and INFOPATH. But just add an auxiliary variable is simpler and much more straight forward.

[carlocab]

Only check whether brew is in PATH is not sufficient. MANPATH and INFOPATH should be checked as well.

Yes. That's what the "etc" stood for. Thanks for the update; I'll have a look shortly.

[MikeMcQuaid]

I have another question: what are the advantages of this approach over, say, making the output of brew shellenv itself idempotent? i.e. the logic for checking whether brew is in PATH is in the code emitted by brew shellenv, rather than brew shellenv itself checking that brew is in PATH, etc.

The implementation is way simpler. Checking whether something is in the PATH reliably is somewhat non-trivial.

[MikeMcQuaid]

This makes sense to me. The only case I can imagine it might have issues is if you're intentionally running /opt/homebrew/bin/brew shellenv and /usr/local/bin/brew shellenv. I wonder if HOMEBREW_SHELLENV_SET could be something like HOMEBREW_SHELLENV_PREFIX=$HOMEBREW_PREFIX and then do a comparison to check the values match?

Happy to merge this as-is, though.

[MikeMcQuaid]

May as well move this above the HOMEBREW_PREFIX etc.

[XuehaiPan]

I add a new variable HOMEBREW_SHELLENV_PREFIX to make sure the current Homebrew is initialized correctly.

eval "(/opt/homebrew/bin/brew shellenv)"  # add `/opt/homebrew/bin` to `PATH` and set `MANPATH`, `INFOPATH`
eval "(/opt/homebrew/bin/brew shellenv)"  # no effect
eval "(/opt/homebrew/bin/brew shellenv)"  # no effect
eval "(/usr/local/bin/brew shellenv)"     # add `/opt/local/bin` to `PATH` and set `MANPATH`, `INFOPATH`
eval "(/usr/local/bin/brew shellenv)"     # no effect
eval "(/usr/local/bin/brew shellenv)"     # no effect

eval "(/opt/homebrew/bin/brew shellenv)"  # add `/opt/homebrew/bin` to `PATH` and set `MANPATH`, `INFOPATH`
eval "(/usr/local/bin/brew shellenv)"     # add `/opt/local/bin` to `PATH` and set `MANPATH`, `INFOPATH`
eval "(/opt/homebrew/bin/brew shellenv)"  # add `/opt/homebrew/bin` to `PATH` at the front (but it is duplicate)
eval "(/usr/local/bin/brew shellenv)"     # add `/opt/local/bin` to `PATH` at the front (but it is duplicate)

Or may only keep HOMEBREW_SHELLENV_PREFIX and remove HOMEBREW_SHELLENV_SET to keep duplicate entries in MANPATH and INFOPATH as well.

[MikeMcQuaid]

Looking good so far!

[MikeMcQuaid]

Great work here, thanks again @XuehaiPan!

[XuehaiPan]

I am sorry that this PR causes inconsistent shell behavior by introducing a new variable after brew update.

You can replace eval "$(/path/to/brew shellenv)" with:

eval "$(HOMEBREW_SHELLENV_PREFIX='' /path/to/brew shellenv)"

to always get all export statements for Homebrew entries.

[carlocab]

Do we want to reconsider the answers to either of my two questions/suggestions above? I think doing either of them might help resolve some of the issues people are having.

For clarity, I'm referring to

I wonder if it's worth emitting a warning when brew shellenv is called with HOMEBREW_SHELLENV_SET set

and

[W]hat are the advantages of this approach over, say, making the output of brew shellenv itself idempotent? i.e. the logic for checking whether brew is in PATH is in the code emitted by brew shellenv, rather than brew shellenv itself checking that brew is in PATH, etc.

[XuehaiPan]

I wonder if it's worth emitting a warning when brew shellenv is called with HOMEBREW_SHELLENV_SET set.

What are the advantages of this approach over, say, making the output of brew shellenv itself idempotent? i.e. the logic for checking whether brew is in PATH is in the code emitted by brew shellenv, rather than brew shellenv itself checking that brew is in PATH, etc.

I have created a new PR #11930 for that the variable HOMEBREW_SHELLENV_PREFIX is inherited from the parent shell but the users override the PATH.

The warning should only arise on the HOMEBREW_SHELLENV_PREFIX is set but with an unexpected PATH variable. Duplicate eval "$(brew shellenv)" should be allowed as commented in #11789 (comment) for variable inheritecement.