KEMBAR78
cmd/shellenv: echo path helper command rather than run it. by MikeMcQuaid · Pull Request #20675 · Homebrew/brew · GitHub
Skip to content

Conversation

MikeMcQuaid
Copy link
Member

This is faster at both execution and evaling than the existing version and aids caching.

Fixes #20671

This is faster at both execution and `eval`ing than the existing version
and aids caching.
The output of `path_helper` needs to be `eval`ed for it to work.
@carlocab carlocab force-pushed the shellenv_echo_path_helper branch from 40e6236 to 57578cf Compare September 13, 2025 05:32
@carlocab
Copy link
Member

The output of path_helper needs to be evaled for it to work; pushed a fix. This essentially brings us back to the original implementation at #18188.

However, doing eval may well break caching still, so not sure this can fix #20671.

@MikeMcQuaid
Copy link
Member Author

Ok, thanks, agreed this seems mostly pointless then. Thanks @carlocab!

@ericbn
Copy link

ericbn commented Sep 13, 2025

@carlocab @MikeMcQuaid, my bad, I should have enclosed the path_helper call in eval when reporting #20671. It's fixed correctly fixed here. This does solve the caching issue and arguably gives a clearer output on what the script it actually doing without hiding the path_helper call.

@MikeMcQuaid MikeMcQuaid reopened this Sep 13, 2025
@MikeMcQuaid MikeMcQuaid added this pull request to the merge queue Sep 13, 2025
Merged via the queue into main with commit 8304edb Sep 13, 2025
70 checks passed
@MikeMcQuaid MikeMcQuaid deleted the shellenv_echo_path_helper branch September 13, 2025 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"path_helper" version of the brew shellenv script is not cacheable

4 participants