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

From: Alexander Riesen <alexander.riesen@cetitec.com>
Date: Oct 22, 2015, 13:59 UTC
Message-ID: <5628EBAF.1030205@cetitec.com>
In-Reply-To: <xmqqpp085cth.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/21/2015 07:52 PM, Junio C Hamano wrote:

> The readers of this change need  to be enlightened with a log message

 > to remind them that "--reference" is not the only way. Namely, if
 > you start from a repository with $GIT_DIR/objects/info/alternates,
 > i.e. the original already borrows from somewhere, and bypass the
 > normal "Git aware" transport mechanism, i.e. "git clone --local",
 > then the resulting repository would also become dependent of the
 > object store that the original depended on before the clone. In
 > order to make it free-standing, you would need "--dissociate", but
 > there is no "--reference" involved in that use case.
 >
 > And once that is clarified, it becomes very clear why it is wrong to
 > blindly require "--reference" to be there on the command line when
 > "--dissociate" is given.

Indeed. Log message improved.

> As to the patch, I think this  one is much simpler and preferrable.

 > It would hurt those who make a clone without bypassing the normal
 > "Git aware" transport mechanism and pass "--dissociate" without
 > "--reference".  They will end up making a clone that does not need
 > repacking to dissociate, but with this patch they would spend extra
 > cycles to run an unnecessary repack.  To avoid that, I think you can
 > throw in an check at the beginning of dissociate_from_references()
 > to see if git_path("objects/info/alternates") is there and make the
 > function a no-op if there isn't.

I think I understand. How about this?

  builtin/clone.c | 10 +++++-----
  1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 9eaecd9..a7d0c07 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -801,11 +801,15 @@ 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)
+        return;
      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"));
+    free(alts);
  }

  int cmd_clone(int argc, const char **argv, const char *prefix)
@@ -954,10 +958,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);

-- 
2.6.1.151.ge74ab91