| Message ID | 20190214144241.11240-1-peartben@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] read-cache: add post-indexchanged hook | expand |
On 14/02/2019 14:42, Ben Peart wrote: > From: Ben Peart <benpeart@microsoft.com> > > Add a post-indexchanged hook that is invoked after the index is written in s/post-indexchanged/post-index-changed/ > do_write_locked_index(). > > This hook is meant primarily for notification, and cannot affect > the outcome of git commands that trigger the index write. > > The hook is passed a flag to indicate whether the working directory was > updated or not and a flag indicating if a skip-worktree bit could have > changed. These flags enable the hook to optmize its response to the s/optmize/optimize/ ATB, Ramsay Jones
Ramsay Jones <ramsay@ramsayjones.plus.com> writes: > On 14/02/2019 14:42, Ben Peart wrote: >> From: Ben Peart <benpeart@microsoft.com> >> >> Add a post-indexchanged hook that is invoked after the index is written in > > s/post-indexchanged/post-index-changed/ Good. I wasn't paying close attention to the previous round, but is that the only name-related bikeshedding? I somehow feel that without s/changed/change/ the name does not roll well on my tongue and does not sit well together with existing ones like post-receive (which is not post-received). I dunno. Will queue. Thanks. >> do_write_locked_index(). >> >> This hook is meant primarily for notification, and cannot affect >> the outcome of git commands that trigger the index write. >> >> The hook is passed a flag to indicate whether the working directory was >> updated or not and a flag indicating if a skip-worktree bit could have >> changed. These flags enable the hook to optmize its response to the > > s/optmize/optimize/ > > ATB, > Ramsay Jones
On 2/14/2019 3:33 PM, Junio C Hamano wrote: > Ramsay Jones <ramsay@ramsayjones.plus.com> writes: > >> On 14/02/2019 14:42, Ben Peart wrote: >>> From: Ben Peart <benpeart@microsoft.com> >>> >>> Add a post-indexchanged hook that is invoked after the index is written in >> >> s/post-indexchanged/post-index-changed/ > > Good. I wasn't paying close attention to the previous round, but is > that the only name-related bikeshedding? I somehow feel that > without s/changed/change/ the name does not roll well on my tongue > and does not sit well together with existing ones like post-receive > (which is not post-received). I dunno. > > Will queue. Thanks. Would you like me to submit another version with the above spelling corrections in the commit message or is it easier to fix it up yourself? > >>> do_write_locked_index(). >>> >>> This hook is meant primarily for notification, and cannot affect >>> the outcome of git commands that trigger the index write. >>> >>> The hook is passed a flag to indicate whether the working directory was >>> updated or not and a flag indicating if a skip-worktree bit could have >>> changed. These flags enable the hook to optmize its response to the >> >> s/optmize/optimize/ >> >> ATB, >> Ramsay Jones
Ben Peart <peartben@gmail.com> writes: > On 2/14/2019 3:33 PM, Junio C Hamano wrote: >> Ramsay Jones <ramsay@ramsayjones.plus.com> writes: >> >>> On 14/02/2019 14:42, Ben Peart wrote: >>>> From: Ben Peart <benpeart@microsoft.com> >>>> >>>> Add a post-indexchanged hook that is invoked after the index is written in >>> >>> s/post-indexchanged/post-index-changed/ >> >> Good. I wasn't paying close attention to the previous round, but is >> that the only name-related bikeshedding? I somehow feel that >> without s/changed/change/ the name does not roll well on my tongue >> and does not sit well together with existing ones like post-receive >> (which is not post-received). I dunno. >> >> Will queue. Thanks. > > Would you like me to submit another version with the above spelling > corrections in the commit message or is it easier to fix it up > yourself? I've already done s/indexchanged/index-changed/ before queuing (there was only one IIRC in the log message), and also the 'optimize' typofix. I didn't do anything about dropping 'd' at the end, as I haven't heard any feedback on that from anybody yet. >>>> do_write_locked_index(). >>>> >>>> This hook is meant primarily for notification, and cannot affect >>>> the outcome of git commands that trigger the index write. >>>> >>>> The hook is passed a flag to indicate whether the working directory was >>>> updated or not and a flag indicating if a skip-worktree bit could have >>>> changed. These flags enable the hook to optmize its response to the >>> >>> s/optmize/optimize/ >>> >>> ATB, >>> Ramsay Jones
> -----Original Message----- > From: Junio C Hamano <jch2355@gmail.com> On Behalf Of Junio C Hamano > Sent: Friday, February 15, 2019 12:50 PM > To: Ben Peart <peartben@gmail.com> > Cc: Ramsay Jones <ramsay@ramsayjones.plus.com>; git@vger.kernel.org; > Ben Peart <Ben.Peart@microsoft.com>; Kevin Willford > <kewillf@microsoft.com>; sandals@crustytoothpaste.net > Subject: Re: [PATCH v2] read-cache: add post-indexchanged hook > > Ben Peart <peartben@gmail.com> writes: > > > On 2/14/2019 3:33 PM, Junio C Hamano wrote: > >> Ramsay Jones <ramsay@ramsayjones.plus.com> writes: > >> > >>> On 14/02/2019 14:42, Ben Peart wrote: > >>>> From: Ben Peart <benpeart@microsoft.com> > >>>> > >>>> Add a post-indexchanged hook that is invoked after the index is > >>>> written in > >>> > >>> s/post-indexchanged/post-index-changed/ > >> > >> Good. I wasn't paying close attention to the previous round, but is > >> that the only name-related bikeshedding? I somehow feel that without > >> s/changed/change/ the name does not roll well on my tongue and does > >> not sit well together with existing ones like post-receive (which is > >> not post-received). I dunno. > >> > >> Will queue. Thanks. > > > > Would you like me to submit another version with the above spelling > > corrections in the commit message or is it easier to fix it up > > yourself? > > I've already done s/indexchanged/index-changed/ before queuing (there > was only one IIRC in the log message), and also the 'optimize' typofix. > > I didn't do anything about dropping 'd' at the end, as I haven't heard any > feedback on that from anybody yet. > I'm ok with either. post-index-changed sounded clearer to me but you're right, none of the other hooks use the post tense. I've submitted one with 'post-index-change' - feel free to keep/user either. > >>>> do_write_locked_index(). > >>>> > >>>> This hook is meant primarily for notification, and cannot affect > >>>> the outcome of git commands that trigger the index write. > >>>> > >>>> The hook is passed a flag to indicate whether the working directory > >>>> was updated or not and a flag indicating if a skip-worktree bit > >>>> could have changed. These flags enable the hook to optmize its > >>>> response to the > >>> > >>> s/optmize/optimize/ > >>> > >>> ATB, > >>> Ramsay Jones
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt index 959044347e..94b4dadf30 100644 --- a/Documentation/githooks.txt +++ b/Documentation/githooks.txt @@ -492,6 +492,24 @@ This hook is invoked by `git-p4 submit`. It takes no parameters and nothing from standard input. Exiting with non-zero status from this script prevent `git-p4 submit` from launching. Run `git-p4 submit --help` for details. +post-index-changed +~~~~~~~~~~~~~~~~~ + +This hook is invoked when the index is written in read-cache.c +do_write_locked_index. + +The first parameter passed to the hook is the indicator for the +working directory being updated. "1" meaning working directory +was updated or "0" when the working directory was not updated. + +The second parameter passed to the hook is the indicator for whether +or not the index was updated and the skip-worktree bit could have +changed. "1" meaning skip-worktree bits could have been updated +and "0" meaning they were not. + +Only one parameter should be set to "1" when the hook runs. The hook +running passing "1", "1" should not be possible. + GIT --- Part of the linkgit:git[1] suite diff --git a/builtin/reset.c b/builtin/reset.c index 4d18a461fa..e173afcaac 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -380,6 +380,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix) int flags = quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN; if (read_from_tree(&pathspec, &oid, intent_to_add)) return 1; + the_index.updated_skipworktree = 1; if (!quiet && get_git_work_tree()) { uint64_t t_begin, t_delta_in_ms; diff --git a/builtin/update-index.c b/builtin/update-index.c index 02ace602b9..cf731640fa 100644 --- a/builtin/update-index.c +++ b/builtin/update-index.c @@ -1071,6 +1071,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix) if (entries < 0) die("cache corrupted"); + the_index.updated_skipworktree = 1; + /* * Custom copy of parse_options() because we want to handle * filename arguments as they come. diff --git a/cache.h b/cache.h index 27fe635f62..46eb862d3e 100644 --- a/cache.h +++ b/cache.h @@ -338,7 +338,9 @@ struct index_state { struct cache_time timestamp; unsigned name_hash_initialized : 1, initialized : 1, - drop_cache_tree : 1; + drop_cache_tree : 1, + updated_workdir : 1, + updated_skipworktree : 1; struct hashmap name_hash; struct hashmap dir_hash; struct object_id oid; diff --git a/read-cache.c b/read-cache.c index 0e0c93edc9..b6ead7bf8f 100644 --- a/read-cache.c +++ b/read-cache.c @@ -17,6 +17,7 @@ #include "commit.h" #include "blob.h" #include "resolve-undo.h" +#include "run-command.h" #include "strbuf.h" #include "varint.h" #include "split-index.h" @@ -2999,8 +3000,17 @@ static int do_write_locked_index(struct index_state *istate, struct lock_file *l if (ret) return ret; if (flags & COMMIT_LOCK) - return commit_locked_index(lock); - return close_lock_file_gently(lock); + ret = commit_locked_index(lock); + else + ret = close_lock_file_gently(lock); + + run_hook_le(NULL, "post-index-changed", + istate->updated_workdir ? "1" : "0", + istate->updated_skipworktree ? "1" : "0", NULL); + istate->updated_workdir = 0; + istate->updated_skipworktree = 0; + + return ret; } static int write_split_index(struct index_state *istate, diff --git a/t/t7113-post-index-changed-hook.sh b/t/t7113-post-index-changed-hook.sh new file mode 100755 index 0000000000..6231b88fca --- /dev/null +++ b/t/t7113-post-index-changed-hook.sh @@ -0,0 +1,144 @@ +#!/bin/sh + +test_description='post index changed hook' + +. ./test-lib.sh + +test_expect_success 'setup' ' + mkdir -p dir1 && + touch dir1/file1.txt && + echo testing >dir1/file2.txt && + git add . && + git commit -m "initial" +' + +test_expect_success 'test status, add, commit, others trigger hook without flags set' ' + mkdir -p .git/hooks && + write_script .git/hooks/post-index-changed <<-\EOF && + if test "$1" -eq 1; then + echo "Invalid combination of flags passed to hook; updated_workdir is set." >testfailure + exit 1 + fi + if test "$2" -eq 1; then + echo "Invalid combination of flags passed to hook; updated_skipworktree is set." >testfailure + exit 1 + fi + if test -f ".git/index.lock"; then + echo ".git/index.lock exists" >testfailure + exit 3 + fi + if ! test -f ".git/index"; then + echo ".git/index does not exist" >testfailure + exit 3 + fi + echo "success" >testsuccess + EOF + mkdir -p dir2 && + touch dir2/file1.txt && + touch dir2/file2.txt && + : force index to be dirty && + test-tool chmtime +60 dir1/file1.txt && + git status && + test_path_is_file testsuccess && rm -f testsuccess && + test_path_is_missing testfailure && + git add . && + test_path_is_file testsuccess && rm -f testsuccess && + test_path_is_missing testfailure && + git commit -m "second" && + test_path_is_file testsuccess && rm -f testsuccess && + test_path_is_missing testfailure && + git checkout -- dir1/file1.txt && + test_path_is_file testsuccess && rm -f testsuccess && + test_path_is_missing testfailure && + git update-index && + test_path_is_missing testsuccess && + test_path_is_missing testfailure && + git reset --soft && + test_path_is_missing testsuccess && + test_path_is_missing testfailure +' + +test_expect_success 'test checkout and reset trigger the hook' ' + write_script .git/hooks/post-index-changed <<-\EOF && + if test "$1" -eq 1 && test "$2" -eq 1; then + echo "Invalid combination of flags passed to hook; updated_workdir and updated_skipworktree are both set." >testfailure + exit 1 + fi + if test "$1" -eq 0 && test "$2" -eq 0; then + echo "Invalid combination of flags passed to hook; neither updated_workdir or updated_skipworktree are set." >testfailure + exit 2 + fi + if test "$1" -eq 1; then + if test -f ".git/index.lock"; then + echo "updated_workdir set but .git/index.lock exists" >testfailure + exit 3 + fi + if ! test -f ".git/index"; then + echo "updated_workdir set but .git/index does not exist" >testfailure + exit 3 + fi + else + echo "update_workdir should be set for checkout" >testfailure + exit 4 + fi + echo "success" >testsuccess + EOF + : force index to be dirty && + test-tool chmtime +60 dir1/file1.txt && + git checkout master && + test_path_is_file testsuccess && rm -f testsuccess && + test_path_is_missing testfailure && + test-tool chmtime +60 dir1/file1.txt && + git checkout HEAD && + test_path_is_file testsuccess && rm -f testsuccess && + test_path_is_missing testfailure && + test-tool chmtime +60 dir1/file1.txt && + git reset --hard && + test_path_is_file testsuccess && rm -f testsuccess && + test_path_is_missing testfailure && + git checkout -B test && + test_path_is_file testsuccess && rm -f testsuccess && + test_path_is_missing testfailure +' + +test_expect_success 'test reset --mixed and update-index triggers the hook' ' + write_script .git/hooks/post-index-changed <<-\EOF && + if test "$1" -eq 1 && test "$2" -eq 1; then + echo "Invalid combination of flags passed to hook; updated_workdir and updated_skipworktree are both set." >testfailure + exit 1 + fi + if test "$1" -eq 0 && test "$2" -eq 0; then + echo "Invalid combination of flags passed to hook; neither updated_workdir or updated_skipworktree are set." >testfailure + exit 2 + fi + if test "$2" -eq 1; then + if test -f ".git/index.lock"; then + echo "updated_skipworktree set but .git/index.lock exists" >testfailure + exit 3 + fi + if ! test -f ".git/index"; then + echo "updated_skipworktree set but .git/index does not exist" >testfailure + exit 3 + fi + else + echo "updated_skipworktree should be set for reset --mixed and update-index" >testfailure + exit 4 + fi + echo "success" >testsuccess + EOF + : force index to be dirty && + test-tool chmtime +60 dir1/file1.txt && + git reset --mixed --quiet HEAD~1 && + test_path_is_file testsuccess && rm -f testsuccess && + test_path_is_missing testfailure && + git hash-object -w --stdin <dir1/file2.txt >expect && + git update-index --cacheinfo 100644 "$(cat expect)" dir1/file1.txt && + test_path_is_file testsuccess && rm -f testsuccess && + test_path_is_missing testfailure && + git update-index --skip-worktree dir1/file2.txt && + git update-index --remove dir1/file2.txt && + test_path_is_file testsuccess && rm -f testsuccess && + test_path_is_missing testfailure +' + +test_done diff --git a/unpack-trees.c b/unpack-trees.c index 3563daae1a..8665a4a7c0 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -1637,6 +1637,8 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options WRITE_TREE_SILENT | WRITE_TREE_REPAIR); } + + o->result.updated_workdir = 1; discard_index(o->dst_index); *o->dst_index = o->result; } else {