Quantcast

pulling the root commit overwrites untracked files without warning (1.7.2.3)

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

pulling the root commit overwrites untracked files without warning (1.7.2.3)

Gert Palok-2
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: pulling the root commit overwrites untracked files without warning (1.7.2.3)

Clemens Buchacher
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@...

I hope to finish it this weekend.

Clemens

signature.asc (501 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: pulling the root commit overwrites untracked files without warning (1.7.2.3)

Clemens Buchacher
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@...

Actually, it might not. But since it's related, I'll have a look at
it.

signature.asc (501 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: pulling the root commit overwrites untracked files without warning (1.7.2.3)

Clemens Buchacher
In reply to this post by Gert Palok-2
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

signature.asc (501 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH] do not overwrite untracked during merge from unborn branch

Clemens Buchacher
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 <[hidden email]>
---

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


signature.asc (501 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH w/o PGP] do not overwrite untracked during merge from unborn branch

Clemens Buchacher
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 <[hidden email]>
---

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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH v2] do not overwrite untracked during merge from unborn branch

Clemens Buchacher
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 <[hidden email]>
---

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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH v3] do not overwrite untracked during merge from unborn branch

Clemens Buchacher
In reply to this post by Clemens Buchacher
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 <[hidden email]>
---

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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH v2] do not overwrite untracked during merge from unborn branch

Miles Bader-2
In reply to this post by Clemens Buchacher
Clemens Buchacher <[hidden email]> 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_)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Loading...