Commit Graph

13448 Commits

Author SHA1 Message Date
Junio C Hamano
7913f53b56 Sync with Git 2.17.1
* maint: (25 commits)
  Git 2.17.1
  Git 2.16.4
  Git 2.15.2
  Git 2.14.4
  Git 2.13.7
  fsck: complain when .gitmodules is a symlink
  index-pack: check .gitmodules files with --strict
  unpack-objects: call fsck_finish() after fscking objects
  fsck: call fsck_finish() after fscking objects
  fsck: check .gitmodules content
  fsck: handle promisor objects in .gitmodules check
  fsck: detect gitmodules files
  fsck: actually fsck blob data
  fsck: simplify ".git" check
  index-pack: make fsck error message more specific
  verify_path: disallow symlinks in .gitmodules
  update-index: stat updated files earlier
  verify_dotfile: mention case-insensitivity in comment
  verify_path: drop clever fallthrough
  skip_prefix: add case-insensitive variant
  ...
2018-05-29 17:10:05 +09:00
Junio C Hamano
1e174fdbe6 Merge branch 'em/status-rename-config'
"git status" learned to pay attention to UI related diff
configuration variables such as diff.renames.

* em/status-rename-config:
  wt-status: use settings from git_diff_ui_config
2018-05-23 14:38:23 +09:00
Junio C Hamano
02d11bb5c6 Merge branch 'cc/perf-bisect'
Performance test updates.

* cc/perf-bisect:
  perf/bisect_run_script: disable codespeed
2018-05-23 14:38:23 +09:00
Junio C Hamano
fda537adba Merge branch 'bp/test-drop-caches'
Code simplification.

* bp/test-drop-caches:
  test-drop-caches: simplify delay loading of NtSetSystemInformation
2018-05-23 14:38:22 +09:00
Junio C Hamano
4e0086bea1 Merge branch 'bc/format-patch-cover-no-attach'
"git format-patch --cover --attach" created a broken MIME multipart
message for the cover letter, which has been fixed by keeping the
cover letter as plain text file.

* bc/format-patch-cover-no-attach:
  format-patch: make cover letters always text/plain
2018-05-23 14:38:21 +09:00
Junio C Hamano
71cdbb3d4a Merge branch 'tb/test-apfs-utf8-normalization'
A test to see if the filesystem normalizes UTF-8 filename has been
updated to check what we need to know in a more direct way, i.e. a
path created in NFC form can be accessed with NFD form (or vice
versa) to cope with APFS as well as HFS.

* tb/test-apfs-utf8-normalization:
  test: correct detection of UTF8_NFD_TO_NFC for APFS
2018-05-23 14:38:21 +09:00
Junio C Hamano
2c18e6ae24 Merge branch 'js/rebase-recreate-merge'
"git rebase" learned "--rebase-merges" to transplant the whole
topology of commit graph elsewhere.

* js/rebase-recreate-merge:
  rebase -i --rebase-merges: add a section to the man page
  rebase -i: introduce --rebase-merges=[no-]rebase-cousins
  pull: accept --rebase=merges to recreate the branch topology
  rebase --rebase-merges: avoid "empty merges"
  sequencer: handle post-rewrite for merge commands
  sequencer: make refs generated by the `label` command worktree-local
  rebase --rebase-merges: add test for --keep-empty
  rebase: introduce the --rebase-merges option
  rebase-helper --make-script: introduce a flag to rebase merges
  sequencer: fast-forward `merge` commands, if possible
  sequencer: introduce the `merge` command
  sequencer: introduce new commands to reset the revision
  git-rebase--interactive: clarify arguments
  sequencer: offer helpful advice when a command was rescheduled
  sequencer: refactor how original todo list lines are accessed
  sequencer: make rearrange_squash() a bit more obvious
  sequencer: avoid using errno clobbered by rollback_lock_file()
2018-05-23 14:38:20 +09:00
Junio C Hamano
ad635e82d6 Merge branch 'nd/pack-objects-pack-struct'
"git pack-objects" needs to allocate tons of "struct object_entry"
while doing its work, and shrinking its size helps the performance
quite a bit.

* nd/pack-objects-pack-struct:
  ci: exercise the whole test suite with uncommon code in pack-objects
  pack-objects: reorder members to shrink struct object_entry
  pack-objects: shrink delta_size field in struct object_entry
  pack-objects: shrink size field in struct object_entry
  pack-objects: clarify the use of object_entry::size
  pack-objects: don't check size when the object is bad
  pack-objects: shrink z_delta_size field in struct object_entry
  pack-objects: refer to delta objects by index instead of pointer
  pack-objects: move in_pack out of struct object_entry
  pack-objects: move in_pack_pos out of struct object_entry
  pack-objects: use bitfield for object_entry::depth
  pack-objects: use bitfield for object_entry::dfs_state
  pack-objects: turn type and in_pack_type to bitfields
  pack-objects: a bit of document about struct object_entry
  read-cache.c: make $GIT_TEST_SPLIT_INDEX boolean
2018-05-23 14:38:19 +09:00
Junio C Hamano
c67de747f4 Merge branch 'en/rename-directory-detection-reboot'
Rename detection logic in "diff" family that is used in "merge" has
learned to guess when all of x/a, x/b and x/c have moved to z/a,
z/b and z/c, it is likely that x/d added in the meantime would also
want to move to z/d by taking the hint that the entire directory
'x' moved to 'z'.  A bug causing dirty files involved in a rename
to be overwritten during merge has also been fixed as part of this
work.  Incidentally, this also avoids updating a file in the
working tree after a (non-trivial) merge whose result matches what
our side originally had.

* en/rename-directory-detection-reboot: (36 commits)
  merge-recursive: fix check for skipability of working tree updates
  merge-recursive: make "Auto-merging" comment show for other merges
  merge-recursive: fix remainder of was_dirty() to use original index
  merge-recursive: fix was_tracked() to quit lying with some renamed paths
  t6046: testcases checking whether updates can be skipped in a merge
  merge-recursive: avoid triggering add_cacheinfo error with dirty mod
  merge-recursive: move more is_dirty handling to merge_content
  merge-recursive: improve add_cacheinfo error handling
  merge-recursive: avoid spurious rename/rename conflict from dir renames
  directory rename detection: new testcases showcasing a pair of bugs
  merge-recursive: fix remaining directory rename + dirty overwrite cases
  merge-recursive: fix overwriting dirty files involved in renames
  merge-recursive: avoid clobbering untracked files with directory renames
  merge-recursive: apply necessary modifications for directory renames
  merge-recursive: when comparing files, don't include trees
  merge-recursive: check for file level conflicts then get new name
  merge-recursive: add computation of collisions due to dir rename & merging
  merge-recursive: check for directory level conflicts
  merge-recursive: add get_directory_renames()
  merge-recursive: make a helper function for cleanup for handle_renames
  ...
2018-05-23 14:38:19 +09:00
Junio C Hamano
4a3bf32b6c Merge branch 'js/rebase-i-clean-msg-after-fixup-continue'
"git rebase -i" sometimes left intermediate "# This is a
combination of N commits" message meant for the human consumption
inside an editor in the final result in certain corner cases, which
has been fixed.

* js/rebase-i-clean-msg-after-fixup-continue:
  rebase --skip: clean up commit message after a failed fixup/squash
  sequencer: always commit without editing when asked for
  rebase -i: Handle "combination of <n> commits" with GETTEXT_POISON
  rebase -i: demonstrate bugs with fixup!/squash! commit messages
2018-05-23 14:38:18 +09:00
Junio C Hamano
10174da9f1 Merge branch 'tg/worktree-add-existing-branch'
"git worktree add" learned to check out an existing branch.

* tg/worktree-add-existing-branch:
  worktree: teach "add" to check out existing branches
  worktree: factor out dwim_branch function
  worktree: improve message when creating a new worktree
  worktree: remove extra members from struct add_opts
2018-05-23 14:38:18 +09:00
Junio C Hamano
352cf6cfe1 Merge branch 'js/deprecate-grafts'
The functionality of "$GIT_DIR/info/grafts" has been superseded by
the "refs/replace/" mechanism for some time now, but the internal
code had support for it in many places, which has been cleaned up
in order to drop support of the "grafts" mechanism.

* js/deprecate-grafts:
  Remove obsolete script to convert grafts to replace refs
  technical/shallow: describe why shallow cannot use replace refs
  technical/shallow: stop referring to grafts
  filter-branch: stop suggesting to use grafts
  Deprecate support for .git/info/grafts
  Add a test for `git replace --convert-graft-file`
  replace: introduce --convert-graft-file
  replace: prepare create_graft() for converting graft files wholesale
  replace: "libify" create_graft() and callees
  replace: avoid using die() to indicate a bug
  commit: Let the callback of for_each_mergetag return on error
  argv_array: offer to split a string by whitespace
2018-05-23 14:38:17 +09:00
Junio C Hamano
5002702e48 Merge branch 'js/test-unset-prereq'
Test debugging aid.

* js/test-unset-prereq:
  tests: introduce test_unset_prereq, for debugging
2018-05-23 14:38:17 +09:00
Junio C Hamano
d0f7b22b5b Merge branch 'cc/perf-aggregate-unknown-option'
Perf-test helper updates.

* cc/perf-aggregate-unknown-option:
  perf/aggregate: use Getopt::Long for option parsing
2018-05-23 14:38:15 +09:00
Junio C Hamano
89753dc2b7 Merge branch 'en/git-debugger'
Dev support.

* en/git-debugger:
  Make running git under other debugger-like programs easy
2018-05-23 14:38:15 +09:00
Junio C Hamano
41267e9697 Merge branch 'bw/server-options'
The transport protocol v2 is getting updated further.

* bw/server-options:
  fetch: send server options when using protocol v2
  ls-remote: send server options when using protocol v2
  serve: introduce the server-option capability
2018-05-23 14:38:15 +09:00
Junio C Hamano
30b015bffe Merge branch 'nd/repack-keep-pack'
"git gc" in a large repository takes a lot of time as it considers
to repack all objects into one pack by default.  The command has
been taught to pretend as if the largest existing packfile is
marked with ".keep" so that it is left untouched while objects in
other packs and loose ones are repacked.

* nd/repack-keep-pack:
  pack-objects: show some progress when counting kept objects
  gc --auto: exclude base pack if not enough mem to "repack -ad"
  gc: handle a corner case in gc.bigPackThreshold
  gc: add gc.bigPackThreshold config
  gc: add --keep-largest-pack option
  repack: add --keep-pack option
  t7700: have closing quote of a test at the beginning of line
2018-05-23 14:38:14 +09:00
Junio C Hamano
05682ee270 Merge branch 'nd/term-columns'
The code did not propagate the terminal width to subprocesses via
COLUMNS environment variable, which it now does.  This caused
trouble to "git column" helper subprocess when "git tag --column=row"
tried to list the existing tags on a display with non-default width.

* nd/term-columns:
  column: fix off-by-one default width
  pager: set COLUMNS to term_columns()
2018-05-23 14:38:13 +09:00
Junio C Hamano
798b029da8 Merge branch 'sg/t7005-spaces-in-filenames-cleanup'
Test update.

* sg/t7005-spaces-in-filenames-cleanup:
  t7005-editor: get rid of the SPACES_IN_FILENAMES prereq
2018-05-23 14:38:12 +09:00
Junio C Hamano
c8311980f9 Merge branch 'sg/t5516-fixes'
Test fixes.

* sg/t5516-fixes:
  t5516-fetch-push: fix broken &&-chain
  t5516-fetch-push: fix 'push with dry-run' test
2018-05-23 14:38:12 +09:00
Junio C Hamano
c4e7220f08 Merge branch 'sg/t5310-jgit-bitmap-test'
Test update.

* sg/t5310-jgit-bitmap-test:
  t5310-pack-bitmaps: make JGit tests work with GIT_TEST_SPLIT_INDEX
2018-05-23 14:38:12 +09:00
Junio C Hamano
67b878eba1 Merge branch 'hn/sort-ls-remote'
Hotfix.

* hn/sort-ls-remote:
  t5512: run git fetch inside test
2018-05-23 14:38:09 +09:00
Jeff King
b7b1fca175 fsck: complain when .gitmodules is a symlink
We've recently forbidden .gitmodules to be a symlink in
verify_path(). And it's an easy way to circumvent our fsck
checks for .gitmodules content. So let's complain when we
see it.

Signed-off-by: Jeff King <peff@peff.net>
2018-05-21 23:55:12 -04:00
Jeff King
73c3f0f704 index-pack: check .gitmodules files with --strict
Now that the internal fsck code has all of the plumbing we
need, we can start checking incoming .gitmodules files.
Naively, it seems like we would just need to add a call to
fsck_finish() after we've processed all of the objects. And
that would be enough to cover the initial test included
here. But there are two extra bits:

  1. We currently don't bother calling fsck_object() at all
     for blobs, since it has traditionally been a noop. We'd
     actually catch these blobs in fsck_finish() at the end,
     but it's more efficient to check them when we already
     have the object loaded in memory.

  2. The second pass done by fsck_finish() needs to access
     the objects, but we're actually indexing the pack in
     this process. In theory we could give the fsck code a
     special callback for accessing the in-pack data, but
     it's actually quite tricky:

       a. We don't have an internal efficient index mapping
	  oids to packfile offsets. We only generate it on
	  the fly as part of writing out the .idx file.

       b. We'd still have to reconstruct deltas, which means
          we'd basically have to replicate all of the
	  reading logic in packfile.c.

     Instead, let's avoid running fsck_finish() until after
     we've written out the .idx file, and then just add it
     to our internal packed_git list.

     This does mean that the objects are "in the repository"
     before we finish our fsck checks. But unpack-objects
     already exhibits this same behavior, and it's an
     acceptable tradeoff here for the same reason: the
     quarantine mechanism means that pushes will be
     fully protected.

In addition to a basic push test in t7415, we add a sneaky
pack that reverses the usual object order in the pack,
requiring that index-pack access the tree and blob during
the "finish" step.

This already works for unpack-objects (since it will have
written out loose objects), but we'll check it with this
sneaky pack for good measure.

Signed-off-by: Jeff King <peff@peff.net>
2018-05-21 23:55:12 -04:00
Jeff King
6e328d6cae unpack-objects: call fsck_finish() after fscking objects
As with the previous commit, we must call fsck's "finish"
function in order to catch any queued objects for
.gitmodules checks.

This second pass will be able to access any incoming
objects, because we will have exploded them to loose objects
by now.

This isn't quite ideal, because it means that bad objects
may have been written to the object database (and a
subsequent operation could then reference them, even if the
other side doesn't send the objects again). However, this is
sufficient when used with receive.fsckObjects, since those
loose objects will all be placed in a temporary quarantine
area that will get wiped if we find any problems.

Signed-off-by: Jeff King <peff@peff.net>
2018-05-21 23:55:12 -04:00
Jeff King
1995b5e03e fsck: call fsck_finish() after fscking objects
Now that the internal fsck code is capable of checking
.gitmodules files, we just need to teach its callers to use
the "finish" function to check any queued objects.

With this, we can now catch the malicious case in t7415 with
git-fsck.

Signed-off-by: Jeff King <peff@peff.net>
2018-05-21 23:55:12 -04:00
Jeff King
eedd5949f5 Merge branch 'jk/submodule-name-verify-fix' into jk/submodule-name-verify-fsck
* jk/submodule-name-verify-fix:
  verify_path: disallow symlinks in .gitmodules
  update-index: stat updated files earlier
  verify_path: drop clever fallthrough
  skip_prefix: add icase-insensitive variant
  is_{hfs,ntfs}_dotgitmodules: add tests
  path: match NTFS short names for more .git files
  is_hfs_dotgit: match other .git files
  is_ntfs_dotgit: use a size_t for traversing string
  submodule-config: verify submodule names as paths

Note that this includes two bits of evil-merge:

 - there's a new call to verify_path() that doesn't actually
   have a mode available. It should be OK to pass "0" here,
   since we're just manipulating the untracked cache, not an
   actual index entry.

 - the lstat() in builtin/update-index.c:update_one() needs
   to be updated to handle the fsmonitor case (without this
   it still behaves correctly, but does an unnecessary
   lstat).
2018-05-21 23:54:28 -04:00
Johannes Schindelin
dc2d9ba318 is_{hfs,ntfs}_dotgitmodules: add tests
This tests primarily for NTFS issues, but also adds one example of an
HFS+ issue.

Thanks go to Congyi Wu for coming up with the list of examples where
NTFS would possibly equate the filename with `.gitmodules`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Jeff King <peff@peff.net>
2018-05-21 23:50:11 -04:00
Jeff King
0383bbb901 submodule-config: verify submodule names as paths
Submodule "names" come from the untrusted .gitmodules file,
but we blindly append them to $GIT_DIR/modules to create our
on-disk repo paths. This means you can do bad things by
putting "../" into the name (among other things).

Let's sanity-check these names to avoid building a path that
can be exploited. There are two main decisions:

  1. What should the allowed syntax be?

     It's tempting to reuse verify_path(), since submodule
     names typically come from in-repo paths. But there are
     two reasons not to:

       a. It's technically more strict than what we need, as
          we really care only about breaking out of the
          $GIT_DIR/modules/ hierarchy.  E.g., having a
          submodule named "foo/.git" isn't actually
          dangerous, and it's possible that somebody has
          manually given such a funny name.

       b. Since we'll eventually use this checking logic in
          fsck to prevent downstream repositories, it should
          be consistent across platforms. Because
          verify_path() relies on is_dir_sep(), it wouldn't
          block "foo\..\bar" on a non-Windows machine.

  2. Where should we enforce it? These days most of the
     .gitmodules reads go through submodule-config.c, so
     I've put it there in the reading step. That should
     cover all of the C code.

     We also construct the name for "git submodule add"
     inside the git-submodule.sh script. This is probably
     not a big deal for security since the name is coming
     from the user anyway, but it would be polite to remind
     them if the name they pick is invalid (and we need to
     expose the name-checker to the shell anyway for our
     test scripts).

     This patch issues a warning when reading .gitmodules
     and just ignores the related config entry completely.
     This will generally end up producing a sensible error,
     as it works the same as a .gitmodules file which is
     missing a submodule entry (so "submodule update" will
     barf, but "git clone --recurse-submodules" will print
     an error but not abort the clone.

     There is one minor oddity, which is that we print the
     warning once per malformed config key (since that's how
     the config subsystem gives us the entries). So in the
     new test, for example, the user would see three
     warnings. That's OK, since the intent is that this case
     should never come up outside of malicious repositories
     (and then it might even benefit the user to see the
     message multiple times).

Credit for finding this vulnerability and the proof of
concept from which the test script was adapted goes to
Etienne Stalmans.

Signed-off-by: Jeff King <peff@peff.net>
2018-05-21 23:50:11 -04:00
SZEDER Gábor
4362da078e t7005-editor: get rid of the SPACES_IN_FILENAMES prereq
The last two tests 'editor with a space' and 'core.editor with a
space' in 't7005-editor.sh' need the SPACES_IN_FILENAMES prereq to
ensure that they are only run on filesystems that allow, well, spaces
in filenames.  However, we have been putting a space in the name of
the trash directory for just over a decade now, so we wouldn't be able
to run any of our tests on such a filesystem in the first place.

This prereq is therefore unnecessary, remove it.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-15 12:27:29 +09:00
René Scharfe
7484cf538e t5512: run git fetch inside test
Do the preparatory fetch inside the test of ls-remote --symref to avoid
cluttering the test output and to be able to catch unexpected fetch
failures.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-14 08:49:17 +09:00
Nguyễn Thái Ngọc Duy
b5d5a567fb column: fix off-by-one default width
By default we want to fill the whole screen if possible, but we do not
want to use up _all_ terminal columns because the last character is
going hit the border, push the cursor over and wrap. Keep it at
default value zero, which will make print_columns() set the width at
term_columns() - 1.

This affects the test in t7004 because effective column width before
was 40 but now 39 so we need to compensate it by one or the output at
39 columns has a different layout.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-13 10:45:05 +09:00
SZEDER Gábor
f6b82970aa t5516-fetch-push: fix broken &&-chain
b2dc968e60 (t5516: refactor oddball tests, 2008-11-07) accidentaly
broke the &&-chain in the test 'push does not update local refs on
failure', but since it was in a subshell, chain-lint couldn't notice
it.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-11 12:32:53 +09:00
SZEDER Gábor
cfb482b6c3 t5516-fetch-push: fix 'push with dry-run' test
In a while-at-it cleanup replacing a 'cd dir && <...> && cd ..' with a
subshell, commit 28391a80a9 (receive-pack: allow deletion of corrupt
refs, 2007-11-29) also moved the assignment of the $old_commit
variable to that subshell.  This variable, however, is used outside of
that subshell as a parameter of check_push_result(), to check that a
ref still points to the commit where it is supposed to.  With the
variable remaining unset outside the subshell check_push_result()
doesn't perform that check at all.

Use 'git -C <dir> cmd...', so we don't need to change directory, and
thus don't need the subshell either when setting $old_commit.

Furthermore, change check_push_result() to require at least three
parameters (the repo, the oid, and at least one ref), so it will catch
similar issues earlier should they ever arise.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-11 12:32:51 +09:00
SZEDER Gábor
87a6bb701a t5310-pack-bitmaps: make JGit tests work with GIT_TEST_SPLIT_INDEX
The two JGit tests 'we can read jgit bitmaps' and 'jgit can read our
bitmaps' in 't5310-pack-bitmaps.sh' fail when run with
GIT_TEST_SPLIT_INDEX=YesPlease.  Both tests create a clone of the test
repository to check bitmap interoperability with JGit.  With split
indexes enabled the index in the clone repositories contains the
'link' extension, which JGit doesn't support and, consequently, an
exception aborts it:

  <...>
  org.eclipse.jgit.api.errors.JGitInternalException: DIRC extension 'link' not supported by this version.
          at org.eclipse.jgit.dircache.DirCache.readFrom(DirCache.java:562)
  <...>

Since testing bitmaps doesn't need a worktree in the first place,
let's just create bare clones for the two JGit tests, so the cloned
won't have an index, and these two tests can be executed even with
split index enabled.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-11 12:14:34 +09:00
Elijah Newren
1de70dbd1a merge-recursive: fix check for skipability of working tree updates
The can-working-tree-updates-be-skipped check has had a long and blemished
history.  The update can be skipped iff:
  a) The merge is clean
  b) The merge matches what was in HEAD (content, mode, pathname)
  c) The target path is usable (i.e. not involved in D/F conflict)

Traditionally, we split b into parts:
  b1) The merged result matches the content and mode found in HEAD
  b2) The merged target path existed in HEAD

Steps a & b1 are easy to check; we have always gotten those right.  While
it is easy to overlook step c, this was fixed seven years ago with commit
4ab9a157d0 ("merge_content(): Check whether D/F conflicts are still
present", 2010-09-20).  merge-recursive didn't have a readily available
way to directly check step b2, so various approximations were used:

  * In commit b2c8c0a762 ("merge-recursive: When we detect we can skip
    an update, actually skip it", 2011-02-28), it was noted that although
    the code claimed it was skipping the update, it did not actually skip
    the update.  The code was made to skip it, but used lstat(path, ...)
    as an approximation to path-was-tracked-in-index-before-merge.

  * In commit 5b448b8530 ("merge-recursive: When we detect we can skip
    an update, actually skip it", 2011-08-11), the problem with using
    lstat was noted.  It was changed to the approximation
       path2 && strcmp(path, path2)
    which is also wrong.  !path2 || strcmp(path, path2) would have been
    better, but would have fallen short with directory renames.

  * In c5b761fb27 ("merge-recursive: ensure we write updates for
    directory-renamed file", 2018-02-14), the problem with the previous
    approximation was noted and changed to
       was_tracked(path)
    That looks close to what we were trying to answer, but was_tracked()
    as implemented at the time should have been named is_tracked(); it
    returned something different than what we were looking for.

  * To make matters more complex, fixing was_tracked() isn't sufficient
    because the splitting of b into b1 and b2 is wrong.  Consider the
    following merge with a rename/add conflict:
       side A: modify foo, add unrelated bar
       side B: rename foo->bar (but don't modify the mode or contents)
    In this case, the three-way merge of original foo, A's foo, and B's
    bar will result in a desired pathname of bar with the same
    mode/contents that A had for foo.  Thus, A had the right mode and
    contents for the file, and it had the right pathname present (namely,
    bar), but the bar that was present was unrelated to the contents, so
    the working tree update was not skippable.

Fix this by introducing a new function:
   was_tracked_and_matches(o, path, &mfi.oid, mfi.mode)
and use it to directly check for condition b.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-08 16:11:00 +09:00
Elijah Newren
c04ba51739 t6046: testcases checking whether updates can be skipped in a merge
Add several tests checking whether updates can be skipped in a merge.
Also add several similar testcases for where updates cannot be skipped in
a merge to make sure that we skip if and only if we should.

In particular:

  * Testcase 1a (particularly 1a-check-L) would have pointed out the
    problem Linus has been dealing with for year with his merges[1].

  * Testcase 2a (particularly 2a-check-L) would have pointed out the
    problem with my directory-rename-series before it broke master[2].

  * Testcases 3[ab] (particularly 3a-check-L) provide a simpler testcase
    than 12b of t6043 making that one easier to understand.

  * There are several complementary testcases to make sure we're not just
    fixing those particular issues while regressing in the opposite
    direction.

  * There are also a pair of tests for the special case when a merge
    results in a skippable update AND the user has dirty modifications to
    the path.

[1] https://public-inbox.org/git/CA+55aFzLZ3UkG5svqZwSnhNk75=fXJRkvU1m_RHBG54NOoaZPA@mail.gmail.com/
[2] https://public-inbox.org/git/xmqqmuya43cs.fsf@gitster-ct.c.googlers.com/

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-08 16:11:00 +09:00
Elijah Newren
2f682e21a6 merge-recursive: avoid triggering add_cacheinfo error with dirty mod
If a cherry-pick or merge with a rename results in a skippable update
(due to the merged content matching what HEAD already had), but the
working directory is dirty, avoid trying to refresh the index as that
will fail.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-08 16:11:00 +09:00
Elijah Newren
fd53b7ffd1 merge-recursive: improve add_cacheinfo error handling
Four closely related changes all with the purpose of fixing error handling
in this function:
  - fix reported function name in add_cacheinfo error messages
  - differentiate between the two error messages
  - abort early when we hit the error (stop ignoring return code)
  - mark a test which was hitting this error as failing until we get the
    right fix

In more detail...

In commit 0424138d57 ("Fix bogus error message from merge-recursive
error path", 2007-04-01), it was noted that the name of the function which
the error message claimed it was reported from did not match the actual
function name.  This was changed to something closer to the real function
name, but it still didn't match the actual function name.  Fix the
reported name to match.

Second, the two errors in this function had identical messages, preventing
us from knowing which error had been triggered.  Add a couple words to the
second error message to differentiate the two.

Next, make sure callers do not ignore the return code so that it will stop
processing further entries (processing further entries could result in
more output which could cause the error to scroll off the screen, or at
least be missed by the user) and make it clear the error is the cause of
the early abort.  These errors should never be triggered in production; if
either one is, it represents a bug in the calling path somewhere and is
likely to have resulted in mis-merged content.  The combination of
ignoring of the return code and continuing to print other standard
messages after hitting the error resulted in the following bug report from
Junio: "...the command pretends that everything went well and merged
cleanly in that path...[Behaving] in a buggy and unexplainable way is bad
enough, doing so silently is unexcusable."  Fix this.

Finally, there was one test in the testsuite that did hit this error path,
but was passing anyway.  This would have been easy to miss since it had a
test_must_fail and thus could have failed for the wrong reason, but in a
separate testing step I added an intentional NULL-dereference to the
codepath where these error messages are printed in order to flush out such
cases.  I could modify that test to explicitly check for this error and
fail the test if it is hit, but since this test operates in a bit of a
gray area and needed other changes, I went for a different fix.  The gray
area this test operates in is the following: If the merge of a certain
file results in the same version of the file that existed in HEAD, but
there are dirty modifications to the file, is that an error with a
"Refusing to overwrite existing file" expected, or a case where the merge
should succeed since we shouldn't have to touch the dirty file anyway?
Recent discussion on the list leaned towards saying it should be a
success.  Therefore, change the expected behavior of this test to match.
As a side effect, this makes the failed-due-to-hitting-add_cacheinfo-error
very clear, and we can mark the test as test_expect_failure.  A subsequent
commit will implement the necessary changes to get this test to pass
again.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-08 16:11:00 +09:00
Elijah Newren
6e7e027fe5 merge-recursive: avoid spurious rename/rename conflict from dir renames
If a file on one side of history was renamed, and merely modified on the
other side, then applying a directory rename to the modified side gives us
a rename/rename(1to2) conflict.  We should only apply directory renames to
pairs representing either adds or renames.

Making this change means that a directory rename testcase that was
previously reported as a rename/delete conflict will now be reported as a
modify/delete conflict.

Reviewed-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-08 16:11:00 +09:00
Elijah Newren
bc71c4eebe directory rename detection: new testcases showcasing a pair of bugs
Add a testcase showing spurious rename/rename(1to2) conflicts occurring
due to directory rename detection.

Also add a pair of testcases dealing with moving directory hierarchies
around that were suggested by Stefan Beller as "food for thought" during
his review of an earlier patch series, but which actually uncovered a
bug.  Round things out with a test that is a cross between the two
testcases that showed existing bugs in order to make sure we aren't
merely addressing problems in isolation but in general.

Reviewed-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-08 16:11:00 +09:00
Elijah Newren
18797a3b10 merge-recursive: fix remaining directory rename + dirty overwrite cases
Reviewed-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-08 16:11:00 +09:00
Elijah Newren
64b1abe962 merge-recursive: fix overwriting dirty files involved in renames
This fixes an issue that existed before my directory rename detection
patches that affects both normal renames and renames implied by
directory rename detection.  Additional codepaths that only affect
overwriting of dirty files that are involved in directory rename
detection will be added in a subsequent commit.

Reviewed-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-08 16:11:00 +09:00
Elijah Newren
79c47598f5 merge-recursive: avoid clobbering untracked files with directory renames
Reviewed-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-08 16:11:00 +09:00
Elijah Newren
9c0743fe1e merge-recursive: apply necessary modifications for directory renames
This commit hooks together all the directory rename logic by making the
necessary changes to the rename struct, it's dst_entry, and the
diff_filepair under consideration.

Reviewed-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-08 16:11:00 +09:00
Elijah Newren
f6f7755918 merge-recursive: check for file level conflicts then get new name
Before trying to apply directory renames to paths within the given
directories, we want to make sure that there aren't conflicts at the
file level either.  If there aren't any, then get the new name from
any directory renames.

Reviewed-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-08 16:11:00 +09:00
Junio C Hamano
96f29521a3 Merge branch 'ma/http-walker-no-partial'
"git http-fetch" (deprecated) had an optional and experimental
"feature" to fetch only commits and/or trees, which nobody used.
This has been removed.

* ma/http-walker-no-partial:
  walker: drop fields of `struct walker` which are always 1
  http-fetch: make `-a` standard behaviour
2018-05-08 15:59:35 +09:00
Junio C Hamano
a56fb3dcc0 Merge branch 'js/colored-push-errors'
Error messages from "git push" can be painted for more visibility.

* js/colored-push-errors:
  config: document the settings to colorize push errors/hints
  push: test to verify that push errors are colored
  push: colorize errors
  color: introduce support for colorizing stderr
2018-05-08 15:59:34 +09:00
Junio C Hamano
3915f9a4fa Merge branch 'jc/parseopt-expiry-errors'
"git gc --prune=nonsense" spent long time repacking and then
silently failed when underlying "git prune --expire=nonsense"
failed to parse its command line.  This has been corrected.

* jc/parseopt-expiry-errors:
  parseopt: handle malformed --expire arguments more nicely
  gc: do not upcase error message shown with die()
2018-05-08 15:59:33 +09:00
Junio C Hamano
ad3207e6ff Merge branch 'ma/fast-export-skip-merge-fix'
"git fast-export" had a regression in v2.15.0 era where it skipped
some merge commits in certain cases, which has been corrected.

* ma/fast-export-skip-merge-fix:
  fast-export: fix regression skipping some merge-commits
2018-05-08 15:59:33 +09:00