{"thread":{"id":"23301","subject":"[PATCH 0/4] fix regression that \"git status\" doesn't refresh the index","startedAt":"2010-04-02T12:27:17Z","lastAt":"2010-04-06T06:20:07Z","messageCount":15,"participants":["Markus Heidelberg","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"138420","messageId":"1270211241-10795-1-git-send-email-markus.heidelberg@web.de","threadId":"23301","inReplyTo":null,"subject":"[PATCH 0/4] fix regression that \"git status\" doesn't refresh the index","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2010-04-02T12:27:17Z","receivedAt":"2010-04-02T12:27:17Z","isPatch":true,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"Patches 1 and 2 are minor unrelated fixes, noticed while working on the regression.\nPatches 3 and 4 add a test and fix the regression.\n\nMarkus Heidelberg (4):\n builtin/commit: fix duplicated sentence in a comment\n builtin/commit: remove unnecessary variable definition\n t7508: add test for \"git status\" refreshing the index\n git status: refresh the index\n\n builtin/commit.c | 11 ++++++++---\n t/t7508-status.sh | 10 ++++++++++\n 2 files changed, 18 insertions(+), 3 deletions(-)\n"},{"id":"138421","messageId":"1270211241-10795-2-git-send-email-markus.heidelberg@web.de","threadId":"23301","inReplyTo":"1270211241-10795-1-git-send-email-markus.heidelberg@web.de","subject":"[PATCH 1/4] builtin/commit: fix duplicated sentence in a comment","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2010-04-02T12:27:18Z","receivedAt":"2010-04-02T12:27:18Z","isPatch":true,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"\nSigned-off-by: Markus Heidelberg \n---\n builtin/commit.c | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 8dd104e..8cc9293 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -322,8 +322,8 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int\n \t *\n \t * (1) return the name of the real index file.\n \t *\n-\t * The caller should run hooks on the real index, and run\n-\t * hooks on the real index, and create commit from the_index.\n+\t * The caller should run hooks on the real index,\n+\t * and create commit from the_index.\n \t * We still need to refresh the index here.\n \t */\n \tif (!pathspec || !*pathspec) {\n-- \n1.7.0.4.300.gc535b\n"},{"id":"138424","messageId":"1270211241-10795-3-git-send-email-markus.heidelberg@web.de","threadId":"23301","inReplyTo":"1270211241-10795-1-git-send-email-markus.heidelberg@web.de","subject":"[PATCH 2/4] builtin/commit: remove unnecessary variable definition","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2010-04-02T12:27:19Z","receivedAt":"2010-04-02T12:27:19Z","isPatch":true,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"The file descriptor is already defined at the beginning of the function.\n\nSigned-off-by: Markus Heidelberg \n---\n builtin/commit.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 8cc9293..c5ab683 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -307,7 +307,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int\n \t * (B) on failure, rollback the real index.\n \t */\n \tif (all || (also && pathspec && *pathspec)) {\n-\t\tint fd = hold_locked_index(&index_lock, 1);\n+\t\tfd = hold_locked_index(&index_lock, 1);\n \t\tadd_files_to_cache(also ? prefix : NULL, pathspec, 0);\n \t\trefresh_cache_or_die(refresh_flags);\n \t\tif (write_cache(fd, active_cache, active_nr) ||\n-- \n1.7.0.4.300.gc535b\n"},{"id":"138422","messageId":"1270211241-10795-4-git-send-email-markus.heidelberg@web.de","threadId":"23301","inReplyTo":"1270211241-10795-1-git-send-email-markus.heidelberg@web.de","subject":"[PATCH 3/4] t7508: add test for \"git status\" refreshing the index","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2010-04-02T12:27:20Z","receivedAt":"2010-04-02T12:27:20Z","isPatch":true,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"\nSigned-off-by: Markus Heidelberg \n---\n t/t7508-status.sh | 10 ++++++++++\n 1 files changed, 10 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t7508-status.sh b/t/t7508-status.sh\nindex 556d0fa..086ec3a 100755\n--- a/t/t7508-status.sh\n+++ b/t/t7508-status.sh\n@@ -496,6 +496,16 @@ test_expect_success 'dry-run of partial commit excluding new file in index' '\n \ttest_cmp expect output\n '\n \n+cat >expect <output &&\n+\ttest_cmp expect output\n+'\n+\n test_expect_success 'setup status submodule summary' '\n \ttest_create_repo sm && (\n \t\tcd sm &&\n-- \n1.7.0.4.300.gc535b\n"},{"id":"138423","messageId":"1270211241-10795-5-git-send-email-markus.heidelberg@web.de","threadId":"23301","inReplyTo":"1270211241-10795-1-git-send-email-markus.heidelberg@web.de","subject":"[PATCH 4/4] git status: refresh the index","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2010-04-02T12:27:21Z","receivedAt":"2010-04-02T12:27:21Z","isPatch":true,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"This was already the case before commit 9e4b7ab6 (git status: not\n\"commit --dry-run\" anymore, 2009-08-15) and got lost during the\nconversion, which was meant to only change behaviour when invoked with\narguments.\n\nSigned-off-by: Markus Heidelberg \n---\n builtin/commit.c | 5 +++++\n t/t7508-status.sh | 2 +-\n 2 files changed, 6 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex c5ab683..2262734 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1017,6 +1017,7 @@ static int git_status_config(const char *k, const char *v, void *cb)\n int cmd_status(int argc, const char **argv, const char *prefix)\n {\n \tstruct wt_status s;\n+\tint fd;\n \tunsigned char sha1[20];\n \tstatic struct option builtin_status_options[] = {\n \t\tOPT__VERBOSE(&verbose),\n@@ -1050,6 +1051,10 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n \n \tread_cache_preload(s.pathspec);\n \trefresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, s.pathspec, NULL, NULL);\n+\tfd = hold_locked_index(&index_lock, 1);\n+\tif (write_cache(fd, active_cache, active_nr) ||\n+\t commit_locked_index(&index_lock))\n+\t\tdie(\"unable to write new_index file\");\n \ts.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;\n \ts.in_merge = in_merge;\n \twt_status_collect(&s);\ndiff --git a/t/t7508-status.sh b/t/t7508-status.sh\nindex 086ec3a..c317bde 100755\n--- a/t/t7508-status.sh\n+++ b/t/t7508-status.sh\n@@ -499,7 +499,7 @@ test_expect_success 'dry-run of partial commit excluding new file in index' '\n cat >expect <output &&\n-- \n1.7.0.4.300.gc535b\n"},{"id":"138438","messageId":"20100402165759.GB18576@coredump.intra.peff.net","threadId":"23301","inReplyTo":"1270211241-10795-5-git-send-email-markus.heidelberg@web.de","subject":"Re: [PATCH 4/4] git status: refresh the index","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-04-02T16:57:59Z","receivedAt":"2010-04-02T16:57:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Apr 02, 2010 at 02:27:21PM +0200, Markus Heidelberg wrote:\n\n> +\tfd = hold_locked_index(&index_lock, 1);\n> +\tif (write_cache(fd, active_cache, active_nr) ||\n> +\t commit_locked_index(&index_lock))\n> +\t\tdie(\"unable to write new_index file\");\n\nDoes this mean we will fail to run in a read-only repository? I think\nthat status, like diff, should refresh the index on disk if it _can_,\nbut as that refresh is a side effect of the main purpose (which is to\noutput information), it should not be fatal if it cannot do so.\n\n-Peff\n"},{"id":"138445","messageId":"7v6349bs52.fsf@alter.siamese.dyndns.org","threadId":"23301","inReplyTo":"1270211241-10795-5-git-send-email-markus.heidelberg@web.de","subject":"Re: [PATCH 4/4] git status: refresh the index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-04-02T18:56:09Z","receivedAt":"2010-04-02T18:56:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Markus Heidelberg writes:\n\n> This was already the case before commit 9e4b7ab6 (git status: not\n> \"commit --dry-run\" anymore, 2009-08-15) and got lost during the\n> conversion, which was meant to only change behaviour when invoked with\n> arguments.\n>\n> Signed-off-by: Markus Heidelberg \n> ---\n> builtin/commit.c | 5 +++++\n> t/t7508-status.sh | 2 +-\n> 2 files changed, 6 insertions(+), 1 deletions(-)\n>\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index c5ab683..2262734 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -1017,6 +1017,7 @@ static int git_status_config(const char *k, const char *v, void *cb)\n> int cmd_status(int argc, const char **argv, const char *prefix)\n> {\n> \tstruct wt_status s;\n> +\tint fd;\n> \tunsigned char sha1[20];\n> \tstatic struct option builtin_status_options[] = {\n> \t\tOPT__VERBOSE(&verbose),\n> @@ -1050,6 +1051,10 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n> \n> \tread_cache_preload(s.pathspec);\n> \trefresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, s.pathspec, NULL, NULL);\n> +\tfd = hold_locked_index(&index_lock, 1);\n> +\tif (write_cache(fd, active_cache, active_nr) ||\n> +\t commit_locked_index(&index_lock))\n> +\t\tdie(\"unable to write new_index file\");\n\nThis is a regression, I think.\n\nThe first two patches are trivially correct and I'll queue them to\n'master'.\n\nThanks.\n"},{"id":"138458","messageId":"201004022237.04130.markus.heidelberg@web.de","threadId":"23301","inReplyTo":"20100402165759.GB18576@coredump.intra.peff.net","subject":"Re: [PATCH 4/4] git status: refresh the index","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2010-04-02T20:37:03Z","receivedAt":"2010-04-02T20:37:03Z","isPatch":true,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"Jeff King, 2010-04-02 18:57:\n> On Fri, Apr 02, 2010 at 02:27:21PM +0200, Markus Heidelberg wrote:\n> \n> > +\tfd = hold_locked_index(&index_lock, 1);\n> > +\tif (write_cache(fd, active_cache, active_nr) ||\n> > +\t commit_locked_index(&index_lock))\n> > +\t\tdie(\"unable to write new_index file\");\n> \n> Does this mean we will fail to run in a read-only repository?\n\nYou're right.\nBut that was already the case when \"status\" was \"commit --dry-run\".\nI have to admit, I didn't think about this scenario, but simply looked\nfor the differences between these two commands.\n\n> I think\n> that status, like diff, should refresh the index on disk if it _can_,\n> but as that refresh is a side effect of the main purpose (which is to\n> output information), it should not be fatal if it cannot do so.\n\nSounds sensible.\n\nMarkus\n"},{"id":"138459","messageId":"201004022239.46139.markus.heidelberg@web.de","threadId":"23301","inReplyTo":"7v6349bs52.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/4] git status: refresh the index","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2010-04-02T20:39:45Z","receivedAt":"2010-04-02T20:39:45Z","isPatch":true,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"Junio C Hamano, 2010-04-02 20:56:\n> Markus Heidelberg writes:\n> \n> > This was already the case before commit 9e4b7ab6 (git status: not\n> > \"commit --dry-run\" anymore, 2009-08-15) and got lost during the\n> > conversion, which was meant to only change behaviour when invoked with\n> > arguments.\n> >\n> > Signed-off-by: Markus Heidelberg \n> > ---\n> > builtin/commit.c | 5 +++++\n> > t/t7508-status.sh | 2 +-\n> > 2 files changed, 6 insertions(+), 1 deletions(-)\n> >\n> > diff --git a/builtin/commit.c b/builtin/commit.c\n> > index c5ab683..2262734 100644\n> > --- a/builtin/commit.c\n> > +++ b/builtin/commit.c\n> > @@ -1017,6 +1017,7 @@ static int git_status_config(const char *k, const char *v, void *cb)\n> > int cmd_status(int argc, const char **argv, const char *prefix)\n> > {\n> > \tstruct wt_status s;\n> > +\tint fd;\n> > \tunsigned char sha1[20];\n> > \tstatic struct option builtin_status_options[] = {\n> > \t\tOPT__VERBOSE(&verbose),\n> > @@ -1050,6 +1051,10 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n> > \n> > \tread_cache_preload(s.pathspec);\n> > \trefresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, s.pathspec, NULL, NULL);\n> > +\tfd = hold_locked_index(&index_lock, 1);\n> > +\tif (write_cache(fd, active_cache, active_nr) ||\n> > +\t commit_locked_index(&index_lock))\n> > +\t\tdie(\"unable to write new_index file\");\n> \n> This is a regression, I think.\n\nA regressions in comparison with the current behaviour, but not with the\nformer \"commit --dry-run\".\n\nI'll send a second attempt without regression.\n\nMarkus\n"},{"id":"138463","messageId":"20100402212120.GA28352@coredump.intra.peff.net","threadId":"23301","inReplyTo":"201004022237.04130.markus.heidelberg@web.de","subject":"Re: [PATCH 4/4] git status: refresh the index","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-04-02T21:21:20Z","receivedAt":"2010-04-02T21:21:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Apr 02, 2010 at 09:37:03PM +0100, Markus Heidelberg wrote:\n\n> > Does this mean we will fail to run in a read-only repository?\n> \n> You're right.\n> But that was already the case when \"status\" was \"commit --dry-run\".\n> I have to admit, I didn't think about this scenario, but simply looked\n> for the differences between these two commands.\n\nSort of. See ab68545 (status: don't require the repository to be\nwritable, 2010-01-19), which went onto maint while the \"status is no\nlonger commit --dry-run\" topic was cooking elsewhere. So I think it\nwould be a regression from 1.6.6.2 onwards.\n\nAt any rate, I think we all agree on what it _should_ do, so if you're\nwilling to do an updated patch, that would be great. The patch from\nab68545 may be helpful, as the code it changed is quite similar to what\nyou posted in this series.\n\n-Peff\n"},{"id":"138465","messageId":"1270244661-24173-1-git-send-email-markus.heidelberg@web.de","threadId":"23301","inReplyTo":"1270211241-10795-5-git-send-email-markus.heidelberg@web.de","subject":"[PATCH v2 4/4] git status: refresh the index if possible","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2010-04-02T21:44:21Z","receivedAt":"2010-04-02T21:44:21Z","isPatch":true,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"This was already the case before commit 9e4b7ab6 (git status: not\n\"commit --dry-run\" anymore, 2009-08-15) with the difference that it died\nat failure.\nIt got lost during the new implementation of \"git status\", which was\nmeant to only change behaviour when invoked with arguments.\n\nSigned-off-by: Markus Heidelberg \n---\n\nv2:\nDoesn't die when writing the index fails and so works for read-only\nrepositories.\nIs rollback_lock_file(&index_lock) necessary? It isn't used in\n\"git commit --dry-run\" when commit_style is COMMIT_AS_IS.\n\n builtin/commit.c | 9 +++++++++\n t/t7508-status.sh | 2 +-\n 2 files changed, 10 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex c5ab683..3c14ade 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1017,6 +1017,7 @@ static int git_status_config(const char *k, const char *v, void *cb)\n int cmd_status(int argc, const char **argv, const char *prefix)\n {\n \tstruct wt_status s;\n+\tint fd;\n \tunsigned char sha1[20];\n \tstatic struct option builtin_status_options[] = {\n \t\tOPT__VERBOSE(&verbose),\n@@ -1050,6 +1051,14 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n \n \tread_cache_preload(s.pathspec);\n \trefresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, s.pathspec, NULL, NULL);\n+\n+\tfd = hold_locked_index(&index_lock, 0);\n+\tif (0 <= fd) {\n+\t\tif (!write_cache(fd, active_cache, active_nr))\n+\t\t\tcommit_locked_index(&index_lock);\n+\t\trollback_lock_file(&index_lock);\n+\t}\n+\n \ts.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;\n \ts.in_merge = in_merge;\n \twt_status_collect(&s);\ndiff --git a/t/t7508-status.sh b/t/t7508-status.sh\nindex 086ec3a..c317bde 100755\n--- a/t/t7508-status.sh\n+++ b/t/t7508-status.sh\n@@ -499,7 +499,7 @@ test_expect_success 'dry-run of partial commit excluding new file in index' '\n cat >expect <output &&\n-- \n1.7.0.4.300.ge0630\n"},{"id":"138489","messageId":"7v1vex9mur.fsf@alter.siamese.dyndns.org","threadId":"23301","inReplyTo":"1270244661-24173-1-git-send-email-markus.heidelberg@web.de","subject":"Re: [PATCH v2 4/4] git status: refresh the index if possible","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-04-03T04:33:16Z","receivedAt":"2010-04-03T04:33:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Markus Heidelberg writes:\n\n> Is rollback_lock_file(&index_lock) necessary? It isn't used in\n> \"git commit --dry-run\" when commit_style is COMMIT_AS_IS.\n\nThat is because AS_IS commit does not even lock anything for writing, as\nAS_IS means just that: \"git commit\" does not touch the index but just\nwrites tree out of the index.\n\nUpon program exit (unless you get an uncontrolled crash), the lockfile API\narranges atexit(3) to roll back the lockfiles, so it probably may not make\nmuch of a difference if you omitted rollback_lock_file(&index_lock)\nyourself, but it is a good idea to clean up the mess you made after you\nare done, especially if the mess is not something the operating system\nwill clean up for us (e.g. open file descriptors, malloc'ed region of\nmemory etc.)\n\nTo make sure that the failure case is covered, you may also want to add a\ntest case where you run \"chmod a-w $GIT_DIR\" and then run status (but that\ntest needs to be conditional on POSIXPERM).\n\nThanks.\n"},{"id":"138495","messageId":"1270289517-32680-1-git-send-email-markus.heidelberg@web.de","threadId":"23301","inReplyTo":"7v1vex9mur.fsf@alter.siamese.dyndns.org","subject":"[PATCH] t7508: add a test for \"git status\" in a read-only repository","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2010-04-03T10:11:57Z","receivedAt":"2010-04-03T10:11:57Z","isPatch":true,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"\nSigned-off-by: Markus Heidelberg \n---\n t/t7508-status.sh | 10 ++++++++++\n 1 files changed, 10 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t7508-status.sh b/t/t7508-status.sh\nindex c317bde..baa8d7b 100755\n--- a/t/t7508-status.sh\n+++ b/t/t7508-status.sh\n@@ -703,4 +703,14 @@ test_expect_success 'commit --dry-run submodule summary (--amend)' '\n \ttest_cmp expect output\n '\n \n+test_expect_success POSIXPERM 'status succeeds in a read-only repository' '\n+\t(\n+\t\tchmod a-w .git &&\n+\t\tgit status\n+\t)\n+\tstatus=$?\n+\tchmod 775 .git\n+\t(exit $status)\n+'\n+\n test_done\n-- \n1.7.0.4.304.gc2d16\n"},{"id":"138497","messageId":"201004031233.46258.markus.heidelberg@web.de","threadId":"23301","inReplyTo":"7v1vex9mur.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 4/4] git status: refresh the index if possible","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2010-04-03T10:33:46Z","receivedAt":"2010-04-03T10:33:46Z","isPatch":true,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"Junio C Hamano, 2010-04-03 06:33:\n> Markus Heidelberg writes:\n> \n> > Is rollback_lock_file(&index_lock) necessary? It isn't used in\n> > \"git commit --dry-run\" when commit_style is COMMIT_AS_IS.\n> \n> That is because AS_IS commit does not even lock anything for writing, as\n> AS_IS means just that: \"git commit\" does not touch the index but just\n> writes tree out of the index.\n\nHmm, it does lock and write the index, doesn't it?\n\n\t/*\n\t * As-is commit.\n\t *\n\t * (1) return the name of the real index file.\n\t *\n\t * The caller should run hooks on the real index,\n\t * and create commit from the_index.\n\t * We still need to refresh the index here.\n\t */\n\tif (!pathspec || !*pathspec) {\n\t\tfd = hold_locked_index(&index_lock, 1);\n\t\trefresh_cache_or_die(refresh_flags);\n\t\tif (write_cache(fd, active_cache, active_nr) ||\n\t\t commit_locked_index(&index_lock))\n\t\t\tdie(\"unable to write new_index file\");\n\t\tcommit_style = COMMIT_AS_IS;\n\t\treturn get_index_file();\n\t}\n\n$ stat .git/index\nAccess: 2010-04-03 12:31:11.000000000 +0200\nModify: 2010-04-03 12:31:11.000000000 +0200\nChange: 2010-04-03 12:31:11.000000000 +0200\n$ git commit --dry-run\n$ stat .git/index\nAccess: 2010-04-03 12:31:52.000000000 +0200\nModify: 2010-04-03 12:31:52.000000000 +0200\nChange: 2010-04-03 12:31:52.000000000 +0200\n\n$ chmod a-w .git\n$ git commit --dry-run\nfatal: Unable to create '/home/markus/git/git/.git/index.lock': Permission denied\n\n> Upon program exit (unless you get an uncontrolled crash), the lockfile API\n> arranges atexit(3) to roll back the lockfiles, so it probably may not make\n> much of a difference if you omitted rollback_lock_file(&index_lock)\n> yourself, but it is a good idea to clean up the mess you made after you\n> are done, especially if the mess is not something the operating system\n> will clean up for us (e.g. open file descriptors, malloc'ed region of\n> memory etc.)\n\nThanks for the explanation!\n\n> To make sure that the failure case is covered, you may also want to add a\n> test case where you run \"chmod a-w $GIT_DIR\" and then run status (but that\n> test needs to be conditional on POSIXPERM).\n\nPatch has been sent.\n\nMarkus\n"},{"id":"138714","messageId":"7vochxje5k.fsf@alter.siamese.dyndns.org","threadId":"23301","inReplyTo":"1270289517-32680-1-git-send-email-markus.heidelberg@web.de","subject":"Re: [PATCH] t7508: add a test for \"git status\" in a read-only repository","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-04-06T06:20:07Z","receivedAt":"2010-04-06T06:20:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks, will queue with a minor tweak.\n"}]}