Commit Graph

101285 Commits

Author SHA1 Message Date
Johannes Schindelin
74c73441bf Merge pull request #971 from jeffhostetler/jeffhostetler/add_preload_fscache
add: use preload-index and fscache for performance
2019-07-30 10:55:26 +02:00
Johannes Schindelin
990650aba3 Merge pull request #994 from jeffhostetler/jeffhostetler/fscache_nfd
fscache: add not-found directory cache to fscache
2019-07-30 10:55:26 +02:00
Johannes Schindelin
71865b70e0 Merge branch 'fscache' 2019-07-30 10:55:25 +02:00
Johannes Schindelin
29146dc830 Merge 'add-p-many-files'
This topic branch allows `add -p` and `add -i` with a large number of
files. It is kind of a hack that was never really meant to be
upstreamed. Let's see if we can do better in the built-in `add -p`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:55:25 +02:00
Johannes Schindelin
98bf98c94e Merge branch 'inherit-only-stdhandles'
When spawning child processes, we do want them to inherit the standard
handles so that we can talk to them. We do *not* want them to inherit
any other handle, as that would hold a lock to the respective files
(preventing them from being renamed, modified or deleted), and the child
process would not know how to access that handle anyway.

Happily, there is an API to make that happen. It is supported in Windows
Vista and later, which is exactly what we promise to support in Git for
Windows for the time being.

This also means that we lift, at long last, the target Windows version
from Windows XP to Windows Vista.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:55:24 +02:00
Johannes Schindelin
ff47f4ced2 Merge branch 'gitk-and-git-gui-patches'
These are Git for Windows' Git GUI and gitk patches. We will have to
decide at some point what to do about them, but that's a little lower
priority (as Git GUI seems to be unmaintained for the time being, and
the gitk maintainer keeps a very low profile on the Git mailing list,
too).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:55:24 +02:00
Johannes Schindelin
9b33c0acd7 Merge branch 'ready-for-upstream'
This is the branch thicket of patches in Git for Windows that are
considered ready for upstream. To keep them in a ready-to-submit shape,
they are kept as close to the beginning of the branch thicket as
possible.
2019-07-30 10:55:24 +02:00
Jeff Hostetler
f39ed38f71 add: use preload-index and fscache for performance
Teach "add" to use preload-index and fscache features
to improve performance on very large repositories.

During an "add", a call is made to run_diff_files()
which calls check_remove() for each index-entry.  This
calls lstat().  On Windows, the fscache code intercepts
the lstat() calls and builds a private cache using the
FindFirst/FindNext routines, which are much faster.

Somewhat independent of this, is the preload-index code
which distributes some of the start-up costs across
multiple threads.

We need to keep the call to read_cache() before parsing the
pathspecs (and hence cannot use the pathspecs to limit any preload)
because parse_pathspec() is using the index to determine whether a
pathspec is, in fact, in a submodule. If we would not read the index
first, parse_pathspec() would not error out on a path that is inside
a submodule, and t7400-submodule-basic.sh would fail with

	not ok 47 - do not add files from a submodule

We still want the nice preload performance boost, though, so we simply
call read_cache_preload(&pathspecs) after parsing the pathspecs.

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:55:10 +02:00
Johannes Schindelin
de36811faa fscache: add a test for the dir-not-found optimization
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:55:10 +02:00
Karsten Blees
a7d3c7da93 fscache: load directories only once
If multiple threads access a directory that is not yet in the cache, the
directory will be loaded by each thread. Only one of the results is added
to the cache, all others are leaked. This wastes performance and memory.

On cache miss, add a future object to the cache to indicate that the
directory is currently being loaded. Subsequent threads register themselves
with the future object and wait. When the first thread has loaded the
directory, it replaces the future object with the result and notifies
waiting threads.

Signed-off-by: Karsten Blees <blees@dcon.de>
2019-07-30 10:55:10 +02:00
Jeff Hostetler
54c81149ca fscache: remember not-found directories
Teach FSCACHE to remember "not found" directories.

This is a performance optimization.

FSCACHE is a performance optimization available for Windows.  It
intercepts Posix-style lstat() calls into an in-memory directory
using FindFirst/FindNext.  It improves performance on Windows by
catching the first lstat() call in a directory, using FindFirst/
FindNext to read the list of files (and attribute data) for the
entire directory into the cache, and short-cut subsequent lstat()
calls in the same directory.  This gives a major performance
boost on Windows.

However, it does not remember "not found" directories.  When STATUS
runs and there are missing directories, the lstat() interception
fails to find the parent directory and simply return ENOENT for the
file -- it does not remember that the FindFirst on the directory
failed. Thus subsequent lstat() calls in the same directory, each
re-attempt the FindFirst.  This completely defeats any performance
gains.

This can be seen by doing a sparse-checkout on a large repo and
then doing a read-tree to reset the skip-worktree bits and then
running status.

This change reduced status times for my very large repo by 60%.

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:55:10 +02:00
Karsten Blees
33bb735240 Win32: add a cache below mingw's lstat and dirent implementations
Checking the work tree status is quite slow on Windows, due to slow lstat
emulation (git calls lstat once for each file in the index). Windows
operating system APIs seem to be much better at scanning the status
of entire directories than checking single files.

Add an lstat implementation that uses a cache for lstat data. Cache misses
read the entire parent directory and add it to the cache. Subsequent lstat
calls for the same directory are served directly from the cache.

Also implement opendir / readdir / closedir so that they create and use
directory listings in the cache.

The cache doesn't track file system changes and doesn't plug into any
modifying file APIs, so it has to be explicitly enabled for git functions
that don't modify the working copy.

Note: in an earlier version of this patch, the cache was always active and
tracked file system changes via ReadDirectoryChangesW. However, this was
much more complex and had negative impact on the performance of modifying
git commands such as 'git checkout'.

Signed-off-by: Karsten Blees <blees@dcon.de>
2019-07-30 10:55:10 +02:00
Jeff Hostetler
45ef1bc435 fscache: add key for GIT_TRACE_FSCACHE
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:55:10 +02:00
Karsten Blees
f99a8c90c0 add infrastructure for read-only file system level caches
Add a macro to mark code sections that only read from the file system,
along with a config option and documentation.

This facilitates implementation of relatively simple file system level
caches without the need to synchronize with the file system.

Enable read-only sections for 'git status' and preload_index.

Signed-off-by: Karsten Blees <blees@dcon.de>
2019-07-30 10:55:10 +02:00
Karsten Blees
0b99c1b6b6 Win32: make the lstat implementation pluggable
Emulating the POSIX lstat API on Windows via GetFileAttributes[Ex] is quite
slow. Windows operating system APIs seem to be much better at scanning the
status of entire directories than checking single files. A caching
implementation may improve performance by bulk-reading entire directories
or reusing data obtained via opendir / readdir.

Make the lstat implementation pluggable so that it can be switched at
runtime, e.g. based on a config option.

Signed-off-by: Karsten Blees <blees@dcon.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:55:10 +02:00
Johannes Schindelin
0562925c61 t3701: verify that we can add *lots* of files interactively
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:55:09 +02:00
Johannes Schindelin
826ffa3bfa mingw: restrict file handle inheritance only on Windows 7 and later
Turns out that it don't work so well on Vista, see
https://github.com/git-for-windows/git/issues/1742 for details.

According to https://devblogs.microsoft.com/oldnewthing/?p=8873, it
*should* work on Windows Vista and later.

But apparently there are issues on Windows Vista when pipes are
involved. Given that Windows Vista is past its end of life (official
support ended on April 11th, 2017), let's not spend *too* much time on
this issue and just disable the file handle inheritance restriction on
any Windows version earlier than Windows 7.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:55:09 +02:00
Karsten Blees
ce31a5afb3 Win32: Make the dirent implementation pluggable
Emulating the POSIX dirent API on Windows via FindFirstFile/FindNextFile is
pretty staightforward, however, most of the information provided in the
WIN32_FIND_DATA structure is thrown away in the process. A more
sophisticated implementation may cache this data, e.g. for later reuse in
calls to lstat.

Make the dirent implementation pluggable so that it can be switched at
runtime, e.g. based on a config option.

Define a base DIR structure with pointers to readdir/closedir that match
the opendir implementation (i.e. similar to vtable pointers in OOP).
Define readdir/closedir so that they call the function pointers in the DIR
structure. This allows to choose the opendir implementation on a
call-by-call basis.

Move the fixed sized dirent.d_name buffer to the dirent-specific DIR
structure, as d_name may be implementation specific (e.g. a caching
implementation may just set d_name to point into the cache instead of
copying the entire file name string).

Signed-off-by: Karsten Blees <blees@dcon.de>
2019-07-30 10:55:09 +02:00
Kelly Heller
30d87b43f7 Allow add -p and add -i with a large number of files
This fixes https://github.com/msysgit/git/issues/182.

Inspired by Pull Request 218 using code from @PhilipDavis.

[jes: simplified code quite a bit]

Signed-off-by: Kelly Heller <kkheller@cedrus.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:55:09 +02:00
Johannes Schindelin
6fd7b0de78 mingw: spawned processes need to inherit only standard handles
By default, CreateProcess() does not inherit any open file handles,
unless the bInheritHandles parameter is set to TRUE. Which we do need to
set because we need to pass in stdin/stdout/stderr to talk to the child
processes. Sadly, this means that all file handles (unless marked via
O_NOINHERIT) are inherited.

This lead to problems in GVFS Git, where a long-running read-object hook
is used to hydrate missing objects, and depending on the circumstances,
might only be called *after* Git opened a file handle.

Ideally, we would not open files without O_NOINHERIT unless *really*
necessary (i.e. when we want to pass the opened file handle as standard
handle into a child process), but apparently it is all-too-easy to
introduce incorrect open() calls: this happened, and prevented updating
a file after the read-object hook was started because the hook still
held a handle on said file.

Happily, there is a solution: as described in the "Old New Thing"
https://blogs.msdn.microsoft.com/oldnewthing/20111216-00/?p=8873 there
is a way, starting with Windows Vista, that lets us define precisely
which handles should be inherited by the child process.

And since we bumped the minimum Windows version for use with Git for
Windows to Vista with v2.10.1 (i.e. a *long* time ago), we can use this
method. So let's do exactly that.

We need to make sure that the list of handles to inherit does not
contain duplicates; Otherwise CreateProcessW() would fail with
ERROR_INVALID_ARGUMENT.

While at it, stop setting errno to ENOENT unless it really is the
correct value.

Also, fall back to not limiting handle inheritance under certain error
conditions (e.g. on Windows 7, which is a lot stricter in what handles
you can specify to limit to).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:55:09 +02:00
Karsten Blees
e28182a5c3 Win32: dirent.c: Move opendir down
Move opendir down in preparation for the next patch.

Signed-off-by: Karsten Blees <blees@dcon.de>
2019-07-30 10:55:09 +02:00
Johannes Schindelin
0100c5d5fe mingw: work around incorrect standard handles
For some reason, when being called via TortoiseGit the standard handles,
or at least what is returned by _get_osfhandle(0) for standard input,
can take on the value (HANDLE)-2 (which is not a legal value, according
to the documentation).

Even if this value is not documented anywhere, CreateProcess() seems to
work fine without complaints if hStdInput set to this value.

In contrast, the upcoming code to restrict which file handles get
inherited by spawned processes would result in `ERROR_INVALID_PARAMETER`
when including such handle values in the list.

To help this, special-case the value (HANDLE)-2 returned by
_get_osfhandle() and replace it with INVALID_HANDLE_VALUE, which will
hopefully let the handle inheritance restriction work even when called
from TortoiseGit.

This fixes https://github.com/git-for-windows/git/issues/1481

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:55:09 +02:00
Karsten Blees
ee8618de28 Win32: make FILETIME conversion functions public
We will use them in the upcoming "FSCache" patches (to accelerate
sequential lstat() calls).

Signed-off-by: Karsten Blees <blees@dcon.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:55:09 +02:00
Johannes Schindelin
115b618fb1 mingw: demonstrate that all file handles are inherited by child processes
When spawning child processes, we really should be careful which file
handles we let them inherit.

This is doubly important on Windows, where we cannot rename, delete, or
modify files if there is still a file handle open.

Sadly, we have to guard this test inside #ifdef WIN32: we need to use
the value of the HANDLE directly, and that concept does not exist on
Linux/Unix.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:55:09 +02:00
Johannes Schindelin
64f5bd684e Merge branch 'work-around-isilon'
It would appear that least the Isilon network filesystem (and possibly
other network filesystems, too), report non-standard error values when
trying to access a non-existing directory.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:55:06 +02:00
Johannes Schindelin
01feddcec8 Merge branch 'dont-clean-junctions'
This topic branch teaches `git clean` to respect NTFS junctions and Unix
bind mounts: it will now stop at those boundaries.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:55:05 +02:00
Johannes Schindelin
f6e99c5c27 Merge branch 'msys2-python'
In MSYS2, we have two Python interpreters at our disposal, so we can
include the Python stuff in the build.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:55:05 +02:00
Johannes Schindelin
435951fdc7 Merge branch 'msys2-strace'
Debugging support on MSYS2.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:55:05 +02:00
Johannes Schindelin
b665671028 Merge branch 'robustify-is-hidden-tests'
In Git for Windows, there is an option to mark the .git directory as
hidden. Our test cases verify this by using the system utility
`attrib.exe`.

This file name is unfortunately quite generic, and overlaps with a
Unix-y utility that might be hiding the system one in the `PATH`.

Let's specify explicitly which `attrib` to use.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:55:04 +02:00
Johannes Schindelin
231ab46a5b Merge branch 'fflush-in-git-clean'
After writing to `stdout` and before reading from `stdin`, it is a good
idea to flush the former.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:55:04 +02:00
Johannes Schindelin
a563dd9516 Merge branch 'avoid-pipes-in-svn-tests'
It is a good idea in general to avoid pipes in test cases, as it makes
things more debuggable to have files to inspect (instead of ephemereal
piped data that is long gone).

This also seemed to work around a problem where MSYS2' Perl would
segfault which may, or may not, still be present today.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:55:03 +02:00
Johannes Schindelin
0ff2b2133a Merge pull request #2170 from dscho/gitk-long-cmdline
Fix gitk (long cmdline)
2019-07-30 10:55:03 +02:00
Johannes Schindelin
6bb3d776b7 Merge branch 'fsync-object-files-always'
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:55:03 +02:00
Johannes Schindelin
8750d27656 Merge branch 'program-data-config'
This branch introduces support for reading the "Windows-wide" Git
configuration from `%PROGRAMDATA%\Git\config`. As these settings are
intended to be shared between *all* Git-related software, that config
file takes an even lower precedence than `$(prefix)/etc/gitconfig`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:55:02 +02:00
Johannes Schindelin
12fd5fc41f Merge pull request #2148 from dscho/azure-pipelines-msvc
Let the MSVC build also be tested in the Azure Pipeline
2019-07-30 10:55:02 +02:00
Jeff Hostetler
591c299cbf Merge branch 'visual-studio'
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:55:01 +02:00
Johannes Schindelin
1ae3e93d3a Merge remote-tracking branch 'dscho/add-p' into add-p-g4w
Let's test this for a while.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:55:01 +02:00
Johannes Schindelin
ff3feb22fd Merge branch 'stash-p-corner-case'
This topic branch fixes a corner case that is amazingly common in this
developer's workflow: in a `git stash -p`, splitting a hunk and stashing
only part of it runs into a (known) bug where the partial hunk cannot be
applied in reverse.

It is one of those "good enough" fixes, not a full fix, though, as the
full fix would require a 3-way merge between `stash^` and the *worktree*
(not `HEAD`), with `stash` as merge base (i.e. a `git revert`, but on
top of the current worktree).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:54:55 +02:00
Johannes Schindelin
cb62da441d Merge branch 'add-p-in-c-config-settings'
This is the final leg of the journey to a fully built-in `git add`: the
`git add -i` and `git add -p` modes were re-implemented in C, but they
lacked support for a couple of config settings.

The one that sticks out most is the `interactive.singleKey` setting: it
was not only particularly hard to get to work, especially on Windows. It
is also the setting that seems to be incomplete already in the Perl
version: while the name suggests that it applies to the main loop of
`git add --interactive`, or to the file selections in that command, it
does not. Only the `git add --patch` mode respects that setting.

As it is outside the purpose of the conversion of
`git-add--interactive.perl` to C, we will leave that loose end for some
future date.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:54:55 +02:00
Johannes Schindelin
15f615988f Merge branch 'other-command-p-in-c'
At this stage on the journey to a fully built-in `git add`, we already
have everything we need, including the `--interactive` and `--patch`
options, as long as the `add.interactive.useBuiltin` setting is set to
`true` (kind of a "turned off feature flag", which it will be for a
while, until we get confident enough that the built-in version does the
job, and retire the Perl script).

However, the internal `add--interactive` helper is also used to back the
`--patch` option of `git stash`, `git reset` and `git checkout`.

This patch series brings them "online".

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:54:54 +02:00
Johannes Schindelin
d60c2096a1 Merge branch 'add-p-in-c'
Out of all the patch series on the journey to provide `git add
--interactive` and `git add --patch` in a built-in versions, this is the
big one, as can be expected from the fact that the `git add --patch`
functionality makes up over half of the 1,800+ lines of
`git-add--interactive.perl`.

The two patches that stick out are of course the ones to implement hunk
splitting and hunk editing: these operations are fundamentally more
complicated, and less obvious, than the rest of the operations.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:54:54 +02:00
Johannes Schindelin
e6cde3d9c5 Merge branch 'add-i-fixes'
While re-implementing `git add -i` and `git add -p` in C, I tried to
make sure that there is test coverage for all of the features I convert
from Perl to C, to give me some confidence in the correctness from
running the test suite both with `GIT_TEST_ADD_I_USE_BUILTIN=true` and
with `GIT_TEST_ADD_I_USE_BUILTIN=false`.

However, I discovered that there are a couple of gaps. This patch series
intends to close them.

The first patch might actually not be considered a gap by some: it
basically removes the need for the `TTY` prerequisite in the `git add
-i` tests to verify that the output is colored all right. This change is
rather crucial for me, though: on Windows, where the conversion to a
built-in shows the most obvious benefits, there are no pseudo terminals
(yet), therefore `git.exe` cannot work with them (even if the MSYS2 Perl
interpreter used by Git for Windows knows about some sort of pty
emulation). And I *really* wanted to make sure that the colors work on
Windows, as I personally get a lot out of those color cues.

The patch series ends by addressing two issues that are not exactly
covering testing gaps:

- While adding a test case, I noticed that `git add -p` exited with
  *success* when it could not even generate a diff. This is so obviously
  wrong that I had to fix it right away (I noticed, actually, because my
  in-progress built-in `git add -p` failed, and the Perl version did
  not), and I used the same test case to verify that this is fixed once
  and for all.

- While working on covering those test gaps, I noticed a problem in an
  early version of the built-in version of `git add -p` where the `git
  apply --allow-overlap` mode failed to work properly, for little
  reason, and I fixed it real quick.

  It would seem that the `--allow-overlap` function is not only
  purposefully under-documented, but also purposefully under-tested,
  probably to discourage its use. I do not quite understand what I
  perceive to be Junio's aversion to that option, but I did not feel
  like I should put up a battle here, so I did not accompany this fix
  with a new test script.

  In the end, the built-in version of `git add -p` does not use the
  `--allow-overlap` function at all, anyway. Which should make everybody
  a lot happier.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:54:54 +02:00
Johannes Schindelin
00ff55476b stash -p: (partially) fix bug concerning split hunks
When trying to stash part of the worktree changes by splitting a hunk
and then only partially accepting the split bits and pieces, the user
is presented with a rather cryptic error:

	error: patch failed: <file>:<line>
	error: test: patch does not apply
	Cannot remove worktree changes

and the command would fail to stash the desired parts of the worktree
changes (even if the `stash` ref was actually updated correctly).

We even have a test case demonstrating that failure, carrying it for
four years already.

The explanation: when splitting a hunk, the changed lines are no longer
separated by more than 3 lines (which is the amount of context lines
Git's diffs use by default), but less than that. So when staging only
part of the diff hunk for stashing, the resulting diff that we want to
apply to the worktree in reverse will contain those changes to be
dropped surrounded by three context lines, but since the diff is
relative to HEAD rather than to the worktree, these context lines will
not match.

Example time. Let's assume that the file README contains these lines:

	We
	the
	people

and the worktree added some lines so that it contains these lines
instead:

	We
	are
	the
	kind
	people

and the user tries to stash the line containing "are", then the command
will internally stage this line to a temporary index file and try to
revert the diff between HEAD and that index file. The diff hunk that
`git stash` tries to revert will look somewhat like this:

	@@ -1776,3 +1776,4
	 We
	+are
	 the
	 people

It is obvious, now, that the trailing context lines overlap with the
part of the original diff hunk that the user did *not* want to stash.

Keeping in mind that context lines in diffs serve the primary purpose of
finding the exact location when the diff does not apply precisely (but
when the exact line number in the file to be patched differs from the
line number indicated in the diff), we work around this by reducing the
amount of context lines: the diff was just generated.

Note: this is not a *full* fix for the issue. Just as demonstrated in
t3701's 'add -p works with pathological context lines' test case, there
are ambiguities in the diff format. It is very rare in practice, of
course, to encounter such repeated lines.

The full solution for such cases would be to replace the approach of
generating a diff from the stash and then applying it in reverse by
emulating `git revert` (i.e. doing a 3-way merge). However, in `git
stash -p` it would not apply to `HEAD` but instead to the worktree,
which makes this non-trivial to implement as long as we also maintain a
scripted version of `add -i`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:54:53 +02:00
Johannes Schindelin
4a0ff4a72c Merge branch 'add-i-in-c-all-except-patch'
This patch series implements the rest of the commands in `git add -i`'s
main loop: `update`, `revert`, `add_untracked`, `patch`, `diff`, and
`quit`. Apart from `quit`, these commands are all very similar in that
they first build a list of files, display it, let the user choose which
ones to act on, and then perform the action.

Note that the `patch` command is not actually converted to C, not
completely at least: the built-in version simply hands off to `git
add--interactive` after letting the user select which files to act on.

The reason for this omission is practicality. Out of the 1,800+ lines of
`git-add--interactive.perl`, over a thousand deal *just* with the `git
add -p` part. I did convert that functionality already (to be
contributed in a separate patch series), discovering that there is so
little overlap between the `git add --patch` part and the rest of `git
add --interactive` that I could put the former into a totally different
file: `add-patch.c`. Just a teaser ;-)

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:54:53 +02:00
Johannes Schindelin
375fa0207e Merge branch 'add-i-in-c-status-and-help'
The first part of the long journey to a fully built-in `git add -i`.

It reflects the part that was submitted a couple of times (see
https://github.com/gitgitgadget/git/pull/103) during the Outreachy
project by Slavica Đukić that continued the journey based on an initial
patch series by Daniel Ferreira.

This part only implements the `status` and the `help` part, like
Slavica's last iteration did, in the interest of making the review
remotely more reviewable. I fear that this attempt of making it a bit
more reviewable is pretty futile, as so many things changed. So I will
ask the reviewers for forgiveness: please be kind, and give this sort of
a fresh review.

I threw in a couple of major changes on top of that iteration, though:

- The original plan was to add a helper (`git add--helper`) that takes
  over more and more responsibility from the Perl script over the course
  of the conversion.

  This plan is no longer in effect, as I encountered a serious problem
  with that: the MSYS2 runtime used by the Perl interpreter which Git
  for Windows employs to run `git add -i` has a curious bug (that is
  safely outside the purview of this here patch series) where it fails
  to read from standard input after it spawned a non-MSYS2 program that
  reads from standard input. To keep my `git add -i` in a working state,
  I therefore adopted a different strategy:

  Just like `git difftool` was converted by starting with a built-in
  that did nothing but handing off to the scripted version, guarded by
  the (opt-in) `difftool.useBuiltin` config setting, I start this patch
  series by a built-in `add -i` that does nothing else but state that it
  is not implemented yet, guarded by the (opt-in)
  `add.interactive.useBuiltin` config setting.

  In contrast to the `git difftool` situation, it is quite a bit easier
  here, as we do not even have to rename the script to
  `git-legacy-add--interactive.perl`: the `add--interactive` command is
  an implementation detail that users are not even supposed to know
  about. Therefore, we can implement that road fork between the built-in
  and the scripted version in `builtin/add.c`, i.e. in the user-facing
  `git add` command.

  This will also naturally help with the transition to a fully built-in
  `git add -i`/`git add -p`, as we saw with the built-in `git rebase`
  how important it is for end users to have an escape hatch (and for
  that reason, tried our best to provide the same with the built-in `git
  stash`).

- The `help` command was actually not hooked up in `git add -i`, but was
  only available as a special option of the `git add--helper` command.
  As that command no longer exists, I kind of *had* to implement
  some way to let the built-in `git add -i` show the help text.

- The main loop of `git add -i` (i.e. the thing that lets you choose
  `status` or `help`) is now implemented (but only lists `status` and
  `help`, of course), as it makes use of that feature that took the main
  chunk of the Outreachy project: the function to determine unique
  prefixes of a list of strings.

- Speaking of the unique prefixes: the functionality to determine those
  is now encapsulated in the `prefix-map.c` file, and I also added a
  regression test.

- Speaking of the tests: I also implemented support for the environment
  variable `GIT_TEST_ADD_I_USE_BUILTIN`: by setting it, the test suite
  can be forced to use the built-in, or the Perl script, version of `git
  add -i`. Needless to say: by the end of this patch series, running the
  test suite with `GIT_TEST_ADD_I_USE_BUILTIN=true` will still result in
  a ton of test failures due to not-yet-implemented commands, but it
  will also demonstrate what *already* works.

- Since the main loop starts not only by showing the status, but
  refreshes the index before that, I added that, and I actually
  refactored that code into a new function
  (`repo_refresh_and_write_index()`), as it will be used a couple of
  times by the end of the complete conversion of `git add -i` into a
  built-in command.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:54:53 +02:00
Johannes Schindelin
4fe442ad76 ci: include the built-in git add -i in the linux-gcc job
This job runs the test suite twice, once in regular mode, and once with
a whole slew of `GIT_TEST_*` variables set.

Now that the built-in version of `git add --interactive` is
feature-complete, let's also throw `GIT_TEST_MULTI_PACK_INDEX` into that
fray.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:54:52 +02:00
Johannes Schindelin
68797cea83 t3904: fix incorrect demonstration of a bug
In 7e9e048661 (stash -p: demonstrate failure of split with mixed y/n,
2015-04-16), a regression test for a known breakage that was added to
the test script `t3904-stash-patch.sh` that demonstrated that splitting
a hunk and trying to stash only part of that split hunk fails (but
shouldn't).

As expected, it still fails, but for the wrong reason: once the bug is
fixed, we would expect stderr to show nothing, yet the regression test
expects stderr to show something.

Let's fix that by telling that regression test case to expect nothing to
be printed to stderr.

While at it, also drop the obvious left-over from debugging where the
regression test did not mind `git stash -p` to return a non-zero exit
status.

Of course, the regression test still fails, but this time for the
correct reason.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:54:52 +02:00
Johannes Schindelin
45f4e4de35 built-in add -p: handle Escape sequences more efficiently
When `interactive.singlekey = true`, we react immediately to keystrokes,
even to Escape sequences (e.g. when pressing a cursor key).

The problem with Escape sequences is that we do not really know when
they are done, and as a heuristic we poll standard input for half a
second to make sure that we got all of it.

While waiting half a second is not asking for a whole lot, it can become
quite annoying over time, therefore with this patch, we read the
terminal capabilities (if available) and extract known Escape sequences
from there, then stop polling immediately when we detected that the user
pressed a key that generated such a known sequence.

This recapitulates the remaining part of b5cc003253 (add -i: ignore
terminal escape sequences, 2011-05-17).

Note: We do *not* query the terminal capabilities directly. That would
either require a lot of platform-specific code, or it would require
linking to a library such as ncurses.

Linking to a library in the built-ins is something we try very hard to
avoid (we even kicked the libcurl dependency to a non-built-in remote
helper, just to shave off a tiny fraction of a second from Git's startup
time). And the platform-specific code would be a maintenance nightmare.

Even worse: in Git for Windows' case, we would need to query MSYS2
pseudo terminals, which `git.exe` simply cannot do (because it is
intentionally *not* an MSYS2 program).

To address this, we simply spawn `infocmp -L -1` and parse its output
(which works even in Git for Windows, because that helper is included in
the end-user facing installations).

This is done only once, as in the Perl version, but it is done only when
the first Escape sequence is encountered, not upon startup of `git add
-i`; This saves on startup time, yet makes reacting to the first Escape
sequence slightly more sluggish. But it allows us to keep the
terminal-related code encapsulated in the `compat/terminal.c` file.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:54:52 +02:00
Johannes Schindelin
bae5c09cbe built-in add -p: handle Escape sequences in interactive.singlekey mode
This recapitulates part of b5cc003253 (add -i: ignore terminal escape
sequences, 2011-05-17):

    add -i: ignore terminal escape sequences

    On the author's terminal, the up-arrow input sequence is ^[[A, and
    thus fat-fingering an up-arrow into 'git checkout -p' is quite
    dangerous: git-add--interactive.perl will ignore the ^[ and [
    characters and happily treat A as "discard everything".

    As a band-aid fix, use Term::Cap to get all terminal capabilities.
    Then use the heuristic that any capability value that starts with ^[
    (i.e., \e in perl) must be a key input sequence.  Finally, given an
    input that starts with ^[, read more characters until we have read a
    full escape sequence, then return that to the caller.  We use a
    timeout of 0.5 seconds on the subsequent reads to avoid getting stuck
    if the user actually input a lone ^[.

    Since none of the currently recognized keys start with ^[, the net
    result is that the sequence as a whole will be ignored and the help
    displayed.

Note that we leave part for later which uses "Term::Cap to get all
terminal capabilities", for several reasons:

1. it is actually not really necessary, as the timeout of 0.5 seconds
   should be plenty sufficient to catch Escape sequences,

2. it is cleaner to keep the change to special-case Escape sequences
   separate from the change that reads all terminal capabilities to
   speed things up, and

3. in practice, relying on the terminal capabilities is a bit overrated,
   as the information could be incomplete, or plain wrong. For example,
   in this developer's tmux sessions, the terminal capabilities claim
   that the "cursor up" sequence is ^[M, but the actual sequence
   produced by the "cursor up" key is ^[[A.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:54:52 +02:00
Johannes Schindelin
0435c001cb built-in add -p: respect the interactive.singlekey config setting
The Perl version of `git add -p` supports this config setting to allow
users to input commands via single characters (as opposed to having to
press the <Enter> key afterwards).

This is an opt-in feature because it requires Perl packages
(Term::ReadKey and Term::Cap, where it tries to handle an absence of the
latter package gracefully) to work. Note that at least on Ubuntu, that
Perl package is not installed by default (it needs to be installed via
`sudo apt-get install libterm-readkey-perl`), so this feature is
probably not used a whole lot.

In C, we obviously do not have these packages available, but we just
introduced `read_single_keystroke()` that is similar to what
Term::ReadKey provides, and we use that here.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2019-07-30 10:54:52 +02:00