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

In a non-interactive environment, git commit doesn't work #171

Open
PW999 opened this issue Dec 20, 2020 · 5 comments
Open

In a non-interactive environment, git commit doesn't work #171

PW999 opened this issue Dec 20, 2020 · 5 comments

Comments

@PW999
Copy link
Contributor

PW999 commented Dec 20, 2020

In a non-interactive git environment (like Visual Studio Code), the git hook hangs while waiting for input from the user.

I've installed gitlint v0.15.0 on OpenSuse in WSL and added it to my repo using install-hook with a default config file using generate-config. However, if I want to commit in Visual Studio Code (using the Remote - WSL extension), the commit process hangs if there's something wrong with my commit message. The only way to proceed is by killing the script

The output

> gitlint --staged --msg-filename "$1" run-hook
> git config --get-all user.name
> git config --get-all user.email
> git -c user.useConfigOnly=true commit --quiet --allow-empty-message --file -
> git ls-files --stage -- /home/phillip/src/100-days-of-code/.gitlint
> git cat-file -s 317c4136f08cafaeb8f577a043aa4291684e6424
> git show --textconv :.gitlint
gitlint: checking commit message...
-----------------------------------------------
gitlint: Your commit message contains the above violations.
Continue with commit anyways (this keeps the current commit message)? [y(es)/n(no)/e(dit)] .git/hooks/commit-msg: line 22:  5628 Terminated 

I hoped that adding --silent and --ignore-stdin to the commit hook would help but unfortunately it did not.

@jorisroovers
Copy link
Owner

Hi Phillip, nice to see you here :-)

Interesting case, since I have tested this on VScode on Mac when rewriting the commit-msg hook recently (ced7bde and #157).

I think this might be related to WSL + Remote extension - I've never used those. My gut reaction is the root-cause is probably related to either:

  1. The vodoo we do to check whether we have a TTY connected to stdin is somehow failing on WSL + VSCode. I think it's less likely this is the cause.
  2. The stdin input is not correctly detected, similar to Gitlint hangs on Cygwin #91, for which the workaround is to use --ignore-stdin. You mentioned having already tried this, but I'd like to double check whether gitlint actually did it right. Can you try enabling debug by adding both --ignore-stdin as well as --debug to the gitlint invocation in the .git/hooks/commit-msg hook (line 22) and then posting output here?

Thanks!

@PW999
Copy link
Contributor Author

PW999 commented Dec 21, 2020

Hi Joris 😃 . Glad I can finally try out gitlint ... If I get the hang of it I might introduce it at work.

Verified that it was running with the correct arguments using ps ax (since I have to kill it), this is the complete command
/usr/bin/python3 /usr/bin/gitlint --staged --ignore-stdin --debug --msg-filename .git/COMMIT_EDITMSG run-hook

This is the output from the git log in VSCode

Looking for git in: git
Using git 2.26.2 from git
> git rev-parse --git-dir
Open repository: /home/phillip/src/100-days-of-code
> git status -z -u
> git symbolic-ref --short HEAD
> git rev-parse master
> git rev-parse --symbolic-full-name master@{u}
> git rev-list --left-right master...refs/remotes/origin/master
> git for-each-ref --sort -committerdate --format %(refname) %(objectname) %(*objectname)
> git remote --verbose
> git config --get commit.template
> git check-ignore -v -z --stdin
> git -c user.useConfigOnly=true commit --quiet --allow-empty-message --file -
DEBUG: gitlint.cli To report issues, please visit https://github.com/jorisroovers/gitlint/issues
DEBUG: gitlint.cli Platform: Linux-4.4.0-19041-Microsoft-x86_64-with-glibc2.3.4
DEBUG: gitlint.cli Python version: 3.6.10 (default, Jan 16 2020, 09:12:04) [GCC]
DEBUG: gitlint.git ('--version',)
DEBUG: gitlint.cli Git version: git version 2.26.2
DEBUG: gitlint.cli Gitlint version: 0.15.0
DEBUG: gitlint.cli GITLINT_USE_SH_LIB: [NOT SET]
DEBUG: gitlint.cli DEFAULT_ENCODING: UTF-8
DEBUG: gitlint.cli Configuration
config-path: /home/phillip/src/100-days-of-code/.gitlint
[GENERAL]
extra-path: None
contrib: []
ignore: 
ignore-merge-commits: True
ignore-fixup-commits: True
ignore-squash-commits: True
ignore-revert-commits: True
ignore-stdin: True
staged: True
verbosity: 3
debug: True
target: /home/phillip/src/100-days-of-code
[RULES]
  I1: ignore-by-title
     ignore=all
     regex=None
  I2: ignore-by-body
     ignore=all
     regex=None
  I3: ignore-body-lines
     regex=None
  T1: title-max-length
     line-length=72
  T2: title-trailing-whitespace
  T6: title-leading-whitespace
  T3: title-trailing-punctuation
  T4: title-hard-tab
  T5: title-must-not-contain-word
     words=WIP
  T7: title-match-regex
     regex=None
  T8: title-min-length
     min-length=5
  B1: body-max-line-length
     line-length=80
  B5: body-min-length
     min-length=20
  B6: body-is-missing
     ignore-merge-commits=True
  B2: body-trailing-whitespace
  B3: body-hard-tab
  B4: body-first-line-empty
  B7: body-changed-file-mention
     files=
  B8: body-match-regex
     regex=None
  M1: author-valid-email
     regex=[^@ ]+@[^@ ]+\.[^@ ]+

gitlint: checking commit message...
DEBUG: gitlint.cli Fetching additional meta-data from staged commit
DEBUG: gitlint.cli Using --msg-filename.
DEBUG: gitlint.git ('config', '--get', 'core.commentchar')
DEBUG: gitlint.cli Linting 1 commit(s)
DEBUG: gitlint.lint Linting commit [SHA UNKNOWN]
DEBUG: gitlint.git ('config', '--get', 'user.name')
DEBUG: gitlint.git ('config', '--get', 'user.email')
DEBUG: gitlint.git ('rev-parse', '--abbrev-ref', 'HEAD')
DEBUG: gitlint.git ('diff', '--staged', '--name-only', '-r')
DEBUG: gitlint.lint Commit Object
--- Commit Message ----
Add gitlint
--- Meta info ---------
Author: Phillip <[email protected]>
Date:   2020-12-21 10:05:40 +0100
is-merge-commit:  False
is-fixup-commit:  False
is-squash-commit: False
is-revert-commit: False
Branches: ['master']
Changed Files: ['.gitlint']
-----------------------
3: B6 Body message is missing
DEBUG: gitlint.cli Exit Code = 1
-----------------------------------------------
gitlint: Your commit message contains the above violations.
Continue with commit anyways (this keeps the current commit message)? [y(es)/n(no)/e(dit)] .git/hooks/commit-msg: line 22: 16948 Terminated              gitlint --staged --ignore-stdin --debug --msg-filename "$1" run-hook
> git config --get-all user.name
> git config --get-all user.email

I've been looking at the code and it seems like the --ignore-stdin isn't checked here: https://github.com/jorisroovers/gitlint/blob/main/gitlint/cli.py#L371

I tried hard-coding value to "n" and then VSCode immediately fails the commit message (as expected). I'm not sure if you intended on handling the --ignore-stdin elsewhere ?

I can try to make a pull request but I'm not exactly a seasoned Python developer 😄

@jorisroovers
Copy link
Owner

jorisroovers commented Dec 21, 2020

Ok, most obvious question first (just so we can rule it out): did you actually try to answer the question (y/n/e), or is the prompt not interactive at all?

Secondly, I think our focus on the --ignore-stdin might be taking us in the wrong direction, although the confusion is understandable.

We're conflating gitlint's --ignore-stdin general option with enabling/disabling the interactive gitlint commit-msg hook prompt. Those are 2 different things, although both have to do with stdin and TTYs.

Consider these examples of --ignore-stdin:

# This will lint "foobar" as a commit message, using the interactive hook
# Because the stdin = pipe (i.e. not interactive), this will abort immediately after the first user-prompt
echo "foobar" | gitlint run-hook

# This will force gitlint to ignore the piped input "foobar", and just lint the last commit message
# Note however that because stdin = (pipe i.e. not interactive), gitlint will still abort after the first user-prompt
# We might debate whether this is expected behavior
echo "foobar" | gitlint --ignore-stdin run-hook

Normally, Python's input() is what causes the abort to happen if stdin is non-interactive. But clearly that's not happening in your case (i.e. why I asked whether it's definitely not possible to just answer y/n/e on the prompt in VSCode).

value = input()

Ideally, we should be able to detect this scenario reliably, but from experience I know that introducing a 'force ignore' option is not a bad idea so we always have a work-around. However, if we do this, I think it should be an option specific to run-hook and not re-use the generic --ignore-stdin option that already has a different meaning. Something like --non-interactive:

# Note that `--non-interactive` is specified *after* `run-hook`, and not before like `--ignore-stdin`
gitlint run-hook --non-interactive

# This will still work and lint "foobar", but the hook will be forced to be non-interactive
echo "foobar" | gitlint run-hook --non-interactive

--non-interactive could then also be set using a .gitlint option hook-non-interactive=True and env var GITLINT_HOOK_NON_INTERACTIVE=1.

Of course, because it's a force flagm it means the hook will always be non-interactive, even when you invoke it from a regular CLI.

PS: christmas time 🎄 , so delayed responses are very likely. I suggest not spending/wasting too much time on PRs on this until we've figured out the approach first. Thanks for your help!

@eugen1j
Copy link

eugen1j commented Sep 29, 2021

@jorisroovers

I want to introduce gitlint at work project. I'm going to insert checks in CI pipeline and pre-commit hook.

There is an option to skip checks in pre-commit hook: Continue with commit anyways (this keeps the current commit message)? [y(es)/n(no)/e(dit)] If I continue I will break the CI.

It seems, that new --non-interactive option that you have described is great solution. Any plans to implement this feature?

I suggest not spending/wasting too much time on PRs on this until we've figured out the approach first.

Did you figure out is this approach is good enough? In this case I can try to send a PR for this feature.

@eugen1j
Copy link

eugen1j commented Sep 30, 2021

I came up with custom commit-msg git hook like this

gitlint --msg-filename $1

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

No branches or pull requests

3 participants