Skip to content
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

Enable the use of environment variables in configuration #139

Open
giggio opened this issue Jan 23, 2024 · 19 comments
Open

Enable the use of environment variables in configuration #139

giggio opened this issue Jan 23, 2024 · 19 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@giggio
Copy link

giggio commented Jan 23, 2024

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
Copy link
Owner

gabyx commented Jan 23, 2024

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 gabyx self-assigned this Jan 23, 2024
@gabyx gabyx added the enhancement New feature or request label Jan 23, 2024
@gabyx
Copy link
Owner

gabyx commented Jan 23, 2024

See for a possible solution #141

@giggio
Copy link
Author

giggio commented Jan 23, 2024

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
Copy link
Author

giggio commented Jan 23, 2024

See for a possible solution #141

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

@gabyx
Copy link
Owner

gabyx commented Jan 23, 2024

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...

  • 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?
  • 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...

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
Copy link
Owner

gabyx commented Jan 23, 2024

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

@giggio
Copy link
Author

giggio commented Jan 23, 2024

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
Copy link
Author

giggio commented Jan 23, 2024

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
Copy link
Author

giggio commented Jan 23, 2024

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
Copy link
Author

giggio commented Jan 23, 2024

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

@giggio
Copy link
Author

giggio commented Jan 23, 2024

  • 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
Copy link
Author

giggio commented Jan 23, 2024

  • 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
Copy link
Owner

gabyx commented Jan 23, 2024

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:

  • I think changing the run-wrapper.sh which you should not. You have not given me a reason why you need this? Anyway if you want to change the runner we can just do this: You set GITHOOKS_RUNNER to something you want as your env variable and we do:
if [ -z "$GITHOOKS_RUNNER" ]; then
  GITHOOKS_RUNNER=$(git config githooks.runner)
fi

inside the run-wrapper.sh.

  • Without a envsubst inside run-wrapper.sh (I want to keep that as slick and fast as possible).
    there is need for an absolute existing path in githooks.runner. So the installer sets it (fully expanded).

  • Substituting env variables when the config is read in Githooks installer/runner/cli can be done (feat: Add expansion of env. variables in Git config ⚓ #141), but writting unexpanded stuff back to the config is a hassle (not saying it cant be done but is it worth implementing, Githooks already has too many features...). 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?

@giggio
Copy link
Author

giggio commented Jan 23, 2024

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
Copy link
Owner

gabyx commented Jan 23, 2024

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
Copy link
Author

giggio commented Jan 23, 2024

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
Copy link
Author

giggio commented Jan 23, 2024

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

@gabyx gabyx added this to the Version 3.1 milestone Mar 30, 2024
@giggio
Copy link
Author

giggio commented May 2, 2024

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

@gabyx
Copy link
Owner

gabyx commented May 2, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants