Markus Heidelberg [off-list ref] writes:
Is rollback_lock_file(&index_lock) necessary? It isn't used in
"git commit --dry-run" when commit_style is COMMIT_AS_IS.
That is because AS_IS commit does not even lock anything for writing, as
AS_IS means just that: "git commit" does not touch the index but just
writes tree out of the index.
Upon program exit (unless you get an uncontrolled crash), the lockfile API
arranges atexit(3) to roll back the lockfiles, so it probably may not make
much of a difference if you omitted rollback_lock_file(&index_lock)
yourself, but it is a good idea to clean up the mess you made after you
are done, especially if the mess is not something the operating system
will clean up for us (e.g. open file descriptors, malloc'ed region of
memory etc.)
To make sure that the failure case is covered, you may also want to add a
test case where you run "chmod a-w $GIT_DIR" and then run status (but that
test needs to be conditional on POSIXPERM).
Thanks.
Signed-off-by: Markus Heidelberg <redacted>
---
t/t7508-status.sh | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index c317bde..baa8d7b 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -703,4 +703,14 @@ test_expect_success 'commit --dry-run submodule summary (--amend)' '
test_cmp expect output
'
+test_expect_success POSIXPERM 'status succeeds in a read-only repository' '
+ (
+ chmod a-w .git &&
+ git status
+ )
+ status=$?
+ chmod 775 .git
+ (exit $status)
+'
+
test_done
--
1.7.0.4.304.gc2d16
Junio C Hamano, 2010-04-03 06:33:
Markus Heidelberg [off-list ref] writes:
quoted
Is rollback_lock_file(&index_lock) necessary? It isn't used in
"git commit --dry-run" when commit_style is COMMIT_AS_IS.
That is because AS_IS commit does not even lock anything for writing, as
AS_IS means just that: "git commit" does not touch the index but just
writes tree out of the index.
Hmm, it does lock and write the index, doesn't it?
/*
* As-is commit.
*
* (1) return the name of the real index file.
*
* The caller should run hooks on the real index,
* and create commit from the_index.
* We still need to refresh the index here.
*/
if (!pathspec || !*pathspec) {
fd = hold_locked_index(&index_lock, 1);
refresh_cache_or_die(refresh_flags);
if (write_cache(fd, active_cache, active_nr) ||
commit_locked_index(&index_lock))
die("unable to write new_index file");
commit_style = COMMIT_AS_IS;
return get_index_file();
}
$ stat .git/index
Access: 2010-04-03 12:31:11.000000000 +0200
Modify: 2010-04-03 12:31:11.000000000 +0200
Change: 2010-04-03 12:31:11.000000000 +0200
$ git commit --dry-run
$ stat .git/index
Access: 2010-04-03 12:31:52.000000000 +0200
Modify: 2010-04-03 12:31:52.000000000 +0200
Change: 2010-04-03 12:31:52.000000000 +0200
$ chmod a-w .git
$ git commit --dry-run
fatal: Unable to create '/home/markus/git/git/.git/index.lock': Permission denied
Upon program exit (unless you get an uncontrolled crash), the lockfile API
arranges atexit(3) to roll back the lockfiles, so it probably may not make
much of a difference if you omitted rollback_lock_file(&index_lock)
yourself, but it is a good idea to clean up the mess you made after you
are done, especially if the mess is not something the operating system
will clean up for us (e.g. open file descriptors, malloc'ed region of
memory etc.)
Thanks for the explanation!
To make sure that the failure case is covered, you may also want to add a
test case where you run "chmod a-w $GIT_DIR" and then run status (but that
test needs to be conditional on POSIXPERM).
Patch has been sent.
Markus