pre-commit hook leaves changes in my staging area and working directory

[simon04]

After this commit, https://github.com/simon04/pkgbuilds/commit/5b853c5ee54dfd9d27ec94c633e1a693dca562a6, I have changes in my staging area as well in the working directory. Presumably this is due to the pre-commit hook.

$ git status .
On branch master
Your branch is up to date with 'origin/master'.

Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
        modified:   .SRCINFO

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   .SRCINFO

$ git diff --staged
diff --git a/wikibase-cli/.SRCINFO b/wikibase-cli/.SRCINFO
index 06a7263..428104e 100644
--- a/wikibase-cli/.SRCINFO
+++ b/wikibase-cli/.SRCINFO
@@ -1,6 +1,6 @@
 pkgbase = wikibase-cli
        pkgdesc = The command-line interface to Wikibase (Wikidata)
-       pkgver = 9.0.2
+       pkgver = 9.0.1
        pkgrel = 1
        url = https://github.com/maxlath/wikibase-cli#readme
        arch = any
@@ -8,10 +8,10 @@ pkgbase = wikibase-cli
        makedepends = npm
        depends = nodejs
        replaces = nodejs-wikibase-cli
-       noextract = wikibase-cli-9.0.2.tgz
+       noextract = wikibase-cli-9.0.1.tgz
        options = !strip
-       source = https://registry.npmjs.org/wikibase-cli/-/wikibase-cli-9.0.2.tgz
-       sha256sums = 1e6b60998f1c636f86755d58ac98f8c1cd14ffab8484a713d5418b1bb6e1f164
+       source = https://registry.npmjs.org/wikibase-cli/-/wikibase-cli-9.0.1.tgz
+       sha256sums = 278fe41bf562d0150154eb25080e7a1f03a9e534086229b02f5a6ec6ecccfb40

 pkgname = wikibase-cli

$ git diff 
diff --git a/wikibase-cli/.SRCINFO b/wikibase-cli/.SRCINFO
index 428104e..06a7263 100644
--- a/wikibase-cli/.SRCINFO
+++ b/wikibase-cli/.SRCINFO
@@ -1,6 +1,6 @@
 pkgbase = wikibase-cli
        pkgdesc = The command-line interface to Wikibase (Wikidata)
-       pkgver = 9.0.1
+       pkgver = 9.0.2
        pkgrel = 1
        url = https://github.com/maxlath/wikibase-cli#readme
        arch = any
@@ -8,10 +8,10 @@ pkgbase = wikibase-cli
        makedepends = npm
        depends = nodejs
        replaces = nodejs-wikibase-cli
-       noextract = wikibase-cli-9.0.1.tgz
+       noextract = wikibase-cli-9.0.2.tgz
        options = !strip
-       source = https://registry.npmjs.org/wikibase-cli/-/wikibase-cli-9.0.1.tgz
-       sha256sums = 278fe41bf562d0150154eb25080e7a1f03a9e534086229b02f5a6ec6ecccfb40
+       source = https://registry.npmjs.org/wikibase-cli/-/wikibase-cli-9.0.2.tgz
+       sha256sums = 1e6b60998f1c636f86755d58ac98f8c1cd14ffab8484a713d5418b1bb6e1f164

 pkgname = wikibase-cli

To fix this, I have to run git reset HEAD.

[eli-schwartz]

Somehow it is rolling back the .SRCINFO changes, but only in the staging index? How odd. I'm not sure what would cause this, though, as I don't remember seeing such an issue before. Does this often happen to you, or just the once? Can you figure out how to reliably trigger it?

[simon04]

Hi, it happens every time on my system (Arch Linux, git version 2.25.0). The behaviour is reproducible with an empty global gitconfig (normally I use this config: https://github.com/simon04/dotfiles/blob/master/.gitconfig).

[rafasc]

Are you using: git commit packagename/PKGBUILD to commit instead of just git commit?

I've seen this reported before, not with aurpublish, but with other hooks that perform git-add in a pre-commit hook. An workaround for those cases would be to put git update-index -g on the post-commit.

[simon04]

I've been using git commit . inside packagename/ – this leaves the repository in the state described above. However, when doing git commit --all, everything is fine.

Awesome, thank you, @rafasc, adding git update-index -g to .git/hooks/post-commit did the trick.

[rafasc]

The behaviour is a consequence of using git commit with paths.

3. by listing files as arguments to the commit command (without --interactive or --patch switch), in which case the commit will ignore changes staged in the index, and instead record the current content of the listed files (which must already be known to Git);

@simon04 in your situation, I would use git add . ; git commit; or --all like you mentioned, without the need for the extra hook.

[eli-schwartz]

@rafasc thanks for pointing that out. Can you point me to the other projects that did similar things?

[eli-schwartz]

I'm trying to decide whether update-index -g is the "correct" tool for the job. What it seems to do is take every file for which there are staged changes, and git add them. It is true that immediately after a commit, the only files which are staged would ordinarily be ones erroneously staged, but I'm wondering if it makes more sense to e.g. explicitly git checkout HEAD ${pkgbase}/.SRCINFO to reset it to the version in the commit, since that's the only files we modified.

[eli-schwartz]

One edge case this would avoid is git add foo/PKGBUILD; vim foo/PKGBUILD; git commit bar/PKGBUILD, which would only commit bar/PKGBUILD, leave foo/PKGBUILD staged, and then git add changes which should not be applied. And sure, it feels silly for someone to actually do that, but I'd rather not break on it.

[rafasc]

There is: https://youtrack.jetbrains.com/issue/IDEA-135454#focus=streamItem-27-3068711-0-0 which was causing the same issue because it used --only internally. The stopped doing it.

But I remembered someone complaining at #git. It was a mixture of using linters, hook managers like https://github.com/typicode/husky and git for windows. If you search github for "update-index -g" you'll see that's a common strategy done by people.

It is true that immediately after a commit, the only files which are staged would ordinarily be ones erroneously staged

Not necessarily, Using --only (which is what commit with paths defaults to), that situation isn't erroneously at all, it is the intended state.

In this example, changes to b, both staged and not staged should be left as is.

touch a b
git add a b
echo something >b
git commit a
git status

Having update-index -g on post-commit will break this.

And adding the .SRCINFO to the commit is also a violation of the promise made by --only.

Doing git checkout HEAD ${pkgbase}/.SRCINFO would not work on pre-commit because when --only is used, a temporary index/staging area is used. c.f. GIT_INDEX_FILE. So any changes to the index we do there will be discarded after the commit. That's why it looked like changes were being rolled back, but the truth was that they were never made into the "official" index in the first place.

I am not sure if there's a "correct" way that does not compromise one thing or another.

Maybe a warning is enough if GIT_INDEX_FILE != GIT_DIR/index ?

[eli-schwartz]

Doing git checkout HEAD ${pkgbase}/.SRCINFO would not work on pre-commit because

I meant in post-commit. :p

...

Anyway, I can get the list of files from the previous commit with git diff --name-only HEAD^..HEAD --diff-filter=AM (adapted from pre-commit but with a commit range rather than --cached), then do a similar loop over them in the post-commit but with git checkout HEAD rather than git add. Thoughts?

And adding the .SRCINFO to the commit is also a violation of the promise made by --only.

The pre-commit hook is an enhancement of the behavior of --only, and the hook can be disabled using --no-verify if one really wishes, so I don't think this is a problem. I just want it to work consistently. :D

[eli-schwartz]

@rafasc wrote some nice plumbing commands so I didn't have to. :D

I have uploaded a new snapshot to the [community] repository which fixes this, you can run aurpublish setup to update/reinstall the githooks, particularly, to add the new one.

[rafasc]

It was a combined effort :)

[simon04]

Awesome, thank you so much, @eli-schwartz and @rafasc!