Skip to content

Commit 4e52008

Browse files
gitstergitster
committed
Merge branch 'jh/notes-merge-in-git-dir-worktree'
Running "notes merge --commit" failed to perform correctly when run from any directory inside $GIT_DIR/. When "notes merge" stops with conflicts, $GIT_DIR/NOTES_MERGE_WORKTREE is the place a user edits to resolve it. By Johan Herland (3) and Junio C Hamano (1) * jh/notes-merge-in-git-dir-worktree: notes-merge: Don't remove .git/NOTES_MERGE_WORKTREE; it may be the user's cwd notes-merge: use opendir/readdir instead of using read_directory() t3310: illustrate failure to "notes merge --commit" inside $GIT_DIR/ remove_dir_recursively(): Add flag for skipping removal of toplevel dir
2 parents 3bec29b + dabba59 commit 4e52008

File tree

4 files changed

+73
-32
lines changed

4 files changed

+73
-32
lines changed

‎dir.c‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1178,16 +1178,22 @@ int remove_dir_recursively(struct strbuf *path, int flag)
11781178
struct dirent *e;
11791179
int ret = 0, original_len = path->len, len;
11801180
int only_empty = (flag & REMOVE_DIR_EMPTY_ONLY);
1181+
int keep_toplevel = (flag & REMOVE_DIR_KEEP_TOPLEVEL);
11811182
unsigned char submodule_head[20];
11821183

11831184
if ((flag & REMOVE_DIR_KEEP_NESTED_GIT) &&
11841185
!resolve_gitlink_ref(path->buf, "HEAD", submodule_head))
11851186
/* Do not descend and nuke a nested git work tree. */
11861187
return 0;
11871188

1189+
flag &= ~(REMOVE_DIR_KEEP_TOPLEVEL|REMOVE_DIR_KEEP_NESTED_GIT);
11881190
dir = opendir(path->buf);
1189-
if (!dir)
1190-
return rmdir(path->buf);
1191+
if (!dir) {
1192+
if (!keep_toplevel)
1193+
return rmdir(path->buf);
1194+
else
1195+
return -1;
1196+
}
11911197
if (path->buf[original_len - 1] != '/')
11921198
strbuf_addch(path, '/');
11931199

@@ -1202,7 +1208,7 @@ int remove_dir_recursively(struct strbuf *path, int flag)
12021208
if (lstat(path->buf, &st))
12031209
; /* fall thru */
12041210
else if (S_ISDIR(st.st_mode)) {
1205-
if (!remove_dir_recursively(path, only_empty))
1211+
if (!remove_dir_recursively(path, flag))
12061212
continue; /* happy */
12071213
} else if (!only_empty && !unlink(path->buf))
12081214
continue; /* happy, too */
@@ -1214,7 +1220,7 @@ int remove_dir_recursively(struct strbuf *path, int flag)
12141220
closedir(dir);
12151221

12161222
strbuf_setlen(path, original_len);
1217-
if (!ret)
1223+
if (!ret && !keep_toplevel)
12181224
ret = rmdir(path->buf);
12191225
return ret;
12201226
}

‎dir.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ extern void setup_standard_excludes(struct dir_struct *dir);
102102

103103
#define REMOVE_DIR_EMPTY_ONLY 01
104104
#define REMOVE_DIR_KEEP_NESTED_GIT 02
105+
#define REMOVE_DIR_KEEP_TOPLEVEL 04
105106
extern int remove_dir_recursively(struct strbuf *path, int flag);
106107

107108
/* tries to remove the path with empty directories along it, ignores ENOENT */

‎notes-merge.c‎

Lines changed: 39 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,8 @@ static void check_notes_merge_worktree(struct notes_merge_options *o)
267267
* Must establish NOTES_MERGE_WORKTREE.
268268
* Abort if NOTES_MERGE_WORKTREE already exists
269269
*/
270-
if (file_exists(git_path(NOTES_MERGE_WORKTREE))) {
270+
if (file_exists(git_path(NOTES_MERGE_WORKTREE)) &&
271+
!is_empty_dir(git_path(NOTES_MERGE_WORKTREE))) {
271272
if (advice_resolve_conflict)
272273
die("You have not concluded your previous "
273274
"notes merge (%s exists).\nPlease, use "
@@ -687,51 +688,60 @@ int notes_merge_commit(struct notes_merge_options *o,
687688
{
688689
/*
689690
* Iterate through files in .git/NOTES_MERGE_WORKTREE and add all
690-
* found notes to 'partial_tree'. Write the updates notes tree to
691+
* found notes to 'partial_tree'. Write the updated notes tree to
691692
* the DB, and commit the resulting tree object while reusing the
692693
* commit message and parents from 'partial_commit'.
693694
* Finally store the new commit object SHA1 into 'result_sha1'.
694695
*/
695-
struct dir_struct dir;
696-
char *path = xstrdup(git_path(NOTES_MERGE_WORKTREE "/"));
697-
int path_len = strlen(path), i;
696+
DIR *dir;
697+
struct dirent *e;
698+
struct strbuf path = STRBUF_INIT;
698699
char *msg = strstr(partial_commit->buffer, "\n\n");
699700
struct strbuf sb_msg = STRBUF_INIT;
701+
int baselen;
700702

703+
strbuf_addstr(&path, git_path(NOTES_MERGE_WORKTREE));
701704
if (o->verbosity >= 3)
702-
printf("Committing notes in notes merge worktree at %.*s\n",
703-
path_len - 1, path);
705+
printf("Committing notes in notes merge worktree at %s\n",
706+
path.buf);
704707

705708
if (!msg || msg[2] == '\0')
706709
die("partial notes commit has empty message");
707710
msg += 2;
708711

709-
memset(&dir, 0, sizeof(dir));
710-
read_directory(&dir, path, path_len, NULL);
711-
for (i = 0; i < dir.nr; i++) {
712-
struct dir_entry *ent = dir.entries[i];
712+
dir = opendir(path.buf);
713+
if (!dir)
714+
die_errno("could not open %s", path.buf);
715+
716+
strbuf_addch(&path, '/');
717+
baselen = path.len;
718+
while ((e = readdir(dir)) != NULL) {
713719
struct stat st;
714-
const char *relpath = ent->name + path_len;
715720
unsigned char obj_sha1[20], blob_sha1[20];
716721

717-
if (ent->len - path_len != 40 || get_sha1_hex(relpath, obj_sha1)) {
722+
if (is_dot_or_dotdot(e->d_name))
723+
continue;
724+
725+
if (strlen(e->d_name) != 40 || get_sha1_hex(e->d_name, obj_sha1)) {
718726
if (o->verbosity >= 3)
719-
printf("Skipping non-SHA1 entry '%s'\n",
720-
ent->name);
727+
printf("Skipping non-SHA1 entry '%s%s'\n",
728+
path.buf, e->d_name);
721729
continue;
722730
}
723731

732+
strbuf_addstr(&path, e->d_name);
724733
/* write file as blob, and add to partial_tree */
725-
if (stat(ent->name, &st))
726-
die_errno("Failed to stat '%s'", ent->name);
727-
if (index_path(blob_sha1, ent->name, &st, HASH_WRITE_OBJECT))
728-
die("Failed to write blob object from '%s'", ent->name);
734+
if (stat(path.buf, &st))
735+
die_errno("Failed to stat '%s'", path.buf);
736+
if (index_path(blob_sha1, path.buf, &st, HASH_WRITE_OBJECT))
737+
die("Failed to write blob object from '%s'", path.buf);
729738
if (add_note(partial_tree, obj_sha1, blob_sha1, NULL))
730739
die("Failed to add resolved note '%s' to notes tree",
731-
ent->name);
740+
path.buf);
732741
if (o->verbosity >= 4)
733742
printf("Added resolved note for object %s: %s\n",
734743
sha1_to_hex(obj_sha1), sha1_to_hex(blob_sha1));
744+
strbuf_setlen(&path, baselen);
735745
}
736746

737747
strbuf_attach(&sb_msg, msg, strlen(msg), strlen(msg) + 1);
@@ -740,20 +750,25 @@ int notes_merge_commit(struct notes_merge_options *o,
740750
if (o->verbosity >= 4)
741751
printf("Finalized notes merge commit: %s\n",
742752
sha1_to_hex(result_sha1));
743-
free(path);
753+
strbuf_release(&path);
754+
closedir(dir);
744755
return 0;
745756
}
746757

747758
int notes_merge_abort(struct notes_merge_options *o)
748759
{
749-
/* Remove .git/NOTES_MERGE_WORKTREE directory and all files within */
760+
/*
761+
* Remove all files within .git/NOTES_MERGE_WORKTREE. We do not remove
762+
* the .git/NOTES_MERGE_WORKTREE directory itself, since it might be
763+
* the current working directory of the user.
764+
*/
750765
struct strbuf buf = STRBUF_INIT;
751766
int ret;
752767

753768
strbuf_addstr(&buf, git_path(NOTES_MERGE_WORKTREE));
754769
if (o->verbosity >= 3)
755-
printf("Removing notes merge worktree at %s\n", buf.buf);
756-
ret = remove_dir_recursively(&buf, 0);
770+
printf("Removing notes merge worktree at %s/*\n", buf.buf);
771+
ret = remove_dir_recursively(&buf, REMOVE_DIR_KEEP_TOPLEVEL);
757772
strbuf_release(&buf);
758773
return ret;
759774
}

‎t/t3310-notes-merge-manual-resolve.sh‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ y and z notes on 4th commit
324324
EOF
325325
git notes merge --commit &&
326326
# No .git/NOTES_MERGE_* files left
327-
test_must_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&
327+
test_might_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&
328328
test_cmp /dev/null output &&
329329
# Merge commit has pre-merge y and pre-merge z as parents
330330
test "$(git rev-parse refs/notes/m^1)" = "$(cat pre_merge_y)" &&
@@ -386,7 +386,7 @@ test_expect_success 'redo merge of z into m (== y) with default ("manual") resol
386386
test_expect_success 'abort notes merge' '
387387
git notes merge --abort &&
388388
# No .git/NOTES_MERGE_* files left
389-
test_must_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&
389+
test_might_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&
390390
test_cmp /dev/null output &&
391391
# m has not moved (still == y)
392392
test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)" &&
@@ -453,7 +453,7 @@ EOF
453453
# Finalize merge
454454
git notes merge --commit &&
455455
# No .git/NOTES_MERGE_* files left
456-
test_must_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&
456+
test_might_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&
457457
test_cmp /dev/null output &&
458458
# Merge commit has pre-merge y and pre-merge z as parents
459459
test "$(git rev-parse refs/notes/m^1)" = "$(cat pre_merge_y)" &&
@@ -542,7 +542,7 @@ EOF
542542
test_expect_success 'resolve situation by aborting the notes merge' '
543543
git notes merge --abort &&
544544
# No .git/NOTES_MERGE_* files left
545-
test_must_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&
545+
test_might_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&
546546
test_cmp /dev/null output &&
547547
# m has not moved (still == w)
548548
test "$(git rev-parse refs/notes/m)" = "$(git rev-parse refs/notes/w)" &&
@@ -553,4 +553,23 @@ test_expect_success 'resolve situation by aborting the notes merge' '
553553
verify_notes z
554554
'
555555

556+
cat >expect_notes <<EOF
557+
foo
558+
bar
559+
EOF
560+
561+
test_expect_success 'switch cwd before committing notes merge' '
562+
git notes add -m foo HEAD &&
563+
git notes --ref=other add -m bar HEAD &&
564+
test_must_fail git notes merge refs/notes/other &&
565+
(
566+
cd .git/NOTES_MERGE_WORKTREE &&
567+
echo "foo" > $(git rev-parse HEAD) &&
568+
echo "bar" >> $(git rev-parse HEAD) &&
569+
git notes merge --commit
570+
) &&
571+
git notes show HEAD > actual_notes &&
572+
test_cmp expect_notes actual_notes
573+
'
574+
556575
test_done

0 commit comments

Comments
 (0)