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

commit-msg hook with gitlint makes the text get lost on failure #833

Open
rico-chet opened this issue Sep 24, 2018 · 11 comments
Open

commit-msg hook with gitlint makes the text get lost on failure #833

rico-chet opened this issue Sep 24, 2018 · 11 comments

Comments

@rico-chet
Copy link

@rico-chet rico-chet commented Sep 24, 2018

When using gitlint as a commit-msg hook with pre-commit, I run into issue of commit message getting lost easily when gitlint finds that it's not good enough.

Repro: on a clean minimal debian 9.5.0, I run:

apt install --assume-yes curl git python3
curl https://bootstrap.pypa.io/get-pip.py | python3 - pre-commit

git clone --mirror https://github.com/jorisroovers/gitlint

mkdir gitlint-test
cd gitlint-test
git init
git config user.email "you@example.com"
git config user.name "Your Name"
git commit --allow-empty --message="Initial empty commit"
cat > .pre-commit-config.yaml <<EOF
repos:
  - repo: ../gitlint.git
    rev: v0.10.0
    hooks:
    - id: gitlint
      stages:
      - commit-msg
EOF
git add .pre-commit-config.yaml
git commit --message="Add pre-commit config"
pre-commit install --hook-type commit-msg
touch foo
git add foo
git commit
*enter a lengthy commit message*
*find that gitlint keeps commit from succeeding for some valid reason*
git commit # try to fix the lengthy message
*find that all message text is gone :-(*

One can retrieve the text from .git/COMMIT_EDITMSG after the gitlint test, but once you hit git commit for the second time, all is lost. I'd expect that you get a second chance without risk of losing all your text.

IIRC, I didn't observe this issue with gitlint alone.

@asottile
Copy link
Member

@asottile asottile commented Sep 24, 2018

I can't reproduce "IIRC, I didn't observe this issue with gitlint alone."

Here's my session:

$ git commit
gitlint: checking commit message...
2: B4 Second line is not empty: "asdf"
3: B5 Body message is too short (8<20): "asdfasdf"
-----------------------------------------------
gitlint: Your commit message contains the above violations.
Continue with commit anyways (this keeps the current commit message)? [y(es)/n(no)/e(dit)] n
Commit aborted.
Your commit message: 
-----------------------------------------------
asdf
asdf
asdf

# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit.
#
# On branch master
# Changes to be committed:
#	new file:   foo
#
# Untracked files:
#	venv/
#
-----------------------------------------------
$ git commit

I'm then presented with:


# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit.
#
# On branch master
# Changes to be committed:
#   new file:   foo
#
# Untracked files:
#   venv/
#

There's nothing particularly special about how pre-commit invokes gitlint, as far as I know it should be the same in or out

@rico-chet
Copy link
Author

@rico-chet rico-chet commented Sep 25, 2018

You're right, it's the default git behavior with the commit-msg hooks which have to take care of the rejected content on their own. Apparently, it takes a backup file and a prepare-commit-msg hook to make the rejected message re-appear. Looks like a new feature request for pre-commit, right?

@asottile
Copy link
Member

@asottile asottile commented Sep 26, 2018

Could be a neat little feature yeah! Need to be sure it plays nicely if someone defines their own prepare-commit-msg hook and probably mesh with the rest of the framework but definitely sounds like a sharp edge in git that pre-commit could smooth over!

@asottile
Copy link
Member

@asottile asottile commented Apr 29, 2019

Now that prepare-commit-msg will land in the next pre-commit release, it should be pretty easy to make something like this!

@asottile
Copy link
Member

@asottile asottile commented Jun 9, 2019

Here's a small demo of this idea, this can probably be cleaned up and turned into a repo: meta hook

.pre-commit-config.yaml

repos:
-   repo: local
    hooks:
    -   id: save-commit-msg
        name: save commit message
        entry: ./bin/save-commit-msg
        verbose: true
        language: script
        stages: [commit-msg]
    -   id: restore-commit-msg
        name: restore commit message
        entry: ./bin/restore-commit-msg
        verbose: true
        language: script
        stages: [prepare-commit-msg]
    -   id: derp
        name: derp
        entry: derp
        language: fail
        stages: [commit-msg]

bin/restore-commit-msg

#!/usr/bin/env python3
import os
import shutil
import sys


def main():
    if os.path.exists('.git/pre-commit-saved-commit-msg'):
        with open(sys.argv[1]) as f:
            lines = [x for x in f if x.strip() if not x.startswith('#')]
        if not lines:
            print('restoring old commit message...')
            shutil.copy('.git/pre-commit-saved-commit-msg', sys.argv[1])


if __name__ == '__main__':
    exit(main())

bin/save-commit-msg

#!/usr/bin/env python3
import shutil
import sys


def main():
    shutil.copy(sys.argv[1], '.git/pre-commit-saved-commit-msg')


if __name__ == '__main__':
    exit(main())

@rico-chet
Copy link
Author

@rico-chet rico-chet commented Jun 10, 2019

It's a good start, lacking:

  • support for gitdir file, e.g. doesn't work in a submodule or a git worktree
  • support for git commit --amend, dropping message of the last rejected edit

which I tried to solve, here's what I got so far (using e.g. Play with Docker):

docker run --interactive --tty debian:stable bash

export DEBIAN_FRONTEND=noninteractive
apt update
apt-get dist-upgrade --assume-yes --quiet < /dev/null
apt-get install --assume-yes --quiet curl git python3 python3-setuptools vim < /dev/null
curl https://bootstrap.pypa.io/get-pip.py | python3 - pre-commit

# make Click happy
export LC_ALL=C.UTF-8
export LANG=C.UTF-8

cd
git clone --mirror https://github.com/jorisroovers/gitlint

mkdir gitlint-test
cd gitlint-test
git init
git config user.email "you@example.com"
git config user.name "Your Name"
git commit --allow-empty --message="Initial empty commit"

cat > save-commit-msg <<'EOF'
#!/usr/bin/env sh

cp "${1}" "$(git rev-parse --git-path pre-commit-saved-commit-msg)"
EOF

cat > restore-commit-msg <<'EOF'
#!/usr/bin/env sh

set -e
set -u

msg_file=$(git rev-parse --git-path pre-commit-saved-commit-msg)
[ -r "${msg_file}" ] || exit 0

nr_of_non_empty_lines=$(sed -e '/^#/d' -e '/^[[:space:]]*$/d' "${1}" | wc -l)
if [ "${nr_of_non_empty_lines}" -ne 0 ]
then
    echo "# last commit message:" >> "${1}"
    sed -e '/^#/d' -e 's/^/# /' "${msg_file}" >> "${1}"
    exit 0
fi

echo 'restoring commit message...'
cp "${msg_file}" "${1}"
EOF

chmod +x save-commit-msg restore-commit-msg
git add save-commit-msg restore-commit-msg
git commit --message='Add {save,restore}-commit-msg'

cat > .pre-commit-config.yaml <<EOF
repos:
  - repo: local
    hooks:
    - id: restore-commit-msg
      entry: restore-commit-msg
      language: script
      name: Restore commit message
      stages:
      - prepare-commit-msg
  - repo: ../gitlint.git
    rev: v0.11.0
    hooks:
    - id: gitlint
      stages:
      - commit-msg
  - repo: local
    hooks:
    - id: save-commit-msg
      entry: save-commit-msg
      language: script
      name: Save commit message
      stages:
      - commit-msg
EOF
git add .pre-commit-config.yaml
git commit --message="Add pre-commit config"

for hook_type in prepare-commit-msg commit-msg
do
    pre-commit install --hook-type "${hook_type}"
done
pre-commit install-hooks

touch foo
git add foo
git commit --message=$'This is a good one\n\nMessage body Message body Message body Message body Message body'
git commit --amend --message=$'This is a bad one\nbooooooo'
git commit --amend

The last amend will bring up the last rejected message as a comment in the new one.

@asottile
Copy link
Member

@asottile asottile commented Jul 20, 2019

One way to bring the commit message back is the suggestion here by @pylipp

git commit --edit --file $(git rev-parse --git-path COMMIT_EDITMSG)

@billsioros

This comment has been minimized.

@asottile
Copy link
Member

@asottile asottile commented Aug 6, 2021

@billsioros off topic

@billsioros
Copy link

@billsioros billsioros commented Aug 8, 2021

@billsioros off topic

@asottile Sorry for being off topic. This was the most relevant issue that I was able to find. Should I maybe open a separate issue?

@asottile
Copy link
Member

@asottile asottile commented Aug 8, 2021

unless your issue is identical to the issue you should always make a new issue -- otherwise it's off topic

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

No branches or pull requests

3 participants