Re: [PATCH v2 4/4] git status: refresh the index if possible

From: Markus Heidelberg <markus.heidelberg@web.de>
Date: Apr 3, 2010, 10:33 UTC
Message-ID: <201004031233.46258.markus.heidelberg@web.de>
In-Reply-To: <7v1vex9mur.fsf@alter.siamese.dyndns.org>

Junio C Hamano, 2010-04-03 06:33:

Show 8 quoted lines

> Markus Heidelberg <markus.heidelberg@web.de> 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.

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

Show 7 quoted lines

> 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