diff --git a/Documentation/RelNotes/2.50.0.adoc b/Documentation/RelNotes/2.50.0.adoc index b1083c5193a54d..07759cf98b3cca 100644 --- a/Documentation/RelNotes/2.50.0.adoc +++ b/Documentation/RelNotes/2.50.0.adoc @@ -251,6 +251,15 @@ Fixes since v2.49 * Update to arm64 Windows port. (merge 436a42215e js/windows-arm64 later to maint). + * hashmap API clean-up to ensure hashmap_clear() leaves a cleared map + in a reusable state. + (merge 9481877de3 en/hashmap-clear-fix later to maint). + + * "git mv a a/b dst" would ask to move the directory 'a' itself, as + well as its contents, in a single destination directory, which is + a contradicting request that is impossible to satisfy. This case is + now detected and the command errors out. + (merge 974f0d4664 ps/mv-contradiction-fix later to maint). * Other code cleanup, docfix, build fix, etc. (merge 227c4f33a0 ja/doc-block-delimiter-markup-fix later to maint). @@ -277,3 +286,4 @@ Fixes since v2.49 (merge 25292c301d lo/remove-log-reencode-from-rev-info later to maint). (merge 1aa50636fd jk/p5332-testfix later to maint). (merge 42cf4ac552 ps/ci-resurrect-p4-on-github later to maint). + (merge 104add8368 js/diff-codeql-false-positive-workaround later to maint). diff --git a/builtin/mv.c b/builtin/mv.c index 54b323fff72a27..07548fe96aeb49 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -39,6 +39,13 @@ enum update_mode { INDEX = (1 << 2), SPARSE = (1 << 3), SKIP_WORKTREE_DIR = (1 << 4), + /* + * A file gets moved implicitly via a move of one of its parent + * directories. This flag causes us to skip the check that we don't try + * to move a file and any of its parent directories at the same point + * in time. + */ + MOVE_VIA_PARENT_DIR = (1 << 5), }; #define DUP_BASENAME 1 @@ -183,6 +190,21 @@ static void remove_empty_src_dirs(const char **src_dir, size_t src_dir_nr) strbuf_release(&a_src_dir); } +struct pathmap_entry { + struct hashmap_entry ent; + const char *path; +}; + +static int pathmap_cmp(const void *cmp_data UNUSED, + const struct hashmap_entry *a, + const struct hashmap_entry *b, + const void *key UNUSED) +{ + const struct pathmap_entry *e1 = container_of(a, struct pathmap_entry, ent); + const struct pathmap_entry *e2 = container_of(b, struct pathmap_entry, ent); + return fspathcmp(e1->path, e2->path); +} + int cmd_mv(int argc, const char **argv, const char *prefix, @@ -213,6 +235,8 @@ int cmd_mv(int argc, struct cache_entry *ce; struct string_list only_match_skip_worktree = STRING_LIST_INIT_DUP; struct string_list dirty_paths = STRING_LIST_INIT_DUP; + struct hashmap moved_dirs = HASHMAP_INIT(pathmap_cmp, NULL); + struct strbuf pathbuf = STRBUF_INIT; int ret; git_config(git_default_config, NULL); @@ -331,6 +355,7 @@ int cmd_mv(int argc, dir_check: if (S_ISDIR(st.st_mode)) { + struct pathmap_entry *entry; char *dst_with_slash; size_t dst_with_slash_len; int j, n; @@ -348,6 +373,11 @@ int cmd_mv(int argc, goto act_on_entry; } + entry = xmalloc(sizeof(*entry)); + entry->path = src; + hashmap_entry_init(&entry->ent, fspathhash(src)); + hashmap_add(&moved_dirs, &entry->ent); + /* last - first >= 1 */ modes[i] |= WORKING_DIRECTORY; @@ -368,8 +398,7 @@ int cmd_mv(int argc, strvec_push(&sources, path); strvec_push(&destinations, prefixed_path); - memset(modes + argc + j, 0, sizeof(enum update_mode)); - modes[argc + j] |= ce_skip_worktree(ce) ? SPARSE : INDEX; + modes[argc + j] = MOVE_VIA_PARENT_DIR | (ce_skip_worktree(ce) ? SPARSE : INDEX); submodule_gitfiles[argc + j] = NULL; free(prefixed_path); @@ -465,6 +494,32 @@ int cmd_mv(int argc, } } + for (i = 0; i < argc; i++) { + const char *slash_pos; + + if (modes[i] & MOVE_VIA_PARENT_DIR) + continue; + + strbuf_reset(&pathbuf); + strbuf_addstr(&pathbuf, sources.v[i]); + + slash_pos = strrchr(pathbuf.buf, '/'); + while (slash_pos > pathbuf.buf) { + struct pathmap_entry needle; + + strbuf_setlen(&pathbuf, slash_pos - pathbuf.buf); + + needle.path = pathbuf.buf; + hashmap_entry_init(&needle.ent, fspathhash(pathbuf.buf)); + + if (hashmap_get_entry(&moved_dirs, &needle, ent, NULL)) + die(_("cannot move both '%s' and its parent directory '%s'"), + sources.v[i], pathbuf.buf); + + slash_pos = strrchr(pathbuf.buf, '/'); + } + } + if (only_match_skip_worktree.nr) { advise_on_updating_sparse_paths(&only_match_skip_worktree); if (!ignore_errors) { @@ -507,7 +562,8 @@ int cmd_mv(int argc, continue; pos = index_name_pos(the_repository->index, src, strlen(src)); - assert(pos >= 0); + if (pos < 0) + BUG("could not find source in index: '%s'", src); if (!(mode & SPARSE) && !lstat(src, &st)) sparse_and_dirty = ie_modified(the_repository->index, the_repository->index->cache[pos], @@ -589,6 +645,8 @@ int cmd_mv(int argc, strvec_clear(&dest_paths); strvec_clear(&destinations); strvec_clear(&submodule_gitfiles_to_free); + hashmap_clear_and_free(&moved_dirs, struct pathmap_entry, ent); + strbuf_release(&pathbuf); free(submodule_gitfiles); free(modes); return ret; diff --git a/diff.c b/diff.c index 63e9ecb30c6d72..90e8003dd11e4d 100644 --- a/diff.c +++ b/diff.c @@ -892,7 +892,7 @@ static void fill_es_indent_data(struct emitted_diff_symbol *es) /* skip any \v \f \r at start of indentation */ while (s[off] == '\f' || s[off] == '\v' || - (s[off] == '\r' && off < len - 1)) + (off < len - 1 && s[off] == '\r')) off++; /* calculate the visual width of indentation */ diff --git a/hashmap.c b/hashmap.c index ee45ef00852c86..a711377853f185 100644 --- a/hashmap.c +++ b/hashmap.c @@ -205,8 +205,9 @@ void hashmap_clear_(struct hashmap *map, ssize_t entry_offset) return; if (entry_offset >= 0) /* called by hashmap_clear_and_free */ free_individual_entries(map, entry_offset); - free(map->table); - memset(map, 0, sizeof(*map)); + FREE_AND_NULL(map->table); + map->tablesize = 0; + map->private_size = 0; } struct hashmap_entry *hashmap_get(const struct hashmap *map, diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh index 25334b506228f0..920479e925620a 100755 --- a/t/t7001-mv.sh +++ b/t/t7001-mv.sh @@ -550,16 +550,32 @@ test_expect_success 'moving nested submodules' ' git status ' -test_expect_failure 'nonsense mv triggers assertion failure and partially updated index' ' +test_expect_success 'moving file and its parent directory at the same time fails' ' test_when_finished git reset --hard HEAD && git reset --hard HEAD && mkdir -p a && mkdir -p b && >a/a.txt && git add a/a.txt && - test_must_fail git mv a/a.txt a b && - git status --porcelain >actual && - grep "^A[ ]*a/a.txt$" actual + cat >expect <<-EOF && + fatal: cannot move both ${SQ}a/a.txt${SQ} and its parent directory ${SQ}a${SQ} + EOF + test_must_fail git mv a/a.txt a b 2>err && + test_cmp expect err +' + +test_expect_success 'moving nested directory and its parent directory at the same time fails' ' + test_when_finished git reset --hard HEAD && + git reset --hard HEAD && + mkdir -p a/b/c && + >a/b/c/file.txt && + git add a && + mkdir target && + cat >expect <<-EOF && + fatal: cannot move both ${SQ}a/b/c${SQ} and its parent directory ${SQ}a${SQ} + EOF + test_must_fail git mv a/b/c a target 2>err && + test_cmp expect err ' test_done