Re: [PATCH 4/6] add -u: only show pathless 'add -u' warning when changes exist outside cwd

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 20, 2013, 15:10 UTC
Message-ID: <7v620mgl9y.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20130319225050.GE19014@google.com>

Jonathan Nieder <jrnieder@gmail.com> writes:

Show 29 quoted lines

> A common workflow in large projects is to chdir into a subdirectory of
> interest and only do work there:
>
> 	cd src
> 	vi foo.c
> 	make test
> 	git add -u
> 	git commit
>
> The upcoming change to 'git add -u' behavior would not affect such a
> workflow: when the only changes present are in the current directory,
> 'git add -u' will add all changes, and whether that happens via an
> implicit "." or implicit ":/" parameter is an unimportant
> implementation detail.
>
> The warning about use of 'git add -u' with no pathspec is annoying
> because it seemingly serves no purpose in this case.  So suppress the
> warning unless there are changes outside the cwd that are not being
> added.
>
> A previous version of this patch ran two I/O-intensive diff-files
> passes: one to find changes outside the cwd, and another to find
> changes to add to the index within the cwd.  This version runs one
> full-tree diff and decides for each change whether to add it or warn
> and suppress it in update_callback.  As a result, even on very large
> repositories "git add -u" will not be significantly slower than the
> future default behavior ("git add -u :/"), and the slowdown relative
> to "git add -u ." should be a useful clue to users of such
> repositories to get into the habit of explicitly passing '.'.

I'll queue this as-is for now, but the point Peff raised on IMPLICIT_DOT needs to be addressed.

This is a tangent, but I would also suggest at least considering to measure how big the working tree is relative to the area covered by the implicit dot [*1*]. If you are running "add -u" in a project with 30k paths but are working with only a few dozen files, you may want more encouragement to use an explicit "." than the command mysteriously and silently getting slower at Git version boundary.

IOW, an advice message issued only when (1) you are indeed working in a narrow directory, (2) you did not touch anything outside, and (3) you did not give an explicit ".". We would want to keep the advice in place even after Git 2.0 switches the default to ":/". In fact, it would become even more valuable after the transition. And make it protected under advice.addUAuseexplicitdot or something.

By the way, I am not a big fan of the approach taken in [3/6]. It may be a lot cleaner to separate the "do we need to warn?" logic that flips a global (or a field in a callback, if we make some of these codepaths callable from other places) and the code to issue the warning, no?

[Footnote]

*1* The former you can get an approximation from active_nr, the latter you can count after first finding where prefix would insert to the active_cache with index_name_pos(). I suspect that you also could add a new read-only API to cache-tree to see if it already knows how many index entries a given subtree for the prefix covers.