Git development
 help / color / mirror / Atom feed
* Re: Test failures in contrib/remote-helpers
From: Felipe Contreras @ 2012-11-13  0:35 UTC (permalink / raw)
  To: Ramkumar Ramachandra; +Cc: Git List
In-Reply-To: <CALkWK0k9trxx8NC1GWw-yYzBKhFchrvg2JLeBtyoAkokmv9A0w@mail.gmail.com>

On Sun, Nov 11, 2012 at 1:48 PM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> Felipe Contreras wrote:
>> On Sun, Nov 11, 2012 at 11:32 AM, Ramkumar Ramachandra
>> <artagnon@gmail.com> wrote:
>>> I'm experiencing test failures in contrib/remote-helpers.
>>
>> Which are your versions of hg, and bzr?
>
> Mercurial Distributed SCM (version 1.9.1)
> Bazaar (bzr) 2.4.1

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.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* Re: [PATCH 2/2] send-email: add series-cc-cmd option
From: Junio C Hamano @ 2012-11-13  0:37 UTC (permalink / raw)
  To: Joe Perches
  Cc: Ramkumar Ramachandra, Felipe Contreras, git, Jonathan Nieder,
	Pascal Obry
In-Reply-To: <1352760759.18715.7.camel@joe-AO722>

Joe Perches <joe@perches.com> writes:

> On Tue, 2012-11-13 at 03:21 +0530, Ramkumar Ramachandra wrote:
>> Felipe Contreras wrote:
>> > cc-cmd is only per-file, and many times receipients get lost without
>> > seing the full patch series.
>> 
>> s/seing/seeing
>> 
>> > [...]
>> 
>> Looks good otherwise.
>
> s/receipients/recipients/ too
>
> Practically this is ok but I think it's unnecessary.
>
> Output from git format-patch is always in a single
> directory.

Sorry, but I do not see how the usefulness (or necessity) of this
new option is connected to the fact that you can tell the command to
write the patches into a single (possibly new) directory.  Care to
explain?

> My work flow is to use a script for --to and --cc
> lines that can be set to emit the same addresses for
> all files in a patch series or generate different
> addresses per patch file.

^ permalink raw reply

* Re: [PATCH] gitweb: git_summary - show $project in title
From: Junio C Hamano @ 2012-11-13  0:46 UTC (permalink / raw)
  To: Jeff King; +Cc: Henrich Schuchardt, git
In-Reply-To: <20121112232513.GF10531@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Sun, Nov 11, 2012 at 06:20:58AM +0100, Henrich Schuchardt wrote:
>
>> Gitweb pages are structured by divs of class title with grey background.
>> The shortlog, and the log page show the project name as the first title.
>> Page summary only shows an empty grey box above the project details.
>> This provides an inconstent user experience.
>> 
>> This patch adds the missing project title.
>> 
>> Signed-off-by: Henrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>>  gitweb/gitweb.perl |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> index 10ed9e5..3e1c452 100755
>> --- a/gitweb/gitweb.perl
>> +++ b/gitweb/gitweb.perl
>> @@ -6451,7 +6451,7 @@ sub git_summary {
>>  	git_header_html();
>>  	git_print_page_nav('summary','', $head);
>>  
>> -	print "<div class=\"title\">&nbsp;</div>\n";
>> +	print "<div class=\"title\">$project</div>\n";
>
> I do not have any opinion on whether the intent of the change is good or
> not, but shouldn't $project be run through esc_html() here?

I think the answer is yes.  And if $project needs to be escaped, the
git_feed function you fixed today has another codepath that needs to
be fixed.  When git_get_project_description($project) returns undef,
the description is taken from $project without any escaping.

^ permalink raw reply

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

On Tue, Nov 13, 2012 at 1:02 AM, Jeff King <peff@peff.net> wrote:
> On Tue, Nov 13, 2012 at 12:42:02AM +0100, Felipe Contreras wrote:
>
>> > Why not use Git::ident_person() here? It saves some code, and would also
>> > respect environment variables. Or better yet...
>>
>> I assume there was a reason why that code was asking for input;
>> precisely because it would use the environment variables. For some
>> reason the user might have exported GIT_AUTHOR_EMAIL, or maybe EMAIL
>> is not right, or the full name config.
>>
>> OTOH user.name/.email configurations come clearly from the user.
>
> 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)>.

> The one distinction that would make sense to me is pausing to ask when
> we use "implicit" methods to look up the ident, like concatenating the
> username with the hostname to get the email.
>
> Git::ident uses "git var" to do its lookup, which will use IDENT_STRICT;
> that stops most junk like empty names and bogus domains. But I think we
> would want to go one step further and actually check
> user_ident_sufficiently_given.  Unfortunately, that is not currently
> available outside of C. You'd probably want something like:
>
> diff --git a/builtin/var.c b/builtin/var.c
> index aedbb53..eaf324e 100644
> --- a/builtin/var.c
> +++ b/builtin/var.c
> @@ -26,6 +26,12 @@ static const char *pager(int flag)
>         return pgm;
>  }
>
> +static const char *explicit_ident(int flag)
> +{
> +       git_committer_info(flag);
> +       return user_ident_sufficiently_given() ? "1" : "0";
> +}
> +
>  struct git_var {
>         const char *name;
>         const char *(*read)(int);
> @@ -35,6 +41,7 @@ static struct git_var git_vars[] = {
>         { "GIT_AUTHOR_IDENT",   git_author_info },
>         { "GIT_EDITOR", editor },
>         { "GIT_PAGER", pager },
> +       { "GIT_EXPLICIT_IDENT", explicit_ident },
>         { "", NULL },
>  };

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.

-- 
Felipe Contreras

^ permalink raw reply

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

Jeff King <peff@peff.net> writes:

> On Mon, Nov 12, 2012 at 07:02:17PM -0500, Jeff King wrote:
>
>> The one distinction that would make sense to me is pausing to ask when
>> we use "implicit" methods to look up the ident, like concatenating the
>> username with the hostname to get the email.
>
> By the way, I suspect this is the answer to "what do people type for
> this prompt". It is probably more about a safety on bad ident than it is
> about people routinely updating the information. I actually think it
> would make more sense to drop the prompt entirely and just die when the
> user has not given us a usable ident.

Yeah, I agree with pretty much everything you said in this thread
(including that environment and config are equally likely to reflect
user's wish and it does not make much sense to treat environment as
more suspicious).

> But maybe people who do one-off format-patches would rather type
> their name in a prompt than set an environment variable and re-run
> the program.

s/one-off format-patches/one-off send-email/.  I think dying will
force them to configure their names once (so that later invocations
do not have to stop) while prompting will force them to type their
names every time, so the current behaviour is probably a false
economy.  As long as we caution users in the release notes, it
probably is OK to change the command to die.

^ permalink raw reply

* Re: [PATCH 2/2] send-email: add series-cc-cmd option
From: Felipe Contreras @ 2012-11-13  1:01 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Joe Perches, Ramkumar Ramachandra, git, Jonathan Nieder,
	Pascal Obry
In-Reply-To: <7vd2zigwx6.fsf@alter.siamese.dyndns.org>

On Tue, Nov 13, 2012 at 1:37 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Joe Perches <joe@perches.com> writes:
>
>> On Tue, 2012-11-13 at 03:21 +0530, Ramkumar Ramachandra wrote:
>>> Felipe Contreras wrote:
>>> > cc-cmd is only per-file, and many times receipients get lost without
>>> > seing the full patch series.
>>>
>>> s/seing/seeing
>>>
>>> > [...]
>>>
>>> Looks good otherwise.
>>
>> s/receipients/recipients/ too
>>
>> Practically this is ok but I think it's unnecessary.
>>
>> Output from git format-patch is always in a single
>> directory.
>
> Sorry, but I do not see how the usefulness (or necessity) of this
> new option is connected to the fact that you can tell the command to
> write the patches into a single (possibly new) directory.  Care to
> explain?

Basically 'git send-email' would do something like this:
git format-patch -10 -o /tmp
for x in /tmp/*; do cc_cmd $x; done

And the cc-cmd would do something like:
cc_cmd() { do_stuff_with $(dirname $1)/*.patch }

-- 
Felipe Contreras

^ permalink raw reply

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

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

* 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

* 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: [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 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: [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 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: [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] 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: 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: 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: 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: 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: [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: [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: 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/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


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