Git development
 help / color / mirror / Atom feed
* Re: [PATCH 1/4] remote-hg: add missing config for basic tests
From: Felipe Contreras @ 2012-11-13  7:12 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Ramkumar Ramachandra
In-Reply-To: <20121113054826.GC10995@sigill.intra.peff.net>

On Tue, Nov 13, 2012 at 6:48 AM, Jeff King <peff@peff.net> wrote:

> Any objection to me marking it up as I apply?

Nope.

-- 
Felipe Contreras

^ permalink raw reply

* Re: [PATCH 2/4] remote-hg: fix compatibility with older versions of hg
From: Felipe Contreras @ 2012-11-13  7:11 UTC (permalink / raw)
  To: Ramkumar Ramachandra; +Cc: git
In-Reply-To: <CALkWK0k9Ta-SOo43Knz9kMV0ePKf7wjK-3Zm6ocDQpe7VzFz+A@mail.gmail.com>

On Tue, Nov 13, 2012 at 6:25 AM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> This patch fixes my original problem. So,
> Reported-by: Ramkumar Ramachandra <artagnon@gmail.com>
> Tested-by: Ramkumar Ramachandra <artagnon@gmail.com>
>
> However, test 4 in test-hg.sh still fails for me:
>
> --- expected    2012-11-13 05:22:57.946637384 +0000
> +++ actual      2012-11-13 05:22:57.946637384 +0000
> @@ -1,2 +1,2 @@
> -zero
> -refs/heads/master
> +feature-a
> +refs/heads/feature-a
>
> What is going on?

Probably a bug in mercurial, but can be worked around:

--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -429,10 +429,18 @@ def get_branch_tip(repo, branch):

     return heads[0]

+def get_current_bookmark(repo):
+    head = bookmarks.readcurrent(repo)
+    if not head:
+        return None
+    if repo[head] != repo['.']:
+        return None
+    return head
+
 def list_head(repo, cur):
     global g_head, bmarks

-    head = bookmarks.readcurrent(repo)
+    head = get_current_bookmark(repo)
     if head:
         node = repo[head]
     else:

-- 
Felipe Contreras

^ permalink raw reply

* Re: [PATCH] send-email: add proper default sender
From: Felipe Contreras @ 2012-11-13  6:42 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <20121113040104.GA9361@sigill.intra.peff.net>

On Tue, Nov 13, 2012 at 5:01 AM, Jeff King <peff@peff.net> wrote:
> On Tue, Nov 13, 2012 at 04:55:25AM +0100, Felipe Contreras wrote:
>
>> > No, it's not. Those broken names do not come from the environment, but
>> > from our last-resort guess of the hostname.
>>
>> That depends how you define environment, but fine, the point is that
>> it happens.
>
> If you have a strawman definition that does not have anything to do with
> what I said in my original email, then yes, it could happen.

It happens, I've seen commits with (none) not that long ago.

> But as I
> said already, "git var" uses IDENT_STRICT and will not allow such broken
> names.

Since 1.7.11, sure. But not everyone is using such a recent version of
git, and people with fully qualified domains would still get unwanted
behavior.

>> > We long ago switched to
>> > printing the name as a warning when we have made such a guess (bb1ae3f),
>> > then more recently started rejecting them outright (8c5b1ae).
>>
>> Right, but these would still happen:
>>
>> michael <michael@michael-laptop.michael.org>
>
> Did you read my email? I explicitly proposed that we would _not_ allow
> send-email to use implicit email addresses constructed in that way.

I'm not talking about git send-email, I'm talking about your comment
'it has always been the case that you can use git without setting
user.*', which has caused issues with wrong author/commmitter names in
commits, and will probably continue to do so.

>> > But in the meantime you are causing a regression for anybody who expects
>> > GIT_AUTHOR_NAME to override user.email when running git-send-email (and
>> > you have taken away the prompt that they could have used to notice and
>> > correct it).
>>
>> I think they can survive. If anybody like this exists.
>
> Sorry, but that is not how things work on this project. You do not get
> to cause regressions because you are too lazy to implement the feature
> _you_ want in a way that does not break other people.

That doesn't change the fact that they would survive, and the fact
that those users don't actually exist.

But let's look at the current situation closely:

PERL5LIB=~/dev/git/perl ./git-send-email.perl --confirm=always -1

1) No information at all

fatal: empty ident name (for <felipec@nysa.(none)>) not allowed

2) Full Name + full hostname

Who should the emails appear to be from? [Felipe Contreras
<felipec@nysa.felipec.org>]

That's right, ident doesn't fail, and that's not the mail address I
specified, it's *implicit*.

3) Full Name + EMAIL

Who should the emails appear to be from? [Felipe Contreras
<felipe.contreras@gmail.com>]

4) config user

Who should the emails appear to be from? [Felipe Contreras 2nd
<felipe.contreras+2@gmail.com>]

5) GIT_COMMITTER

Who should the emails appear to be from? [Felipe Contreras 2nd
<felipe.contreras+2@gmail.com>]

Whoa, what happened there?

Well:

  $sender = $repoauthor || $repocommitter || '';
  ($repoauthor) = Git::ident_person(@repo, 'author');
  % ./git var GIT_AUTHOR_IDENT
  Felipe Contreras 2nd <felipe.contreras+2@gmail.com> 1352783223 +0100

That's right, AUTHOR_IDENT would fall back to the default email and full name.

Hmm, I wonder...

5.1) GIT_COMMITER without anything else

fatal: empty ident name (for <felipec@nysa.(none)>) not allowed
var GIT_AUTHOR_IDENT: command returned error: 128

Why? Because:

% PERL5LIB=~/dev/git/perl perl -e 'use Git; printf("%s\n",
Git::ident_person(@repo, 'author'));'
fatal: empty ident name (for <felipec@nysa.(none)>) not allowed

($repoauthor) = Git::ident_person(@repo, 'author');
($repocommitter) = Git::ident_person(@repo, 'committer');

So $repoauthor || $repocommiter is pointless.

6) GIT_AUTHOR

Who should the emails appear to be from? [Felipe Contreras 4th
<felipe.contreras+4@gmail.com>]

What about after my change?

6.1) GIT_AUTHOR without anything else

fatal: empty ident name (for <felipec@nysa.(none)>) not allowed
var GIT_COMMITTER_IDENT: command returned error: 128

4) config user

From: Felipe Contreras 2nd <felipe.contreras+2@gmail.com>

5) GIT_COMMITTER

From: Felipe Contreras 2nd <felipe.contreras+2@gmail.com>

6) GIT_AUTHOR

From: Felipe Contreras 2nd <felipe.contreras+2@gmail.com>

And what about your proposed change?

2) Full Name + full hostname

./git var GIT_EXPLICIT_IDENT
0

6.1) GIT_AUTHOR without anything else

Even if the previous problem was solved:

export GIT_AUTHOR_NAME='Felipe Contreras 4th'; export
GIT_AUTHOR_EMAIL='felipe.contreras+4@gmail.com'
./git var GIT_EXPLICIT_IDENT
0

No explicit ident? This is most certainly not what the user would expect.

And then:

5.2) GIT_COMMITTER with Full Name and full hostname

export GIT_COMMITTER_NAME='Felipe Contreras 3nd'; export
GIT_COMMITTER_EMAIL='felipe.contreras+3@gmail.com'
./git var GIT_EXPLICIT_IDENT
1

From: Felipe Contreras <felipec@nysa.felipec.org>

It is explicit, yeah, but 'git send-email' would not be picking the
committer, it would pick the author.

> I tried to help you by pointing you in the right direction and even
> providing a sample "git var" patch.

Are you 100% sure that was the right direction?

I think the right approach is more along these lines:

--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -748,16 +748,11 @@ if (!$force) {
        }
 }

-my $prompting = 0;
 if (!defined $sender) {
        $sender = $repoauthor || $repocommitter || '';
-       $sender = ask("Who should the emails appear to be from? [$sender] ",
-                     default => $sender,
-                     valid_re => qr/\@.*\./, confirm_only => 1);
-       print "Emails will be sent from: ", $sender, "\n";
-       $prompting++;
 }

+my $prompting = 0;
 if (!@initial_to && !defined $to_cmd) {
        my $to = ask("Who should the emails be sent to (if any)? ",
                     default => "",
diff --git a/ident.c b/ident.c
index a4bf206..c73ba82 100644
--- a/ident.c
+++ b/ident.c
@@ -291,9 +291,9 @@ const char *fmt_ident(const char *name, const char *email,
        }

        if (strict && email == git_default_email.buf &&
-           strstr(email, "(none)")) {
+               !(user_ident_explicitly_given & IDENT_MAIL_GIVEN)) {
                fputs(env_hint, stderr);
-               die("unable to auto-detect email address (got '%s')", email);
+               die("no explicit email address");
        }

        if (want_date) {

With that we get:

2) Full Name + full hostname

fatal: no explicit email address

3) Full Name + EMAIL

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

4) config user

From: Felipe Contreras 2nd <felipe.contreras+2@gmail.com>

5) GIT_COMMITTER

From: Felipe Contreras 2nd <felipe.contreras+2@gmail.com>

(as buggy as before)

6) GIT_AUTHOR

From: Felipe Contreras 4th <felipe.contreras+4@gmail.com>

Not only will this fix 'git send-email', but it will also fix 'git
commit' so that we don't end up with authors such as 'Felipe Contreras
<felipec@nysa.felipec.org>' ever again.

> But it is not my itch to scratch.

Suit yourself, it's only git users that would get hurt. I can always
use my own 'git send-email' (as I am doing right now).

Cheers.

-- 
Felipe Contreras

^ permalink raw reply related

* Re: Re: [PATCH] git tag --contains : avoid stack overflow
From: Jeff King @ 2012-11-13  6:08 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johannes Schindelin, Jean-Jacques Lafay, René Scharfe,
	msysgit, Git List, Philip Oakley
In-Reply-To: <7vwqxqf6li.fsf@alter.siamese.dyndns.org>

On Mon, Nov 12, 2012 at 08:51:37PM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > Yeah. We tolerate a certain amount of skew (24 hours for --name-rev, and
> > 5 broken commits in a row for --since). But the big ones are usually
> > software bugs (the big kernel ones were from broken "guilt", I think) or
> > broken imports (when I published a bunch of skew statistics last year,
> > the interesting ones were all imports; I don't know if they were
> > software bugs, or just garbage in, garbage out).
> 
> I was hoping that 2e6bdd3 (test-generation: compute generation
> numbers and clock skews, 2012-09-04) may be a good first step to
> come up with a practical and cheap solution on top of it.
>
> The traversal can be fooled by clock skews when it sees a commit
> that has a timestamp that is older than it should, causing it to
> give up, incorrectly thinking that there won't be newer commits that
> it is interested in behind the problematic commit.

I wrote a similar skew-finding tool last year, though some of the
numbers it came up with were different (I remember having many fewer
skewed commits in the kernel repo).

One problem is that it identifies commits which behave badly with
certain algorithms, but it does not identify commits which are wrong.
If I skew backwards, it will find my commit. But if I skew forwards, it
will label my children as wrong.

> The logic implemented by the change is to identify these problematic
> commits, and we could record these commits with the value of the
> timestamps they should have had (e.g. the timestamp of the newest
> ancestor for each of these commits) in a notes tree.  Then the
> traversal logic (commit-list-insert-by-date) could be updated use
> that "corrected" timestamp instead not to be fooled by the clock
> skew.
> 
> Such a notes tree can be built once and updated by only "appending",
> as a commit will never acquire more ancestors in its parents chain
> once it is made.
> 
> Is it too simplistic, or too costly?  In git.git we have three such
> commits whose timestamp need to be corrected, while in the Linux
> kernel there were 2.2k skewed commits when I counted them a few
> months ago.

This came up in the big generations discussion last summer, and I think
I even implemented a proof of concept. I couldn't find the actual code,
though but only that I got "pleasing performance results using a notes
tree to store a list of commits with bogus timestamps":

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

It is a little wasteful in space if you have a lot of skewed commits
(the notes tree stores a 160-bit hash pointing to a blob object storing
a 32-bit integer).

My personal preference at this point would be:

  1. introduce an auxiliary metadata file that would live alongside the
     pack index and contain generation numbers

  2. generate the metadata file during pack indexing.

  3. If we have a generation metadata file, but a particular object is
     not in it, compute the generation; this should be quick because we
     will hit a file with a stored generation eventually

  4. If we do not have any generation metadata files, or if grafts or
     replace objects are in use, do not use cutoffs in algorithms. Be
     safe but slow.

On the other hand, just switching to doing a single traversal instead of
one merge-base computation per tag already got rid of the really awful
performance cases. Nobody has complained since that went in, so maybe
nobody cares about shaving a few seconds per operation down to a few
tens of milliseconds. The real win was shaving tens of seconds down to a
few seconds.

-Peff

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply

* Re: [PATCH 1/4] remote-hg: add missing config for basic tests
From: Jeff King @ 2012-11-13  5:48 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git, Ramkumar Ramachandra
In-Reply-To: <CAMP44s2GzX_+6+eNU5n6kGue_-MWvU9qNXAr3Caw3A7nWhkawA@mail.gmail.com>

On Tue, Nov 13, 2012 at 04:46:36AM +0100, Felipe Contreras wrote:

> >> +setup () {
> >> +     (
> >> +     echo "[ui]"
> >> +     echo "username = A U Thor <author@example.com>"
> >> +     ) >> "$HOME"/.hgrc
> >> +}
> >
> > This makes sense, but I wonder if we should use something different from
> > the git author ident set up by the test scripts, just to double check
> > that we do not have any bugs in confusing the two during the import.
> 
> I don't know, but these tests would not check for any of those issues.
> When such tests are added I would prefer the author to use to be
> explicitly defined, but lets see.

It's OK if we do not add more explicit tests at this point. I'd just
rather set a safer precedent on the off chance that it might catch
something in a later test, just as we use separate GIT_AUTHOR_* and
GIT_COMMITTER_* in the rest of the test suite. If the choice were not
completely arbitrary and had some maintenance cost, I might be more
concerned, but as far as I can tell, one name is as good as another at
this point.

Any objection to me marking it up as I apply?

-Peff

^ permalink raw reply

* Re: [PATCH 3/3] submodule: display summary header in bold
From: Jeff King @ 2012-11-13  5:44 UTC (permalink / raw)
  To: Ramkumar Ramachandra; +Cc: Git List, Jens Lehmann
In-Reply-To: <1352653146-3932-4-git-send-email-artagnon@gmail.com>

On Sun, Nov 11, 2012 at 10:29:06PM +0530, Ramkumar Ramachandra wrote:

> Currently, 'git diff --submodule' displays output with a bold diff
> header for non-submodules.  So this part is in bold:
> 
>     diff --git a/file1 b/file1
>     index 30b2f6c..2638038 100644
>     --- a/file1
>     +++ b/file1
> 
> For submodules, the header looks like this:
> 
>     Submodule submodule1 012b072..248d0fd:
> 
> Unfortunately, it's easy to miss in the output because it's not bold.
> Change this.

Reading this, I was at first concerned that you were using "bold" and
not DIFF_METAINFO. But:

> diff --git a/diff.c b/diff.c
> index b486070..51c0d6c 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -2267,7 +2267,7 @@ static void builtin_diff(const char *name_a,
>  		const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
>  		show_submodule_summary(o->file, one ? one->path : two->path,
>  				one->sha1, two->sha1, two->dirty_submodule,
> -				del, add, reset);
> +				set, del, add, reset);

This is correctly passing the DIFF_METAINFO color, which is in the "set"
variable. Good. I looked at the output on a sample repository, and it is
much easier to read (though I use magenta for my metainfo).

"set" is a terrible name for that variable (not your fault, of course).
When you re-roll (and I think you need to because of the diff_ui thing
in patch 2), would you mind throwing this cleanup in the middle?

-- >8 --
Subject: builtin_diff: rename "set" variable

Once upon a time the builtin_diff function used one color, and the color
variables were called "set" and "reset". Nowadays it is a much longer
function and we use several colors (e.g., "add", "del"). Rename "set" to
"meta" to show that it is the color for showing diff meta-info (it still
does not indicate that it is a "color", but at least it matches the
scheme of the other color variables).

Signed-off-by: Jeff King <peff@peff.net>
---
 diff.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/diff.c b/diff.c
index e89a201..978e1af 100644
--- a/diff.c
+++ b/diff.c
@@ -2222,7 +2222,7 @@ static void builtin_diff(const char *name_a,
 	mmfile_t mf1, mf2;
 	const char *lbl[2];
 	char *a_one, *b_two;
-	const char *set = diff_get_color_opt(o, DIFF_METAINFO);
+	const char *meta = diff_get_color_opt(o, DIFF_METAINFO);
 	const char *reset = diff_get_color_opt(o, DIFF_RESET);
 	const char *a_prefix, *b_prefix;
 	struct userdiff_driver *textconv_one = NULL;
@@ -2269,24 +2269,24 @@ static void builtin_diff(const char *name_a,
 	b_two = quote_two(b_prefix, name_b + (*name_b == '/'));
 	lbl[0] = DIFF_FILE_VALID(one) ? a_one : "/dev/null";
 	lbl[1] = DIFF_FILE_VALID(two) ? b_two : "/dev/null";
-	strbuf_addf(&header, "%s%sdiff --git %s %s%s\n", line_prefix, set, a_one, b_two, reset);
+	strbuf_addf(&header, "%s%sdiff --git %s %s%s\n", line_prefix, meta, a_one, b_two, reset);
 	if (lbl[0][0] == '/') {
 		/* /dev/null */
-		strbuf_addf(&header, "%s%snew file mode %06o%s\n", line_prefix, set, two->mode, reset);
+		strbuf_addf(&header, "%s%snew file mode %06o%s\n", line_prefix, meta, two->mode, reset);
 		if (xfrm_msg)
 			strbuf_addstr(&header, xfrm_msg);
 		must_show_header = 1;
 	}
 	else if (lbl[1][0] == '/') {
-		strbuf_addf(&header, "%s%sdeleted file mode %06o%s\n", line_prefix, set, one->mode, reset);
+		strbuf_addf(&header, "%s%sdeleted file mode %06o%s\n", line_prefix, meta, one->mode, reset);
 		if (xfrm_msg)
 			strbuf_addstr(&header, xfrm_msg);
 		must_show_header = 1;
 	}
 	else {
 		if (one->mode != two->mode) {
-			strbuf_addf(&header, "%s%sold mode %06o%s\n", line_prefix, set, one->mode, reset);
-			strbuf_addf(&header, "%s%snew mode %06o%s\n", line_prefix, set, two->mode, reset);
+			strbuf_addf(&header, "%s%sold mode %06o%s\n", line_prefix, meta, one->mode, reset);
+			strbuf_addf(&header, "%s%snew mode %06o%s\n", line_prefix, meta, two->mode, reset);
 			must_show_header = 1;
 		}
 		if (xfrm_msg)

^ permalink raw reply related

* Re: [PATCH 2/3] diff: introduce diff.submodule configuration variable
From: Jeff King @ 2012-11-13  5:33 UTC (permalink / raw)
  To: Ramkumar Ramachandra; +Cc: Git List, Jens Lehmann
In-Reply-To: <1352653146-3932-3-git-send-email-artagnon@gmail.com>

On Sun, Nov 11, 2012 at 10:29:05PM +0530, Ramkumar Ramachandra wrote:

> +static int parse_submodule_params(struct diff_options *options, const char *value,
> +				struct strbuf *errmsg)
> +{
> +	if (!strcmp(value, "log"))
> +		DIFF_OPT_SET(options, SUBMODULE_LOG);
> +	else if (!strcmp(value, "short"))
> +		DIFF_OPT_CLR(options, SUBMODULE_LOG);
> +	else {
> +		strbuf_addf(errmsg, _("'%s'"), value);
> +		return 1;
> +	}
> +	return 0;
> +}

I think "-1" would be the more normal error return.

> @@ -223,6 +238,15 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
>  		return 0;
>  	}
>  
> +	if (!strcmp(var, "diff.submodule")) {

Shouldn't this be in git_diff_ui_config so it does not affect scripts
calling plumbing?

> +		struct strbuf errmsg = STRBUF_INIT;
> +		if (parse_submodule_params(&default_diff_options, value, &errmsg))
> +			warning(_("Unknown value for 'diff.submodule' config variable: %s"),
> +				errmsg.buf);
> +		strbuf_release(&errmsg);
> +		return 0;
> +	}

Hmm. This strbuf error handling strikes me as very clunky, considering
that it does not pass any useful information out of the parse function
(it always just adds '$value' to the error string).  Wouldn't it be
simpler to just have parse_submodule_params return -1, and then let the
caller warn or generate an error as appropriate?

-Peff

^ permalink raw reply

* Re: Test failures in contrib/remote-helpers
From: Ramkumar Ramachandra @ 2012-11-13  5:29 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Git List
In-Reply-To: <CAMP44s1E7qKCPTH1MMuOwJkW8opmD+Q6bzD6j1-KM039g-Uq2g@mail.gmail.com>

Felipe Contreras wrote:
> And here's the one for bzr:
>
> --- a/contrib/remote-helpers/git-remote-bzr
> +++ b/contrib/remote-helpers/git-remote-bzr
> @@ -646,12 +646,12 @@ def get_repo(url, alias):
>      global dirname, peer
>
>      clone_path = os.path.join(dirname, 'clone')
> -    origin = bzrlib.controldir.ControlDir.open(url)
> +    origin = bzrlib.bzrdir.BzrDir.open(url)
>      remote_branch = origin.open_branch()
>
>      if os.path.exists(clone_path):
>          # pull
> -        d = bzrlib.controldir.ControlDir.open(clone_path)
> +        d = bzrlib.bzrdir.BzrDir.open(clone_path)
>          branch = d.open_branch()
>          result = branch.pull(remote_branch, [], None, False)
>      else:
>
> I have other fixes to run up to 2.0.

Works for me.  All four tests pass now.
Reported-by: Ramkumar Ramachandra <artagnon@gmail.com>
Tested-by: Ramkumar Ramachandra <artagnon@gmail.com>

Ram

^ permalink raw reply

* Re: [PATCH 2/4] remote-hg: fix compatibility with older versions of hg
From: Ramkumar Ramachandra @ 2012-11-13  5:25 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git
In-Reply-To: <1352742068-15346-3-git-send-email-felipe.contreras@gmail.com>

This patch fixes my original problem. So,
Reported-by: Ramkumar Ramachandra <artagnon@gmail.com>
Tested-by: Ramkumar Ramachandra <artagnon@gmail.com>

However, test 4 in test-hg.sh still fails for me:

--- expected    2012-11-13 05:22:57.946637384 +0000
+++ actual      2012-11-13 05:22:57.946637384 +0000
@@ -1,2 +1,2 @@
-zero
-refs/heads/master
+feature-a
+refs/heads/feature-a

What is going on?

Ram

^ permalink raw reply

* Re: [PATCH v3 0/3] Introduce diff.submodule
From: Ramkumar Ramachandra @ 2012-11-13  5:17 UTC (permalink / raw)
  To: Git List; +Cc: Jens Lehmann, Jeff King, Junio C Hamano
In-Reply-To: <1352653146-3932-1-git-send-email-artagnon@gmail.com>

Ramkumar Ramachandra wrote:
> v1 is here: http://mid.gmane.org/1349196670-2844-1-git-send-email-artagnon@gmail.com
> v2 is here: http://mid.gmane.org/1351766630-4837-1-git-send-email-artagnon@gmail.com
>
> This version was prepared in response to Peff's review of v2.  As
> suggested, I've created a separate function which both '--submodule'
> and 'diff.submodule' use to set/ unset SUBMODULE_OPT.

Junio, can we pick this up?

Ram

^ permalink raw reply

* Re: Re: [PATCH] git tag --contains : avoid stack overflow
From: Johannes Schindelin @ 2012-11-13  4:52 UTC (permalink / raw)
  To: Jeff King
  Cc: Jean-Jacques Lafay, René Scharfe, msysgit, Git List,
	Philip Oakley
In-Reply-To: <20121113040541.GA9439@sigill.intra.peff.net>

Hi Peff,

On Mon, 12 Nov 2012, Jeff King wrote:

> On Tue, Nov 13, 2012 at 04:01:11AM +0000, Johannes Schindelin wrote:
> 
> > > Note that name-rev will produce wrong answers in the face of clock skew.
> > > And I think that you even wrote that code. :)
> > 
> > IIRC the cute code to short-circuit using the date is not from me. If it
> > is, I am very ashamed.
> 
> Sorry, but it was:
> 
>   $ git blame -L'/commit->date < cutoff/',+1  builtin/name-rev.c
>   bd321bcc name-rev.c (Johannes Schindelin 2005-10-26 15:10:20 +0200 32)
>   if (commit->date < cutoff)
> 
> But it is never too late to fix it. :)

I will now go and find a hole to hide in. Or alternatively finally go to
sleep.

Ciao,
Johannes

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply

* Re: Re: [PATCH] git tag --contains : avoid stack overflow
From: Junio C Hamano @ 2012-11-13  4:51 UTC (permalink / raw)
  To: Jeff King
  Cc: Johannes Schindelin, Jean-Jacques Lafay, René Scharfe,
	msysgit, Git List, Philip Oakley
In-Reply-To: <20121113034605.GB8387@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Yeah. We tolerate a certain amount of skew (24 hours for --name-rev, and
> 5 broken commits in a row for --since). But the big ones are usually
> software bugs (the big kernel ones were from broken "guilt", I think) or
> broken imports (when I published a bunch of skew statistics last year,
> the interesting ones were all imports; I don't know if they were
> software bugs, or just garbage in, garbage out).

I was hoping that 2e6bdd3 (test-generation: compute generation
numbers and clock skews, 2012-09-04) may be a good first step to
come up with a practical and cheap solution on top of it.

The traversal can be fooled by clock skews when it sees a commit
that has a timestamp that is older than it should, causing it to
give up, incorrectly thinking that there won't be newer commits that
it is interested in behind the problematic commit.

The logic implemented by the change is to identify these problematic
commits, and we could record these commits with the value of the
timestamps they should have had (e.g. the timestamp of the newest
ancestor for each of these commits) in a notes tree.  Then the
traversal logic (commit-list-insert-by-date) could be updated use
that "corrected" timestamp instead not to be fooled by the clock
skew.

Such a notes tree can be built once and updated by only "appending",
as a commit will never acquire more ancestors in its parents chain
once it is made.

Is it too simplistic, or too costly?  In git.git we have three such
commits whose timestamp need to be corrected, while in the Linux
kernel there were 2.2k skewed commits when I counted them a few
months ago.

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply

* Re: Re: [PATCH] git tag --contains : avoid stack overflow
From: Jeff King @ 2012-11-13  4:05 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Jean-Jacques Lafay, René Scharfe, msysgit, Git List,
	Philip Oakley
In-Reply-To: <alpine.DEB.1.00.1211130400030.13573@bonsai2>

On Tue, Nov 13, 2012 at 04:01:11AM +0000, Johannes Schindelin wrote:

> > Note that name-rev will produce wrong answers in the face of clock skew.
> > And I think that you even wrote that code. :)
> 
> IIRC the cute code to short-circuit using the date is not from me. If it
> is, I am very ashamed.

Sorry, but it was:

  $ git blame -L'/commit->date < cutoff/',+1  builtin/name-rev.c
  bd321bcc name-rev.c (Johannes Schindelin 2005-10-26 15:10:20 +0200 32)
  if (commit->date < cutoff)

But it is never too late to fix it. :)

-Peff

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply

* Re: Re: [PATCH] git tag --contains : avoid stack overflow
From: Johannes Schindelin @ 2012-11-13  4:01 UTC (permalink / raw)
  To: Jeff King
  Cc: Jean-Jacques Lafay, René Scharfe, msysgit, Git List,
	Philip Oakley
In-Reply-To: <20121113034605.GB8387@sigill.intra.peff.net>

Hi Peff,

On Mon, 12 Nov 2012, Jeff King wrote:

> On Tue, Nov 13, 2012 at 01:16:01AM +0000, Johannes Schindelin wrote:
> 
> > > We can do much better than O(number of commits), though, if we stop
> > > traversing down a path when its timestamp shows that it is too old to
> > > contain the commits we are searching for. The problem is that the
> > > timestamps cannot always be trusted, because they are generated on
> > > machines with wrong clocks, or by buggy software. This could be solved
> > > by calculating and caching a "generation" number, but last time it was
> > > discussed there was a lot of arguing and nothing got done.
> > 
> > Sadly, not only machines with skewed clocks, but in particular buggy
> > 3rd-party SCMs make this more than just problematic. In a git-svn clone
> > that was used as base for heavy Git development, I encountered quite a lot
> > of Jan 1, 1970 commits.
> 
> Yeah. We tolerate a certain amount of skew (24 hours for --name-rev, and
> 5 broken commits in a row for --since). But the big ones are usually
> software bugs (the big kernel ones were from broken "guilt", I think) or
> broken imports (when I published a bunch of skew statistics last year,
> the interesting ones were all imports; I don't know if they were
> software bugs, or just garbage in, garbage out).
> 
> > It just cannot be helped, we must distrust timestamps completely.
> 
> Note that name-rev will produce wrong answers in the face of clock skew.
> And I think that you even wrote that code. :)

IIRC the cute code to short-circuit using the date is not from me. If it
is, I am very ashamed.

Ciao,
Johannes

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply

* Re: [PATCH] send-email: add proper default sender
From: Jeff King @ 2012-11-13  4:01 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <CAMP44s0SDHVzSd-8Rq7Z1sbiQ6m0pxX+2pgx16_DoWnHeyNsNQ@mail.gmail.com>

On Tue, Nov 13, 2012 at 04:55:25AM +0100, Felipe Contreras wrote:

> > No, it's not. Those broken names do not come from the environment, but
> > from our last-resort guess of the hostname.
> 
> That depends how you define environment, but fine, the point is that
> it happens.

If you have a strawman definition that does not have anything to do with
what I said in my original email, then yes, it could happen. But as I
said already, "git var" uses IDENT_STRICT and will not allow such broken
names.

> > We long ago switched to
> > printing the name as a warning when we have made such a guess (bb1ae3f),
> > then more recently started rejecting them outright (8c5b1ae).
> 
> Right, but these would still happen:
> 
> michael <michael@michael-laptop.michael.org>

Did you read my email? I explicitly proposed that we would _not_ allow
send-email to use implicit email addresses constructed in that way.

> > But in the meantime you are causing a regression for anybody who expects
> > GIT_AUTHOR_NAME to override user.email when running git-send-email (and
> > you have taken away the prompt that they could have used to notice and
> > correct it).
> 
> I think they can survive. If anybody like this exists.

Sorry, but that is not how things work on this project. You do not get
to cause regressions because you are too lazy to implement the feature
_you_ want in a way that does not break other people.

I tried to help you by pointing you in the right direction and even
providing a sample "git var" patch. But it is not my itch to scratch.

-Peff

^ permalink raw reply

* Re: [PATCH] send-email: add proper default sender
From: Felipe Contreras @ 2012-11-13  3:55 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <20121113032727.GA8387@sigill.intra.peff.net>

On Tue, Nov 13, 2012 at 4:27 AM, Jeff King <peff@peff.net> wrote:
> On Tue, Nov 13, 2012 at 01:54:59AM +0100, Felipe Contreras wrote:
>
>> > But we use the environment to default the field, so the distinction
>> > doesn't make much sense to me.  Plus, it has always been the case that
>> > you can use git without setting user.*, but instead only using the
>> > environment. I don't see any reason not to follow that principle here,
>> > too.
>>
>> And that's why a lot of commits end up like michael
>> <michael@michael-laptop.(none)>.
>
> No, it's not. Those broken names do not come from the environment, but
> from our last-resort guess of the hostname.

That depends how you define environment, but fine, the point is that it happens.

> We long ago switched to
> printing the name as a warning when we have made such a guess (bb1ae3f),
> then more recently started rejecting them outright (8c5b1ae).

Right, but these would still happen:

michael <michael@michael-laptop.michael.org>

>> Probably. But what I really want is to stop 'git send-email' from
>> asking. I think the one next step further can be done later.
>
> But in the meantime you are causing a regression for anybody who expects
> GIT_AUTHOR_NAME to override user.email when running git-send-email (and
> you have taken away the prompt that they could have used to notice and
> correct it).

I think they can survive. If anybody like this exists.

-- 
Felipe Contreras

^ permalink raw reply

* Re: [PATCH 1/4] remote-hg: add missing config for basic tests
From: Felipe Contreras @ 2012-11-13  3:46 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Ramkumar Ramachandra
In-Reply-To: <20121112203207.GF4623@sigill.intra.peff.net>

On Mon, Nov 12, 2012 at 9:32 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Nov 12, 2012 at 06:41:05PM +0100, Felipe Contreras wrote:
>
>> From: Ramkumar Ramachandra <artagnon@gmail.com>
>>
>> 'hg commit' fails otherwise in some versiosn of mercurial because of
>
> s/versiosn/versions/
>
>> +setup () {
>> +     (
>> +     echo "[ui]"
>> +     echo "username = A U Thor <author@example.com>"
>> +     ) >> "$HOME"/.hgrc
>> +}
>
> This makes sense, but I wonder if we should use something different from
> the git author ident set up by the test scripts, just to double check
> that we do not have any bugs in confusing the two during the import.

I don't know, but these tests would not check for any of those issues.
When such tests are added I would prefer the author to use to be
explicitly defined, but lets see.

-- 
Felipe Contreras

^ permalink raw reply

* Re: [msysGit] Re: [PATCH] git tag --contains : avoid stack overflow
From: Jeff King @ 2012-11-13  3:46 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Jean-Jacques Lafay, René Scharfe, msysgit, Git List,
	Philip Oakley
In-Reply-To: <alpine.DEB.1.00.1211130114180.13573@bonsai2>

On Tue, Nov 13, 2012 at 01:16:01AM +0000, Johannes Schindelin wrote:

> > We can do much better than O(number of commits), though, if we stop
> > traversing down a path when its timestamp shows that it is too old to
> > contain the commits we are searching for. The problem is that the
> > timestamps cannot always be trusted, because they are generated on
> > machines with wrong clocks, or by buggy software. This could be solved
> > by calculating and caching a "generation" number, but last time it was
> > discussed there was a lot of arguing and nothing got done.
> 
> Sadly, not only machines with skewed clocks, but in particular buggy
> 3rd-party SCMs make this more than just problematic. In a git-svn clone
> that was used as base for heavy Git development, I encountered quite a lot
> of Jan 1, 1970 commits.

Yeah. We tolerate a certain amount of skew (24 hours for --name-rev, and
5 broken commits in a row for --since). But the big ones are usually
software bugs (the big kernel ones were from broken "guilt", I think) or
broken imports (when I published a bunch of skew statistics last year,
the interesting ones were all imports; I don't know if they were
software bugs, or just garbage in, garbage out).

> It just cannot be helped, we must distrust timestamps completely.

Note that name-rev will produce wrong answers in the face of clock skew.
And I think that you even wrote that code. :)

-Peff

^ permalink raw reply

* Re: [PATCH v5 00/14] New remote-hg helper
From: Felipe Contreras @ 2012-11-13  3:45 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: <20121111221712.GB2774@arachsys.com>

On Sun, Nov 11, 2012 at 11:17 PM, Chris Webb <chris@arachsys.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> Implemented now. I'm not handling the 'tip' revision, but most likely
>> it's also the '.' revision. In this case a fake 'master' bookmark will
>> be created to track that revision.
>
> Hi Felipe. Sorry for the slow response, I've been snowed under with work and
> have only just got around to testing your latest version.
>
> The new remote-hg.track-branches=false option is great and does exactly what
> I was hoping for. For the benefit of the list archives, one natural way to
> use it is
>
>   git clone -c remote-hg.track-branches=false hg::foo
>
> when cloning the relevant repositories, if you don't want the setting
> globally for every hg-remote clone.

Cool :)

> During testing, I've seen some strange behaviour which I think is caused by
> using the . revision instead of tip:
>
> $ hg init h
> $ hg init h2
> $ ( cd h && touch foo && hg add foo && hg commit -m foo && hg push ../h2 )
> pushing to ../h2
> searching for changes
> adding changesets
> adding manifests
> adding file changes
> added 1 changesets with 1 changes to 1 files
> $ git clone hg::h g
> Cloning into 'g'...
> $ git clone hg::h2 g2
> Cloning into 'g2'...
> warning: remote HEAD refers to nonexistent ref, unable to checkout.
> $
>
> The reason for this is that by default . == null (not tip) in the repo h2
> which we pushed into from h. The hg equivalent of a bare repo typically has a
> null checkout like this. (Actually, the checkout of HEAD seems to break
> whenever . is different from tip, not just when it's null as in this example.)

Well, I thought in those cases we didn't want HEAD to be updated.
People can still use the repo and checkout whatever branch they want.
But '.' is not  really the equivalent of HEAD. Since there's no
equivalent, I think it makes sense to try first '.', and then 'tip'.
This means that we would never have a HEAD pointing to nowhere.

I've added a patch for that.

Cheers.
-- 
Felipe Contreras

^ permalink raw reply

* Re: [PATCH] send-email: add proper default sender
From: Jeff King @ 2012-11-13  3:40 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <20121113032727.GA8387@sigill.intra.peff.net>

On Mon, Nov 12, 2012 at 10:27:27PM -0500, Jeff King wrote:

> On Tue, Nov 13, 2012 at 01:54:59AM +0100, Felipe Contreras wrote:
> 
> > > But we use the environment to default the field, so the distinction
> > > doesn't make much sense to me.  Plus, it has always been the case that
> > > you can use git without setting user.*, but instead only using the
> > > environment. I don't see any reason not to follow that principle here,
> > > too.
> > 
> > And that's why a lot of commits end up like michael
> > <michael@michael-laptop.(none)>.
> 
> No, it's not. Those broken names do not come from the environment, but
> from our last-resort guess of the hostname. We long ago switched to
> printing the name as a warning when we have made such a guess (bb1ae3f),
> then more recently started rejecting them outright (8c5b1ae).
> 
> And I have proposed exactly the same behavior here: respect the
> environment and the config, but do not trust the implicit guesses.

Re-reading this, I think "them" at the end of the second paragraph is
slightly unclear. Let me rephrase.

The lack of a name or the presence of an obviously bogus email address
(e.g., with "(none)") is disallowed by commit. We still allow implicit
idents on commit as long they are not obviously wrong, but show them to
the user so that they can notice and correct via "commit --amend".

So if it dies on an implicit ident, send-email would actually be taking
an even stronger stance against bogus idents. Which makes sense, since
there is no "--amend" for fixing a broken email that has been sent.

-Peff

^ permalink raw reply

* Re: [PATCH] send-email: add proper default sender
From: Jeff King @ 2012-11-13  3:27 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <CAMP44s16y9WSmnTdb04EMSzXVgzfYP7pSMo6qZi0HY0bjouA0w@mail.gmail.com>

On Tue, Nov 13, 2012 at 01:54:59AM +0100, Felipe Contreras wrote:

> > But we use the environment to default the field, so the distinction
> > doesn't make much sense to me.  Plus, it has always been the case that
> > you can use git without setting user.*, but instead only using the
> > environment. I don't see any reason not to follow that principle here,
> > too.
> 
> And that's why a lot of commits end up like michael
> <michael@michael-laptop.(none)>.

No, it's not. Those broken names do not come from the environment, but
from our last-resort guess of the hostname. We long ago switched to
printing the name as a warning when we have made such a guess (bb1ae3f),
then more recently started rejecting them outright (8c5b1ae).

And I have proposed exactly the same behavior here: respect the
environment and the config, but do not trust the implicit guesses.

> Probably. But what I really want is to stop 'git send-email' from
> asking. I think the one next step further can be done later.

But in the meantime you are causing a regression for anybody who expects
GIT_AUTHOR_NAME to override user.email when running git-send-email (and
you have taken away the prompt that they could have used to notice and
correct it).

-Peff

^ permalink raw reply

* Re: Re: [PATCH] git tag --contains : avoid stack overflow
From: Johannes Schindelin @ 2012-11-13  1:16 UTC (permalink / raw)
  To: Jeff King
  Cc: Jean-Jacques Lafay, René Scharfe, msysgit, Git List,
	Philip Oakley
In-Reply-To: <20121112231453.GA21679@sigill.intra.peff.net>

Hi Peff,

On Mon, 12 Nov 2012, Jeff King wrote:

> On Mon, Nov 12, 2012 at 11:27:14PM +0100, Jean-Jacques Lafay wrote:
> 
> > 2012/11/11 Jeff King <peff@peff.net>:
> > > On Sun, Nov 11, 2012 at 05:46:32PM +0100, René Scharfe wrote:
> > >
> > > Ultimately, I have some ideas for doing this in a breadth-first way,
> > > which would make it more naturally iterative. It would involve
> > > having N bits of storage per commit to check N tags, but it would
> > > mean that we could get accurate answers in the face of clock skew
> > > (like the merge-base calculation, it would merely get slower in the
> > > face of skew).
> > 
> > I guess the optimal algorithm may also depend on the commit graph
> > general shape, but intuitively, I'd say that the critical factor is
> > the number and distribution of tags. As soon as you have a significant
> > number of tags (let's say 1% of the commits are tagged, evenly
> > distributed), you'll quickly end up with every commit marked as
> > containing or not the target commit, so that each additional tag check
> > is cheap.
> > 
> > This suggests a complexity of O(number of commits) more often then
> > not, however you choose to traverse the graph.
> 
> We can do much better than O(number of commits), though, if we stop
> traversing down a path when its timestamp shows that it is too old to
> contain the commits we are searching for. The problem is that the
> timestamps cannot always be trusted, because they are generated on
> machines with wrong clocks, or by buggy software. This could be solved
> by calculating and caching a "generation" number, but last time it was
> discussed there was a lot of arguing and nothing got done.

Sadly, not only machines with skewed clocks, but in particular buggy
3rd-party SCMs make this more than just problematic. In a git-svn clone
that was used as base for heavy Git development, I encountered quite a lot
of Jan 1, 1970 commits.

It just cannot be helped, we must distrust timestamps completely.

Ciao,
Dscho

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply

* [PATCH 2/2] remote-bzr: detect local repositories
From: Felipe Contreras @ 2012-11-13  1:03 UTC (permalink / raw)
  To: git; +Cc: Felipe Contreras
In-Reply-To: <1352768608-20703-1-git-send-email-felipe.contreras@gmail.com>

So we don't create a clone  unnecessarily.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-bzr | 48 ++++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 20 deletions(-)

diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
index 6cdfac6..c5822e4 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -24,6 +24,7 @@ import bzrlib.plugin
 bzrlib.plugin.load_plugins()
 
 import bzrlib.generate_ids
+import bzrlib.transport
 
 import sys
 import os
@@ -613,11 +614,14 @@ def do_export(parser):
         if ref == 'refs/heads/master':
             repo.generate_revision_history(revid, marks.get_tip('master'))
             revno, revid = repo.last_revision_info()
-            if hasattr(peer, "import_last_revision_info_and_tags"):
-                peer.import_last_revision_info_and_tags(repo, revno, revid)
+            if peer:
+                if hasattr(peer, "import_last_revision_info_and_tags"):
+                    peer.import_last_revision_info_and_tags(repo, revno, revid)
+                else:
+                    peer.import_last_revision_info(repo.repository, revno, revid)
+                wt = peer.bzrdir.open_workingtree()
             else:
-                peer.import_last_revision_info(repo.repository, revno, revid)
-            wt = peer.bzrdir.open_workingtree()
+                wt = repo.bzrdir.open_workingtree()
             wt.update()
         print "ok %s" % ref
     print
@@ -649,24 +653,28 @@ def do_list(parser):
 def get_repo(url, alias):
     global dirname, peer
 
-    clone_path = os.path.join(dirname, 'clone')
     origin = bzrlib.bzrdir.BzrDir.open(url)
-    remote_branch = origin.open_branch()
-
-    if os.path.exists(clone_path):
-        # pull
-        d = bzrlib.bzrdir.BzrDir.open(clone_path)
-        branch = d.open_branch()
-        result = branch.pull(remote_branch, [], None, False)
+    branch = origin.open_branch()
+
+    if not isinstance(origin.transport, bzrlib.transport.local.LocalTransport):
+        clone_path = os.path.join(dirname, 'clone')
+        remote_branch = branch
+        if os.path.exists(clone_path):
+            # pull
+            d = bzrlib.bzrdir.BzrDir.open(clone_path)
+            branch = d.open_branch()
+            result = branch.pull(remote_branch, [], None, False)
+        else:
+            # clone
+            d = origin.sprout(clone_path, None,
+                    hardlink=True, create_tree_if_local=False,
+                    source_branch=remote_branch)
+            branch = d.open_branch()
+            branch.bind(remote_branch)
+
+        peer = remote_branch
     else:
-        # clone
-        d = origin.sprout(clone_path, None,
-                hardlink=True, create_tree_if_local=False,
-                source_branch=remote_branch)
-        branch = d.open_branch()
-        branch.bind(remote_branch)
-
-    peer = remote_branch
+        peer = None
 
     return branch
 
-- 
1.8.0

^ permalink raw reply related

* [PATCH 1/2] remote-bzr: add support for older versions of bzr
From: Felipe Contreras @ 2012-11-13  1:03 UTC (permalink / raw)
  To: git; +Cc: Felipe Contreras
In-Reply-To: <1352768608-20703-1-git-send-email-felipe.contreras@gmail.com>

At least as old as 2.0.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-bzr | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
index f8919f4..6cdfac6 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -17,7 +17,8 @@
 import sys
 
 import bzrlib
-bzrlib.initialize()
+if hasattr(bzrlib, "initialize"):
+    bzrlib.initialize()
 
 import bzrlib.plugin
 bzrlib.plugin.load_plugins()
@@ -553,7 +554,7 @@ def parse_commit(parser):
 
     repo.lock_write()
     try:
-        builder = repo.get_commit_builder(parents, None, date, tz, committer, props, revid, False)
+        builder = repo.get_commit_builder(parents, None, date, tz, committer, props, revid)
         try:
             list(builder.record_iter_changes(mtree, mtree.last_revision(), changes))
             builder.finish_inventory()
@@ -612,7 +613,10 @@ def do_export(parser):
         if ref == 'refs/heads/master':
             repo.generate_revision_history(revid, marks.get_tip('master'))
             revno, revid = repo.last_revision_info()
-            peer.import_last_revision_info_and_tags(repo, revno, revid)
+            if hasattr(peer, "import_last_revision_info_and_tags"):
+                peer.import_last_revision_info_and_tags(repo, revno, revid)
+            else:
+                peer.import_last_revision_info(repo.repository, revno, revid)
             wt = peer.bzrdir.open_workingtree()
             wt.update()
         print "ok %s" % ref
@@ -646,12 +650,12 @@ def get_repo(url, alias):
     global dirname, peer
 
     clone_path = os.path.join(dirname, 'clone')
-    origin = bzrlib.controldir.ControlDir.open(url)
+    origin = bzrlib.bzrdir.BzrDir.open(url)
     remote_branch = origin.open_branch()
 
     if os.path.exists(clone_path):
         # pull
-        d = bzrlib.controldir.ControlDir.open(clone_path)
+        d = bzrlib.bzrdir.BzrDir.open(clone_path)
         branch = d.open_branch()
         result = branch.pull(remote_branch, [], None, False)
     else:
-- 
1.8.0

^ permalink raw reply related

* [PATCH 0/2] remote-bzr: trivial updates
From: Felipe Contreras @ 2012-11-13  1:03 UTC (permalink / raw)
  To: git; +Cc: Felipe Contreras

Hi,

Here's a fix for older versions of bzr, and a check for local repositories.

Felipe Contreras (2):
  remote-bzr: add support for older versions of bzr
  remote-bzr: detect local repositories

 contrib/remote-helpers/git-remote-bzr | 54 +++++++++++++++++++++--------------
 1 file changed, 33 insertions(+), 21 deletions(-)

-- 
1.8.0

^ 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