Re: [PATCH] fetch-pack: do not remove .git/shallow file when --depth is not specified

2013-08-26 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy  pclo...@gmail.com writes:

   The two possible fixes which I see are:
  
   1) Replace back if (alternate_shallow_file) condition in fetch pack with
  if (args-depth  0)
  
   2) alternate_shallow_file should be copy of shallow_lock.filename not a
  reference to it

  3) Move alternate_shallow_file to struct fetch_pack_args, which will
 always be zero'd by memset

  I think #1 is better. It's the original condition before 6035d6a
  replaces it with if (alternate_shallow_file). Apparently I did not
  see that fetch_pack() could be called twice. #3 is also an option,
  but we still need static shallow_lock anyway, so I disregarded it.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] fetch-pack: do not remove .git/shallow file when --depth is not specified

2013-08-25 Thread Nguyễn Thái Ngọc Duy
fetch_pack() can remove .git/shallow file when a shallow repository
becomes a full one again. This behavior is triggered incorrectly when
tags are also fetched because fetch_pack() will be called twice. At
the first fetch_pack() call:

 - shallow_lock is set up
 - alternate_shallow_file points to shallow_lock.filename, which is
   shallow.lock
 - commit_lock_file is called, which sets shallow_lock.filename to .
   alternate_shallow_file also becomes  because it points to the
   same memory.

At the second call, setup_alternate_shallow() is not called and
alternate_shallow_file remains . It's mistaken as unshallow case and
.git/shallow is removed. The end result is a broken repository.

Fix this by always initializing alternate_shallow_file when
fetch_pack() is called. As an extra measure, check if args-depth  0
before commit/rollback shallow file.

Reported-by: Kacper Kornet kor...@camk.edu.pl
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
  The two possible fixes which I see are:
 
  1) Replace back if (alternate_shallow_file) condition in fetch pack with
     if (args-depth  0)
 
  2) alternate_shallow_file should be copy of shallow_lock.filename not a
     reference to it

 3) Move alternate_shallow_file to struct fetch_pack_args, which will
always be zero'd by memset

 I think #1 is better. It's the original condition before 6035d6a
 replaces it with if (alternate_shallow_file). Apparently I did not
 see that fetch_pack() could be called twice. #3 is also an option,
 but we still need static shallow_lock anyway, so I disregarded it.

 fetch-pack.c  |  4 +++-
 t/t5500-fetch-pack.sh | 16 
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/fetch-pack.c b/fetch-pack.c
index 6b5467c..76190a8 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -888,6 +888,8 @@ static struct ref *do_fetch_pack(struct fetch_pack_args 
*args,
packet_flush(fd[1]);
if (args-depth  0)
setup_alternate_shallow();
+   else
+   alternate_shallow_file = NULL;
if (get_pack(args, fd, pack_lockfile))
die(git fetch-pack: fetch failed.);
 
@@ -978,7 +980,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
}
ref_cpy = do_fetch_pack(args, fd, ref, sought, nr_sought, 
pack_lockfile);
 
-   if (alternate_shallow_file) {
+   if (args-depth  0  alternate_shallow_file) {
if (*alternate_shallow_file == '\0') { /* --unshallow */
unlink_or_warn(git_path(shallow));
rollback_lock_file(shallow_lock);
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index fd2598e..a80584e 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -505,4 +505,20 @@ test_expect_success 'test --all, --depth, and explicit 
tag' '
) out-adt 2error-adt
 '
 
+test_expect_success 'shallow fetch with tags does not break the repository' '
+   mkdir repo1 
+   (
+   cd repo1 
+   git init 
+   test_commit 1 
+   test_commit 2 
+   test_commit 3 
+   mkdir repo2 
+   cd repo2 
+   git init 
+   git fetch --depth=2 ../.git master:branch 
+   git fsck
+   )
+'
+
 test_done
-- 
1.8.2.82.gc24b958

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html