Commit Graph

15966 Commits

Author SHA1 Message Date
Elijah Newren
73113c5922 t3433: new rebase testcase documenting a stat-dirty-like failure
A user discovered a case where they had a stack of 20 simple commits to
rebase, and the rebase would succeed in picking the first commit and
then error out with a pair of "Could not execute the todo command" and
"Your local changes to the following files would be overwritten by
merge" messages.

Their steps actually made use of the -i flag, but I switched it over to
-m to make it simpler to trigger the bug.  With that flag, it bisects
back to commit 68aa495b59 (rebase: implement --merge via the
interactive machinery, 2018-12-11), but that's misleading.  If you
change the -m flag to --keep-empty, then the problem persists and will
bisect back to 356ee4659b (sequencer: try to commit without forking
'git commit', 2017-11-24)

After playing with the testcase for a bit, I discovered that added
--exec "sleep 1" to the command line makes the rebase succeed, making me
suspect there is some kind of discard and reloading of caches that lead
us to believe that something is stat dirty, but I didn't succeed in
digging any further than that.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-19 10:13:27 -08: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
Derrick Stolee
a1087c9367 graph: fix lack of color in horizontal lines
In some cases, horizontal lines in rendered graphs can lose their
coloring. This is due to a use of graph_line_addch() instead of
graph_line_write_column(). Using a ternary operator to pick the
character is nice for compact code, but we actually need a column to
provide the color.

Add a test to t4215-log-skewed-merges.sh to prevent regression.

Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-08 09:37:18 -08:00
Derrick Stolee
0d251c3291 graph: drop assert() for merge with two collapsing parents
When "git log --graph" shows a merge commit that has two collapsing
lines, like:

    | | | | *
    | |_|_|/|
    |/| | |/
    | | |/|
    | |/| |
    | * | |
    * | | |

we trigger an assert():

        graph.c:1228: graph_output_collapsing_line: Assertion
                      `graph->mapping[i - 3] == target' failed.

The assert was introduced by eaf158f8 ("graph API: Use horizontal
lines for more compact graphs", 2009-04-21), which is quite old.
This assert is trying to say that when we complete a horizontal
line with a single slash, it is because we have reached our target.

It is actually the _second_ collapsing line that hits this assert.
The reason we are in this code path is because we are collapsing
the first line, and in that case we are hitting our target now
that the horizontal line is complete. However, the second line
cannot be a horizontal line, so it will collapse without horizontal
lines. In this case, it is inappropriate to assert that we have
reached our target, as we need to continue for another column
before reaching the target. Dropping the assert is safe here.

The new behavior in 0f0f389f12 (graph: tidy up display of
left-skewed merges, 2019-10-15) caused the behavior change that
made this assertion failure possible. In addition to making the
assert possible, it also changed how multiple edges collapse.

In a larger example, the current code will output a collapse
as follows:

	| | | | | | *
	| |_|_|_|_|/|\
	|/| | | | |/ /
	| | | | |/| /
	| | | |/| |/
	| | |/| |/|
	| |/| |/| |
	| | |/| | |
	| | * | | |

However, the intended collapse should allow multiple horizontal lines
as follows:

	| | | | | | *
	| |_|_|_|_|/|\
	|/| | | | |/ /
	| | |_|_|/| /
	| |/| | | |/
	| | | |_|/|
	| | |/| | |
	| | * | | |

This behavior is not corrected by this change, but is noted for a later
update.

Helped-by: Jeff King <peff@peff.net>
Reported-by: Bradley Smith <brad@brad-smith.co.uk>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-08 09:35:07 -08:00
Jeff King
4d8cab95cc transport: don't flush when disconnecting stateless-rpc helper
Since ba227857d2 (Reduce the number of connects when fetching,
2008-02-04), when we disconnect a git transport, we send a final flush
packet. This cleanly tells the other side that we're done, and avoids
the other side complaining "the remote end hung up unexpectedly" (though
we'd only see that for transports that pass along the server stderr,
like ssh or local-host).

But when we've initiated a v2 stateless-connect session over a transport
helper, there's no point in sending this flush packet. Each operation
we've performed is self-contained, and the other side is fine with us
hanging up between operations.

But much worse, by sending the flush packet we may cause the helper to
issue an entirely new request _just_ to send the flush packet. So we can
incur an extra network request just to say "by the way, we have nothing
more to send".

Let's drop this extra flush packet. As the test shows, this reduces the
number of POSTs required for a v2 ls-remote over http from 2 to 1.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-08 09:32:38 -08:00
Jeff King
e701bab3e9 restore: invalidate cache-tree when removing entries with --staged
When "git restore --staged <path>" removes a path that's in the index,
it marks the entry with CE_REMOVE, but we don't do anything to
invalidate the cache-tree. In the non-staged case, we end up in
checkout_worktree(), which calls remove_marked_cache_entries(). That
actually drops the entries from the index, as well as invalidating the
cache-tree and untracked-cache.

But with --staged, we never call checkout_worktree(), and the CE_REMOVE
entries remain. Interestingly, they are dropped when we write out the
index, but that means the resulting index is inconsistent: its
cache-tree will not match the actual entries, and running "git commit"
immediately after will create the wrong tree.

We can solve this by calling remove_marked_cache_entries() ourselves
before writing out the index. Note that we can't just hoist it out of
checkout_worktree(); that function needs to iterate over the CE_REMOVE
entries (to drop their matching worktree files) before removing them.

One curiosity about the test: without this patch, it actually triggers a
BUG() when running git-restore:

  BUG: cache-tree.c:810: new1 with flags 0x4420000 should not be in cache-tree

But in the original problem report, which used a similar recipe,
git-restore actually creates the bogus index (and the commit is created
with the wrong tree). I'm not sure why the test here behaves differently
than my out-of-suite reproduction, but what's here should catch either
symptom (and the fix corrects both cases).

Reported-by: Torsten Krah <krah.tm@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-08 09:03:36 -08:00
Junio C Hamano
c20d4fd44a Merge branch 'ds/sparse-list-in-cone-mode'
"git sparse-checkout list" subcommand learned to give its output in
a more concise form when the "cone" mode is in effect.

* ds/sparse-list-in-cone-mode:
  sparse-checkout: document interactions with submodules
  sparse-checkout: list directories in cone mode
2020-01-06 14:17:51 -08:00
Junio C Hamano
a578ef9e63 Merge branch 'js/mingw-loosen-overstrict-tree-entry-checks'
An earlier update to Git for Windows declared that a tree object is
invalid if it has a path component with backslash in it, which was
overly strict, which has been corrected.  The only protection the
Windows users need is to prevent such path (or any path that their
filesystem cannot check out) from entering the index.

* js/mingw-loosen-overstrict-tree-entry-checks:
  mingw: only test index entries for backslashes, not tree entries
2020-01-06 14:17:50 -08:00
Johannes Schindelin
224c7d70fa mingw: only test index entries for backslashes, not tree entries
During a clone of a repository that contained a file with a backslash in
its name in the past, as of v2.24.1(2), Git for Windows prints errors
like this:

	error: filename in tree entry contains backslash: '\'

The idea is to prevent Git from even trying to write files with
backslashes in their file names: while these characters are valid in
file names on other platforms, on Windows it is interpreted as directory
separator (which would obviously lead to ambiguities, e.g. when there is
a file `a\b` and there is also a file `a/b`).

Arguably, this is the wrong layer for that error: As long as the user
never checks out the files whose names contain backslashes, there should
not be any problem in the first place.

So let's loosen the requirements: we now leave tree entries with
backslashes in their file names alone, but we do require any entries
that are added to the Git index to contain no backslashes on Windows.

Note: just as before, the check is guarded by `core.protectNTFS` (to
allow overriding the check by toggling that config setting), and it
is _only_ performed on Windows, as the backslash is not a directory
separator elsewhere, even when writing to NTFS-formatted volumes.

An alternative approach would be to try to prevent creating files with
backslashes in their file names. However, that comes with its own set of
problems. For example, `git config -f C:\ProgramData\Git\config ...` is
a very valid way to specify a custom config location, and we obviously
do _not_ want to prevent that. Therefore, the approach chosen in this
patch would appear to be better.

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

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-02 12:56:08 -08:00
Junio C Hamano
a82027e9e6 Merge branch 'js/use-test-tool-on-path'
Test fix.

* js/use-test-tool-on-path:
  t3008: find test-tool through path lookup
2020-01-02 12:38:30 -08:00
Junio C Hamano
13432fc6dd Merge branch 'js/mingw-reserved-filenames'
Forbid pathnames that the platform's filesystem cannot represent on
MinGW.

* js/mingw-reserved-filenames:
  mingw: refuse paths containing reserved names
  mingw: short-circuit the conversion of `/dev/null` to UTF-16
2020-01-02 12:38:30 -08:00
Derrick Stolee
4fd683b6a3 sparse-checkout: document interactions with submodules
Using 'git submodule (init|deinit)' a user can select a subset of
submodules to populate. This behaves very similar to the sparse-checkout
feature, but those directories contain their own .git directory
including an object database and ref space. To have the sparse-checkout
file also determine if those files should exist would easily cause
problems. Therefore, keeping these features independent in this way
is the best way forward.

Also create a test that demonstrates this behavior to make sure
it doesn't change as the sparse-checkout feature evolves.

Reported-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-30 09:07:20 -08:00
Derrick Stolee
de11951b03 sparse-checkout: list directories in cone mode
When core.sparseCheckoutCone is enabled, the 'git sparse-checkout set'
command takes a list of directories as input, then creates an ordered
list of sparse-checkout patterns such that those directories are
recursively included and all sibling entries along the parent directories
are also included. Listing the patterns is less user-friendly than the
directories themselves.

In cone mode, and as long as the patterns match the expected cone-mode
pattern types, change the output of 'git sparse-checkout list' to only
show the directories that created the patterns.

With this change, the following piped commands would not change the
working directory:

	git sparse-checkout list | git sparse-checkout set --stdin

The only time this would not work is if core.sparseCheckoutCone is
true, but the sparse-checkout file contains patterns that do not
match the expected pattern types for cone mode.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-30 09:07:18 -08:00
Johannes Sixt
20a67e8ce9 t3008: find test-tool through path lookup
Do not use $GIT_BUILD_DIR without quotes; it may contain spaces and be
split into fields. But it is not necessary to access test-tool with an
absolute path in the first place as it can be found via path lookup.
Remove the explicit path.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-27 14:15:01 -08:00
Junio C Hamano
d2189a721c Merge branch 'en/fill-directory-fixes'
Assorted fixes to the directory traversal API.

* en/fill-directory-fixes:
  dir.c: use st_add3() for allocation size
  dir: consolidate similar code in treat_directory()
  dir: synchronize treat_leading_path() and read_directory_recursive()
  dir: fix checks on common prefix directory
  dir: break part of read_directory_recursive() out for reuse
  dir: exit before wildcard fall-through if there is no wildcard
  dir: remove stray quote character in comment
  Revert "dir.c: make 'git-status --ignored' work within leading directories"
  t3011: demonstrate directory traversal failures
2019-12-25 11:22:02 -08:00
Junio C Hamano
8be0a428d6 Merge branch 'rs/test-cleanup'
Test cleanup.

* rs/test-cleanup:
  t6030: don't create unused file
  t5580: don't create unused file
  t3501: don't create unused file
  t7004: don't create unused file
  t4256: don't create unused file
2019-12-25 11:22:01 -08:00
Junio C Hamano
c0c6a74594 Merge branch 'rs/xdiff-ignore-ws-w-func-context'
Extend test coverage for a recent fix.

* rs/xdiff-ignore-ws-w-func-context:
  t4015: improve coverage of function context test
2019-12-25 11:22:01 -08:00
Junio C Hamano
45b96a6fa1 Merge branch 'js/add-p-in-c'
The effort to move "git-add--interactive" to C continues.

* js/add-p-in-c:
  built-in add -p: show helpful hint when nothing can be staged
  built-in add -p: only show the applicable parts of the help text
  built-in add -p: implement the 'q' ("quit") command
  built-in add -p: implement the '/' ("search regex") command
  built-in add -p: implement the 'g' ("goto") command
  built-in add -p: implement hunk editing
  strbuf: add a helper function to call the editor "on an strbuf"
  built-in add -p: coalesce hunks after splitting them
  built-in add -p: implement the hunk splitting feature
  built-in add -p: show different prompts for mode changes and deletions
  built-in app -p: allow selecting a mode change as a "hunk"
  built-in add -p: handle deleted empty files
  built-in add -p: support multi-file diffs
  built-in add -p: offer a helpful error message when hunk navigation failed
  built-in add -p: color the prompt and the help text
  built-in add -p: adjust hunk headers as needed
  built-in add -p: show colored hunks by default
  built-in add -i: wire up the new C code for the `patch` command
  built-in add -i: start implementing the `patch` functionality in C
2019-12-25 11:22:01 -08:00
Junio C Hamano
bd72a08d6c Merge branch 'ds/sparse-cone'
Management of sparsely checked-out working tree has gained a
dedicated "sparse-checkout" command.

* ds/sparse-cone: (21 commits)
  sparse-checkout: improve OS ls compatibility
  sparse-checkout: respect core.ignoreCase in cone mode
  sparse-checkout: check for dirty status
  sparse-checkout: update working directory in-process for 'init'
  sparse-checkout: cone mode should not interact with .gitignore
  sparse-checkout: write using lockfile
  sparse-checkout: use in-process update for disable subcommand
  sparse-checkout: update working directory in-process
  sparse-checkout: sanitize for nested folders
  unpack-trees: add progress to clear_ce_flags()
  unpack-trees: hash less in cone mode
  sparse-checkout: init and set in cone mode
  sparse-checkout: use hashmaps for cone patterns
  sparse-checkout: add 'cone' mode
  trace2: add region in clear_ce_flags
  sparse-checkout: create 'disable' subcommand
  sparse-checkout: add '--stdin' option to set subcommand
  sparse-checkout: 'set' subcommand
  clone: add --sparse mode
  sparse-checkout: create 'init' subcommand
  ...
2019-12-25 11:21:58 -08:00
Junio C Hamano
f3c520e17f Merge branch 'sg/name-rev-wo-recursion'
Redo "git name-rev" to avoid recursive calls.

* sg/name-rev-wo-recursion:
  name-rev: cleanup name_ref()
  name-rev: eliminate recursion in name_rev()
  name-rev: use 'name->tip_name' instead of 'tip_name'
  name-rev: drop name_rev()'s 'generation' and 'distance' parameters
  name-rev: restructure creating/updating 'struct rev_name' instances
  name-rev: restructure parsing commits and applying date cutoff
  name-rev: pull out deref handling from the recursion
  name-rev: extract creating/updating a 'struct name_rev' into a helper
  t6120: add a test to cover inner conditions in 'git name-rev's name_rev()
  name-rev: use sizeof(*ptr) instead of sizeof(type) in allocation
  name-rev: avoid unnecessary cast in name_ref()
  name-rev: use strbuf_strip_suffix() in get_rev_name()
  t6120-describe: modernize the 'check_describe' helper
  t6120-describe: correct test repo history graph in comment
2019-12-25 11:21:58 -08:00
Junio C Hamano
6514ad40a1 Merge branch 'ra/t5150-depends-on-perl'
Some Porcelain commands are written in Perl, and tests on them are
expected not to work when the platform lacks a working perl.

* ra/t5150-depends-on-perl:
  t5150: skip request-pull test if Perl is disabled
2019-12-25 11:21:58 -08:00
Junio C Hamano
17066bea38 Merge branch 'dl/format-patch-notes-config-fixup'
"git format-patch" can take a set of configured format.notes values
to specify which notes refs to use in the log message part of the
output.  The behaviour of this was not consistent with multiple
--notes command line options, which has been corrected.

* dl/format-patch-notes-config-fixup:
  notes.h: fix typos in comment
  notes: break set_display_notes() into smaller functions
  config/format.txt: clarify behavior of multiple format.notes
  format-patch: move git_config() before repo_init_revisions()
  format-patch: use --notes behavior for format.notes
  notes: extract logic into set_display_notes()
  notes: create init_display_notes() helper
  notes: rename to load_display_notes()
2019-12-25 11:21:58 -08:00
Junio C Hamano
135365dd99 Merge branch 'am/pathspec-f-f-checkout'
A few more commands learned the "--pathspec-from-file" command line
option.

* am/pathspec-f-f-checkout:
  checkout, restore: support the --pathspec-from-file option
  doc: restore: synchronize <pathspec> description
  doc: checkout: synchronize <pathspec> description
  doc: checkout: fix broken text reference
  doc: checkout: remove duplicate synopsis
  add: support the --pathspec-from-file option
  cmd_add: prepare for next patch
2019-12-25 11:21:57 -08:00
Junio C Hamano
ff0cb70d45 Merge branch 'am/pathspec-from-file'
An earlier series to teach "--pathspec-from-file" to "git commit"
forgot to make the option incompatible with "--all", which has been
corrected.

* am/pathspec-from-file:
  commit: forbid --pathspec-from-file --all
2019-12-25 11:21:57 -08:00
Johannes Schindelin
4dc42c6c18 mingw: refuse paths containing reserved names
There are a couple of reserved names that cannot be file names on
Windows, such as `AUX`, `NUL`, etc. For an almost complete list, see
https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file

If one would try to create a directory named `NUL`, it would actually
"succeed", i.e. the call would return success, but nothing would be
created.

Worse, even adding a file extension to the reserved name does not make
it a valid file name. To understand the rationale behind that behavior,
see https://devblogs.microsoft.com/oldnewthing/20031022-00/?p=42073

Let's just disallow them all.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-21 16:09:07 -08:00
Ed Maste
761e3d26bb sparse-checkout: improve OS ls compatibility
On FreeBSD, when executed by root ls enables the '-A' option:

  -A  Include directory entries whose names begin with a dot (`.')
      except for . and ...  Automatically set for the super-user unless
      -I is specified.

As a result the .git directory appeared in the output when run as root.
Simulate no-dotfile ls behaviour using a shell glob.

Helped-by: Eric Wong <e@80x24.org>
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Ed Maste <emaste@FreeBSD.org>
Acked-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-20 12:47:08 -08:00
Elijah Newren
777b420347 dir: synchronize treat_leading_path() and read_directory_recursive()
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.

Alternatively, we could try to change treat_path to return path_recurse
for an untracked directory under the given special circumstances that
this logic checks for, but a simple switch results in many test failures
such as 'git clean -d' not wiping out untracked but empty directories.
To work around that, we'd need the caller of treat_path to check for
path_recurse and sometimes special case it into path_untracked.  In
other words, we'd still have extra logic in both places.

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.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-19 13:45:47 -08:00
Elijah Newren
b9670c1f5e dir: fix checks on common prefix directory
Many years ago, the directory traversing logic had an optimization that
would always recurse into any directory that was a common prefix of all
the pathspecs without walking the leading directories to get down to
the desired directory.  Thus,
   git ls-files -o .git/                        # case A
would notice that .git/ was a common prefix of all pathspecs (since
it is the only pathspec listed), and then traverse into it and start
showing unknown files under that directory.  Unfortunately, .git/ is not
a directory we should be traversing into, which made this optimization
problematic.  This also affected cases like
   git ls-files -o --exclude-standard t/        # case B
where t/ was in the .gitignore file and thus isn't interesting and
shouldn't be recursed into.  It also affected cases like
   git ls-files -o --directory untracked_dir/   # case C
where untracked_dir/ is indeed untracked and thus interesting, but the
--directory flag means we only want to show the directory itself, not
recurse into it and start listing untracked files below it.

The case B class of bugs were noted and fixed in commits 16e2cfa909
("read_directory(): further split treat_path()", 2010-01-08) and
48ffef966c ("ls-files: fix overeager pathspec optimization",
2010-01-08), with the idea being that we first wanted to check whether
the common prefix was interesting.  The former patch noted that
treat_path() couldn't be used when checking the common prefix because
treat_path() requires a dir_entry() and we haven't read any directories
at the point we are checking the common prefix.  So, that patch split
treat_one_path() out of treat_path().  The latter patch then created a
new treat_leading_path() which duplicated by hand the bits of
treat_path() that couldn't be broken out and then called
treat_one_path() for the remainder.  There were three problems with this
approach:

  * The duplicated logic in treat_leading_path() accidentally missed the
    check for special paths (such as is_dot_or_dotdot and matching
    ".git"), causing case A types of bugs to continue to be an issue.
  * The treat_leading_path() logic assumed we should traverse into
    anything where path_treatment was not path_none, i.e. it perpetuated
    class C types of bugs.
  * It meant we had split logic that needed to kept in sync, running the
    risk that people introduced new inconsistencies (such as in commit
    be8a84c526, which we reverted earlier in this series, or in commit
    df5bcdf83a which we'll fix in a subsequent commit)

Fix most these problems by making treat_leading_path() not only loop
over each leading path component, but calling treat_path() directly on
each.  To do so, we have to create a synthetic dir_entry, but that only
takes a few lines.  Then, pay attention to the path_treatment result we
get from treat_path() and don't treat path_excluded, path_untracked, and
path_recurse all the same as path_recurse.

This leaves one remaining problem, the new inconsistency from commit
df5bcdf83a.  That will be addressed in a subsequent commit.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-19 13:45:47 -08:00
Heba Waly
5c4f55f1f6 commit: honor advice.statusHints when rejecting an empty commit
In ea9882bfc4 (commit: disable status hints when writing to
COMMIT_EDITMSG, 2013-09-12) the intent was to disable status hints
when writing to COMMIT_EDITMSG, because giving the hints in the "git
status" like output in the commit message template are too late to
be useful (they say things like "'git add' to stage", but that is
only possible after aborting the current "git commit" session).

But there is one case that the hints can be useful: When the current
attempt to commit is rejected because no change is recorded in the
index.  The message is given and "git commit" errors out, so the
hints can immediately be followed by the user.  Teach the codepath
to honor the configuration variable.

Signed-off-by: Heba Waly <heba.waly@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-19 11:58:08 -08:00
René Scharfe
124a895811 t4015: improve coverage of function context test
Add a test that includes an actual function line in the test file to
check if context is expanded to include the whole function, and add an
ignored change before function context to check if that one stays hidden
while the originally ignored change within function context is shown.

This differs from the existing test, which is concerned with the case
where there is no function line at all in the file (and we might look
past the beginning of the file).

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-19 10:35:21 -08:00
Alexandr Miloslavskiy
509efef789 commit: forbid --pathspec-from-file --all
I forgot this in my previous patch `--pathspec-from-file` for
`git commit` [1]. When both `--pathspec-from-file` and `--all` were
specified, `--all` took precedence and `--pathspec-from-file` was
ignored. Before `--pathspec-from-file` was implemented, this case was
prevented by this check in `parse_and_validate_options()` :

    die(_("paths '%s ...' with -a does not make sense"), argv[0]);

It is unfortunate that these two cases are disconnected. This came as
result of how the code was laid out before my patches, where `pathspec`
is parsed outside of `parse_and_validate_options()`. This branch is
already full of refactoring patches and I did not dare to go for another
one.

Fix by mirroring `die()` for `--pathspec-from-file` as well.

[1] Commit e440fc58 ("commit: support the --pathspec-from-file option" 2019-11-19)

Reported-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-18 14:14:14 -08:00