silviucpp / erlkaf

Erlang kafka driver based on librdkafka
MIT License
84 stars 41 forks source link

`brew install` causing undesired updates and crashing other Homebrew pkgs (macOS-only) #68

Closed joeljuca closed 7 months ago

joeljuca commented 7 months ago

I recently had issues while building erlkaf on macOS due to its attempt to install its dependencies through a forced brew install.

During the process, Homebrew ran some auto-update (brew update --auto-update) which caused some unrelated packages to break. I had to spend some minutes figuring out what had happened, and how a mix deps.get could potentially break my Homebrew – and then I could this snippet:

case $1 in
  $LIBRDKAFKA_DESTINATION)
    case $OS in
      Darwin)
        brew install openssl@1.1 lz4 zstd curl
        OPENSSL_ROOT_DIR=$(brew --prefix openssl@1.1)
        export CPPFLAGS=-I$OPENSSL_ROOT_DIR/include/
        export LDFLAGS=-L$OPENSSL_ROOT_DIR/lib
        ;;
    esac

  # (...)
esac

See: https://github.com/silviucpp/erlkaf/blob/master/build_deps.sh#L67-L72

It's pretty bad to touch system packages during the build of a library since it can potentially affect other packages like the scenario I ran into. So, I would recommend removing this snippet from the build scripts, replacing it with a clear doc section about system requirements (in macOS specifically, suggest Homebrew packages as a recommendation – though people could also use alternative methods like nixpkgs, etc.), and possibly failing the build when necessary with some informative error messages stating that system requirements were not met.

I think we could accomplish this by checking if some binaries and/or environment variables are set. I do similar things in my .zshrc to configure my shell if/when it's on a Homebrew-powered macOS (eg.: https://github.com/joeljuca/cli/blob/main/config/zsh/zshrc.sh#L70-L77).

if ! which openssl >/dev/null 2>&1; then
  echo "You must have OpenSSL installed in your system."
  echo "See: https://(...)"
  exit 1
fi

if [ "${LDFLAGS}" == "" ]; then
  echo "Required config LDFLAGS is not set"
  echo "See: https://(...)"
  exit 1
fi

# (...)

# if everything looks good, proceed with the build...

PS: I'm not a great shell programmer, but I imagine a couple of if blocks could do the trick.

silviucpp commented 7 months ago

You can disable to brew auto-update behavior.

### Bash ###
$ vim ~/.bashrc
export HOMEBREW_NO_AUTO_UPDATE=1

### Zsh ###
$ vim ~/.zshrc
export HOMEBREW_NO_AUTO_UPDATE=1
silviucpp commented 7 months ago

Or better I can probably add HOMEBREW_NO_AUTO_UPDATE=1 brew install X into the script itself.

joeljuca commented 7 months ago

I'm definitely adding this HOMEBREW_NO_AUTO_UPDATE setting to my zshrc – Thanks! But I still believe it's pretty bad to have your build script touching my system pkg manager like that.

I would never expect an Erlang library to touch my Homebrew setup without my consent. It's a system pkg manager, users have custom setups on top of it. A library should not assume it's OK to touch system pkgs and there will be undesired side effects. If it needs the presence of some packages (which is the case), the best option is to inform the user and delegate him the task of installing it (running the necessary brew install and getting the system ready for the build).

silviucpp commented 7 months ago

Yes I will add HOMEBREW_NO_AUTO_UPDATE=1 brew install X insead brew install. If you ask me brew behavior sucks. when you want to install something by default will update all installed libs.

silviucpp commented 7 months ago

Fixed on master