Commit Graph

15995 Commits

Author SHA1 Message Date
Ben Peart
ce0202ba22 fscache: add GIT_TEST_FSCACHE support
Add support to fscache to enable running the entire test suite with the
fscache enabled.

Signed-off-by: Ben Peart <benpeart@microsoft.com>
2020-02-17 10:49:29 +01:00
Takuto Ikuta
dfe8709ad1 checkout.c: enable fscache for checkout again
This is retry of #1419.

I added flush_fscache macro to flush cached stats after disk writing
with tests for regression reported in #1438 and #1442.

git checkout checks each file path in sorted order, so cache flushing does not
make performance worse unless we have large number of modified files in
a directory containing many files.

Using chromium repository, I tested `git checkout .` performance when I
delete 10 files in different directories.
With this patch:
TotalSeconds: 4.307272
TotalSeconds: 4.4863595
TotalSeconds: 4.2975562
Avg: 4.36372923333333

Without this patch:
TotalSeconds: 20.9705431
TotalSeconds: 22.4867685
TotalSeconds: 18.8968292
Avg: 20.7847136

I confirmed this patch passed all tests in t/ with core_fscache=1.

Signed-off-by: Takuto Ikuta <tikuta@chromium.org>
2020-02-17 10:49:29 +01:00
Johannes Schindelin
25a6c49191 fscache: add a test for the dir-not-found optimization
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2020-02-17 10:49:28 +01:00
Johannes Schindelin
2d1660c7d9 Merge pull request #2506 from dscho/issue-2283
Allow running Git directly from `C:\Program Files\Git\mingw64\bin\git.exe`
2020-02-17 10:48:54 +01:00
Johannes Schindelin
0fca0fd110 Merge pull request #2504 from dscho/access-repo-via-junction
Handle `git add <file>` where <file> traverses an NTFS junction
2020-02-17 10:48:53 +01:00
Johannes Schindelin
3cf20031fa Merge pull request #2473 from dscho/com0-is-not-a-reserved-name
Do not mistake `COM0` for a reserved file name
2020-02-17 10:48:52 +01:00
Johannes Schindelin
f1a05963ab Merge pull request #2472 from dscho/builtin-add-i-fixes
Two fixes for the built-in `git add -i`
2020-02-17 10:48:52 +01:00
Johannes Schindelin
af29274d54 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>
2020-02-17 10:48:50 +01:00
Johannes Schindelin
5f3e9f0c7a 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>
2020-02-17 10:48:50 +01:00
Johannes Schindelin
90c05270f2 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>
2020-02-17 10:48:49 +01:00
Johannes Schindelin
9fabe42fe2 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>
2020-02-17 10:48:48 +01:00
Johannes Schindelin
86a66d8705 tests: exercise the RUNTIME_PREFIX feature
Originally, we refrained from adding a regression test in 7b6c649637
(system_path(): Add prefix computation at runtime if RUNTIME_PREFIX set,
2008-08-10), and in 226c0ddd0d (exec_cmd: RUNTIME_PREFIX on some POSIX
systems, 2018-04-10).

The reason was that it was deemed too tricky to test.

Turns out that it is not tricky to test at all: we simply create a
pseudo-root, copy the `git` executable into the `git/` subdirectory of
that pseudo-root, then copy a script into the `libexec/git-core/`
directory and expect that to be picked up.

As long as the trash directory is in a location where binaries can be
executed, this works.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2020-02-17 10:48:47 +01:00
Johannes Schindelin
5abc9c247f Merge branch 'test-unc-fetch'
Fix fetching from UNC paths.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2020-02-17 10:48:47 +01:00
Johannes Schindelin
b54d68fec7 Merge branch 'drive-prefix'
This topic branch allows us to specify absolute paths without the drive
prefix e.g. when cloning.

Example:

	C:\Users\me> git clone https://github.com/git/git \upstream-git

This will clone into a new directory C:\upstream-git, in line with how
Windows interprets absolute paths.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2020-02-17 10:48:47 +01:00
Johannes Schindelin
1f986c4d88 mingw: allow git.exe to be used instead of the "Git wrapper"
Git for Windows wants to add `git.exe` to the users' `PATH`, without
cluttering the latter with unnecessary executables such as `wish.exe`.
To that end, it invented the concept of its "Git wrapper", i.e. a tiny
executable located in `C:\Program Files\Git\cmd\git.exe` (originally a
CMD script) whose sole purpose is to set up a couple of environment
variables and then spawn the _actual_ `git.exe` (which nowadays lives in
`C:\Program Files\Git\mingw64\bin\git.exe` for 64-bit, and the obvious
equivalent for 32-bit installations).

Currently, the following environment variables are set unless already
initialized:

- `MSYSTEM`, to make sure that the MSYS2 Bash and the MSYS2 Perl
  interpreter behave as expected, and

- `PLINK_PROTOCOL`, to force PuTTY's `plink.exe` to use the SSH
  protocol instead of Telnet,

- `PATH`, to make sure that the `bin` folder in the user's home
  directory, as well as the `/mingw64/bin` and the `/usr/bin`
  directories are included. The trick here is that the `/mingw64/bin/`
  and `/usr/bin/` directories are relative to the top-level installation
  directory of Git for Windows (which the included Bash interprets as
  `/`, i.e. as the MSYS pseudo root directory).

Using the absence of `MSYSTEM` as a tell-tale, we can detect in
`git.exe` whether these environment variables have been initialized
properly. Therefore we can call `C:\Program Files\Git\mingw64\bin\git`
in-place after this change, without having to call Git through the Git
wrapper.

Obviously, above-mentioned directories must be _prepended_ to the `PATH`
variable, otherwise we risk picking up executables from unrelated Git
installations. We do that by constructing the new `PATH` value from
scratch, appending `$HOME/bin` (if `HOME` is set), then the MSYS2 system
directories, and then appending the original `PATH`.

Side note: this modification of the `PATH` variable is independent of
the modification necessary to reach the executables and scripts in
`/mingw64/libexec/git-core/`, i.e. the `GIT_EXEC_PATH`. That
modification is still performed by Git, elsewhere, long after making the
changes described above.

While we _still_ cannot simply hard-link `mingw64\bin\git.exe` to `cmd`
(because the former depends on a couple of `.dll` files that are only in
`mingw64\bin`, i.e. calling `...\cmd\git.exe` would fail to load due to
missing dependencies), at least we can now avoid that extra process of
running the Git wrapper (which then has to wait for the spawned
`git.exe` to finish) by calling `...\mingw64\bin\git.exe` directly, via
its absolute path.

Testing this is in Git's test suite tricky: we set up a "new" MSYS
pseudo-root and copy the `git.exe` file into the appropriate location,
then verify that `MSYSTEM` is set properly, and also that the `PATH` is
modified so that scripts can be found in `$HOME/bin`, `/mingw64/bin/`
and `/usr/bin/`.

This addresses https://github.com/git-for-windows/git/issues/2283

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2020-02-17 10:48:12 +01:00
Johannes Schindelin
605e73f7dc mingw: implement a platform-specific strbuf_realpath()
There is a Win32 API function to resolve symbolic links, and we can use
that instead of resolving them manually. Even better, this function also
resolves NTFS junction points (which are somewhat similar to bind
mounts).

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

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2020-02-17 10:44:36 +01:00
Johannes Schindelin
dfb13ee950 mingw: demonstrate a git add issue with NTFS junctions
NTFS junctions are somewhat similar in spirit to Unix bind mounts: they
point to a different directory and are resolved by the filesystem
driver. As such, they appear to `lstat()` as if they are directories,
not as if they are symbolic links.

_Any_ user can create junctions, while symbolic links can only be
created by non-administrators in Developer Mode on Windows 10. Hence
NTFS junctions are much more common "in the wild" than NTFS symbolic
links.

It was reported in https://github.com/git-for-windows/git/issues/2481
that adding files via an absolute path that traverses an NTFS junction:
since 1e64d18 (mingw: do resolve symlinks in `getcwd()`), we resolve not
only symbolic links but also NTFS junctions when determining the
absolute path of the current directory. The same is not true for `git
add <file>`, where symbolic links are resolved in `<file>`, but not NTFS
junctions.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2020-02-17 10:44:36 +01:00
Johannes Schindelin
da99b41098 mingw: do not treat COM0 as a reserved file name
In 4dc42c6c18 (mingw: refuse paths containing reserved names,
2019-12-21), we started disallowing file names that are reserved, e.g.
`NUL`, `CONOUT$`, etc.

This included `COM<n>` where `<n>` is a digit. Unfortunately, this
includes `COM0` but only `COM1`, ..., `COM9` are reserved, according to
the official documentation, `COM0` is mentioned in the "NT Namespaces"
section but it is explicitly _omitted_ from the list of reserved names:
https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file#naming-conventions

Tests corroborate this: it is totally possible to write a file called
`com0.c` on Windows 10, but not `com1.c`.

So let's tighten the code to disallow only the reserved `COM<n>` file
names, but to allow `COM0` again.

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

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2020-02-17 10:44:35 +01:00
Johannes Schindelin
881c8a10dd built-in add -i: accept open-ended ranges again
The interactive `add` command allows selecting multiple files for some
of its sub-commands, via unique prefixes, indices or index ranges.

When re-implementing `git add -i` in C, we even added a code comment
talking about ranges with a missing end index, such as `2-`, but the
code did not actually accept those, as pointed out in
https://github.com/git-for-windows/git/issues/2466#issuecomment-574142760.

Let's fix this, and add a test case to verify that this stays fixed
forever.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2020-02-17 10:44:35 +01:00
Johannes Schindelin
b4f6b9680e clean: remove mount points when possible
Windows' equivalent to "bind mounts", NTFS junction points, can be
unlinked without affecting the mount target. This is clearly what users
expect to happen when they call `git clean -dfx` in a worktree that
contains NTFS junction points: the junction should be removed, and the
target directory of said junction should be left alone (unless it is
inside the worktree).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2020-02-17 10:41:11 +01:00
Johannes Schindelin
0fee73da43 clean: do not traverse mount points
It seems to be not exactly rare on Windows to install NTFS junction
points (the equivalent of "bind mounts" on Linux/Unix) in worktrees,
e.g. to map some development tools into a subdirectory.

In such a scenario, it is pretty horrible if `git clean -dfx` traverses
into the mapped directory and starts to "clean up".

Let's just not do that. Let's make sure before we traverse into a
directory that it is not a mount point (or junction).

This addresses https://github.com/git-for-windows/git/issues/607

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2020-02-17 10:41:08 +01:00
Johannes Schindelin
ce01a525eb mingw: make is_hidden tests in t0001/t5611 more robust
We should not actually expect the first `attrib.exe` in the PATH to
be the one we are looking for. Or that it is in the PATH, for that
matter.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2020-02-17 10:40:54 +01:00
Johannes Schindelin
d86d18efd8 t9001, t9116: avoid pipes
When grepping through the output of a command in the test suite, there
is always a chance that something goes wrong, in which case there would
not be anything useful to debug.

Let's redirect the output into a file instead, and grep that file, so
that the log can be inspected easily if the grep fails.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2020-02-17 10:40:54 +01:00
Johannes Schindelin
897863393f 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>
2020-02-17 10:40:53 +01:00
Johannes Schindelin
f14f271f10 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>
2020-02-17 10:40:53 +01:00
Johannes Schindelin
e1be3d28df t5580: test cloning without file://, test fetching via UNC paths
It gets a bit silly to add the commands to the name of the test script,
so let's just rename it while we're testing more UNC stuff.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2020-02-17 10:40:51 +01:00
Johannes Schindelin
0dce005eb9 mingw: allow absolute paths without drive prefix
When specifying an absolute path without a drive prefix, we convert that
path internally. Let's make sure that we handle that case properly, too
;-)

This fixes the command

	git clone https://github.com/git-for-windows/git \G4W

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2020-02-17 10:40:51 +01:00
Johannes Schindelin
995f11ea90 mingw: demonstrate a problem with certain absolute paths
On Windows, there are several categories of absolute paths. One such
category starts with a backslash and is implicitly relative to the
drive associated with the current working directory. Example:

	c:
	git clone https://github.com/git-for-windows/git \G4W

should clone into C:\G4W.

There is currently a problem with that, in that mingw_mktemp() does not
expect the _wmktemp() function to prefix the absolute path with the
drive prefix, and as a consequence, the resulting path does not fit into
the originally-passed string buffer. The symptom is a "Result too large"
error.

Reported by Juan Carlos Arevalo Baeza.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2020-02-17 10:40:51 +01:00
Sverre Rabbelier
894ac66ebf remote-helper: check helper status after import/export
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
2020-02-17 10:40:51 +01:00
Sverre Rabbelier
8df266dfb0 t9350: point out that refs are not updated correctly
This happens only when the corresponding commits are not exported in
the current fast-export run. This can happen either when the relevant
commit is already marked, or when the commit is explicitly marked
as UNINTERESTING with a negative ref by another argument.

This breaks fast-export basec remote helpers.

Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
2020-02-17 10:40:51 +01:00
Junio C Hamano
0ecc7d62f4 Merge branch 'jb/parse-options-message-fix' into maint
Error message fix.

* jb/parse-options-message-fix:
  parse-options: lose an unnecessary space in an error message
2020-02-14 12:42:32 -08:00
Junio C Hamano
153a1b46f1 Merge branch 'pb/do-not-recurse-grep-no-index' into maint
"git grep --no-index" should not get affected by the contents of
the .gitmodules file but when "--recurse-submodules" is given or
the "submodule.recurse" variable is set, it did.  Now these
settings are ignored in the "--no-index" mode.

* pb/do-not-recurse-grep-no-index:
  grep: ignore --recurse-submodules if --no-index is given
2020-02-14 12:42:31 -08:00
Junio C Hamano
8857657cc9 Merge branch 'jt/t5616-robustify' into maint
Futureproofing a test not to depend on the current implementation
detail.

* jt/t5616-robustify:
  t5616: make robust to delta base change
2020-02-14 12:42:31 -08:00
Junio C Hamano
1f7609b520 Merge branch 'en/fill-directory-fixes-more' into maint
Corner case bugs in "git clean" that stems from a (necessarily for
performance reasons) awkward calling convention in the directory
enumeration API has been corrected.

* en/fill-directory-fixes-more:
  dir: point treat_leading_path() warning to the right place
  dir: restructure in a way to avoid passing around a struct dirent
  dir: treat_leading_path() and read_directory_recursive(), round 2
  clean: demonstrate a bug with pathspecs
2020-02-14 12:42:31 -08:00
Junio C Hamano
650ed395be Merge branch 'ds/refmap-doc' into maint
"git fetch --refmap=" option has got a better documentation.

* ds/refmap-doc:
  fetch: document and test --refmap=""
2020-02-14 12:42:30 -08:00
Junio C Hamano
8a17eb7972 Merge branch 'jk/test-fixes' into maint
Test fixes.

* jk/test-fixes:
  t7800: don't rely on reuse_worktree_file()
  t4018: drop "debugging" cat from hunk-header tests
2020-02-14 12:42:30 -08:00
Junio C Hamano
e361f36f61 Merge branch 'nd/switch-and-restore' into maint
"git restore --staged" did not correctly update the cache-tree
structure, resulting in bogus trees to be written afterwards, which
has been corrected.

* nd/switch-and-restore:
  restore: invalidate cache-tree when removing entries with --staged
2020-02-14 12:42:29 -08:00
Junio C Hamano
4a60c63a75 Merge branch 'jk/no-flush-upon-disconnecting-slrpc-transport' into maint
Reduce unnecessary round-trip when running "ls-remote" over the
stateless RPC mechanism.

* jk/no-flush-upon-disconnecting-slrpc-transport:
  transport: don't flush when disconnecting stateless-rpc helper
2020-02-14 12:42:28 -08:00
Junio C Hamano
3bba763373 Merge branch 'hw/commit-advise-while-rejecting' into maint
"git commit" gives output similar to "git status" when there is
nothing to commit, but without honoring the advise.statusHints
configuration variable, which has been corrected.

* hw/commit-advise-while-rejecting:
  commit: honor advice.statusHints when rejecting an empty commit
2020-02-14 12:42:27 -08:00
Jacques Bodin-Hullin
395518cf7a parse-options: lose an unnecessary space in an error message
Signed-off-by: Jacques Bodin-Hullin <j.bodinhullin@monsieurbiz.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-05 10:49:29 -08:00
Philippe Blain
c56c48dd07 grep: ignore --recurse-submodules if --no-index is given
Since grep learned to recurse into submodules in 0281e487fd
(grep: optionally recurse into submodules, 2016-12-16),
using --recurse-submodules along with --no-index makes Git
die().

This is unfortunate because if submodule.recurse is set in a user's
~/.gitconfig, invoking `git grep --no-index` either inside or outside
a Git repository results in

    fatal: option not supported with --recurse-submodules

Let's allow using these options together, so that setting submodule.recurse
globally does not prevent using `git grep --no-index`.

Using `--recurse-submodules` should not have any effect if `--no-index`
is used inside a repository, as Git will recurse into the checked out
submodule directories just like into regular directories.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-30 10:15:58 -08:00
Jonathan Tan
b54128bb0b t5616: make robust to delta base change
Commit 6462d5eb9a ("fetch: remove fetch_if_missing=0", 2019-11-08)
contains a test that relies on having to lazily fetch the delta base of
a blob, but assumes that the tree being fetched (as part of the test) is
sent as a non-delta object. This assumption may not hold in the future;
for example, a change in the length of the object hash might result in
the tree being sent as a delta instead.

Make the test more robust by relying on having to lazily fetch the delta
base of the tree instead, and by making no assumptions on whether the
blobs are sent as delta or non-delta.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-27 12:42:40 -08:00
Derrick Stolee
b40a50264a fetch: document and test --refmap=""
To prevent long blocking time during a 'git fetch' call, a user
may want to set up a schedule for background 'git fetch' processes.
However, these runs will update the refs/remotes branches due to
the default refspec set in the config when Git adds a remote.
Hence the user will not notice when remote refs are updated during
their foreground fetches. In fact, they may _want_ those refs to
stay put so they can work with the refs from their last foreground
fetch call.

This can be accomplished by overriding the configured refspec using
'--refmap=' along with a custom refspec:

  git fetch --refmap='' <remote> +refs/heads/*:refs/hidden/<remote>/*

to populate a custom ref space and download a pack of the new
reachable objects. This kind of call allows a few things to happen:

1. We download a new pack if refs have updated.
2. Since the refs/hidden branches exist, GC will not remove the
   newly-downloaded data.
3. With fetch.writeCommitGraph enabled, the refs/hidden refs are
   used to update the commit-graph file.

To avoid the refs/hidden directory from filling without bound, the
--prune option can be included. When providing a refspec like this,
the --prune option does not delete remote refs and instead only
deletes refs in the target refspace.

Update the documentation to clarify how '--refmap=""' works and
create tests to guarantee this behavior remains in the future.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-21 10:24:48 -08:00
Jeff King
e4837b4406 t7800: don't rely on reuse_worktree_file()
A test in t7800 tries to make sure that when git-difftool runs an
external tool that fails, it stops looking at files. Our fake failing
tool prints the file name it was asked to diff before exiting non-zero,
and then we confirm the output contains only that file.

However, this subtly relies on our internal reuse_worktree_file().
Because we're diffing between branches, the command run by difftool
might see:

  - the git-stored filename (e.g., "file"), if we decided that the
    working tree contents were up-to-date with the object in the index
    and HEAD, and we could reuse them

  - a temporary filename (e.g. "/tmp/abc123_file") if we had to dump the
    contents from the object database

If the latter case happens, then the test fails, because it's expecting
the string "file". I discovered this when debugging something unrelated
with reuse_worktree_file(). I _thought_ it should be able to be
triggered by a racy-git situation, but running:

  ./t7800-difftool.sh --stress --run=2,13

never seems to fail. However, by my reading of reuse_worktree_file(),
this would probably always fail under Cygwin, because it sets
NO_FAST_WORKING_DIRECTORY. At any rate, since reuse_worktree_file()
is meant to be an optimization that may or may not trigger, our test
should be robust either way.

Instead of checking the filename, let's just make sure we got a single
line of output (which would not be true if we continued after the first
failure).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-16 14:48:26 -08:00
Jeff King
fbce03d329 t4018: drop "debugging" cat from hunk-header tests
We run a series of hunk-header tests in a loop, and each one does this:

  test_when_finished 'cat actual' &&      # for debugging only

This is pretty pointless. When the test succeeds, we waste time running
a useless cat process. If you're debugging a failure with "-i", then we
won't run the when-finished part at all. So it helps only if you're
running with something like "--verbose-log".

Since we expect the tests to succeed most of the time, a better way to
do this would be a helper that checks the output and dumps "actual" only
when it fails. But it's probably not even worth the effort, as anyone
debugging a failure could just run with "-i" and investigate the
"actual" file themselves.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-16 14:46:28 -08:00
Elijah Newren
22705334b9 dir: treat_leading_path() and read_directory_recursive(), round 2
I was going to title this "dir: more synchronizing of
treat_leading_path() and read_directory_recursive()", a nod to commit
777b420347 ("dir: synchronize treat_leading_path() and
read_directory_recursive()", 2019-12-19), but the title was too long.

Anyway, first the backstory...

fill_directory() has always had a slightly error-prone interface: it
returns a subset of paths which *might* match the specified pathspec; it
was intended to prune away some paths which didn't match the specified
pathspec and keep at least all the ones that did match it.  Given this
interface, callers were responsible to post-process the results and
check whether each actually matched the pathspec.

builtin/clean.c did this.  It would first prune out duplicates (e.g. if
"dir" was returned as well as all files under "dir/", then it would
simplify this to just "dir"), and after pruning duplicates it would
compare the remaining paths to the specified pathspec(s).  This
post-processing itself could run into problems, though, as noted in
commit 404ebceda0 ("dir: also check directories for matching
pathspecs", 2019-09-17):

    For the case of git-clean and a set of pathspecs of "dir/file" and
    "more", this caused a problem because we'd end up with dir entries
    for both of
      "dir"
      "dir/file"
    Then correct_untracked_entries() would try to helpfully prune
    duplicates for us by removing "dir/file" since it's under "dir",
    leaving us with
      "dir"
    Since the original pathspec only had "dir/file", the only entry left
    doesn't match and leaves nothing to be removed.  (Note that if only
    one pathspec was specified, e.g. only "dir/file", then the
    common_prefix_len optimizations in fill_directory would cause us to
    bypass this problem, making it appear in simple tests that we could
    correctly remove manually specified pathspecs.)

That commit fixed the issue -- when multiple pathspecs were specified --
by making sure fill_directory() wouldn't return both "dir" and
"dir/file" outside the common_prefix_len optimization path.  This is
where it starts to get fun.

In commit b9670c1f5e ("dir: fix checks on common prefix directory",
2019-12-19), we noticed that the common_prefix_len wasn't doing
appropriate checks and letting all kinds of stuff through, resulting in
recursing into .git/ directories and other craziness.  So it started
locking down and doing checks on pathnames within that code path.  That
continued with commit 777b420347 ("dir: synchronize
treat_leading_path() and read_directory_recursive()", 2019-12-19), which
noted the following:

    Our optimization to avoid calling into read_directory_recursive()
    when all pathspecs have a common leading directory mean that we need
    to match the logic that read_directory_recursive() would use if we
    had just called it from the root.  Since it does more than call
    treat_path() we need to copy that same logic.

...and then it more forcefully addressed the issue with this wonderfully
ironic statement:

    Needing to duplicate logic like this means it is guaranteed someone
    will eventually need to make further changes and forget to update
    both locations.  It is tempting to just nuke the leading_directory
    special casing to avoid such bugs and simplify the code, but
    unpack_trees' verify_clean_subdirectory() also calls
    read_directory() and does so with a non-empty leading path, so I'm
    hesitant to try to restructure further.  Add obnoxious warnings to
    treat_leading_path() and read_directory_recursive() to try to warn
    people of such problems.

You would think that with such a strongly worded description, that its
author would have actually ensured that the logic in
treat_leading_path() and read_directory_recursive() did actually match
and that *everything* that was needed had at least been copied over at
the time that this paragraph was written.  But you'd be wrong, I messed
it up by missing part of the logic.

Copy the missing bits to fix the new final test in t7300.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-16 12:56:13 -08:00
Derrick Stolee
f365bf40a0 clean: demonstrate a bug with pathspecs
b9670c1f5e (dir: fix checks on common prefix directory, 2019-12-19)
modified the way pathspecs are handled when handling a directory
during "git clean -f <path>". While this improved the behavior for
known test breakages, it also regressed in how the clean command
handles cleaning a specified file.

Add a test case that demonstrates this behavior. This test passes
before b9670c1f5e then fails after.

Helped-by: Kevin Willford <Kevin.Willford@microsoft.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Reviewed-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-16 12:56:12 -08:00
Junio C Hamano
4d924528d8 Revert "Merge branch 'ra/rebase-i-more-options'"
This reverts commit 5d9324e0f4, reversing
changes made to c58ae96fc4.

The topic turns out to be too buggy for real use.

cf. <f2fe7437-8a48-3315-4d3f-8d51fe4bb8f1@gmail.com>
2020-01-12 13:25:18 -08:00
Junio C Hamano
1cf4836865 Merge branch 'js/mingw-loosen-overstrict-tree-entry-checks'
Further tweak to a "no backslash in indexed paths" for Windows port
we applied earlier.

* js/mingw-loosen-overstrict-tree-entry-checks:
  mingw: safeguard better against backslashes in file names
2020-01-10 14:45:27 -08:00
Johannes Schindelin via GitGitGadget
49e268e23e mingw: safeguard better against backslashes in file names
In 224c7d70fa (mingw: only test index entries for backslashes, not tree
entries, 2019-12-31), we relaxed the check for backslashes in tree
entries to check only index entries.

However, the code change was incorrect: it was added to
`add_index_entry_with_check()`, not to `add_index_entry()`, so under
certain circumstances it was possible to side-step the protection.

Besides, the description of that commit purported that all index entries
would be checked when in fact they were only checked when being added to
the index (there are code paths that do not do that, constructing
"transient" index entries).

In any case, it was pointed out in one insightful review at
https://github.com/git-for-windows/git/pull/2437#issuecomment-566771835
that it would be a much better idea to teach `verify_path()` to perform
the check for a backslash. This is safer, even if it comes with two
notable drawbacks:

- `verify_path()` cannot say _what_ is wrong with the path, therefore
  the user will no longer be told that there was a backslash in the
  path, only that the path was invalid.

- The `git apply` command also calls the `verify_path()` function, and
  might have been able to handle Windows-style paths (i.e. with
  backslashes instead of forward slashes). This will no longer be
  possible unless the user (temporarily) sets `core.protectNTFS=false`.

Note that `git add <windows-path>` will _still_ work because
`normalize_path_copy_len()` will convert the backslashes to forward
slashes before hitting the code path that creates an index entry.

The clear advantage is that `verify_path()`'s purpose is to check the
validity of the file name, therefore we naturally tap into all the code
paths that need safeguarding, also implicitly into future code paths.

The benefits of that approach outweigh the downsides, so let's move the
check from `add_index_entry_with_check()` to `verify_path()`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-10 12:29:07 -08:00