Created attachment 688395 [details] git output When trying to push a tag in the proj/emacs-tools.git repository: $ git push -v origin emacs-common-gentoo-1.7 [...] remote: Please make sure to read the copyright policy before adding Signed-off-by! remote: https://www.gentoo.org/glep/glep-0076.html remote: error: hook declined to update refs/tags/emacs-common-gentoo-1.7 To git+ssh://git.gentoo.org/proj/emacs-tools.git ! [remote rejected] emacs-common-gentoo-1.7 -> emacs-common-gentoo-1.7 (hook declined) error: failed to push some refs to 'git+ssh://git.gentoo.org/proj/emacs-tools.git' However, pushing the actual commits (including the tagged commit) succeeded. Arguably, the hook should not run this checks when a tag is being pushed. Alternatively, it should not not check for S-o-B on old commits (e.g., committer date before 2019-01-01).
How about adding an additional date check? Like this: commit_date=$(git show -q --pretty=format:'%ct') if [[ ${commit_date} -lt 1540166400 ]]; then ... fi (1540166400 is 2018-10-22 in seconds since the epoch, which is the day after GLEP 76 being marked as active.) That would also help when importing old repos converted from cvs/svn, for which I had to ask infra to disable the hook in the past.
Created attachment 688620 [details, diff] Check only commits unreachable from any refs This patch should fix the problem.
Are you sure we won't hit some limits passing all these revisions to rev-list? $ git rev-parse --all | wc -l 11643
You have a repo with 11643 branches and tags? :)
(In reply to Ulrich Müller from comment #4) > You have a repo with 11643 branches and tags? :) Oh, these are only ref heads. Then I guess it's fine.
@other infra members, any opinion on the patch?
Created attachment 688623 [details, diff] Check only commits unreachable from any refs [v2] If there's really a concern about the length of the command line: git rev-list can read the list of revisions from stdin.
I wonder if you could use <(...) to store the fd in the variable instead of the list, and just let git read straight from the pipe. No, but seriously, it's great work, thank you!
Not sure if I understand that suggestion. That would be nested <(...) ? If you dislike the pipe, that could be trivially changed to a here-string: <(git rev-list ${rev_list_args} <<< "${allrefs}")
(In reply to Ulrich Müller from comment #9) > Not sure if I understand that suggestion. That would be nested <(...) ? > > If you dislike the pipe, that could be trivially changed to a here-string: > <(git rev-list ${rev_list_args} <<< "${allrefs}") No, my point was to avoiding storing the list in memory at all. But as I said, the optimization shouldn't really be necessary, and I don't seem to be able to get it to work.
Created attachment 688689 [details, diff] Check only commits unreachable from any refs [v3] Let's go for the here-string anyway. :)
Created attachment 688707 [details, diff] version with output piped straight to git How about this version?
Created attachment 688761 [details, diff] Check only commits unreachable from any refs [v4] (In reply to Michał Górny from comment #12) > How about this version? The tradeoff is less memory usage versus an additional sed process. Which isn't actually needed, because "git rev-parse --not" can itself do that. :) Updated version attached.
Created attachment 693306 [details, diff] local/update-*: Check for unreachable commits also in other hooks I still get "No common commits with master!" warnings from the update-04-utf8 hooks. Attached patch syncs that hook with the approach from update-06-copyright. It also updates two other hooks where the "new branch" case can never happen.
These fixes are in the repo, and antarus has deployed them yesterday. Closing. commit ba53933add13608c6fa01f3146cd316a2f20b8be (HEAD -> master, tag: githooks-20210324T103327Z, origin/master, origin/HEAD) Author: Ulrich Müller <ulm@gentoo.org> Date: Wed Mar 24 11:04:35 2021 +0100 local/update-*: Check for unreachable commits also in other hooks To this end, sync update-04-utf8 from update-06-copyright. update-03-filename and update-05-manifest check commits only in master, so we need not consider branch creation. Signed-off-by: Ulrich Müller <ulm@gentoo.org> Closes: https://bugs.gentoo.org/772926 Signed-off-by: Michał Górny <mgorny@gentoo.org> commit 0213949276f9bd4ab33641f3988df225fc391b06 (tag: githooks-20210314T213611Z) Author: Ulrich Müller <ulm@gentoo.org> Date: Sat Feb 27 14:44:32 2021 +0100 local/update-06-copyright: Check only commits unreachable from any refs Closes: https://bugs.gentoo.org/772926 Signed-off-by: Ulrich Müller <ulm@gentoo.org> Signed-off-by: Michał Górny <mgorny@gentoo.org>
