gabyx / Githooks

🦎 Githooks: per-repo and shared Git hooks with version control and auto update. [✩Star] if you're using it!
MIT License
102 stars 4 forks source link

Enable the use of environment variables in configuration #139

Open giggio opened 6 months ago

giggio commented 6 months ago

I commit my gitconfig, and, with the installation happening at $HOME, is creating a problema with the path.

I'd like to be able to use environment variable in config, like so:

[githooks]
  installDir = $HOME/.githooks
  runner = $HOME/.githooks/bin/runner
  dialog = $HOME/.githooks/bin/dialog
  manualTemplateDir = $HOME/.gittemplate
[init]
  templateDir = $HOME/.githooks/templates

The alias is already working. Setting it as hooks = !\"$HOME/.githooks/bin/cli\" already works.

gabyx commented 6 months ago

Hi @giggio : Thats true.

We do not yet substitute envvars in the variable. But since we read in all Git config variables at startup anyway (we cache them) we could easily do a env substitute I guess on some relevant variables.

Are you doing this because you manage your dotfiles? I do it with chezmoi here: https://github.com/gabyx/dotfiles/blob/main/config/dot_gitconfig.tmpl#L37

gabyx commented 6 months ago

See for a possible solution #141

giggio commented 6 months ago

Yes, that is exactly what I'm doing. When I opened this issue I was not familiar with home.templateDir, I was not aware this is a global git config, not from this project. It cannot be configured by it. So I'm using the GIT_TEMPLATE_DIR environment variable, and that works as expected. I added it to all my shells (I use Bash and Nushell). The workarounds I'm using: For githooks.installDir: I deleted it, so it uses the default, which already is good enough, $HOME/.githooks. For githooks.dialog: I am not using the dialog at the moment, so I deleted it. For githooks.runner: I am using a custom hook template, first line:

GITHOOKS_RUNNER=$(git config githooks.runner | envsubst)

But envsubst is not posix compliant, if that is a problem.

giggio commented 6 months ago

See for a possible solution #141

We'd still need to change the hook script, the [ ! -x "$GITHOOKS_RUNNER" ] check will not work.

gabyx commented 6 months ago

Ah yeah correct. hm...

The installer/updater at the moment will always overwrite the variables githooks.installDir and dialog and runner. So if we use #141, you can leave $HOME in the varaibles in your dotfiles and when the installer runs or the updater it will just replace the them in your actual config. which is suboptimal

The fact that you want to change the githooks.runner yourself is a bit unfortunate. This variable githooks.runner is only there to somehow get the runner from the install for the run-wrapper.sh. Now you want to change githooks.runner such that it does not get overwritten by the installer, so its an externally managed settings suddenly... hm...

edit:: My thougth do not work I guess, since we need envsubst and I dont want to add non-posix stuff into the run-wrapper.sh, I cannot have githooks.runner have env variable substitution...

gabyx commented 6 months ago

Could you use GITHOOKS_RUNNER env variable to adjust your custom run wrapper? and set this in your shell etc?

giggio commented 6 months ago

Regarding envsubst not being posix, I have written a substitute, a posix compatible scripts using awk. It will replace simple variables in the syntax $VAR (starting with the dollar sign), it should be enough. The alternative would be to use eval, but that is too dangerous.

This is it:

{
  finalstr = $0
  while (1) {
    if (match(finalstr, /\$([A-Za-z0-9_]+)/)) {
      newstr = substr(finalstr, 1, RSTART - 1)
      newstr = newstr ENVIRON[substr(finalstr, RSTART + 1, RLENGTH - 1)]
      newstr = newstr substr(finalstr, RSTART + RLENGTH)
      finalstr = newstr
    } else {
      break
    }
  }
  print finalstr
}

We could add this (or a minimized version of it) to the default hooks script.

giggio commented 6 months ago

Here is the full script:

#!/usr/bin/env sh

# Read the runner script from the local/global or system config and expands environment variables
GITHOOKS_RUNNER=`git config githooks.runner | awk '{
  finalstr = $0
  while (1) {
    if (match(finalstr, /\$([A-Za-z0-9_]+)/)) {
      newstr = substr(finalstr, 1, RSTART - 1)
      newstr = newstr ENVIRON[substr(finalstr, RSTART + 1, RLENGTH - 1)]
      newstr = newstr substr(finalstr, RSTART + RLENGTH)
      finalstr = newstr
    } else {
      break
    }
  }
  print finalstr
}'`

if [ ! -x "$GITHOOKS_RUNNER" ]; then
  echo "! Githooks runner points to a non existing location" >&2
  echo "   \`$GITHOOKS_RUNNER\`" >&2
  echo " or it is not executable!" >&2
  echo " Please run the Githooks install script again to fix it." >&2
  exit 1
fi

exec "$GITHOOKS_RUNNER" "$0" "$@"
giggio commented 6 months ago

Regarding the installer, yes, it is a problem that it keeps changing the values in .gitconfig. I'll open another issue about that.

Opened: #142.

giggio commented 6 months ago

If we use the above script (which expands environment variables) I believe we could close #141. What do you think?

giggio commented 6 months ago
  • What if we use githooks.runnerCustom which gets checked first in the run-wrapper.sh, which is a way of customizing having a custom run-wrapper.sh. Still dont know why you need this?

My main problem here is the variable expansion. I can't use it with the default run-wrapper.sh. I also can't remove it. If variable expansion works I don't need to change it.

This is another reason I opened #140, besides consistency.

giggio commented 6 months ago
  • Then also store by default $HOME/.githooks as prefix to all paths in the Git config not the expanded version if feat: Add expansion of env. variables in Git config ⚓ #141 is going to get used. So --prefix and other CLI arguments to the installer can incorporate env variables which will get written unexpanded to the Git config, and read correctly afterwards...

This is a good idea, it would simplify the configuration. You could make it extensible, e.g.: if a config has a relative path, use the prefix, if not use it as an absolute path. If it is not present, use a default. And make the prefix understand variable expansion.

But, if you do that, you still need to change the hooks (the run-wrapper.sh), as it has no idea about the prefix. The default and variable expansion could also be added there.

gabyx commented 6 months ago

Ok. thanks for sharing the awk snippet. Honestly, I do not favor this approach. All this only to have environment variables subst in some config files:

if [ -z "$GITHOOKS_RUNNER" ]; then
  GITHOOKS_RUNNER=$(git config githooks.runner)
fi

inside the run-wrapper.sh.

giggio commented 6 months ago

But your use case: Have a correct gitconfig file in your dotfiles repo (yes with $HOME) and deploying it, and then installing Githooks (fresh), should work as it substitutes envs when reading them but then writes full paths back. Whats wrong with this approach?

That is what I'm looking for.

gabyx commented 6 months ago

But your use case: Have a correct gitconfig file in your dotfiles repo (yes with $HOME) and deploying it, and then installing Githooks (fresh), should work as it substitutes envs when reading them but then writes full paths back. Whats wrong with this approach?

That is what I'm looking for.

Are you fine with having githooks.installDir = "$HOME/..." and the other variables githooks.runner and githooks.dialog unset in you rdotfiles? Then the installer just changes these values to full paths resulting in a diff from your dotfile's gitconfig to the real one...

giggio commented 6 months ago

Are you fine with having githooks.installDir = "$HOME/..." and the other variables githooks.runner and githooks.dialog unset in you rdotfiles? Then the installer just changes these values to full paths resulting in a diff from your dotfile's gitconfig to the real one...

Yes, this should be enough.

giggio commented 6 months ago

I just added to the issue, .githooks.manualTemplateDir also need to work with environment variables.

giggio commented 2 months ago

With the new 3.0 release I see a new property which needs to support environment variables: pathForUseCoreHooksPath.

gabyx commented 2 months ago

With version 3, I think the only ones needing env. support is: githooks.pathforusecorehookspath and githooks.installdir.