# pulling the root commit overwrites untracked files without warning (1.7.2.3) 9 messages from 2010-10-21 to 2010-11-15. Participants: Gert Palok, Clemens Buchacher, Miles Bader. Thread: https://gitlist.dev/t/25496 ## Gert Palok, 2010-10-21 13:18 Subject: pulling the root commit overwrites untracked files without warning (1.7.2.3) Message-ID: URL: https://gitlist.dev/e/AANLkTimDnyzyV1FEEwEuxLfG1nSOcNa7Hzwt7DDssjba%40mail.gmail.com ``` Hi On 1.7.2.3, pulling the root commit overwrites untracked files Minimal working example: $ git init wc1 $ cd wc1 $ echo a > a $ git add a $ git commit -m "added a" $ mkdir ../wc2 $ cd ../wc2 $ echo b > a $ git init $ git remote add origin ../wc1 $ git pull origin master $ # expected: error: Untracked working tree file 'a' would be overwritten by merge. Aborting $ # actual: no error $ cat a $ # expected: b $ # actual: a An excerpt from the session: gert@planc ~/test/wc2 git[master?] 768$ git pull origin master remote: Counting objects: 3, done. remote: Total 3 (delta 0), reused 0 (delta 0) Unpacking objects: 100% (3/3), done. From ../wc1 * branch master -> FETCH_HEAD gert@planc ~/test/wc2 git[master] 769$ cat a a -- Gert Palok ``` ## Clemens Buchacher, 2010-10-22 21:14 Subject: Re: pulling the root commit overwrites untracked files without warning (1.7.2.3) Message-ID: <20101022211400.GA1774@localhost> URL: https://gitlist.dev/e/20101022211400.GA1774%40localhost In-Reply-To: ``` Hi, On Thu, Oct 21, 2010 at 04:18:19PM +0300, Gert Palok wrote: > > On 1.7.2.3, pulling the root commit overwrites untracked files Thanks. This is probably fixed by the following series. http://mid.gmane.org/1286632380-7002-1-git-send-email-drizzd@aon.at I hope to finish it this weekend. Clemens ``` ## Clemens Buchacher, 2010-10-22 21:17 Subject: Re: pulling the root commit overwrites untracked files without warning (1.7.2.3) Message-ID: <20101022211709.GB1774@localhost> URL: https://gitlist.dev/e/20101022211709.GB1774%40localhost In-Reply-To: <20101022211400.GA1774@localhost> ``` On Fri, Oct 22, 2010 at 11:14:00PM +0200, Clemens Buchacher wrote: > Hi, > > On Thu, Oct 21, 2010 at 04:18:19PM +0300, Gert Palok wrote: > > > > On 1.7.2.3, pulling the root commit overwrites untracked files > > Thanks. This is probably fixed by the following series. > > http://mid.gmane.org/1286632380-7002-1-git-send-email-drizzd@aon.at Actually, it might not. But since it's related, I'll have a look at it. ``` ## Clemens Buchacher, 2010-11-14 21:34 Subject: Re: pulling the root commit overwrites untracked files without warning (1.7.2.3) Message-ID: <20101114213453.GA29287@localhost> URL: https://gitlist.dev/e/20101114213453.GA29287%40localhost In-Reply-To: ``` Hi, On Thu, Oct 21, 2010 at 04:18:19PM +0300, Gert Palok wrote: > > On 1.7.2.3, pulling the root commit overwrites untracked files > > Minimal working example: > $ git init wc1 > $ cd wc1 > $ echo a > a > $ git add a > $ git commit -m "added a" > $ mkdir ../wc2 > $ cd ../wc2 > $ echo b > a > $ git init > $ git remote add origin ../wc1 > $ git pull origin master > $ # expected: error: Untracked working tree file 'a' would be > overwritten by merge. Aborting > $ # actual: no error > $ cat a > $ # expected: b > $ # actual: a I finally got a chance to look at this. Contrary to my initial suspicion, it's not a problem with the merge algorithm at all. It doesn't even do a merge. Instead, git merge finds that it has no valid HEAD and therefore does a hard reset, which obviously overwrites any files already there. Patch to follow. Clemens ``` ## Clemens Buchacher, 2010-11-14 21:46 Subject: [PATCH] do not overwrite untracked during merge from unborn branch Message-ID: <20101114214601.GB29287@localhost> URL: https://gitlist.dev/e/20101114214601.GB29287%40localhost In-Reply-To: <20101114213453.GA29287@localhost> ``` In case HEAD does not point to a valid commit yet, merge is implemented as a hard reset. This will cause untracked files to be overwritten. Instead, assume the empty tree for HEAD and do a regular merge. An untracked file will cause the merge to abort and do nothing. If no conflicting files are present, the merge will have the same effect as a hard reset. Signed-off-by: Clemens Buchacher --- This is based on next and does not apply cleanly to master because of t7607. I can rebase it to master, but then it will conflict with next later. Clemens builtin/merge.c | 20 +++++++++++++++++++- t/t7607-merge-overwrite.sh | 16 ++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletions(-) diff --git a/builtin/merge.c b/builtin/merge.c index 10f091b..7571c93 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -234,6 +234,24 @@ static void save_state(void) die("not a valid object: %s", buffer.buf); } +static void read_empty(unsigned const char *sha1, int verbose) +{ + int i = 0; + const char *args[7]; + + args[i++] = "read-tree"; + if (verbose) + args[i++] = "-v"; + args[i++] = "-m"; + args[i++] = "-u"; + args[i++] = EMPTY_TREE_SHA1_HEX; + args[i++] = sha1_to_hex(sha1); + args[i] = NULL; + + if (run_command_v_opt(args, RUN_GIT_CMD)) + die("read-tree failed"); +} + static void reset_hard(unsigned const char *sha1, int verbose) { int i = 0; @@ -985,7 +1003,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix) die("%s - not something we can merge", argv[0]); update_ref("initial pull", "HEAD", remote_head->sha1, NULL, 0, DIE_ON_ERR); - reset_hard(remote_head->sha1, 0); + read_empty(remote_head->sha1, 0); return 0; } else { struct strbuf merge_names = STRBUF_INIT; diff --git a/t/t7607-merge-overwrite.sh b/t/t7607-merge-overwrite.sh index e49dd80..d4a499d 100755 --- a/t/t7607-merge-overwrite.sh +++ b/t/t7607-merge-overwrite.sh @@ -127,4 +127,20 @@ test_expect_success SYMLINKS 'will not be confused by symlink in leading path' ' git checkout sub ' +cat >expect <<\EOF +error: Untracked working tree file 'c0.c' would be overwritten by merge. +fatal: read-tree failed +EOF + +test_expect_success 'will not overwrite untracked file on unborn branch' ' + git reset --hard c0 && + git rm -fr . && + git checkout --orphan new && + cp important c0.c && + test_must_fail git merge c0 2>out && + test_cmp out expect && + test_path_is_missing .git/MERGE_HEAD && + test_cmp important c0.c +' + test_done -- 1.7.3.1.105.g84915 ``` ## Clemens Buchacher, 2010-11-14 21:49 Subject: [PATCH w/o PGP] do not overwrite untracked during merge from unborn branch Message-ID: <20101114214927.GA29579@localhost> URL: https://gitlist.dev/e/20101114214927.GA29579%40localhost In-Reply-To: <20101114214601.GB29287@localhost> ``` In case HEAD does not point to a valid commit yet, merge is implemented as a hard reset. This will cause untracked files to be overwritten. Instead, assume the empty tree for HEAD and do a regular merge. An untracked file will cause the merge to abort and do nothing. If no conflicting files are present, the merge will have the same effect as a hard reset. Signed-off-by: Clemens Buchacher --- On Sun, Nov 14, 2010 at 10:46:02PM +0100, Clemens Buchacher wrote: > --MfFXiAuoTsnnDAfZ > Content-Type: text/plain; charset=us-ascii > Content-Disposition: inline > Content-Transfer-Encoding: quoted-printable > > This is based on next and does not apply cleanly to master because > of t7607. I can rebase it to master, but then it will conflict with > next later. Without signature this time. Oops. Clemens builtin/merge.c | 20 +++++++++++++++++++- t/t7607-merge-overwrite.sh | 16 ++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletions(-) diff --git a/builtin/merge.c b/builtin/merge.c index 10f091b..7571c93 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -234,6 +234,24 @@ static void save_state(void) die("not a valid object: %s", buffer.buf); } +static void read_empty(unsigned const char *sha1, int verbose) +{ + int i = 0; + const char *args[6]; + + args[i++] = "read-tree"; + if (verbose) + args[i++] = "-v"; + args[i++] = "-m"; + args[i++] = "-u"; + args[i++] = EMPTY_TREE_SHA1_HEX; + args[i++] = sha1_to_hex(sha1); + args[i] = NULL; + + if (run_command_v_opt(args, RUN_GIT_CMD)) + die("read-tree failed"); +} + static void reset_hard(unsigned const char *sha1, int verbose) { int i = 0; @@ -985,7 +1003,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix) die("%s - not something we can merge", argv[0]); update_ref("initial pull", "HEAD", remote_head->sha1, NULL, 0, DIE_ON_ERR); - reset_hard(remote_head->sha1, 0); + read_empty(remote_head->sha1, 0); return 0; } else { struct strbuf merge_names = STRBUF_INIT; diff --git a/t/t7607-merge-overwrite.sh b/t/t7607-merge-overwrite.sh index e49dd80..d4a499d 100755 --- a/t/t7607-merge-overwrite.sh +++ b/t/t7607-merge-overwrite.sh @@ -127,4 +127,20 @@ test_expect_success SYMLINKS 'will not be confused by symlink in leading path' ' git checkout sub ' +cat >expect <<\EOF +error: Untracked working tree file 'c0.c' would be overwritten by merge. +fatal: read-tree failed +EOF + +test_expect_success 'will not overwrite untracked file on unborn branch' ' + git reset --hard c0 && + git rm -fr . && + git checkout --orphan new && + cp important c0.c && + test_must_fail git merge c0 2>out && + test_cmp out expect && + test_path_is_missing .git/MERGE_HEAD && + test_cmp important c0.c +' + test_done -- 1.7.3.1.105.g84915 ``` ## Clemens Buchacher, 2010-11-14 21:53 Subject: [PATCH v2] do not overwrite untracked during merge from unborn branch Message-ID: <20101114215315.GB29579@localhost> URL: https://gitlist.dev/e/20101114215315.GB29579%40localhost In-Reply-To: <20101114214927.GA29579@localhost> ``` In case HEAD does not point to a valid commit yet, merge is implemented as a hard reset. This will cause untracked files to be overwritten. Instead, assume the empty tree for HEAD and do a regular merge. An untracked file will cause the merge to abort and do nothing. If no conflicting files are present, the merge will have the same effect as a hard reset. Signed-off-by: Clemens Buchacher --- On Sun, Nov 14, 2010 at 10:49:27PM +0100, Clemens Buchacher wrote: > On Sun, Nov 14, 2010 at 10:46:02PM +0100, Clemens Buchacher wrote: > > --MfFXiAuoTsnnDAfZ > > Content-Type: text/plain; charset=us-ascii > > Content-Disposition: inline > > Content-Transfer-Encoding: quoted-printable > > > > This is based on next and does not apply cleanly to master because > > of t7607. I can rebase it to master, but then it will conflict with > > next later. > > Without signature this time. Oops. Wrong again! (With args[6] instead of args[7].) I wonder how many times I can get it wrong. Clemens builtin/merge.c | 20 +++++++++++++++++++- t/t7607-merge-overwrite.sh | 16 ++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletions(-) diff --git a/builtin/merge.c b/builtin/merge.c index 10f091b..613543e 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -234,6 +234,24 @@ static void save_state(void) die("not a valid object: %s", buffer.buf); } +static void read_empty(unsigned const char *sha1, int verbose) +{ + int i = 0; + const char *args[7]; + + args[i++] = "read-tree"; + if (verbose) + args[i++] = "-v"; + args[i++] = "-m"; + args[i++] = "-u"; + args[i++] = EMPTY_TREE_SHA1_HEX; + args[i++] = sha1_to_hex(sha1); + args[i] = NULL; + + if (run_command_v_opt(args, RUN_GIT_CMD)) + die("read-tree failed"); +} + static void reset_hard(unsigned const char *sha1, int verbose) { int i = 0; @@ -985,7 +1003,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix) die("%s - not something we can merge", argv[0]); update_ref("initial pull", "HEAD", remote_head->sha1, NULL, 0, DIE_ON_ERR); - reset_hard(remote_head->sha1, 0); + read_empty(remote_head->sha1, 0); return 0; } else { struct strbuf merge_names = STRBUF_INIT; diff --git a/t/t7607-merge-overwrite.sh b/t/t7607-merge-overwrite.sh index e49dd80..d4a499d 100755 --- a/t/t7607-merge-overwrite.sh +++ b/t/t7607-merge-overwrite.sh @@ -127,4 +127,20 @@ test_expect_success SYMLINKS 'will not be confused by symlink in leading path' ' git checkout sub ' +cat >expect <<\EOF +error: Untracked working tree file 'c0.c' would be overwritten by merge. +fatal: read-tree failed +EOF + +test_expect_success 'will not overwrite untracked file on unborn branch' ' + git reset --hard c0 && + git rm -fr . && + git checkout --orphan new && + cp important c0.c && + test_must_fail git merge c0 2>out && + test_cmp out expect && + test_path_is_missing .git/MERGE_HEAD && + test_cmp important c0.c +' + test_done -- 1.7.3.1.105.g84915 ``` ## Clemens Buchacher, 2010-11-14 22:07 Subject: [PATCH v3] do not overwrite untracked during merge from unborn branch Message-ID: <20101114220749.GA29786@localhost> URL: https://gitlist.dev/e/20101114220749.GA29786%40localhost In-Reply-To: <20101114214953.GB16413@burratino> ``` In case HEAD does not point to a valid commit yet, merge is implemented as a hard reset. This will cause untracked files to be overwritten. Instead, assume the empty tree for HEAD and do a regular merge. An untracked file will cause the merge to abort and do nothing. If no conflicting files are present, the merge will have the same effect as a hard reset. Signed-off-by: Clemens Buchacher --- On Sun, Nov 14, 2010 at 03:49:53PM -0600, Jonathan Nieder wrote: > > Naturally, this is the sort of patch that would want to be > fast-tracked to master (or even maint) if it is in good shape. Ok, no problem. This applies to master and maint. Clemens builtin/merge.c | 20 +++++++++++++++++++- t/t7607-merge-overwrite.sh | 16 ++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletions(-) diff --git a/builtin/merge.c b/builtin/merge.c index 10f091b..613543e 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -234,6 +234,24 @@ static void save_state(void) die("not a valid object: %s", buffer.buf); } +static void read_empty(unsigned const char *sha1, int verbose) +{ + int i = 0; + const char *args[7]; + + args[i++] = "read-tree"; + if (verbose) + args[i++] = "-v"; + args[i++] = "-m"; + args[i++] = "-u"; + args[i++] = EMPTY_TREE_SHA1_HEX; + args[i++] = sha1_to_hex(sha1); + args[i] = NULL; + + if (run_command_v_opt(args, RUN_GIT_CMD)) + die("read-tree failed"); +} + static void reset_hard(unsigned const char *sha1, int verbose) { int i = 0; @@ -985,7 +1003,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix) die("%s - not something we can merge", argv[0]); update_ref("initial pull", "HEAD", remote_head->sha1, NULL, 0, DIE_ON_ERR); - reset_hard(remote_head->sha1, 0); + read_empty(remote_head->sha1, 0); return 0; } else { struct strbuf merge_names = STRBUF_INIT; diff --git a/t/t7607-merge-overwrite.sh b/t/t7607-merge-overwrite.sh index d82349a..3988900 100755 --- a/t/t7607-merge-overwrite.sh +++ b/t/t7607-merge-overwrite.sh @@ -84,4 +84,20 @@ test_expect_success 'will not overwrite removed file with staged changes' ' test_cmp important c1.c ' +cat >expect <<\EOF +error: Untracked working tree file 'c0.c' would be overwritten by merge. +fatal: read-tree failed +EOF + +test_expect_success 'will not overwrite untracked file on unborn branch' ' + git reset --hard c0 && + git rm -fr . && + git checkout --orphan new && + cp important c0.c && + test_must_fail git merge c0 2>out && + test_cmp out expect && + test_path_is_missing .git/MERGE_HEAD && + test_cmp important c0.c +' + test_done -- 1.7.3.1.105.g84915 ``` ## Miles Bader, 2010-11-15 01:51 Subject: Re: [PATCH v2] do not overwrite untracked during merge from unborn branch Message-ID: <87r5enbbhx.fsf@catnip.gol.com> URL: https://gitlist.dev/e/87r5enbbhx.fsf%40catnip.gol.com In-Reply-To: <20101114215315.GB29579@localhost> ``` Clemens Buchacher writes: > Wrong again! (With args[6] instead of args[7].) I wonder how many > times I can get it wrong. Hmm, well the use of external processes to implement such seemingly basic functionality doesn't exactly help with the argument checking... ... and now that I look (though only briefly :), builtin/read-tree.c seems almost trivial -- with the bulk of the code devoted to argument parsing, error reporting, and handling different cases...! Maybe it would be better to just rewrite "read_empty" (and "reset_hard"?) in terms of the underlying functions read-tree.c uses? -Miles -- `Cars give people wonderful freedom and increase their opportunities. But they also destroy the environment, to an extent so drastic that they kill all social life' (from _A Pattern Language_) ```