search

jorisroovers / gitlint

Linting for your git commit messages
http://jorisroovers.github.io/gitlint
MIT License
817 stars 100 forks source link

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

Open PW999 opened 3 years ago

PW999 commented 3 years ago

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 commented 3 years ago

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 (ced7bdec8b941785e7815f2bcaee6b1121efd33d 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 #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 commented 3 years ago

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 <blablabla@ymail.com>
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 commented 3 years ago

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). https://github.com/jorisroovers/gitlint/blob/e06dfe02c222b1d4807c17b9c534d7deeed7a8e6/gitlint/cli.py#L386

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 commented 3 years ago

@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 commented 3 years ago

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

gitlint --msg-filename $1