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.

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

cmd/shellenv: use Bash. #4887

Merged
merged 1 commit into from Sep 13, 2018

Conversation

Projects
None yet
3 participants
@MikeMcQuaid
Copy link
Member

MikeMcQuaid commented Sep 12, 2018

This speeds up execution by 3x on my machine and the script is simple enough to warrant this.

@jamescostian

This comment has been minimized.

Copy link
Contributor

jamescostian commented Sep 12, 2018

Glad you're taking over this part, sorry I was out so long lol

I think you have to delete Library/Homebrew/test/cmd/shellenv_spec.rb to get the tests to pass

Also I think there's a typo in the bash script where it exports PATH but says #{HOMEBREW_PREFIX} instead of $HOMEBREW_PREFIX

Also, you had mentioned adding HOMEBREW_PREFIX, something like this I assume:

echo "export HOMEBREW_PREFIX='$HOMEBREW_PREFIX'"

Did you decide against that addition?


homebrew-shellenv() {
echo "export PATH=\"$HOMEBREW_PREFIX/bin:#{HOMEBREW_PREFIX}/sbin:\$PATH\""
echo "export MANPATH=\"$HOMEBREW_PREFIX/share/man:\$MANPATH\""

This comment has been minimized.

@vitorgalvao

vitorgalvao Sep 12, 2018

Member

Using single quotes inside double quotes won’t prevent variable expansion but will avoid the need for escaping the double quotes. For example, this line would become echo "export MANPATH='$HOMEBREW_PREFIX/share/man:\$MANPATH'" which I find marginally clearer.

@vitorgalvao

This comment has been minimized.

Copy link
Member

vitorgalvao commented Sep 12, 2018

Also, you had mentioned adding HOMEBREW_PREFIX, something like this I assume:

echo "export HOMEBREW_PREFIX='$HOMEBREW_PREFIX'"

That would make HOMEBREW_PREFIX be empty (if yet unset). Do you mean:

echo "export HOMEBREW_PREFIX='$(brew --prefix)'"
@jamescostian

This comment has been minimized.

Copy link
Contributor

jamescostian commented Sep 12, 2018

That would make HOMEBREW_PREFIX be empty (if yet unset)

Not quite - notice how the other lines of code in this PR all use $HOMEBREW_PREFIX but still work. The reason is that this code is being run from Library/Homebrew/brew.sh which is loaded from bin/brew which defines $HOMEBREW_PREFIX

If you don't believe me:

  1. Clone Mike's fork and checkout his branch
  2. Open Library/Homebrew/cmd/shellenv.sh in your editor
  3. Right before the }, add echo "export HOMEBREW_PREFIX='$HOMEBREW_PREFIX'";
  4. Run bin/brew shellenv

For example, my machine prints:

export PATH='/Users/james/code/brew/brew/bin:/Users/james/code/brew/brew/sbin':"$PATH"
export MANPATH='/Users/james/code/brew/brew/share/man':"$MANPATH"
export INFOPATH='/Users/james/code/brew/brew/share/man':"$INFOPATH"
export HOMEBREW_PREFIX='/Users/james/code/brew/brew'
cmd/shellenv: use Bash.
This speeds up execution by 3x on my machine and the script is simple
enough to warrant this.

@MikeMcQuaid MikeMcQuaid force-pushed the MikeMcQuaid:shellenv-sh branch from 9b4216c to 44bcf69 Sep 13, 2018

@MikeMcQuaid

This comment has been minimized.

Copy link
Member

MikeMcQuaid commented Sep 13, 2018

Glad you're taking over this part, sorry I was out so long lol

No worries!

I think you have to delete Library/Homebrew/test/cmd/shellenv_spec.rb to get the tests to pass

👍 yep.

Also I think there's a typo in the bash script where it exports PATH but says #{HOMEBREW_PREFIX} instead of $HOMEBREW_PREFIX

Fixed 👍

Did you decide against that addition?

Added now along with HOMEBREW_CELLAR and HOMEBREW_REPOSITORY.

@MikeMcQuaid MikeMcQuaid merged commit 9a9f9c3 into Homebrew:master Sep 13, 2018

3 checks passed

codecov/patch Coverage not affected when comparing 73e8bf6...44bcf69
Details
codecov/project 71.4% (-0.01%) compared to 73e8bf6
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@wafflebot wafflebot bot removed the in progress label Sep 13, 2018

@MikeMcQuaid MikeMcQuaid deleted the MikeMcQuaid:shellenv-sh branch Sep 13, 2018

@lock lock bot added the outdated label Oct 13, 2018

@lock lock bot locked as resolved and limited conversation to collaborators Oct 13, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.