Re: [PATCH] Consider object stores in alternates during a dissociating clone

From: Alexander Riesen <alexander.riesen@cetitec.com>
Date: Oct 22, 2015, 16:41 UTC
Message-ID: <562911AD.50004@cetitec.com>
In-Reply-To: <xmqq7fmeubkf.fsf@gitster.mtv.corp.google.com>

From: Alex Riesen <raa.lkml@gmail.com>

The "--reference" option is not the only way to provide a repository to borrow objects from. For instance, the objects/info/alternates of the origin repository lists object stores which the origin repository borrowed objects from. During clone operations which bypass a git aware transport (i.e. simply copy the things over, like git clone --local) the file is copied into the cloned repository. In such a case, even if there were no reference repositories given in the command-line, there might be still something to "dissociate" the cloned repository from, before it is really independent.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
On 10/22/2015 06:12 PM, Junio C Hamano wrote:

> Alexander Riesen  <alexander.riesen@cetitec.com> writes:

 >> +    if (access(alts, F_OK) < 0)
 >> +        return;
 >
 > You leak alts here.

Fixed.

> Perhaps you would want a new  test somewhere that (1) prepares the

 > ultimate source, (2) prepares a borrowing source with "clone
 > --reference" from the previous, (3) creates a local clone of the
 > previous with "clone --local" without "--reference" but with
 > "--dissociate", and (4) checks the end result by ensuring the
 > absense of $GIT_DIR/objects/info/alternates and runs "fsck" on it.

Added. t5700-clone-reference seemed like a logical place for it.

Regards, Alex

---
  builtin/clone.c            | 11 ++++++-----
  t/t5700-clone-reference.sh | 11 +++++++++++
  2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 9eaecd9..1e14810 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -801,11 +801,16 @@ static void write_refspec_config(const char *src_ref_prefix,
  static void dissociate_from_references(void)
  {
      static const char* argv[] = { "repack", "-a", "-d", NULL };
+    char *alts = git_pathdup("objects/info/alternates");

+    if (access(alts, F_OK) < 0)
+        goto done;
      if (run_command_v_opt(argv, RUN_GIT_CMD|RUN_COMMAND_NO_STDIN))
          die(_("cannot repack to clean up"));
-    if (unlink(git_path("objects/info/alternates")) && errno != ENOENT)
+    if (unlink(alts) && errno != ENOENT)
          die_errno(_("cannot unlink temporary alternates file"));
+done:
+    free(alts);
  }

  int cmd_clone(int argc, const char **argv, const char *prefix)
@@ -954,10 +959,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)

      if (option_reference.nr)
          setup_reference();
-    else if (option_dissociate) {
-        warning(_("--dissociate given, but there is no --reference"));
-        option_dissociate = 0;
-    }

      fetch_pattern = value.buf;
      refspec = parse_fetch_refspec(1, &fetch_pattern);
diff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh
index 2250ef4..dfa1bf7 100755
--- a/t/t5700-clone-reference.sh
+++ b/t/t5700-clone-reference.sh
@@ -210,4 +210,15 @@ test_expect_success 'clone, dissociate from partial 
reference and repack' '
      test_line_count = 1 packs.txt
  '

+test_expect_success 'clone, dissociate from alternates' '
+    rm -fr A B C &&
+    test_create_repo A &&
+    commit_in A file1 &&
+    git clone --reference=A A B &&
+    test_line_count = 1 B/.git/objects/info/alternates &&
+    git clone --local --dissociate B C &&
+    ! test -f C/.git/objects/info/alternates &&
+    ( cd C && git fsck )
+'
+
  test_done

-- 
2.6.2.313.gdf7a1dc