Re: [PATCH] Ensure that commit/status don't stat all files when core.ignoreStat = true

[Junio C Hamano <gitster@pobox.com>]

Marius Storm-Olsen <marius@trolltech.com> writes:

> diff --git a/read-cache.c b/read-cache.c
> index 8b467f8..104e387 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -882,6 +882,16 @@ static struct cache_entry *refresh_cache_ent(struct index_state *istate,
>  	if (ce_uptodate(ce))
>  		return ce;
>  
> +	/*
> +	 * assume_unchanged is used to avoid lstats to check if the
> +	 * file has been modified. When true, the user need to
> +	 * manually update the index.
> +	 */
> +	if (assume_unchanged) {
> +		ce_mark_uptodate(ce);
> +		return ce;
> +	}
> +

The description for core.ignorestat in Documentation/config.txt is quite
bogus.  That single bit does _not_ determine globally if we lstat(2) or
not.  The description in Documentation/git-update-index.txt about it (look
for the section "Using assume unchanged bit") accurately describes what it
is meant to do.  The rules are:

 - (ce->ce_flags & CE_VALID) is the only thing that decides if we can omit
   lstat(2) for _that particular path_.  There is no global "we would
   never ever run lstat(2)" option, and core.ignorestat certainly isn't
   it.

 - you can use the assume-unchanged mechanism without setting
   core.ignorestat.  You flip the CE_VALID bit for selected paths manually
   and forget about them afterwards, when you would want all of your usual
   "active" changes noticed by git, while skipping lstat(2) overhead in
   areas you are not interested in.

 - when you say "git update-index" (or "git add") for a path, if you have
   core.ignorestat set, that path is automatically marked with CE_VALID,
   so that later lstat(2) will be omitted for that particular path.  IOW,
   by having core.ignorestat set, you are promising that you are not going
   to _further_ change the work tree contents _without_ telling git --- or
   at least you are promising that you _will_ tell git if you change it
   when it matters.  But you have to tell git at least once what the
   contents are.

Would it be sufficient for what you are trying to do if you changed that
test to something like this?

        /*
         * CE_VALID means the user promised us that the change to
         * the work tree does not matter and told us not to worry.
         */
	if (!ignore_valid && (ce->ce_flags & CE_VALID)) {
        	ce_mark_uptodate(ce);
		return ce;
	}

> diff --git a/wt-status.c b/wt-status.c
> index a44c543..72db466 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -342,7 +342,14 @@ void wt_status_print(struct wt_status *s)
>  	wt_status_print_changed(s);
>  	if (wt_status_submodule_summary)
>  		wt_status_print_submodule_summary(s);
> -	wt_status_print_untracked(s);
> +
> +	if (assume_unchanged && !s->untracked) {
> +		if (s->commitable)
> +			fprintf(s->fp, "# Untracked files not listed (use -u option to show untracked files)\n");
> +		/* !s->commitable message displayed below */
> +	}
> +	else
> +		wt_status_print_untracked(s);
>  
>  	if (s->verbose && !s->is_initial)
>  		wt_status_print_verbose(s);
> @@ -357,6 +364,8 @@ void wt_status_print(struct wt_status *s)
>  			printf("nothing added to commit but untracked files present (use \"git add\" to track)\n");
>  		else if (s->is_initial)
>  			printf("nothing to commit (create/copy files and use \"git add\" to track)\n");
> +		else if (assume_unchanged && !s->untracked)
> +			printf("nothing to commit (use -u to show untracked files)\n");
>  		else
>  			printf("nothing to commit (working directory clean)\n");
>  	}

The core.ignorestat variable does not have anything to do with showing
untracked files.  It is about "do we mark the added path as CE_VALID,
meaning that we do not have to lstat(2) them?"  IOW, it is about tracked
files.

While it might be useful in certain workflows to ignore untracked files, I
do not think it is a good idea to overload such an unrelated meaning to
the variable.