Git fork

Merge branch 'ps/mv-contradiction-fix'

"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.

* ps/mv-contradiction-fix:
builtin/mv: convert assert(3p) into `BUG()`
builtin/mv: bail out when trying to move child and its parent

+81 -7
+61 -3
builtin/mv.c
··· 39 39 INDEX = (1 << 2), 40 40 SPARSE = (1 << 3), 41 41 SKIP_WORKTREE_DIR = (1 << 4), 42 + /* 43 + * A file gets moved implicitly via a move of one of its parent 44 + * directories. This flag causes us to skip the check that we don't try 45 + * to move a file and any of its parent directories at the same point 46 + * in time. 47 + */ 48 + MOVE_VIA_PARENT_DIR = (1 << 5), 42 49 }; 43 50 44 51 #define DUP_BASENAME 1 ··· 183 190 strbuf_release(&a_src_dir); 184 191 } 185 192 193 + struct pathmap_entry { 194 + struct hashmap_entry ent; 195 + const char *path; 196 + }; 197 + 198 + static int pathmap_cmp(const void *cmp_data UNUSED, 199 + const struct hashmap_entry *a, 200 + const struct hashmap_entry *b, 201 + const void *key UNUSED) 202 + { 203 + const struct pathmap_entry *e1 = container_of(a, struct pathmap_entry, ent); 204 + const struct pathmap_entry *e2 = container_of(b, struct pathmap_entry, ent); 205 + return fspathcmp(e1->path, e2->path); 206 + } 207 + 186 208 int cmd_mv(int argc, 187 209 const char **argv, 188 210 const char *prefix, ··· 213 235 struct cache_entry *ce; 214 236 struct string_list only_match_skip_worktree = STRING_LIST_INIT_DUP; 215 237 struct string_list dirty_paths = STRING_LIST_INIT_DUP; 238 + struct hashmap moved_dirs = HASHMAP_INIT(pathmap_cmp, NULL); 239 + struct strbuf pathbuf = STRBUF_INIT; 216 240 int ret; 217 241 218 242 git_config(git_default_config, NULL); ··· 331 355 332 356 dir_check: 333 357 if (S_ISDIR(st.st_mode)) { 358 + struct pathmap_entry *entry; 334 359 char *dst_with_slash; 335 360 size_t dst_with_slash_len; 336 361 int j, n; ··· 348 373 goto act_on_entry; 349 374 } 350 375 376 + entry = xmalloc(sizeof(*entry)); 377 + entry->path = src; 378 + hashmap_entry_init(&entry->ent, fspathhash(src)); 379 + hashmap_add(&moved_dirs, &entry->ent); 380 + 351 381 /* last - first >= 1 */ 352 382 modes[i] |= WORKING_DIRECTORY; 353 383 ··· 368 398 strvec_push(&sources, path); 369 399 strvec_push(&destinations, prefixed_path); 370 400 371 - memset(modes + argc + j, 0, sizeof(enum update_mode)); 372 - modes[argc + j] |= ce_skip_worktree(ce) ? SPARSE : INDEX; 401 + modes[argc + j] = MOVE_VIA_PARENT_DIR | (ce_skip_worktree(ce) ? SPARSE : INDEX); 373 402 submodule_gitfiles[argc + j] = NULL; 374 403 375 404 free(prefixed_path); ··· 465 494 } 466 495 } 467 496 497 + for (i = 0; i < argc; i++) { 498 + const char *slash_pos; 499 + 500 + if (modes[i] & MOVE_VIA_PARENT_DIR) 501 + continue; 502 + 503 + strbuf_reset(&pathbuf); 504 + strbuf_addstr(&pathbuf, sources.v[i]); 505 + 506 + slash_pos = strrchr(pathbuf.buf, '/'); 507 + while (slash_pos > pathbuf.buf) { 508 + struct pathmap_entry needle; 509 + 510 + strbuf_setlen(&pathbuf, slash_pos - pathbuf.buf); 511 + 512 + needle.path = pathbuf.buf; 513 + hashmap_entry_init(&needle.ent, fspathhash(pathbuf.buf)); 514 + 515 + if (hashmap_get_entry(&moved_dirs, &needle, ent, NULL)) 516 + die(_("cannot move both '%s' and its parent directory '%s'"), 517 + sources.v[i], pathbuf.buf); 518 + 519 + slash_pos = strrchr(pathbuf.buf, '/'); 520 + } 521 + } 522 + 468 523 if (only_match_skip_worktree.nr) { 469 524 advise_on_updating_sparse_paths(&only_match_skip_worktree); 470 525 if (!ignore_errors) { ··· 507 562 continue; 508 563 509 564 pos = index_name_pos(the_repository->index, src, strlen(src)); 510 - assert(pos >= 0); 565 + if (pos < 0) 566 + BUG("could not find source in index: '%s'", src); 511 567 if (!(mode & SPARSE) && !lstat(src, &st)) 512 568 sparse_and_dirty = ie_modified(the_repository->index, 513 569 the_repository->index->cache[pos], ··· 589 645 strvec_clear(&dest_paths); 590 646 strvec_clear(&destinations); 591 647 strvec_clear(&submodule_gitfiles_to_free); 648 + hashmap_clear_and_free(&moved_dirs, struct pathmap_entry, ent); 649 + strbuf_release(&pathbuf); 592 650 free(submodule_gitfiles); 593 651 free(modes); 594 652 return ret;
+20 -4
t/t7001-mv.sh
··· 550 550 git status 551 551 ' 552 552 553 - test_expect_failure 'nonsense mv triggers assertion failure and partially updated index' ' 553 + test_expect_success 'moving file and its parent directory at the same time fails' ' 554 554 test_when_finished git reset --hard HEAD && 555 555 git reset --hard HEAD && 556 556 mkdir -p a && 557 557 mkdir -p b && 558 558 >a/a.txt && 559 559 git add a/a.txt && 560 - test_must_fail git mv a/a.txt a b && 561 - git status --porcelain >actual && 562 - grep "^A[ ]*a/a.txt$" actual 560 + cat >expect <<-EOF && 561 + fatal: cannot move both ${SQ}a/a.txt${SQ} and its parent directory ${SQ}a${SQ} 562 + EOF 563 + test_must_fail git mv a/a.txt a b 2>err && 564 + test_cmp expect err 565 + ' 566 + 567 + test_expect_success 'moving nested directory and its parent directory at the same time fails' ' 568 + test_when_finished git reset --hard HEAD && 569 + git reset --hard HEAD && 570 + mkdir -p a/b/c && 571 + >a/b/c/file.txt && 572 + git add a && 573 + mkdir target && 574 + cat >expect <<-EOF && 575 + fatal: cannot move both ${SQ}a/b/c${SQ} and its parent directory ${SQ}a${SQ} 576 + EOF 577 + test_must_fail git mv a/b/c a target 2>err && 578 + test_cmp expect err 563 579 ' 564 580 565 581 test_done