Skip to content

Commit

Permalink
Merge pull request #3487 from MikeMcQuaid/dev-env-filtering
Browse files Browse the repository at this point in the history 
Enable environment filtering for developers.
  • Loading branch information
@MikeMcQuaid
MikeMcQuaid committed Nov 27, 2017
2 parents fb588f2 + b26a0d4 commit f262cbc
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 10 deletions.
10 changes: 0 additions & 10 deletions Library/Homebrew/brew.sh
View file
Expand Up @@ -246,16 +246,6 @@ case "$HOMEBREW_COMMAND" in
--config) HOMEBREW_COMMAND="config" ;;
esac

if [[ -z "$HOMEBREW_DEVELOPER" ]]
then
export HOMEBREW_GIT_CONFIG_FILE="$HOMEBREW_REPOSITORY/.git/config"
HOMEBREW_GIT_CONFIG_DEVELOPERMODE="$(git config --file="$HOMEBREW_GIT_CONFIG_FILE" --get homebrew.devcmdrun 2>/dev/null)"
if [[ "$HOMEBREW_GIT_CONFIG_DEVELOPERMODE" = "true" ]]
then
export HOMEBREW_DEV_CMD_RUN="1"
fi
fi

if [[ -f "$HOMEBREW_LIBRARY/Homebrew/cmd/$HOMEBREW_COMMAND.sh" ]]
then
HOMEBREW_BASH_COMMAND="$HOMEBREW_LIBRARY/Homebrew/cmd/$HOMEBREW_COMMAND.sh"
Expand Down
1 change: 1 addition & 0 deletions Library/Homebrew/system_config.rb
View file
Expand Up @@ -185,6 +185,7 @@ def dump_verbose_config(f = $stdout)
HOMEBREW_BREW_FILE
HOMEBREW_COMMAND_DEPTH
HOMEBREW_CURL
HOMEBREW_GIT_CONFIG_FILE
HOMEBREW_LIBRARY
HOMEBREW_MACOS_VERSION
HOMEBREW_RUBY_PATH
Expand Down
23 changes: 23 additions & 0 deletions bin/brew
View file
Expand Up @@ -59,6 +59,29 @@ do
export "$VAR_NEW"="${!VAR}"
done

# Set HOMEBREW_DEVELOPER for users who have run a development command
if [[ -z "$HOMEBREW_DEVELOPER" ]]
then
export HOMEBREW_GIT_CONFIG_FILE="$HOMEBREW_REPOSITORY/.git/config"
HOMEBREW_GIT_CONFIG_DEVELOPERMODE="$(git config --file="$HOMEBREW_GIT_CONFIG_FILE" --get homebrew.devcmdrun 2>/dev/null)"
if [[ "$HOMEBREW_GIT_CONFIG_DEVELOPERMODE" = "true" ]]
then
export HOMEBREW_DEV_CMD_RUN="1"
fi
fi

if [[ -z "$HOMEBREW_NO_ENV_FILTERING" ]]
then
if [[ -n "$HOMEBREW_DEVELOPER" || -n "$HOMEBREW_DEV_CMD_RUN" ]]
then
# Use env filtering by default for users who have run a development command
# This will be enabled by default for all users in future.
export HOMEBREW_ENV_FILTERING="1"
fi
else
unset HOMEBREW_ENV_FILTERING
fi

# test-bot sets environment filtering itself
if [[ -n "$HOMEBREW_ENV_FILTERING" && "$1" != "test-bot" ]]
then
Expand Down

11 comments on commit f262cbc

@toonetown
Copy link
Contributor

Choose a reason for hiding this comment

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

For some reason, this commit breaks my custom brewcask-* scripts that I have installed into /usr/local/bin. It causes them to behave as if it is an unknown command (it just spits out the default "help" text).

If I set HOMEBREW_NO_ENV_FILTERING=true prior to running one of my custom brewcask-*. This is an OK workaround for me - and, admittedly, I'm in the minority with my custom scripts...but I thought I'd report the breakage in case it needed to be looked at.

@MikeMcQuaid
Copy link
Member Author

Choose a reason for hiding this comment

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

@toonetown That is intentional. It will likely break anywhere you're relying on custom, non-HOMEBREW_ environment variables being passed to your scripts. That is not supported behaviour so you should not rely on it, sorry.

@toonetown
Copy link
Contributor

@toonetown toonetown commented on f262cbc Nov 27, 2017 •
edited

Choose a reason for hiding this comment

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

That is OK - but it doesn't even look like the scripts are being called in the first place. I'm not relying upon any specific environment variables in the scripts themselves. This can be proven out by having a single script (in this example, called /usr/local/bin/brewcask-ndt):

#!/bin/bash
echo "Hello World"

With that script in place (and executable), I am unable to run brew cask ndt:

$ brew cask ndt
brew-cask provides a friendly homebrew-style CLI workflow for the
administration of macOS applications distributed as binaries.

Commands:

    --version              displays the Homebrew-Cask version
    audit                  verifies installability of Casks
    cat                    dump raw source of the given Cask to the standard output
    cleanup                cleans up cached downloads and tracker symlinks
    create                 creates the given Cask and opens it in an editor
    doctor                 checks for configuration issues
    edit                   edits the given Cask
    fetch                  downloads remote application files to local cache
    home                   opens the homepage of the given Cask
    info                   displays information about the given Cask
    install                installs the given Cask
    list                   with no args, lists installed Casks; given installed Casks, lists staged files
    outdated               list the outdated installed Casks
    reinstall              reinstalls the given Cask
    search                 searches all known Casks
    style                  checks Cask style using RuboCop
    uninstall              uninstalls the given Cask
    zap                    zaps all files associated with the given Cask

See also "man brew-cask"
Error: help does not take arguments.

However, without environment filtering, it runs correctly:

$ export HOMEBREW_NO_ENV_FILTERING=true
$ brew cask ndt
Hello World

Again - if this is intended behavior, then that is OK. I just want to make sure there are no regressions.

@toonetown
Copy link
Contributor

Choose a reason for hiding this comment

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

As an additional note - I can run custom brew-* scripts...just not custom brewcask-* scripts.

@MikeMcQuaid
Copy link
Member Author

Choose a reason for hiding this comment

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

@toonetown It may be those scripts are just filtered out of your PATH. Can you try to add them to a tap temporarily?

@toonetown
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes - do I just place them in a bin directory in any tap? (I've never added custom command scripts to a tap before)

@MikeMcQuaid
Copy link
Member Author

Choose a reason for hiding this comment

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

@toonetown the cmd directory

@toonetown
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't get custom brewcask-* scripts to load from a tap's cmd directory...either with or without the HOMEBREW_NO_ENV_FILTERING. I can get custom brew-* scripts to load from the cmd directory.

I'll keep digging. Sounds like it's likely a problem with my setup.

@MikeMcQuaid
Copy link
Member Author

Choose a reason for hiding this comment

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

@reitermarkus Should they be working from your perspective?

@MikeMcQuaid
Copy link
Member Author

Choose a reason for hiding this comment

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

@toonetown Should be fixed now!

@toonetown
Copy link
Contributor

Choose a reason for hiding this comment

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

@MikeMcQuaid it does, thanks!

Please sign in to comment.