* 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
* 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
* 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] 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] 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\"> </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 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: 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] send-email: add proper default sender
From: Jeff King @ 2012-11-13 0:06 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <20121113000217.GH10531@sigill.intra.peff.net>
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. 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.
-Peff
^ permalink raw reply
* Re: [PATCH] send-email: add proper default sender
From: Jeff King @ 2012-11-13 0:02 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <CAMP44s0emar-C27SX-FDsUVB6Sevuy4fBFHuO2OD6xELCEjmGg@mail.gmail.com>
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.
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 },
};
-Peff
^ permalink raw reply related
* Re: [PATCH] send-email: add proper default sender
From: Felipe Contreras @ 2012-11-12 23:42 UTC (permalink / raw)
To: Jeff King; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <20121112233546.GG10531@sigill.intra.peff.net>
On Tue, Nov 13, 2012 at 12:35 AM, Jeff King <peff@peff.net> wrote:
> On Sun, Nov 11, 2012 at 06:06:50PM +0100, Felipe Contreras wrote:
>
>> There's no point in asking this over and over if the user already
>> properly configured his/her name and email.
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>>
>> I got really tired of 'git send-email' always asking me from which address to send mails... that's already configured.
>
> It should be defaulting to your regular git ident, and you just have to
> hit enter, right?
Yes.
> I think it's probably reasonable to skip that "enter" in most cases. But
> I'm not sure why we ever asked in the first place. What do people input
> there if they are not taking the default?
Beats me.
> 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.
>> my $prompting = 0;
>> if (!defined $sender) {
>> $sender = $repoauthor || $repocommitter || '';
>
> Why not just use $repoauthor or $repocommitter, as the prompt default
> already does?
See above.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 2/2] send-email: add series-cc-cmd option
From: Joe Perches @ 2012-11-12 23:40 UTC (permalink / raw)
To: Felipe Contreras
Cc: Ramkumar Ramachandra, git, Junio C Hamano, Jonathan Nieder,
Pascal Obry
In-Reply-To: <CAMP44s2EyMcYHYGvxZnj90qD5ms2YZ5O6gXv_TTES0H72vUm0g@mail.gmail.com>
On Tue, 2012-11-13 at 00:37 +0100, Felipe Contreras wrote:
> On Tue, Nov 13, 2012 at 12:13 AM, Joe Perches <joe@perches.com> wrote:
> > On Tue, 2012-11-13 at 00:03 +0100, Felipe Contreras wrote:
[]
> >> For --to-cmd and --cc-cmd? So basically you check the dirname of the
> >> argument passed?
> >
> > yes. basename and dirname
>
> Well, the basename is irrelevant, because you don't care witch
> particular patch is being sent, you are going to process all of them
> every time.
Well, I do different actions on cover letter patches
than other patches because for patch sets that touch
lots of files, the cc list can be _very_ long and
that can run afoul of other issues like maximum
recipient counts for various mailing lists.
cheers, Joe
^ permalink raw reply
* Re: [PATCH 2/2] send-email: add series-cc-cmd option
From: Felipe Contreras @ 2012-11-12 23:37 UTC (permalink / raw)
To: Joe Perches
Cc: Ramkumar Ramachandra, git, Junio C Hamano, Jonathan Nieder,
Pascal Obry
In-Reply-To: <1352761982.18715.20.camel@joe-AO722>
On Tue, Nov 13, 2012 at 12:13 AM, Joe Perches <joe@perches.com> wrote:
> On Tue, 2012-11-13 at 00:03 +0100, Felipe Contreras wrote:
>> On Mon, Nov 12, 2012 at 11:52 PM, Joe Perches <joe@perches.com> wrote:
>> > 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.
>>
>> A temporary directory.
>>
>> > 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.
>>
>> For --to-cmd and --cc-cmd? So basically you check the dirname of the
>> argument passed?
>
> yes. basename and dirname
Well, the basename is irrelevant, because you don't care witch
particular patch is being sent, you are going to process all of them
every time.
>> While that works, it means you have to run the same command multiple
>> times, one for each mail.
>
> Shrug. it's not a generally significant cost.
It is when you use 'git blame' and there's a lot of history:
% time git cc-cmd -1 1266686
Peter Krefting <peter@softwolves.pp.se>
junio
Tran Ngoc Quan <vnwildman@gmail.com>
Jiang Xin <worldhello.net@gmail.com>
git cc-cmd -1 1266686 0.23s user 0.16s system 1% cpu 19.991 total
> The script could also output the addresses to yet another file.
Which would be even more hacky.
>> If the command is using something expensive such as 'git blame' and
>> you have many patches, this is particularly bad. Also, it's not
>> elegant :)
>
> Elegant is a beholder viewpoint.
You think a solution that runs the same instructions multiple times
unnecessarily is elegant? All right.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH] send-email: add proper default sender
From: Jeff King @ 2012-11-12 23:35 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <1352653610-2090-1-git-send-email-felipe.contreras@gmail.com>
On Sun, Nov 11, 2012 at 06:06:50PM +0100, Felipe Contreras wrote:
> There's no point in asking this over and over if the user already
> properly configured his/her name and email.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>
> I got really tired of 'git send-email' always asking me from which address to send mails... that's already configured.
It should be defaulting to your regular git ident, and you just have to
hit enter, right?
I think it's probably reasonable to skip that "enter" in most cases. But
I'm not sure why we ever asked in the first place. What do people input
there if they are not taking the default?
> diff --git a/git-send-email.perl b/git-send-email.perl
> index aea66a0..65b8328 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -748,6 +748,17 @@ if (!$force) {
> }
> }
>
> +my $name = Git::config('user.name');
> +my $email = Git::config('user.email');
> +
> +if (defined $email) {
> + if (defined $name) {
> + $sender = sprintf("%s <%s>", $name, $email);
> + } else {
> + $sender = $email;
> + }
> +}
Why not use Git::ident_person() here? It saves some code, and would also
respect environment variables. Or better yet...
> my $prompting = 0;
> if (!defined $sender) {
> $sender = $repoauthor || $repocommitter || '';
Why not just use $repoauthor or $repocommitter, as the prompt default
already does?
-Peff
^ permalink raw reply
* Re: [PATCH] gitweb: git_summary - show $project in title
From: Jeff King @ 2012-11-12 23:25 UTC (permalink / raw)
To: Henrich Schuchardt; +Cc: git
In-Reply-To: <1352611258-11450-1-git-send-email-xypron.glpk@gmx.de>
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\"> </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?
-Peff
^ permalink raw reply
* Re: git log to use .mailmap by default?
From: Jeff King @ 2012-11-12 23:18 UTC (permalink / raw)
To: Jason Timrod; +Cc: git@vger.kernel.org
In-Reply-To: <1352635999.79618.YahooMailNeo@web160306.mail.bf1.yahoo.com>
On Sun, Nov 11, 2012 at 04:13:19AM -0800, Jason Timrod wrote:
> I note that when defining a .mailmap file, that it's honoured by
> git-shotlog(1) by default, but for git-log(1) I have to define an
> entirely new --pretty= formatting option to use it.
>
> Why is this? Why doesn't git-log honour this by default like
> git-shortlog does?
>
> Would there be a way of making .mailmap be the default without needing
> a pretty option each time to redefine the default?
I think it's mostly hysterical raisins. Mailmap grew as a shortlog
feature, and then only later became available via custom format
specifiers like "%aN", in git-blame, etc. And nobody ever updated the
stock formats.
It would make sense to me to add an option to do so, and probably to
turn it on by default for normal human-consumed formats, but probably
not for "--pretty=raw".
-Peff
^ permalink raw reply
* Re: [PATCH] git tag --contains : avoid stack overflow
From: Jeff King @ 2012-11-12 23:14 UTC (permalink / raw)
To: Jean-Jacques Lafay; +Cc: René Scharfe, msysgit, Git List, Philip Oakley
In-Reply-To: <CA+TMuX2p4ck0qXijH+OOcBoveBC42U8PqnXdisau57RXwt1isg@mail.gmail.com>
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.
Another approach, used by the merge-base calculation, is to treat
parents in a breadth-first way, but sort them by timestamp, and walk
backwards to find the merge-base (and if you get to a merge-base, you
can stop). In that case, bad timestamps may cause you to look at extra
commits (because you process a commit prematurely and end up going
deeper than the merge base), but it can never give you a wrong answer.
Thinking on it more, though, the merge-base style of computation would
mean you always have to walk backwards to the oldest tag. Which is in
the same order of magnitude as the number of commits, assuming you have
tags near the start of history. So I think we will always want to keep a
cutoff, anyway, and there is not much point in switching off of a
depth-first approach (but of course it does make sense to use iteration
instead of recursion to do so).
> At least on my almost-real-life repo*, with ~140,000 commits and
> ~2,000 tags, tag --contains takes a few seconds, of course more than
> the worst-case test on a 40,000 commit / 1 tag repo, but still in the
> same order of magnitude.
Try "git rev-list --count --all" to get a sense of how long O(# of
commits) takes. Before the depth-first implementation, "tag --contains"
was O(commits * tags) in the worst case. With depth first, it's
O(commits), and then with the timestamp cutoff, it's really O(commits
since needle), where "needle" is the oldest thing you're looking for.
Here are those numbers on linux-2.6 (with linux-stable tags):
[to get a sense of the repo size]
$ git for-each-ref refs/tags | wc -l
909
$ git rev-list --count --all
363413
[this is O(commits)]
$ time git rev-list --count --all
real 0m4.034s
user 0m3.960s
sys 0m0.056s
[before depth-first, ffc4b80^; you can see that it is much worse than
O(commits), though not as bad as the worst case (because finding the
merge bases for recent tags is not O(commits)]
$ time git tag --contains HEAD~200
real 0m42.838s
user 0m42.527s
sys 0m0.156s
[after depth-first, ffc4b80; you can see that this is O(commits),
because we will go depth-first down to the roots, but do only a
single traversal]
$ time git tag --contains HEAD~200
real 0m3.939s
user 0m3.784s
sys 0m0.140s
[with my generation patches; much faster]
$ time git tag --contains HEAD~200
real 0m0.037s
user 0m0.020s
sys 0m0.012s
I was thinking we had merged the timestamp cutoff (which has the same
performance characteristics as generations, just with the skew issue) to
master, but it looks like we didn't.
-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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox