Git development
 help / color / mirror / Atom feed
* Re: [PATCH v2 3/4] fast-export: don't handle uninteresting refs
From: Johannes Schindelin @ 2012-10-30 19:41 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Jonathan Nieder, Jeff King, Junio C Hamano, Sverre Rabbelier,
	Elijah Newren, git
In-Reply-To: <CAMP44s1W4mwK+cNwBqu2S0=Aw04XX9KBan8w4ghyzqbODdmiLQ@mail.gmail.com>

Hi Felipe,

On Tue, 30 Oct 2012, Felipe Contreras wrote:

> On Tue, Oct 30, 2012 at 8:01 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> > Felipe Contreras wrote:
> >> On Tue, Oct 30, 2012 at 7:47 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> >
> >>> and in the examples listed in the patch
> >>> description the changed behavior does not look like an improvement.
> >>
> >> I disagree.
> >>
> >> % git log master ^master
> >>
> >> What do you expect? Nothing.
> >
> > Yep.
> >
> >> % git fast-export master ^master
> >>
> >> What do you expect? Nothing.
> >
> > Nope.
> 
> That's _your_ opinion. I would like to see what others think.

If you wanted to prove that you can work with others without offending
them, I think that failed.

Ciao,
Johannes

^ permalink raw reply

* Re: [PATCH v4 00/13] New remote-hg helper
From: Johannes Schindelin @ 2012-10-30 19:33 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Jeff King, git, Junio C Hamano, Sverre Rabbelier, Ilari Liusvaara,
	Daniel Barkalow, Michael J Gruber
In-Reply-To: <CAMP44s3CEGqUav-ijnzm7osD70LsjRLyOEeV3bF-LWYTCEPCSQ@mail.gmail.com>

Hi Felipe,

On Tue, 30 Oct 2012, Felipe Contreras wrote:

> But you mentioned something about cooperation, and I've yet to see how
> is it that you are planning to cooperate. If you say you don't have time
> to spend on this, I don't see why I should worry about testing this
> series of patches.

It has been mentioned before that the communication style including all
these snarky and nasty comments is not helpful. It is hardly the first
time that your mails have been insulting, as can be researched easily from
in the public mailing list archives.

In light of the indignation when advised to keep the tone down a little,
it is probable that the mails were never put through the "would I be
insulted or hurt if I was the recipient of this?" test, as in "do you want
me to throw away my work?" when you literally asked us to throw away our
work.

So unlike others, I do not ask you to change your tone, nor your
willingness to work with others. Instead, I prefer to do other things
instead.

Hth,
Johannes

^ permalink raw reply

* Re: [PATCH v2 3/4] fast-export: don't handle uninteresting refs
From: Felipe Contreras @ 2012-10-30 19:17 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: git, Jeff King, Junio C Hamano, Sverre Rabbelier,
	Johannes Schindelin, Elijah Newren
In-Reply-To: <20121030185914.GI15167@elie.Belkin>

(again to the mailing list)

On Tue, Oct 30, 2012 at 7:59 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Felipe Contreras wrote:
>
>> They have been marked as UNINTERESTING for a reason, lets respect that.

That doesn't say anything.

> and in the examples listed in the patch
> description the changed behavior does not look like an improvement.

I disagree.

% git log master ^master

What do you expect? Nothing.

% git fast-export master ^master

What do you expect? Nothing.

> Worse, the description lists a few examples but gives no convincing
> explanation to reassure about the lack of bad behavior for examples
> not listed.

What examples not listed?

-- 
Felipe Contreras

^ permalink raw reply

* Re: git push tags
From: Chris Rorvick @ 2012-10-30 19:11 UTC (permalink / raw)
  To: Angelo Borsotti; +Cc: git
In-Reply-To: <CAB9Jk9CC9wjeyggejkVjKgY2HGAFw70hJo-S0S-W-p4gnd2zug@mail.gmail.com>

On Tue, Oct 30, 2012 at 1:34 PM, Angelo Borsotti
<angelo.borsotti@gmail.com> wrote:
> Hi Cris,
>
> I think a key in the config file of the remote repo is better than an
> option on git-push for what concerns security: it allows the owner of
> the remote repo to enforce the policy not to overwrite tags, which
> would not be possible if any user that has push access can --force
> tags.
>
> -Angelo

Hi Angelo,

Security is orthogonal to what this patch is attempting to resolve.
As Kacper pointed out, you can never be sure you're not going to
clobber an existing tag in the remote repo.  This patch attempts to
give git-push better (i.e., less surprising) semantics for tags.  In
other words, it's should will prevent mistakes, not provide any sort
of security.

So I don't think a config option is better or worse, it's just trying
to solve a different problem.

Thanks,

Chris

^ permalink raw reply

* [PATCH v3 4/4] fast-export: make sure refs are updated properly
From: Felipe Contreras @ 2012-10-30 19:06 UTC (permalink / raw)
  To: git
  Cc: Jeff King, Junio C Hamano, Sverre Rabbelier, Jonathan Nieder,
	Johannes Schindelin, Elijah Newren, Felipe Contreras
In-Reply-To: <1351623987-21012-1-git-send-email-felipe.contreras@gmail.com>

When an object has already been exported (and thus is in the marks) it
is flagged as SHOWN, so it will not be exported again, even if this time
it's exported through a different ref.

We don't need the object to be exported again, but we want the ref
updated, which doesn't happen.

Since we can't know if a ref was exported or not, let's just assume that
if the commit was marked (flags & SHOWN), the user still wants the ref
updated.

So:

 % git branch test master
 % git fast-export $mark_flags master
 % git fast-export $mark_flags test

Would export 'test' properly.

Additionally, this fixes issues with remote helpers; now they can push
refs wich objects have already been exported.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/fast-export.c     | 11 ++++++++---
 t/t5800-remote-helpers.sh | 11 +++++++++++
 t/t9350-fast-export.sh    | 14 ++++++++++++++
 3 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 7fb6fe1..663a93d 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -523,11 +523,16 @@ static void get_tags_and_duplicates(struct object_array *pending,
 				typename(e->item->type));
 			continue;
 		}
-		if (commit->util) {
-			/* more than one name for the same object */
+
+		/*
+		 * This ref will not be updated through a commit, lets make
+		 * sure it gets properly upddated eventually.
+		 */
+		if (commit->util || commit->object.flags & SHOWN) {
 			if (!(commit->object.flags & UNINTERESTING))
 				string_list_append(extra_refs, full_name)->util = commit;
-		} else
+		}
+		if (!commit->util)
 			commit->util = full_name;
 	}
 }
diff --git a/t/t5800-remote-helpers.sh b/t/t5800-remote-helpers.sh
index e7dc668..69a145a 100755
--- a/t/t5800-remote-helpers.sh
+++ b/t/t5800-remote-helpers.sh
@@ -145,4 +145,15 @@ test_expect_failure 'push new branch with old:new refspec' '
 	compare_refs clone HEAD server refs/heads/new-refspec
 '
 
+test_expect_success 'push ref with existing object' '
+	(cd localclone &&
+	git branch point-to-master master &&
+	git push origin point-to-master
+	) &&
+
+	(cd server &&
+	git show-ref refs/heads/point-to-master
+	)
+'
+
 test_done
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 6ea8f6f..a4178e3 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -446,4 +446,18 @@ test_expect_success 'proper extra refs handling' '
 	test_cmp expected actual
 '
 
+cat > expected << EOF
+reset refs/heads/master
+from :13
+
+EOF
+
+test_expect_success 'refs are updated even if no commits need to be exported' '
+	git fast-export --import-marks=tmp-marks \
+		--export-marks=tmp-marks master > /dev/null &&
+	git fast-export --import-marks=tmp-marks \
+		--export-marks=tmp-marks master > actual &&
+	test_cmp expected actual
+'
+
 test_done
-- 
1.8.0

^ permalink raw reply related

* [PATCH v3 3/4] fast-export: don't handle uninteresting refs
From: Felipe Contreras @ 2012-10-30 19:06 UTC (permalink / raw)
  To: git
  Cc: Jeff King, Junio C Hamano, Sverre Rabbelier, Jonathan Nieder,
	Johannes Schindelin, Elijah Newren, Felipe Contreras
In-Reply-To: <1351623987-21012-1-git-send-email-felipe.contreras@gmail.com>

They have been marked as UNINTERESTING for a reason, lets respect that.

Currently the first ref is handled properly, but not the rest, so:

 % git fast-export master ^master

Would currently throw a reset for master (2nd ref), which is not what we
want.

 % git fast-export master ^foo ^bar ^roo
 % git fast-export master salsa..tacos

Even if all these refs point to the same object; foo, bar, roo, salsa,
and tacos would all get a reset.

This is most certainly not what we want. After this patch, nothing gets
exported, because nothing was selected (everything is UNINTERESTING).

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/fast-export.c  | 7 ++++---
 t/t9350-fast-export.sh | 6 ++++++
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 065f324..7fb6fe1 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -523,10 +523,11 @@ static void get_tags_and_duplicates(struct object_array *pending,
 				typename(e->item->type));
 			continue;
 		}
-		if (commit->util)
+		if (commit->util) {
 			/* more than one name for the same object */
-			string_list_append(extra_refs, full_name)->util = commit;
-		else
+			if (!(commit->object.flags & UNINTERESTING))
+				string_list_append(extra_refs, full_name)->util = commit;
+		} else
 			commit->util = full_name;
 	}
 }
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 49bdb44..6ea8f6f 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -440,4 +440,10 @@ test_expect_success 'fast-export quotes pathnames' '
 	)
 '
 
+test_expect_success 'proper extra refs handling' '
+	git fast-export master ^master master..master > actual &&
+	echo -n > expected &&
+	test_cmp expected actual
+'
+
 test_done
-- 
1.8.0

^ permalink raw reply related

* [PATCH v3 2/4] fast-export: fix comparisson in tests
From: Felipe Contreras @ 2012-10-30 19:06 UTC (permalink / raw)
  To: git
  Cc: Jeff King, Junio C Hamano, Sverre Rabbelier, Jonathan Nieder,
	Johannes Schindelin, Elijah Newren, Felipe Contreras
In-Reply-To: <1351623987-21012-1-git-send-email-felipe.contreras@gmail.com>

First the expected, then the actual, otherwise the diff would be the
opposite of what we want.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/t9350-fast-export.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 3e821f9..49bdb44 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -303,7 +303,7 @@ test_expect_success 'dropping tag of filtered out object' '
 (
 	cd limit-by-paths &&
 	git fast-export --tag-of-filtered-object=drop mytag -- there > output &&
-	test_cmp output expected
+	test_cmp expected output
 )
 '
 
@@ -320,7 +320,7 @@ test_expect_success 'rewriting tag of filtered out object' '
 (
 	cd limit-by-paths &&
 	git fast-export --tag-of-filtered-object=rewrite mytag -- there > output &&
-	test_cmp output expected
+	test_cmp expected output
 )
 '
 
@@ -351,7 +351,7 @@ test_expect_failure 'no exact-ref revisions included' '
 	(
 		cd limit-by-paths &&
 		git fast-export master~2..master~1 > output &&
-		test_cmp output expected
+		test_cmp expected output
 	)
 '
 
-- 
1.8.0

^ permalink raw reply related

* [PATCH v3 1/4] fast-export: trivial cleanup
From: Felipe Contreras @ 2012-10-30 19:06 UTC (permalink / raw)
  To: git
  Cc: Jeff King, Junio C Hamano, Sverre Rabbelier, Jonathan Nieder,
	Johannes Schindelin, Elijah Newren, Felipe Contreras
In-Reply-To: <1351623987-21012-1-git-send-email-felipe.contreras@gmail.com>

Setting commit to commit is a no-op. It might have been there to avoid a
compiler warning, but if so, it was the compiler to blame.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/fast-export.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 12220ad..065f324 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -483,7 +483,7 @@ static void get_tags_and_duplicates(struct object_array *pending,
 	for (i = 0; i < pending->nr; i++) {
 		struct object_array_entry *e = pending->objects + i;
 		unsigned char sha1[20];
-		struct commit *commit = commit;
+		struct commit *commit;
 		char *full_name;
 
 		if (dwim_ref(e->name, strlen(e->name), sha1, &full_name) != 1)
-- 
1.8.0

^ permalink raw reply related

* [PATCH v3 0/4] fast-export: general fixes
From: Felipe Contreras @ 2012-10-30 19:06 UTC (permalink / raw)
  To: git
  Cc: Jeff King, Junio C Hamano, Sverre Rabbelier, Jonathan Nieder,
	Johannes Schindelin, Elijah Newren, Felipe Contreras

Hi,

Note: sorry for the noise, the first try (v2) was silently eaten by the mailing
list handler.

First patches are general cleanups and fixes, the last patch fixes a real issue
that affects remote helpers.

Changes since v2:

 * Actually send it to the ml

Changes since v1:

 * Improved commit messages
 * Use /dev/null in tests
 * Add test for remote helpers

Felipe Contreras (4):
  fast-export: trivial cleanup
  fast-export: fix comparisson in tests
  fast-export: don't handle uninteresting refs
  fast-export: make sure refs are updated properly

 builtin/fast-export.c     | 16 +++++++++++-----
 t/t5800-remote-helpers.sh | 11 +++++++++++
 t/t9350-fast-export.sh    | 26 +++++++++++++++++++++++---
 3 files changed, 45 insertions(+), 8 deletions(-)

-- 
1.8.0

^ permalink raw reply

* Re: [PATCH v2 3/4] fast-export: don't handle uninteresting refs
From: Jonathan Nieder @ 2012-10-30 18:59 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: git, Jeff King, Junio C Hamano, Sverre Rabbelier,
	Johannes Schindelin, Elijah Newren
In-Reply-To: <1351617089-13036-4-git-send-email-felipe.contreras@gmail.com>

Felipe Contreras wrote:

> They have been marked as UNINTERESTING for a reason, lets respect that.

This patch looks unsafe, and in the examples listed in the patch
description the changed behavior does not look like an improvement.
Worse, the description lists a few examples but gives no convincing
explanation to reassure about the lack of bad behavior for examples
not listed.

Perhaps this patch has a prerequisite and has come out of order.

Hope that helps,
Jonathan

Patch left unsnipped so we can get a copy in the list archive.

> Currently the first ref is handled properly, but not the rest, so:
> 
>  % git fast-export master ^master
> 
> Would currently throw a reset for master (2nd ref), which is not what we
> want.
> 
>  % git fast-export master ^foo ^bar ^roo
>  % git fast-export master salsa..tacos
> 
> Even if all these refs point to the same object; foo, bar, roo, salsa,
> and tacos would all get a reset.
> 
> This is most certainly not what we want. After this patch, nothing gets
> exported, because nothing was selected (everything is UNINTERESTING).
> 
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  builtin/fast-export.c  | 7 ++++---
>  t/t9350-fast-export.sh | 6 ++++++
>  2 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> index 065f324..7fb6fe1 100644
> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -523,10 +523,11 @@ static void get_tags_and_duplicates(struct object_array *pending,
>  				typename(e->item->type));
>  			continue;
>  		}
> -		if (commit->util)
> +		if (commit->util) {
>  			/* more than one name for the same object */
> -			string_list_append(extra_refs, full_name)->util = commit;
> -		else
> +			if (!(commit->object.flags & UNINTERESTING))
> +				string_list_append(extra_refs, full_name)->util = commit;
> +		} else
>  			commit->util = full_name;
>  	}
>  }
> diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
> index 49bdb44..6ea8f6f 100755
> --- a/t/t9350-fast-export.sh
> +++ b/t/t9350-fast-export.sh
> @@ -440,4 +440,10 @@ test_expect_success 'fast-export quotes pathnames' '
>  	)
>  '
>  
> +test_expect_success 'proper extra refs handling' '
> +	git fast-export master ^master master..master > actual &&
> +	echo -n > expected &&
> +	test_cmp expected actual
> +'
> +
>  test_done
> -- 
> 1.8.0

^ permalink raw reply

* Re: [PATCH v2 2/4] fast-export: fix comparisson in tests
From: Jonathan Nieder @ 2012-10-30 18:57 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: git, Jeff King, Junio C Hamano, Sverre Rabbelier,
	Johannes Schindelin, Elijah Newren
In-Reply-To: <1351617089-13036-3-git-send-email-felipe.contreras@gmail.com>

(actually cc-ing the git list this time.  Sorry for the noise, all.)
Felipe Contreras wrote:

> [Subject: [PATCH v2 2/4] fast-export: fix comparisson in tests]
>
> First the expected, then the actual, otherwise the diff would be the
> opposite of what we want.

Spelling: s/comparisson/comparison/.

Semantics: this isn't actually fixing anything --- it's a cosmetic
thing.  It would be clearer to say:

	fast-export test: swap arguments to test_cmp

	This way if diff output is produced, it describes how the
	actual output differs from what was expected rather than the
	other way around.

> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>

For what it's worth, with amended message,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Patch left unsnipped because it hadn't hit the list.

> ---
>  t/t9350-fast-export.sh | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
> index 3e821f9..49bdb44 100755
> --- a/t/t9350-fast-export.sh
> +++ b/t/t9350-fast-export.sh
> @@ -303,7 +303,7 @@ test_expect_success 'dropping tag of filtered out object' '
>  (
>  	cd limit-by-paths &&
>  	git fast-export --tag-of-filtered-object=drop mytag -- there > output &&
> -	test_cmp output expected
> +	test_cmp expected output
>  )
>  '
>  
> @@ -320,7 +320,7 @@ test_expect_success 'rewriting tag of filtered out object' '
>  (
>  	cd limit-by-paths &&
>  	git fast-export --tag-of-filtered-object=rewrite mytag -- there > output &&
> -	test_cmp output expected
> +	test_cmp expected output
>  )
>  '
>  
> @@ -351,7 +351,7 @@ test_expect_failure 'no exact-ref revisions included' '
>  	(
>  		cd limit-by-paths &&
>  		git fast-export master~2..master~1 > output &&
> -		test_cmp output expected
> +		test_cmp expected output
>  	)
>  '
>  
> -- 
> 1.8.0

^ permalink raw reply

* Re: [PATCH v2 1/4] fast-export: trivial cleanup
From: Jonathan Nieder @ 2012-10-30 18:55 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: git, Jeff King, Junio C Hamano, Sverre Rabbelier,
	Johannes Schindelin, Elijah Newren
In-Reply-To: <1351617089-13036-2-git-send-email-felipe.contreras@gmail.com>

Felipe Contreras wrote:

> Setting commit to commit is a no-op.

Wrong description.  This should say:

	The code uses the idiom of assigning commit to itself to quench a
	"may be used uninitialized" warning.  Luckily at least modern
	versions of gcc do not produce that warning here, so we can drop
	the self-assignment.

	This makes the code clearer to human beings, makes static
	analyzers that do not know that idiom happier, and means that
	if the code some day evolves to use this variable uninitialized
	then we will catch it.

> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>

With that change, for what it's worth,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Patch left unsnipped since it doesn't seem to have hit the list.

> ---
>  builtin/fast-export.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> index 12220ad..065f324 100644
> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -483,7 +483,7 @@ static void get_tags_and_duplicates(struct object_array *pending,
>  	for (i = 0; i < pending->nr; i++) {
>  		struct object_array_entry *e = pending->objects + i;
>  		unsigned char sha1[20];
> -		struct commit *commit = commit;
> +		struct commit *commit;
>  		char *full_name;
>  
>  		if (dwim_ref(e->name, strlen(e->name), sha1, &full_name) != 1)

^ permalink raw reply

* Re: [PATCH v2 4/4] fast-export: make sure refs are updated properly
From: Felipe Contreras @ 2012-10-30 18:47 UTC (permalink / raw)
  To: Sverre Rabbelier
  Cc: >, Jeff King, Junio C Hamano, Jonathan Nieder,
	Johannes Schindelin, Elijah Newren
In-Reply-To: <CAGdFq_j1RROOwxDi1FfJZJ6wiP9y9FWzSpc7MXVSvRmgk0sF9A@mail.gmail.com>

On Tue, Oct 30, 2012 at 7:12 PM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> On Tue, Oct 30, 2012 at 10:11 AM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> When an object has already been exported (and thus is in the marks) it
>> is flagged as SHOWN, so it will not be exported again, even if this time
>> it's exported through a different ref.
>>
>> We don't need the object to be exported again, but we want the ref
>> updated, which doesn't happen.
>>
>> Since we can't know if a ref was exported or not, let's just assume that
>> if the commit was marked (flags & SHOWN), the user still wants the ref
>> updated.
>>
>> So:
>>
>>  % git branch test master
>>  % git fast-export $mark_flags master
>>  % git fast-export $mark_flags test
>>
>> Would export 'test' properly.
>>
>> Additionally, this fixes issues with remote helpers; now they can push
>> refs wich objects have already been exported.
>
> Won't this also export child (or maybe parent) branches that weren't
> mentioned? For example:
>
> $ git branch one
> $ echo foo > content
> $ git commit -m two
> $ git fast-export one
> $ git fast-export two
>
> I suspect that one of those will export both one and two. If not, this
> seems like a great solution to the fast-export problem.

Why would it? We are not changing the way objects are exported, the
only difference is what happens at the end
(handle_tags_and_duplicates()).

And if you are talking about the ref for the reset at the end, it has
to be both in the list of refs selected by the user (initially in
&revs.pending), either marked or the object already referenced by
another ref in the list selected by the user (e.g. fast-export one
two, where one^{commit} == two^{commit}, and not marked as
UNINTERESTING (e.g. ^two).

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* Re: [PATCH] parse_dirstat_params(): use string_list to split comma-separated string
From: Matt Kraai @ 2012-10-30 18:43 UTC (permalink / raw)
  To: git
In-Reply-To: <1351443054-10472-1-git-send-email-mhagger@alum.mit.edu>

Michael Haggerty <mhagger <at> alum.mit.edu> writes:
...
> -static int parse_dirstat_params(struct diff_options *options, const char ...
> +static int parse_dirstat_params(struct diff_options *options, const char ...
>  				struct strbuf *errmsg)
>  {
> -	const char *p = params;
> -	int p_len, ret = 0;
> +	char *params_copy = xstrdup(params_string);
> +	struct string_list params = STRING_LIST_INIT_NODUP;
> +	int ret = 0;
> +	int i;
> 
> -	while (*p) {
> -		p_len = strchrnul(p, ',') - p;
> -		if (!memcmp(p, "changes", p_len)) {
> +	if (*params_copy)

params_copy is set to the value returned by xstrdup, which cannot be NULL.
This check can be removed and if params_string can be NULL, it should be
checked before being passed to xstrdup.

^ permalink raw reply

* Re: [PATCH v5 00/14] New remote-hg helper
From: Felipe Contreras @ 2012-10-30 18:29 UTC (permalink / raw)
  To: Chris Webb
  Cc: git, Junio C Hamano, Sverre Rabbelier, Johannes Schindelin,
	Ilari Liusvaara, Daniel Barkalow, Jeff King, Michael J Gruber
In-Reply-To: <20121030180021.GX26850@arachsys.com>

On Tue, Oct 30, 2012 at 7:00 PM, Chris Webb <chris@arachsys.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> Yes, it seems this is an API issue; repo.branchtip doesn't exist in
>> python 2.2.
>
> Hi. Presumably this is a problem with old mercurial not a problem with old
> python as mentioned in the commit?
>
>> Both issues should be fixed now :)
>
> They are indeed, and it now works nicely on all the repos I've tested it
> with, including http://selenic.com/hg: very impressive!
>
> I wonder whether it's worth ignoring heads with bookmarks pointing to them
> when it comes to considering heads of branches, or at least allowing the
> hg branch tracking to be easily disabled?
>
> A common idiom when working with hg bookmarks is to completely ignore the
> (not very useful) hg branches (i.e. all commits are on the default hg
> branch) and have a bookmark for each line of development used exactly as a
> git branch would be.
>
> On such a repository, at the moment you will always get a warning about
> multiple heads on branches/default, even though you actually don't care
> about branches/default (and would prefer it not to exist) and just want the
> branches coming from the bookmarks.
>
> I've also seen repositories with no hg branches, but with a single
> unbookmarked tip and bookmarks on some other heads to mark non-mainline
> development. It would be nice for branches/default to track the unbookmarked
> tip in this case, without warning about the other, bookmarked heads.
>
> Finally, on a simple repo with no branches and where there's no clash with a
> bookmark called master, I'd love to be able to a get a more idiomatic
> origin/master rather than origin/branches/default.

Sounds like you might want to try this:
% git config --global remote-hg.hg-git-compat true

This would match the behavior of hg-git, which seems to be what you
are looking for: branches will be ignored. But additionally there will
be other obtrusive changes:

 1) Extra information will be stored in the commit message: branch,
renames, extra info.

Personally I don't like this, which is why it's only enabled with the
hg-git compat mode. Eventually there will be support for this in the
normal mode through git notes, and eventually (hopefully), hg-git will
use notes as well.

 2) Authors will be fixed trying to preserve as much information as possible


The problem with this mode is what happens when there are no
bookmarks, and what I decided to do is create a fake bookmark to the
current branch (e.g. default), so you would get 'origin/default', but
you will be warned if there are multiple heads. We could try to create
a 'master' ref to the 'tip', but that might not be what the user
wants: (s)he might switch to a certain branch, and clone the
repository in git; currently the result working directory would be the
same as in hg, but if we point to 'tip', then it would not.

Maybe we should have a branch pointing to 'tip' either way.

The reason I decided on having 'branches/default' on the normal mode
is that users unfamiliar with mercurial might expect to see all the
branches somewhere. But perhaps that should happen only for other
branches, while 'default' gets a special treatment and gets promoted
to 'origin/master' but only if there's no 'master' bookmark, as you
say. Now the tricky part would be handling the case where there was no
'master' bookmark, and suddenly one gets created, but it might be
overkill to think about that right now.

Either way, try the hg-git-compat mode, should be close :)

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* Re: [PATCH v4 6/8] longest_ancestor_length(): require prefix list entries to be normalized
From: Ramsay Jones @ 2012-10-30 18:23 UTC (permalink / raw)
  To: Michael Haggerty
  Cc: Jeff King, Junio C Hamano, Jiang Xin, Lea Wiemann, Johannes Sixt,
	git
In-Reply-To: <1351440987-26636-7-git-send-email-mhagger@alum.mit.edu>

Michael Haggerty wrote:
> Move the responsibility for normalizing prefixes from
> longest_ancestor_length() to its callers. Use slightly different
> normalizations at the two callers:
> 
> In setup_git_directory_gently_1(), use the old normalization, which
> ignores paths that are not usable.  In the next commit we will change
> this caller to also resolve symlinks in the paths from
> GIT_CEILING_DIRECTORIES as part of the normalization.
> 
> In "test-path-utils longest_ancestor_length", use the old
> normalization, but die() if any paths are unusable.  Also change t0060
> to only pass normalized paths to the test program (no empty entries or
> non-absolute paths, strip trailing slashes from the paths, and remove
> tests that thereby become redundant).
> 
> The point of this change is to reduce the scope of the ancestor_length
> tests in t0060 from testing normalization+longest_prefix to testing
> only mostly longest_prefix.  This is necessary because when
> setup_git_directory_gently_1() starts resolving symlinks as part of
> its normalization, it will not be reasonable to do the same in the
> test suite, because that would make the test results depend on the
> contents of the root directory of the filesystem on which the test is
> run.  HOWEVER: under Windows, bash mangles arguments that look like
> absolute POSIX paths into DOS paths.

Just to be clear, this is true for the MinGW port to Windows, but *not*
the cygwin port.
:-P

>                                      So we have to retain the level
> of normalization done by normalize_path_copy() to convert the
> bash-mangled DOS paths (which contain backslashes) into paths that use
> forward slashes.
> 
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
>  path.c                | 26 +++++++++++---------------
>  setup.c               | 23 +++++++++++++++++++++++
>  t/t0060-path-utils.sh | 41 +++++++++++++----------------------------
>  test-path-utils.c     | 45 ++++++++++++++++++++++++++++++++++++++++++++-
>  4 files changed, 91 insertions(+), 44 deletions(-)
> 

[snip]

> diff --git a/test-path-utils.c b/test-path-utils.c
> index acb0560..0092cbf 100644
> --- a/test-path-utils.c
> +++ b/test-path-utils.c
> @@ -1,6 +1,33 @@
>  #include "cache.h"
>  #include "string-list.h"
>  
> +/*
> + * A "string_list_each_func_t" function that normalizes an entry from
> + * GIT_CEILING_DIRECTORIES.  If the path is unusable for some reason,
> + * die with an explanation.
> + */
> +static int normalize_ceiling_entry(struct string_list_item *item, void *unused)
> +{
> +	const char *ceil = item->string;
> +	int len = strlen(ceil);
> +	char buf[PATH_MAX+1];
> +
> +	if (len == 0)
> +		die("Empty path is not supported");
> +	if (len > PATH_MAX)
> +		die("Path \"%s\" is too long", ceil);
> +	if (!is_absolute_path(ceil))
> +		die("Path \"%s\" is not absolute", ceil);
> +	if (normalize_path_copy(buf, ceil) < 0)
> +		die("Path \"%s\" could not be normalized", ceil);
> +	len = strlen(buf);
> +	if (len > 1 && buf[len-1] == '/')
> +		die("Normalized path \"%s\" ended with slash", buf);
> +	free(item->string);
> +	item->string = xstrdup(buf);
> +	return 1;
> +}
> +
>  int main(int argc, char **argv)
>  {
>  	if (argc == 3 && !strcmp(argv[1], "normalize_path_copy")) {
> @@ -33,10 +60,26 @@ int main(int argc, char **argv)
>  	if (argc == 4 && !strcmp(argv[1], "longest_ancestor_length")) {
>  		int len;
>  		struct string_list ceiling_dirs = STRING_LIST_INIT_DUP;
> +		char *path = xstrdup(argv[2]);
>  
> +		/*
> +		 * We have to normalize the arguments because under
> +		 * Windows, bash mangles arguments that look like

ditto

> +		 * absolute POSIX paths or colon-separate lists of
> +		 * absolute POSIX paths into DOS paths (e.g.,
> +		 * "/foo:/foo/bar" might be converted to
> +		 * "D:\Src\msysgit\foo;D:\Src\msysgit\foo\bar"),
> +		 * whereas longest_ancestor_length() requires paths
> +		 * that use forward slashes.
> +		 */
> +		if (normalize_path_copy(path, path))
> +			die("Path \"%s\" could not be normalized", argv[2]);
>  		string_list_split(&ceiling_dirs, argv[3], PATH_SEP, -1);
> -		len = longest_ancestor_length(argv[2], &ceiling_dirs);
> +		filter_string_list(&ceiling_dirs, 0,
> +				   normalize_ceiling_entry, NULL);
> +		len = longest_ancestor_length(path, &ceiling_dirs);
>  		string_list_clear(&ceiling_dirs, 0);
> +		free(path);
>  		printf("%d\n", len);
>  		return 0;
>  	}


HTH

ATB,
Ramsay Jones

^ permalink raw reply

* Re: [PATCH v5 00/14] New remote-hg helper
From: Chris Webb @ 2012-10-30 18:16 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: git, Junio C Hamano, Sverre Rabbelier, Johannes Schindelin,
	Ilari Liusvaara, Daniel Barkalow, Jeff King, Michael J Gruber
In-Reply-To: <20121030180021.GX26850@arachsys.com>

Chris Webb <chris@arachsys.com> writes:

> A common idiom when working with hg bookmarks is to completely ignore the
> (not very useful) hg branches (i.e. all commits are on the default hg
> branch) and have a bookmark for each line of development used exactly as a
> git branch would be.
> 
> On such a repository, at the moment you will always get a warning about
> multiple heads on branches/default, even though you actually don't care
> about branches/default (and would prefer it not to exist) and just want the
> branches coming from the bookmarks.

Something which you can do with hg clone is

  hg clone http://my.repo/foo#master

to clone just the history behind the master bookmark from foo. This works
nicely with git-remote-hg too:

  git clone hg::http://my.repo/foo#master

gives you just origin/master and origin/branches/default, not
origin/otherbookmark. This is a case where it would be particularly nice to
be able to kill origin/branches/default and just keep the identical
origin/master.

Cheers,

Chris.

^ permalink raw reply

* Re: [PATCH v2 4/4] fast-export: make sure refs are updated properly
From: Sverre Rabbelier @ 2012-10-30 18:12 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: >, Jeff King, Junio C Hamano, Jonathan Nieder,
	Johannes Schindelin, Elijah Newren
In-Reply-To: <1351617089-13036-5-git-send-email-felipe.contreras@gmail.com>

On Tue, Oct 30, 2012 at 10:11 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> When an object has already been exported (and thus is in the marks) it
> is flagged as SHOWN, so it will not be exported again, even if this time
> it's exported through a different ref.
>
> We don't need the object to be exported again, but we want the ref
> updated, which doesn't happen.
>
> Since we can't know if a ref was exported or not, let's just assume that
> if the commit was marked (flags & SHOWN), the user still wants the ref
> updated.
>
> So:
>
>  % git branch test master
>  % git fast-export $mark_flags master
>  % git fast-export $mark_flags test
>
> Would export 'test' properly.
>
> Additionally, this fixes issues with remote helpers; now they can push
> refs wich objects have already been exported.

Won't this also export child (or maybe parent) branches that weren't
mentioned? For example:

$ git branch one
$ echo foo > content
$ git commit -m two
$ git fast-export one
$ git fast-export two

I suspect that one of those will export both one and two. If not, this
seems like a great solution to the fast-export problem.

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCH] Enable parallelism in git submodule update.
From: Stefan Zager @ 2012-10-30 18:11 UTC (permalink / raw)
  To: git; +Cc: Jens Lehmann, Heiko Voigt, Junio C Hamano
In-Reply-To: <5090168f.5e+7ZUFKdYL2Qnw7%szager@google.com>

This is a refresh of a conversation from a couple of months ago.

I didn't try to implement all the desired features (e.g., smart logic
for passing a -j parameter to recursive submodule invocations), but I
did address the one issue that Junio insisted on: the code makes a
best effort to detect whether xargs supports parallel execution on the
host platform, and if it doesn't, then it prints a warning and falls
back to serial execution.

Stefan

On Tue, Oct 30, 2012 at 11:03 AM,  <szager@google.com> wrote:
> The --jobs parameter may be used to set the degree of per-submodule
> parallel execution.
>
> Signed-off-by: Stefan Zager <szager@google.com>
> ---
>  Documentation/git-submodule.txt |    8 ++++++-
>  git-submodule.sh                |   40 ++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 46 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index b4683bb..cb23ba7 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -14,7 +14,8 @@ SYNOPSIS
>  'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
>  'git submodule' [--quiet] init [--] [<path>...]
>  'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
> -             [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
> +             [--reference <repository>] [--merge] [--recursive]
> +             [-j|--jobs [jobs]] [--] [<path>...]
>  'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]
>               [commit] [--] [<path>...]
>  'git submodule' [--quiet] foreach [--recursive] <command>
> @@ -146,6 +147,11 @@ If the submodule is not yet initialized, and you just want to use the
>  setting as stored in .gitmodules, you can automatically initialize the
>  submodule with the `--init` option.
>  +
> +By default, each submodule is treated serially.  You may specify a degree of
> +parallel execution with the --jobs flag.  If a parameter is provided, it is
> +the maximum number of jobs to run in parallel; without a parameter, all jobs are
> +run in parallel.
> ++
>  If `--recursive` is specified, this command will recurse into the
>  registered submodules, and update any nested submodules within.
>  +
> diff --git a/git-submodule.sh b/git-submodule.sh
> index ab6b110..60a5f96 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -8,7 +8,7 @@ dashless=$(basename "$0" | sed -e 's/-/ /')
>  USAGE="[--quiet] add [-b branch] [-f|--force] [--reference <repository>] [--] <repository> [<path>]
>     or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
>     or: $dashless [--quiet] init [--] [<path>...]
> -   or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
> +   or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [-j|--jobs [jobs]] [--] [<path>...]
>     or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
>     or: $dashless [--quiet] foreach [--recursive] <command>
>     or: $dashless [--quiet] sync [--] [<path>...]"
> @@ -500,6 +500,7 @@ cmd_update()
>  {
>         # parse $args after "submodule ... update".
>         orig_flags=
> +       jobs="1"
>         while test $# -ne 0
>         do
>                 case "$1" in
> @@ -518,6 +519,20 @@ cmd_update()
>                 -r|--rebase)
>                         update="rebase"
>                         ;;
> +               -j|--jobs)
> +                       case "$2" in
> +                       ''|-*)
> +                               jobs="0"
> +                               ;;
> +                       *)
> +                               jobs="$2"
> +                               shift
> +                               ;;
> +                       esac
> +                       # Don't preserve this arg.
> +                       shift
> +                       continue
> +                       ;;
>                 --reference)
>                         case "$2" in '') usage ;; esac
>                         reference="--reference=$2"
> @@ -551,11 +566,34 @@ cmd_update()
>                 shift
>         done
>
> +       # Correctly handle the case where '-q' came before 'update' on the command line.
> +       if test -n "$GIT_QUIET"
> +       then
> +               orig_flags="$orig_flags -q"
> +       fi
> +
>         if test -n "$init"
>         then
>                 cmd_init "--" "$@" || return
>         fi
>
> +       if test "$jobs" != 1
> +       then
> +               if ( echo test | xargs -P "$jobs" true 2>/dev/null )
> +               then
> +                       if ( echo test | xargs --max-lines=1 true 2>/dev/null ); then
> +                               max_lines="--max-lines=1"
> +                       else
> +                               max_lines="-L 1"
> +                       fi
> +                       module_list "$@" | awk '{print $4}' |
> +                       xargs $max_lines -P "$jobs" git submodule update $orig_flags
> +                       return
> +               else
> +                       echo "Warn: parallel execution is not supported on this platform."
> +               fi
> +       fi
> +
>         cloned_modules=
>         module_list "$@" | {
>         err=
> --
> 1.7.7.3
>

^ permalink raw reply

* Re: [PATCH v4 00/13] New remote-hg helper
From: Felipe Contreras @ 2012-10-30 18:10 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Jeff King, git, Junio C Hamano, Sverre Rabbelier, Ilari Liusvaara,
	Daniel Barkalow, Michael J Gruber
In-Reply-To: <alpine.DEB.1.00.1210301809060.7256@s15462909.onlinehome-server.info>

On Tue, Oct 30, 2012 at 6:20 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:

> P.S.: I would still recommend to have a detailed look at the 'devel'
> branch, in particular the commits starting with "fast-export: do not refer
> to non-existing marks" and ending with "t5801: skip without hg". My
> understanding is that it was completely ignored after a brief and maybe
> too-cursory look. In the least, it has a couple of lessons we learnt the
> hard way, and if git.git is dead set on duplicating the work, making these
> mistakes again could be avoided by learning from our lessons.

% g l --grep="t5801: skip without hg" devel
1e000d4 t5801: skip without hg
bee410c t5801: skip without hg
5cdc7d0 t5801: skip without hg
05b703f t5801: skip without hg
6bb8d90 t5801: skip without hg
c70b4d0 t5801: skip without hg
2f46371 t5801: skip without hg
39bc40f t5801: skip without hg
d0a618b t5801: skip without hg

% g l --grep="fast-export: do not refer" devel
d3ac32c fast-export: do not refer to non-existing marks
bdbb22f fast-export: do not refer to non-existing marks
5d99930 fast-export: do not refer to non-existing marks
381f276 fast-export: do not refer to non-existing marks
b4686c7 fast-export: do not refer to non-existing marks
e3dfe01 fast-export: do not refer to non-existing marks
c00fe59 fast-export: do not refer to non-existing marks
ce357ce fast-export: do not refer to non-existing marks
5c1c7a4 fast-export: do not refer to non-existing marks
9c827d1 fast-export: do not refer to non-existing marks

I'll assume you are referring the latest ones:

% g log --oneline --reverse d3ac32c^..1e000d4
* d3ac32c fast-export: do not refer to non-existing marks

Not needed at all.

* b013fe0 setup_revisions: remember whether a ref was positive or not
* fb89a2c fast-export: do not export negative refs
* 7655869 setup_revisions: remember whether a ref was positive or not

I've fixed this problem already.

The solution proposed in these patches is to convoluted:
1) Requires multiple unrelated changes
2) Proposes change in committish semantics

It's hard to test, because the test to check for this is not in this
patch series, and it's testing for something completely unrelated:

---
cat > expected << EOF
reset refs/heads/master
from $(git rev-parse master)

EOF

test_expect_success 'refs are updated even if no commits need to be exported' '
        git fast-export master..master > actual &&
        test_cmp expected actual
'
---

This is most certainly not what we want.

Notice that in my patch (a single patch) I added the tests at the same
time so it's clear what it's fixing, and I also added a test to the
relevant remote-helper behavior we want:

https://github.com/felipec/git/commit/76e75315bd1bd8d9d8365bb09261a745a10ceae0

* 512cb13 t5800: test pushing a new branch with old content

If this is what the patches above were trying to fix, then yes, my
patch fixes that. Also, it's tainted by changes from another patch.

* a85de2c t5800: point out that deleting branches does not work

Correct, but hardly _necessary_.

* 2412a45 transport-helper: add trailing --

No description what's the problem, or what it's trying to fix, or
tests, so it's not possible to know if this is _needed_ or not. But
probably correct.

* 026d07c remote-helper: check helper status after import/export

Again, no explanation, but the issue was already addressed:

http://article.gmane.org/gmane.comp.version-control.git/208202

The problem is minuscule, not _needed_.

* 5165e26 remote-testgit: factor out RemoteHelper class
* 049f093 git-remote-testgit: make local a function
* f835bb2 git_remote_helpers: add fastimport library
* 088ad33 git-remote-hg: add hgimport, an hg-fast-import equivalent
* 7de6ca0 git-remote-hg: add GitHg, a helper class for converting hg
commits to git
* e3cc5ed git-remote-hg: add hgexport, an hg-fast-export equivalent
* 0edc8e9 git-remote-hg: add GitExporter/GitImporter/NonLocalGit
* 5c73277 remote-hg: adjust to hg 1.9
* 1b47007 git-remote-hg: add the helper
* 4dcc671 git-remote-hg: add tests
* 48b2769 remote-hg: Postel's law dictates we should handle Author<author@mail>
* 2587cc6 remote-hg: another case of Postel's law
* 9f934c9 remote-hg: handle another funny author line from
http://scelenic.com/hg
* a799904 remote-hg: do not interfer with hg's revs() method

All these are specific to this remote-hg version.

* ac77256 Always auto-gc after calling a fast-import transport

This might be a good idea, but not _needed_.

* 1e000d4 t5801: skip without hg

Specific to this remote-hg.


So, yeah, nothing really needed there. Some patches might be nice, but
that's it.

Now, if this is really the latest and greatest remote-hg patch series,
I can try to port them to git's master and see how it fares.

But you mentioned something about cooperation, and I've yet to see how
is it that you are planning to cooperate. If you say you don't have
time to spend on this, I don't see why I should worry about testing
this series of patches.

Also, you seem to be clearly against my implementation, is there any
evidence that will convince you that my version is "good"? Maybe my
version passing more tests than msysgit's? Or is there truly nothing I
can do to change your perception?

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* [PATCH] Enable parallelism in git submodule update.
From: szager @ 2012-10-30 18:03 UTC (permalink / raw)
  To: git; +Cc: jens.lehmann, hvoigt, gitster

The --jobs parameter may be used to set the degree of per-submodule
parallel execution.

Signed-off-by: Stefan Zager <szager@google.com>
---
 Documentation/git-submodule.txt |    8 ++++++-
 git-submodule.sh                |   40 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index b4683bb..cb23ba7 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -14,7 +14,8 @@ SYNOPSIS
 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
 'git submodule' [--quiet] init [--] [<path>...]
 'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
-	      [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
+	      [--reference <repository>] [--merge] [--recursive]
+	      [-j|--jobs [jobs]] [--] [<path>...]
 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]
 	      [commit] [--] [<path>...]
 'git submodule' [--quiet] foreach [--recursive] <command>
@@ -146,6 +147,11 @@ If the submodule is not yet initialized, and you just want to use the
 setting as stored in .gitmodules, you can automatically initialize the
 submodule with the `--init` option.
 +
+By default, each submodule is treated serially.  You may specify a degree of
+parallel execution with the --jobs flag.  If a parameter is provided, it is
+the maximum number of jobs to run in parallel; without a parameter, all jobs are
+run in parallel.
++
 If `--recursive` is specified, this command will recurse into the
 registered submodules, and update any nested submodules within.
 +
diff --git a/git-submodule.sh b/git-submodule.sh
index ab6b110..60a5f96 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -8,7 +8,7 @@ dashless=$(basename "$0" | sed -e 's/-/ /')
 USAGE="[--quiet] add [-b branch] [-f|--force] [--reference <repository>] [--] <repository> [<path>]
    or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] init [--] [<path>...]
-   or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
+   or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [-j|--jobs [jobs]] [--] [<path>...]
    or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
    or: $dashless [--quiet] foreach [--recursive] <command>
    or: $dashless [--quiet] sync [--] [<path>...]"
@@ -500,6 +500,7 @@ cmd_update()
 {
 	# parse $args after "submodule ... update".
 	orig_flags=
+	jobs="1"
 	while test $# -ne 0
 	do
 		case "$1" in
@@ -518,6 +519,20 @@ cmd_update()
 		-r|--rebase)
 			update="rebase"
 			;;
+		-j|--jobs)
+			case "$2" in
+			''|-*)
+				jobs="0"
+				;;
+			*)
+				jobs="$2"
+				shift
+				;;
+			esac
+			# Don't preserve this arg.
+			shift
+			continue
+			;;
 		--reference)
 			case "$2" in '') usage ;; esac
 			reference="--reference=$2"
@@ -551,11 +566,34 @@ cmd_update()
 		shift
 	done
 
+	# Correctly handle the case where '-q' came before 'update' on the command line.
+	if test -n "$GIT_QUIET"
+	then
+		orig_flags="$orig_flags -q"
+	fi
+
 	if test -n "$init"
 	then
 		cmd_init "--" "$@" || return
 	fi
 
+	if test "$jobs" != 1
+	then
+		if ( echo test | xargs -P "$jobs" true 2>/dev/null )
+		then
+			if ( echo test | xargs --max-lines=1 true 2>/dev/null ); then
+				max_lines="--max-lines=1"
+			else
+				max_lines="-L 1"
+			fi
+			module_list "$@" | awk '{print $4}' |
+			xargs $max_lines -P "$jobs" git submodule update $orig_flags
+			return
+		else
+			echo "Warn: parallel execution is not supported on this platform."
+		fi
+	fi
+
 	cloned_modules=
 	module_list "$@" | {
 	err=
-- 
1.7.7.3

^ permalink raw reply related

* [PATCH] Enable parallelism in git submodule update.
From: szager @ 2012-10-30 18:03 UTC (permalink / raw)
  To: git, jens.lehmann, -cc, hvoigt, -cc, gitster; +Cc: c

The --jobs parameter may be used to set the degree of per-submodule
parallel execution.

Signed-off-by: Stefan Zager <szager@google.com>
---
 Documentation/git-submodule.txt |    8 ++++++-
 git-submodule.sh                |   40 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index b4683bb..cb23ba7 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -14,7 +14,8 @@ SYNOPSIS
 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
 'git submodule' [--quiet] init [--] [<path>...]
 'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
-	      [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
+	      [--reference <repository>] [--merge] [--recursive]
+	      [-j|--jobs [jobs]] [--] [<path>...]
 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]
 	      [commit] [--] [<path>...]
 'git submodule' [--quiet] foreach [--recursive] <command>
@@ -146,6 +147,11 @@ If the submodule is not yet initialized, and you just want to use the
 setting as stored in .gitmodules, you can automatically initialize the
 submodule with the `--init` option.
 +
+By default, each submodule is treated serially.  You may specify a degree of
+parallel execution with the --jobs flag.  If a parameter is provided, it is
+the maximum number of jobs to run in parallel; without a parameter, all jobs are
+run in parallel.
++
 If `--recursive` is specified, this command will recurse into the
 registered submodules, and update any nested submodules within.
 +
diff --git a/git-submodule.sh b/git-submodule.sh
index ab6b110..60a5f96 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -8,7 +8,7 @@ dashless=$(basename "$0" | sed -e 's/-/ /')
 USAGE="[--quiet] add [-b branch] [-f|--force] [--reference <repository>] [--] <repository> [<path>]
    or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] init [--] [<path>...]
-   or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
+   or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [-j|--jobs [jobs]] [--] [<path>...]
    or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
    or: $dashless [--quiet] foreach [--recursive] <command>
    or: $dashless [--quiet] sync [--] [<path>...]"
@@ -500,6 +500,7 @@ cmd_update()
 {
 	# parse $args after "submodule ... update".
 	orig_flags=
+	jobs="1"
 	while test $# -ne 0
 	do
 		case "$1" in
@@ -518,6 +519,20 @@ cmd_update()
 		-r|--rebase)
 			update="rebase"
 			;;
+		-j|--jobs)
+			case "$2" in
+			''|-*)
+				jobs="0"
+				;;
+			*)
+				jobs="$2"
+				shift
+				;;
+			esac
+			# Don't preserve this arg.
+			shift
+			continue
+			;;
 		--reference)
 			case "$2" in '') usage ;; esac
 			reference="--reference=$2"
@@ -551,11 +566,34 @@ cmd_update()
 		shift
 	done
 
+	# Correctly handle the case where '-q' came before 'update' on the command line.
+	if test -n "$GIT_QUIET"
+	then
+		orig_flags="$orig_flags -q"
+	fi
+
 	if test -n "$init"
 	then
 		cmd_init "--" "$@" || return
 	fi
 
+	if test "$jobs" != 1
+	then
+		if ( echo test | xargs -P "$jobs" true 2>/dev/null )
+		then
+			if ( echo test | xargs --max-lines=1 true 2>/dev/null ); then
+				max_lines="--max-lines=1"
+			else
+				max_lines="-L 1"
+			fi
+			module_list "$@" | awk '{print $4}' |
+			xargs $max_lines -P "$jobs" git submodule update $orig_flags
+			return
+		else
+			echo "Warn: parallel execution is not supported on this platform."
+		fi
+	fi
+
 	cloned_modules=
 	module_list "$@" | {
 	err=
-- 
1.7.7.3

^ permalink raw reply related

* Re: [PATCH v5 00/14] New remote-hg helper
From: Chris Webb @ 2012-10-30 18:00 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: git, Junio C Hamano, Sverre Rabbelier, Johannes Schindelin,
	Ilari Liusvaara, Daniel Barkalow, Jeff King, Michael J Gruber
In-Reply-To: <CAMP44s1g8rFGP7UOcvp9BEZ1oiSh3+-gYheciqO8Fmghipot8A@mail.gmail.com>

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Yes, it seems this is an API issue; repo.branchtip doesn't exist in
> python 2.2.

Hi. Presumably this is a problem with old mercurial not a problem with old
python as mentioned in the commit?

> Both issues should be fixed now :)

They are indeed, and it now works nicely on all the repos I've tested it
with, including http://selenic.com/hg: very impressive!

I wonder whether it's worth ignoring heads with bookmarks pointing to them
when it comes to considering heads of branches, or at least allowing the
hg branch tracking to be easily disabled?

A common idiom when working with hg bookmarks is to completely ignore the
(not very useful) hg branches (i.e. all commits are on the default hg
branch) and have a bookmark for each line of development used exactly as a
git branch would be.

On such a repository, at the moment you will always get a warning about
multiple heads on branches/default, even though you actually don't care
about branches/default (and would prefer it not to exist) and just want the
branches coming from the bookmarks.

I've also seen repositories with no hg branches, but with a single
unbookmarked tip and bookmarks on some other heads to mark non-mainline
development. It would be nice for branches/default to track the unbookmarked
tip in this case, without warning about the other, bookmarked heads.

Finally, on a simple repo with no branches and where there's no clash with a
bookmark called master, I'd love to be able to a get a more idiomatic
origin/master rather than origin/branches/default.

Just some idle thoughts...

Best wishes,

Chris.

^ permalink raw reply

* Re: [PATCH v5 00/14] New remote-hg helper
From: Johannes Schindelin @ 2012-10-30 17:27 UTC (permalink / raw)
  To: Chris Webb
  Cc: Felipe Contreras, git, Junio C Hamano, Sverre Rabbelier,
	Ilari Liusvaara, Daniel Barkalow, Jeff King, Michael J Gruber
In-Reply-To: <20121030102526.GN4891@arachsys.com>

Hi Chris,

On Tue, 30 Oct 2012, Chris Webb wrote:

> I routinely work with projects in both hg and git, so I'm really
> interested in this. Thanks for working on it! I grabbed the latest
> version from
> 
>   https://github.com/felipec/git/blob/fc-remote-hg/contrib/remote-hg/git-remote-hg
> 
> and have been trying it out.

You might also want to try out the 'devel' branch of
https://github.com/msysgit/git. It is in production use here.

Ciao,
Johannes

^ permalink raw reply

* Re: [PATCH v4 00/13] New remote-hg helper
From: Johannes Schindelin @ 2012-10-30 17:20 UTC (permalink / raw)
  To: Jeff King
  Cc: Felipe Contreras, git, Junio C Hamano, Sverre Rabbelier,
	Ilari Liusvaara, Daniel Barkalow, Michael J Gruber
In-Reply-To: <20121029215631.GF20513@sigill.intra.peff.net>

Hi all,

On Mon, 29 Oct 2012, Jeff King wrote:

> On Mon, Oct 29, 2012 at 10:47:04PM +0100, Felipe Contreras wrote:
> 
> > >> Yeah, the test script is not ready for merging, it needs to check
> > >> for python, hg, and hg-git.
> > >>
> > >> Do you have hg-git installed?
> > >
> > > No. But it's important that it fail gracefully; I can't even take it
> > > in pu if I can't run the test suite in a sane way.
> > 
> > The contrib part is fine for 'pu'. The tests aren't even meant to
> > exercise stuff in 'contrib', right? There might be some exceptions,
> > but either way, there's plenty of stuff in 'contrib' without any
> > tests. The tests I'm providing are simply a little sugar.
> 
> Yeah, contrib is a bit of a wildcard. Most things do not have tests.

Given that the tests of remote-hg as in git://github.com/msysgit/git's
'devel' branch run just fine without additional dependencies (which
probably triggered the not-quite-constructive and unnecessarily-flaming
"bloated" comment of Felipe), and given that the code in said branch is
well-tested and exercised by daily use, and given the fact that my major
concern was not understood (and probably not addressed), and also given
the fact that Sverre indicated that he could finalize the work as a 20%
project, I decided that other projects I have to do unfortunately have a
too-high priority to take care of testing and measuring the performance of
the patch series that is discussed in this thread.

Sorry,
Johannes

P.S.: I would still recommend to have a detailed look at the 'devel'
branch, in particular the commits starting with "fast-export: do not refer
to non-existing marks" and ending with "t5801: skip without hg". My
understanding is that it was completely ignored after a brief and maybe
too-cursory look. In the least, it has a couple of lessons we learnt the
hard way, and if git.git is dead set on duplicating the work, making these
mistakes again could be avoided by learning from our lessons.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox