From 21123ba368d2722044d54e01d2697d5ac9883533 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Wed, 7 Feb 2024 16:15:35 -0400 Subject: [PATCH] assistant, undo: When committing, let the usual git commit hooks run Was doing a Git.Branch.commit for historical reasons to do with direct mode, which no longer apply. Note that the preCommitAnnexHook is no longer called in commitStaged because git-annex installs a pre-commit hook that runs the pre-commit-annex hook. And git commit will run the pre-commit hook. Sponsored-by: the NIH-funded NICEMAN (ReproNim TR&D3) project --- CHANGELOG | 2 + Command/Sync.hs | 16 +++----- ..._e7f759dccbfa134a772f15bc5f9c4b29._comment | 14 +++++++ ..._acd656bf30f7e0b5def7ad7a564cf914._comment | 39 +++++++++++++++++++ ..._0f49414f9b4a68a16a8d47a336242f13._comment | 16 ++++++++ 5 files changed, 77 insertions(+), 10 deletions(-) create mode 100644 doc/todo/make_annex___34__respect__34___.git__47__hooks__47__prepare-commit-msg/comment_1_e7f759dccbfa134a772f15bc5f9c4b29._comment create mode 100644 doc/todo/make_annex___34__respect__34___.git__47__hooks__47__prepare-commit-msg/comment_2_acd656bf30f7e0b5def7ad7a564cf914._comment create mode 100644 doc/todo/make_annex___34__respect__34___.git__47__hooks__47__prepare-commit-msg/comment_3_0f49414f9b4a68a16a8d47a336242f13._comment diff --git a/CHANGELOG b/CHANGELOG index 77f57c0c75..8057fb3c81 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -9,6 +9,8 @@ git-annex (10.20240130) UNRELEASED; urgency=medium already downloaded files when yt-dlp or a special remote was used. * addurl, importfeed: Added --raw-except option. * stack.yaml: Update to lts-22.9 and use crypton. + * assistant, undo: When committing, let the usual git commit + hooks run. -- Joey Hess Mon, 29 Jan 2024 15:59:33 -0400 diff --git a/Command/Sync.hs b/Command/Sync.hs index c4cbb0ccd1..52fe4c4aa0 100644 --- a/Command/Sync.hs +++ b/Command/Sync.hs @@ -36,7 +36,6 @@ import qualified Annex import qualified Annex.Branch import qualified Remote import qualified Types.Remote as Remote -import Annex.Hook import qualified Git.Command import qualified Git.LsFiles as LsFiles import qualified Git.Branch @@ -431,15 +430,12 @@ commitMsg = do return $ "git-annex in " ++ maybe "unknown" fromUUIDDesc (M.lookup u m) commitStaged :: Git.Branch.CommitMode -> String -> Annex Bool -commitStaged commitmode commitmessage = do - runAnnexHook preCommitAnnexHook - mb <- inRepo Git.Branch.currentUnsafe - let (getparent, branch) = case mb of - Just b -> (Git.Ref.sha b, b) - Nothing -> (Git.Ref.headSha, Git.Ref.headRef) - parents <- maybeToList <$> inRepo getparent - void $ inRepo $ Git.Branch.commit commitmode False commitmessage branch parents - return True +commitStaged commitmode commitmessage = + inRepo $ Git.Branch.commitCommand commitmode + (Git.Branch.CommitQuiet True) + [ Param "-m" + , Param commitmessage + ] mergeLocal :: [Git.Merge.MergeConfig] -> SyncOptions -> CurrBranch -> CommandStart mergeLocal mergeconfig o currbranch = stopUnless (notOnlyAnnex o) $ diff --git a/doc/todo/make_annex___34__respect__34___.git__47__hooks__47__prepare-commit-msg/comment_1_e7f759dccbfa134a772f15bc5f9c4b29._comment b/doc/todo/make_annex___34__respect__34___.git__47__hooks__47__prepare-commit-msg/comment_1_e7f759dccbfa134a772f15bc5f9c4b29._comment new file mode 100644 index 0000000000..0a5ff2e239 --- /dev/null +++ b/doc/todo/make_annex___34__respect__34___.git__47__hooks__47__prepare-commit-msg/comment_1_e7f759dccbfa134a772f15bc5f9c4b29._comment @@ -0,0 +1,14 @@ +[[!comment format=mdwn + username="joey" + subject="""comment 1""" + date="2024-02-07T18:59:34Z" + content=""" +Re the default messages, let's just say that any choice git-annex makes +about a default commit message will not be to some people's liking. + +Re the hook not being run, this is due to it using `git commit-tree`. +As well as being used for commits to the git-annex branch, it's also +used in a few other places, including `git-annex import`, and +generating adjusted branches and view branches, and the assistant +and `git-annex undo`. +"""]] diff --git a/doc/todo/make_annex___34__respect__34___.git__47__hooks__47__prepare-commit-msg/comment_2_acd656bf30f7e0b5def7ad7a564cf914._comment b/doc/todo/make_annex___34__respect__34___.git__47__hooks__47__prepare-commit-msg/comment_2_acd656bf30f7e0b5def7ad7a564cf914._comment new file mode 100644 index 0000000000..bf511cc113 --- /dev/null +++ b/doc/todo/make_annex___34__respect__34___.git__47__hooks__47__prepare-commit-msg/comment_2_acd656bf30f7e0b5def7ad7a564cf914._comment @@ -0,0 +1,39 @@ +[[!comment format=mdwn + username="joey" + subject="""comment 2""" + date="2024-02-07T19:09:44Z" + content=""" +Git has two hooks, prepare-commit-msg and commit-msg. prepare-commit-msg +is used prior to interactive editing of the commit message. Since these +commits do not involve interactive editing, I don't think it +necesarily makes sense for git-annex to use that hook. + +The commit-msg hook is simpler, and it would certianly be possible for +git-annex to call it everywhere it does a `git commit-tree`. + +But if git-annex called either of these hooks, how is the hook supposed to +know what is being committed? In the usual case, the hook +is called with `GIT_INDEX_FILE` pointed at an index file that +has been modified to include whatever is going to be in the commit +(which may be different than what is staged in the index), and it knows +that the working directory is being committed. So it can +do things like `git diff --name-stage --cached` to get the files that +are being committed, for example. + +In the git-annex case, `GIT_INDEX_FILE` is pointed at .git/annex/index +when committing the git-annex branch. If a commit-msg hook uses +`git diff --name-stage --cached`, it will diff from the working directory +to new git-annex branch contents, which will not be a useful set of file changes +to summarize in a commit message! + +Even if the hook somehow knows that the git-annex branch is being +committed, the best it could do is diff from git-annex branch to +`GIT_INDEX_FILE` and then it has a bunch of git-annex branch filenames +that have changed, and is supposed to somehow generate a useful commit +message from that. How? + +It seems more likely to me that you know that git-annex branch changes are +associated with an operation, so you might commit with "added 57 photos" +to the master branch, and then use the same message for the git-annex +branch commit. annex.commitmessage seems to already cover that case though? +"""]] diff --git a/doc/todo/make_annex___34__respect__34___.git__47__hooks__47__prepare-commit-msg/comment_3_0f49414f9b4a68a16a8d47a336242f13._comment b/doc/todo/make_annex___34__respect__34___.git__47__hooks__47__prepare-commit-msg/comment_3_0f49414f9b4a68a16a8d47a336242f13._comment new file mode 100644 index 0000000000..1fe2228d48 --- /dev/null +++ b/doc/todo/make_annex___34__respect__34___.git__47__hooks__47__prepare-commit-msg/comment_3_0f49414f9b4a68a16a8d47a336242f13._comment @@ -0,0 +1,16 @@ +[[!comment format=mdwn + username="joey" + subject="""comment 3""" + date="2024-02-07T19:58:11Z" + content=""" +Irrespective of comment #2, I do think that it makes sense for the +assistant and `git-annex undo`, when committing to the HEAD branch, +to run all the usual commit hooks. + +Actually, I don't see any reason for those to not run `git commit`. +It seems that the use of `git commit-tree` in those dates back to +[[!commit 03932212ecb8940e0fe2dd09c04951990bc29422]] which involved +direct mode. + +Fixed those to run commits in the usual way (though noninteractive). +"""]]