Merge branch 'en/merge-recursive-skip-fix'
authorJunio C Hamano <gitster@pobox.com>
Wed, 15 Aug 2018 22:08:25 +0000 (15:08 -0700)
committerJunio C Hamano <gitster@pobox.com>
Wed, 15 Aug 2018 22:08:26 +0000 (15:08 -0700)
When the sparse checkout feature is in use, "git cherry-pick" and
other mergy operations lost the skip_worktree bit when a path that
is excluded from checkout requires content level merge, which is
resolved as the same as the HEAD version, without materializing the
merge result in the working tree, which made the path appear as
deleted. This has been corrected by preserving the skip_worktree
bit (and not materializing the file in the working tree).

* en/merge-recursive-skip-fix:
merge-recursive: preserve skip_worktree bit when necessary
t3507: add a testcase showing failure with sparse checkout

1  2 
merge-recursive.c
diff --combined merge-recursive.c
index 1446e92beaf5a85376684cef0104dcd226a7e539,58f64ff15e355dab7a3d46300b4f50828eafa438..bd053c70dfc518b6265ec273aad2e61c05d60d18
@@@ -8,8 -8,6 +8,8 @@@
  #include "advice.h"
  #include "lockfile.h"
  #include "cache-tree.h"
 +#include "object-store.h"
 +#include "repository.h"
  #include "commit.h"
  #include "blob.h"
  #include "builtin.h"
@@@ -17,7 -15,6 +17,7 @@@
  #include "diff.h"
  #include "diffcore.h"
  #include "tag.h"
 +#include "alloc.h"
  #include "unpack-trees.h"
  #include "string-list.h"
  #include "xdiff-interface.h"
@@@ -158,12 -155,12 +158,12 @@@ static struct tree *shift_tree_object(s
        }
        if (!oidcmp(&two->object.oid, &shifted))
                return two;
 -      return lookup_tree(&shifted);
 +      return lookup_tree(the_repository, &shifted);
  }
  
  static struct commit *make_virtual_commit(struct tree *tree, const char *comment)
  {
 -      struct commit *commit = alloc_commit_node();
 +      struct commit *commit = alloc_commit_node(the_repository);
  
        set_merge_remote_desc(commit, comment, (struct object *)commit);
        commit->maybe_tree = tree;
@@@ -184,7 -181,7 +184,7 @@@ static int oid_eq(const struct object_i
  
  enum rename_type {
        RENAME_NORMAL = 0,
 -      RENAME_DIR,
 +      RENAME_VIA_DIR,
        RENAME_DELETE,
        RENAME_ONE_FILE_TO_ONE,
        RENAME_ONE_FILE_TO_TWO,
@@@ -289,12 -286,10 +289,12 @@@ static void output(struct merge_option
  
  static void output_commit_title(struct merge_options *o, struct commit *commit)
  {
 +      struct merge_remote_desc *desc;
 +
        strbuf_addchars(&o->obuf, ' ', o->call_depth * 2);
 -      if (commit->util)
 -              strbuf_addf(&o->obuf, "virtual %s\n",
 -                      merge_remote_util(commit)->name);
 +      desc = merge_remote_util(commit);
 +      if (desc)
 +              strbuf_addf(&o->obuf, "virtual %s\n", desc->name);
        else {
                strbuf_add_unique_abbrev(&o->obuf, &commit->object.oid,
                                         DEFAULT_ABBREV);
  }
  
  static int add_cacheinfo(struct merge_options *o,
 -              unsigned int mode, const struct object_id *oid,
 -              const char *path, int stage, int refresh, int options)
 +                       unsigned int mode, const struct object_id *oid,
 +                       const char *path, int stage, int refresh, int options)
  {
        struct cache_entry *ce;
        int ret;
  
 -      ce = make_cache_entry(mode, oid ? oid->hash : null_sha1, path, stage, 0);
 +      ce = make_cache_entry(&the_index, mode, oid ? oid : &null_oid, path, stage, 0);
        if (!ce)
                return err(o, _("add_cacheinfo failed for path '%s'; merge aborting."), path);
  
        if (refresh) {
                struct cache_entry *nce;
  
 -              nce = refresh_cache_entry(ce, CE_MATCH_REFRESH | CE_MATCH_IGNORE_MISSING);
 +              nce = refresh_cache_entry(&the_index, ce, CE_MATCH_REFRESH | CE_MATCH_IGNORE_MISSING);
                if (!nce)
                        return err(o, _("add_cacheinfo failed to refresh for path '%s'; merge aborting."), path);
                if (nce != ce)
@@@ -416,14 -411,14 +416,14 @@@ struct tree *write_tree_from_memory(str
                return NULL;
        }
  
 -      result = lookup_tree(&active_cache_tree->oid);
 +      result = lookup_tree(the_repository, &active_cache_tree->oid);
  
        return result;
  }
  
  static int save_files_dirs(const struct object_id *oid,
 -              struct strbuf *base, const char *path,
 -              unsigned int mode, int stage, void *context)
 +                         struct strbuf *base, const char *path,
 +                         unsigned int mode, int stage, void *context)
  {
        struct path_hashmap_entry *entry;
        int baselen = base->len;
@@@ -544,7 -539,7 +544,7 @@@ static void record_df_conflict_files(st
                                     struct string_list *entries)
  {
        /* If there is a D/F conflict and the file for such a conflict
 -       * currently exist in the working tree, we want to allow it to be
 +       * currently exists in the working tree, we want to allow it to be
         * removed to make room for the corresponding directory if needed.
         * The files underneath the directories of such D/F conflicts will
         * be processed before the corresponding file involved in the D/F
@@@ -918,7 -913,7 +918,7 @@@ static int make_room_for_path(struct me
         */
        if (would_lose_untracked(path))
                return err(o, _("refusing to lose untracked file at '%s'"),
 -                           path);
 +                         path);
  
        /* Successful unlink is good.. */
        if (!unlink(path))
@@@ -997,16 -992,16 +997,16 @@@ static int update_file_flags(struct mer
                        unlink(path);
                        if (symlink(lnk, path))
                                ret = err(o, _("failed to symlink '%s': %s"),
 -                                      path, strerror(errno));
 +                                        path, strerror(errno));
                        free(lnk);
                } else
                        ret = err(o,
                                  _("do not know what to do with %06o %s '%s'"),
                                  mode, oid_to_hex(oid), path);
 - free_buf:
 +      free_buf:
                free(buf);
        }
 - update_index:
 +update_index:
        if (!ret && update_cache)
                if (add_cacheinfo(o, mode, oid, path, 0, update_wd,
                                  ADD_CACHE_OK_TO_ADD))
@@@ -1095,7 -1090,7 +1095,7 @@@ static int merge_3way(struct merge_opti
  }
  
  static int find_first_merges(struct object_array *result, const char *path,
 -              struct commit *a, struct commit *b)
 +                           struct commit *a, struct commit *b)
  {
        int i, j;
        struct object_array merges = OBJECT_ARRAY_INIT;
  
        /* get all revisions that merge commit a */
        xsnprintf(merged_revision, sizeof(merged_revision), "^%s",
 -                      oid_to_hex(&a->object.oid));
 +                oid_to_hex(&a->object.oid));
        init_revisions(&revs, NULL);
        rev_opts.submodule = path;
        /* FIXME: can't handle linked worktrees in submodules yet */
@@@ -1192,9 -1187,9 +1192,9 @@@ static int merge_submodule(struct merge
                return 0;
        }
  
 -      if (!(commit_base = lookup_commit_reference(base)) ||
 -          !(commit_a = lookup_commit_reference(a)) ||
 -          !(commit_b = lookup_commit_reference(b))) {
 +      if (!(commit_base = lookup_commit_reference(the_repository, base)) ||
 +          !(commit_a = lookup_commit_reference(the_repository, a)) ||
 +          !(commit_b = lookup_commit_reference(the_repository, b))) {
                output(o, 1, _("Failed to merge submodule %s (commits not present)"), path);
                return 0;
        }
                output(o, 2, _("Found a possible merge resolution for the submodule:\n"));
                print_commit((struct commit *) merges.objects[0].item);
                output(o, 2, _(
 -                      "If this is correct simply add it to the index "
 -                      "for example\n"
 -                      "by using:\n\n"
 -                      "  git update-index --cacheinfo 160000 %s \"%s\"\n\n"
 -                      "which will accept this suggestion.\n"),
 -                      oid_to_hex(&merges.objects[0].item->oid), path);
 +                     "If this is correct simply add it to the index "
 +                     "for example\n"
 +                     "by using:\n\n"
 +                     "  git update-index --cacheinfo 160000 %s \"%s\"\n\n"
 +                     "which will accept this suggestion.\n"),
 +                     oid_to_hex(&merges.objects[0].item->oid), path);
                break;
  
        default:
@@@ -1337,10 -1332,10 +1337,10 @@@ static int merge_file_1(struct merge_op
                        result->clean = (merge_status == 0);
                } else if (S_ISGITLINK(a->mode)) {
                        result->clean = merge_submodule(o, &result->oid,
 -                                                     one->path,
 -                                                     &one->oid,
 -                                                     &a->oid,
 -                                                     &b->oid);
 +                                                      one->path,
 +                                                      &one->oid,
 +                                                      &a->oid,
 +                                                      &b->oid);
                } else if (S_ISLNK(a->mode)) {
                        switch (o->recursive_variant) {
                        case MERGE_RECURSIVE_NORMAL:
@@@ -1415,17 -1410,11 +1415,17 @@@ static int merge_file_one(struct merge_
        return merge_file_1(o, &one, &a, &b, path, branch1, branch2, mfi);
  }
  
 -static int conflict_rename_dir(struct merge_options *o,
 -                             struct diff_filepair *pair,
 -                             const char *rename_branch,
 -                             const char *other_branch)
 +static int handle_rename_via_dir(struct merge_options *o,
 +                               struct diff_filepair *pair,
 +                               const char *rename_branch,
 +                               const char *other_branch)
  {
 +      /*
 +       * Handle file adds that need to be renamed due to directory rename
 +       * detection.  This differs from handle_rename_normal, because
 +       * there is no content merge to do; just move the file into the
 +       * desired final location.
 +       */
        const struct diff_filespec *dest = pair->two;
  
        if (!o->call_depth && would_lose_untracked(dest->path)) {
  }
  
  static int handle_change_delete(struct merge_options *o,
 -                               const char *path, const char *old_path,
 -                               const struct object_id *o_oid, int o_mode,
 -                               const struct object_id *changed_oid,
 -                               int changed_mode,
 -                               const char *change_branch,
 -                               const char *delete_branch,
 -                               const char *change, const char *change_past)
 +                              const char *path, const char *old_path,
 +                              const struct object_id *o_oid, int o_mode,
 +                              const struct object_id *changed_oid,
 +                              int changed_mode,
 +                              const char *change_branch,
 +                              const char *delete_branch,
 +                              const char *change, const char *change_past)
  {
        char *alt_path = NULL;
        const char *update_path = path;
                if (!ret)
                        ret = update_file(o, 0, o_oid, o_mode, update_path);
        } else {
 +              /*
 +               * Despite the four nearly duplicate messages and argument
 +               * lists below and the ugliness of the nested if-statements,
 +               * having complete messages makes the job easier for
 +               * translators.
 +               *
 +               * The slight variance among the cases is due to the fact
 +               * that:
 +               *   1) directory/file conflicts (in effect if
 +               *      !alt_path) could cause us to need to write the
 +               *      file to a different path.
 +               *   2) renames (in effect if !old_path) could mean that
 +               *      there are two names for the path that the user
 +               *      may know the file by.
 +               */
                if (!alt_path) {
                        if (!old_path) {
                                output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s "
        return ret;
  }
  
 -static int conflict_rename_delete(struct merge_options *o,
 -                                 struct diff_filepair *pair,
 -                                 const char *rename_branch,
 -                                 const char *delete_branch)
 +static int handle_rename_delete(struct merge_options *o,
 +                              struct diff_filepair *pair,
 +                              const char *rename_branch,
 +                              const char *delete_branch)
  {
        const struct diff_filespec *orig = pair->one;
        const struct diff_filespec *dest = pair->two;
@@@ -1640,8 -1614,8 +1640,8 @@@ static int handle_file(struct merge_opt
        return ret;
  }
  
 -static int conflict_rename_rename_1to2(struct merge_options *o,
 -                                      struct rename_conflict_info *ci)
 +static int handle_rename_rename_1to2(struct merge_options *o,
 +                                   struct rename_conflict_info *ci)
  {
        /* One file was renamed in both branches, but to different names. */
        struct diff_filespec *one = ci->pair1->one;
        return 0;
  }
  
 -static int conflict_rename_rename_2to1(struct merge_options *o,
 -                                      struct rename_conflict_info *ci)
 +static int handle_rename_rename_2to1(struct merge_options *o,
 +                                   struct rename_conflict_info *ci)
  {
        /* Two files, a & b, were renamed to the same thing, c. */
        struct diff_filespec *a = ci->pair1->one;
@@@ -2445,7 -2419,7 +2445,7 @@@ static void apply_directory_rename_modi
         * "NOTE" in update_stages(), doing so will modify the current
         * in-memory index which will break calls to would_lose_untracked()
         * that we need to make.  Instead, we need to just make sure that
 -       * the various conflict_rename_*() functions update the index
 +       * the various handle_rename_*() functions update the index
         * explicitly rather than relying on unpack_trees() to have done it.
         */
        get_tree_entry(&tree->object.oid,
@@@ -2718,7 -2692,7 +2718,7 @@@ static int process_renames(struct merge
  
                        if (oid_eq(&src_other.oid, &null_oid) &&
                            ren1->add_turned_into_rename) {
 -                              setup_rename_conflict_info(RENAME_DIR,
 +                              setup_rename_conflict_info(RENAME_VIA_DIR,
                                                           ren1->pair,
                                                           NULL,
                                                           branch1,
@@@ -2849,12 -2823,12 +2849,12 @@@ static void initial_cleanup_rename(stru
        free(pairs);
  }
  
 -static int handle_renames(struct merge_options *o,
 -                        struct tree *common,
 -                        struct tree *head,
 -                        struct tree *merge,
 -                        struct string_list *entries,
 -                        struct rename_info *ri)
 +static int detect_and_process_renames(struct merge_options *o,
 +                                    struct tree *common,
 +                                    struct tree *head,
 +                                    struct tree *merge,
 +                                    struct string_list *entries,
 +                                    struct rename_info *ri)
  {
        struct diff_queue_struct *head_pairs, *merge_pairs;
        struct hashmap *dir_re_head, *dir_re_merge;
@@@ -2930,8 -2904,7 +2930,8 @@@ static struct object_id *stage_oid(cons
  }
  
  static int read_oid_strbuf(struct merge_options *o,
 -      const struct object_id *oid, struct strbuf *dst)
 +                         const struct object_id *oid,
 +                         struct strbuf *dst)
  {
        void *buf;
        enum object_type type;
@@@ -2984,10 -2957,10 +2984,10 @@@ error_return
  }
  
  static int handle_modify_delete(struct merge_options *o,
 -                               const char *path,
 -                               struct object_id *o_oid, int o_mode,
 -                               struct object_id *a_oid, int a_mode,
 -                               struct object_id *b_oid, int b_mode)
 +                              const char *path,
 +                              struct object_id *o_oid, int o_mode,
 +                              struct object_id *a_oid, int a_mode,
 +                              struct object_id *b_oid, int b_mode)
  {
        const char *modify_branch, *delete_branch;
        struct object_id *changed_oid;
@@@ -3070,10 -3043,26 +3070,26 @@@ static int merge_content(struct merge_o
        if (mfi.clean &&
            was_tracked_and_matches(o, path, &mfi.oid, mfi.mode) &&
            !df_conflict_remains) {
+               int pos;
+               struct cache_entry *ce;
                output(o, 3, _("Skipped %s (merged same as existing)"), path);
                if (add_cacheinfo(o, mfi.mode, &mfi.oid, path,
                                  0, (!o->call_depth && !is_dirty), 0))
                        return -1;
+               /*
+                * However, add_cacheinfo() will delete the old cache entry
+                * and add a new one.  We need to copy over any skip_worktree
+                * flag to avoid making the file appear as if it were
+                * deleted by the user.
+                */
+               pos = index_name_pos(&o->orig_index, path, strlen(path));
+               ce = o->orig_index.cache[pos];
+               if (ce_skip_worktree(ce)) {
+                       pos = index_name_pos(&the_index, path, strlen(path));
+                       ce = the_index.cache[pos];
+                       ce->ce_flags |= CE_SKIP_WORKTREE;
+               }
                return mfi.clean;
        }
  
        return !is_dirty && mfi.clean;
  }
  
 -static int conflict_rename_normal(struct merge_options *o,
 -                                const char *path,
 -                                struct object_id *o_oid, unsigned int o_mode,
 -                                struct object_id *a_oid, unsigned int a_mode,
 -                                struct object_id *b_oid, unsigned int b_mode,
 -                                struct rename_conflict_info *ci)
 +static int handle_rename_normal(struct merge_options *o,
 +                              const char *path,
 +                              struct object_id *o_oid, unsigned int o_mode,
 +                              struct object_id *a_oid, unsigned int a_mode,
 +                              struct object_id *b_oid, unsigned int b_mode,
 +                              struct rename_conflict_info *ci)
  {
        /* Merge the content and write it out */
        return merge_content(o, path, was_dirty(o, path),
@@@ -3157,37 -3146,37 +3173,37 @@@ static int process_entry(struct merge_o
                switch (conflict_info->rename_type) {
                case RENAME_NORMAL:
                case RENAME_ONE_FILE_TO_ONE:
 -                      clean_merge = conflict_rename_normal(o,
 -                                                           path,
 -                                                           o_oid, o_mode,
 -                                                           a_oid, a_mode,
 -                                                           b_oid, b_mode,
 -                                                           conflict_info);
 +                      clean_merge = handle_rename_normal(o,
 +                                                         path,
 +                                                         o_oid, o_mode,
 +                                                         a_oid, a_mode,
 +                                                         b_oid, b_mode,
 +                                                         conflict_info);
                        break;
 -              case RENAME_DIR:
 +              case RENAME_VIA_DIR:
                        clean_merge = 1;
 -                      if (conflict_rename_dir(o,
 -                                              conflict_info->pair1,
 -                                              conflict_info->branch1,
 -                                              conflict_info->branch2))
 +                      if (handle_rename_via_dir(o,
 +                                                conflict_info->pair1,
 +                                                conflict_info->branch1,
 +                                                conflict_info->branch2))
                                clean_merge = -1;
                        break;
                case RENAME_DELETE:
                        clean_merge = 0;
 -                      if (conflict_rename_delete(o,
 -                                                 conflict_info->pair1,
 -                                                 conflict_info->branch1,
 -                                                 conflict_info->branch2))
 +                      if (handle_rename_delete(o,
 +                                               conflict_info->pair1,
 +                                               conflict_info->branch1,
 +                                               conflict_info->branch2))
                                clean_merge = -1;
                        break;
                case RENAME_ONE_FILE_TO_TWO:
                        clean_merge = 0;
 -                      if (conflict_rename_rename_1to2(o, conflict_info))
 +                      if (handle_rename_rename_1to2(o, conflict_info))
                                clean_merge = -1;
                        break;
                case RENAME_TWO_FILES_TO_ONE:
                        clean_merge = 0;
 -                      if (conflict_rename_rename_2to1(o, conflict_info))
 +                      if (handle_rename_rename_2to1(o, conflict_info))
                                clean_merge = -1;
                        break;
                default:
@@@ -3281,13 -3270,6 +3297,13 @@@ int merge_trees(struct merge_options *o
                struct tree **result)
  {
        int code, clean;
 +      struct strbuf sb = STRBUF_INIT;
 +
 +      if (!o->call_depth && index_has_changes(&the_index, head, &sb)) {
 +              err(o, _("Your local changes to the following files would be overwritten by merge:\n  %s"),
 +                  sb.buf);
 +              return -1;
 +      }
  
        if (o->subtree_shift) {
                merge = shift_tree_object(head, merge, o->subtree_shift);
        }
  
        if (oid_eq(&common->object.oid, &merge->object.oid)) {
 -              struct strbuf sb = STRBUF_INIT;
 -
 -              if (!o->call_depth && index_has_changes(&sb)) {
 -                      err(o, _("Dirty index: cannot merge (dirty: %s)"),
 -                          sb.buf);
 -                      return 0;
 -              }
                output(o, 0, _("Already up to date!"));
                *result = head;
                return 1;
                get_files_dirs(o, merge);
  
                entries = get_unmerged();
 -              clean = handle_renames(o, common, head, merge, entries,
 -                                     &re_info);
 +              clean = detect_and_process_renames(o, common, head, merge,
 +                                                 entries, &re_info);
                record_df_conflict_files(o, entries);
                if (clean < 0)
                        goto cleanup;
                                    entries->items[i].string);
                }
  
 -cleanup:
 +      cleanup:
                final_cleanup_renames(&re_info);
  
                string_list_clear(entries, 1);
@@@ -3427,7 -3416,7 +3443,7 @@@ int merge_recursive(struct merge_option
                /* if there is no common ancestor, use an empty tree */
                struct tree *tree;
  
 -              tree = lookup_tree(the_hash_algo->empty_tree);
 +              tree = lookup_tree(the_repository, the_repository->hash_algo->empty_tree);
                merged_common_ancestors = make_virtual_commit(tree, "ancestor");
        }
  
@@@ -3489,9 -3478,7 +3505,9 @@@ static struct commit *get_ref(const str
  {
        struct object *object;
  
 -      object = deref_tag(parse_object(oid), name, strlen(name));
 +      object = deref_tag(the_repository, parse_object(the_repository, oid),
 +                         name,
 +                         strlen(name));
        if (!object)
                return NULL;
        if (object->type == OBJ_TREE)
@@@ -3522,14 -3509,14 +3538,14 @@@ int merge_recursive_generic(struct merg
                        struct commit *base;
                        if (!(base = get_ref(base_list[i], oid_to_hex(base_list[i]))))
                                return err(o, _("Could not parse object '%s'"),
 -                                      oid_to_hex(base_list[i]));
 +                                         oid_to_hex(base_list[i]));
                        commit_list_insert(base, &ca);
                }
        }
  
        hold_locked_index(&lock, LOCK_DIE_ON_ERROR);
        clean = merge_recursive(o, head_commit, next_commit, ca,
 -                      result);
 +                              result);
        if (clean < 0) {
                rollback_lock_file(&lock);
                return clean;