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

[Junio C Hamano]

Alexander Riesen  writes:

> Incidentally, what does "---" mean in the documentation hunk?

Heh, good eyes.  I was hoping that it would turn into an em-dash,
but it seems I just get three dashes instead.
--
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

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

[Junio C Hamano]

Junio C Hamano  writes:

> Alexander Riesen  writes:
>
>> Incidentally, what does "---" mean in the documentation hunk?
>
> Heh, good eyes.  I was hoping that it would turn into an em-dash,
> but it seems I just get three dashes instead.

Outside relnotes, there seem to be a handful instances of those
that expresses em-dash with "---".

$ git grep -E '[A-Za-z]{2,}---[A-Za-z]{2,}' Documentation/
git-bisect.txt:command not found, 126 is for command found but not 
executable---these
git-clone.txt:  repository---the new repository will borrow objects from the
git-fetch.txt:  the refspecs---they specify which refs to fetch and which local 
refs
git-push.txt:be omitted---such a push will update a ref that `` normally 
updates
technical/index-format.txt:  first subtree---let's call this A---of the root 
level...

I'll leave it for a separate topic to clean these up later.
--
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

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

[Junio C Hamano]

Alexander Riesen  writes:

>> Content-Type: text/plain; charset=windows-1252; format=flowed

I had to hand-munge it as the above lost all tabs and made the patch
unusable for machines X-<.

Re-reading the documentation, I realized that the use case this new
mode of operation allows is totally outside of the original design
space that was described, so I added a note to teach users how the
option can be used in a new way as well.

So here is what I tentatively queued.

Thanks.

-- >8 --
From: Alex Riesen 
Date: Thu, 22 Oct 2015 18:41:17 +0200
Subject: [PATCH] clone: allow "--dissociate" without reference

The "--reference" option is not the only way to provide a repository
to borrow objects from.  A repository that borrows from another
repository can be cloned with "clone --local" and the resulting
repository will borrow from the same repository, which the user
may want to "--dissociate" from.

Signed-off-by: Alex Riesen 
Signed-off-by: Junio C Hamano 
---
 Documentation/git-clone.txt |  9 +++--
 builtin/clone.c | 16 
 t/t5700-clone-reference.sh  | 11 +++
 3 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index f1f2a3f..a8c11e3 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -104,8 +104,13 @@ objects from the source repository into a pack in the 
cloned repository.
 --dissociate::
Borrow the objects from reference repositories specified
with the `--reference` options only to reduce network
-   transfer and stop borrowing from them after a clone is made
-   by making necessary local copies of borrowed objects.
+   transfer, and stop borrowing from them after a clone is made
+   by making necessary local copies of borrowed objects.  This
+   option can also be used when cloning locally from a
+   repository that already borrows objects from another
+   repository---the new repository will borrow objects from the
+   same repository, and this option can be used to stop the
+   borrowing.
 
 --quiet::
 -q::
diff --git a/builtin/clone.c b/builtin/clone.c
index 9eaecd9..caae43e 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 *alternates = git_pathdup("objects/info/alternates");
 
-   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)
-   die_errno(_("cannot unlink temporary alternates file"));
+   if (!access(alternates, F_OK)) {
+   if (run_command_v_opt(argv, RUN_GIT_CMD|RUN_COMMAND_NO_STDIN))
+   die(_("cannot repack to clean up"));
+   if (unlink(alternates) && errno != ENOENT)
+   die_errno(_("cannot unlink temporary alternates file"));
+   }
+   free(alternates);
 }
 
 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, _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-383-g4ea3cbc

--
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

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

[Alexander Riesen]

On 10/22/2015 07:50 PM, Junio C Hamano wrote:

Alexander Riesen  writes:


Content-Type: text/plain; charset=windows-1252; format=flowed

I had to hand-munge it as the above lost all tabs and made the patch
unusable for machines X-<.

I'm very sorry. I don't know why Icedove does that, nor do
I know how to stop it mangling the text. Right now I just
hate the damn thing.

Thank you very much for reformatting the patch, as it would
take quite some time until I configure a sane mail program
to work here.

Incidentally, what does "---" mean in the documentation hunk?

Regards,
Alex


--
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

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

[Johannes Löthberg]

On 22/10, Alexander Riesen wrote:

On 10/22/2015 07:50 PM, Junio C Hamano wrote:

Alexander Riesen  writes:


Content-Type: text/plain; charset=windows-1252; format=flowed

I had to hand-munge it as the above lost all tabs and made the patch
unusable for machines X-<.

I'm very sorry. I don't know why Icedove does that, nor do
I know how to stop it mangling the text. Right now I just
hate the damn thing.

Thank you very much for reformatting the patch, as it would
take quite some time until I configure a sane mail program
to work here.



Tip: Just use git-send-email instead. There's even an example 
configuration for gmail in the manpage.


--
Sincerely,
 Johannes Löthberg
 PGP Key ID: 0x50FB9B273A9D0BB5
 https://theos.kyriasis.com/~kyrias/


signature.asc
Description: PGP signature

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

[Alexander Riesen]

From: Alex Riesen 

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 
---
On 10/22/2015 06:12 PM, Junio C Hamano wrote:

Alexander Riesen   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, _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


--
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

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

[Junio C Hamano]

Alexander Riesen  writes:

> 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;

You leak alts here.

>  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, _pattern);

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.

Other than these two points, the patch looks good to me.
--
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