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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jonathan Nieder <jrnieder@xxxxxxxxx> writes:

> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]