* [PATCH v2 06/10] pretty: use mailmap to display username and email
From: Junio C Hamano @ 2013-01-08 0:10 UTC (permalink / raw)
To: git; +Cc: Antoine Pelisse
In-Reply-To: <1357603821-8647-1-git-send-email-gitster@pobox.com>
From: Antoine Pelisse <apelisse@gmail.com>
Use the mailmap information to display the rewritten
username and email address in all log commands.
Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
pretty.c | 58 +++++++++++++++++++++++++++++++++++++---------------------
1 file changed, 37 insertions(+), 21 deletions(-)
diff --git a/pretty.c b/pretty.c
index dffcade..622275c 100644
--- a/pretty.c
+++ b/pretty.c
@@ -387,9 +387,13 @@ void pp_user_info(const struct pretty_print_context *pp,
const char *what, struct strbuf *sb,
const char *line, const char *encoding)
{
+ struct strbuf name;
+ struct strbuf mail;
struct ident_split ident;
- int linelen, namelen;
+ int linelen;
char *line_end, *date;
+ const char *mailbuf, *namebuf;
+ size_t namelen, maillen;
int max_length = 78; /* per rfc2822 */
unsigned long time;
int tz;
@@ -408,42 +412,54 @@ void pp_user_info(const struct pretty_print_context *pp,
if (split_ident_line(&ident, line, linelen))
return;
- namelen = ident.mail_end - ident.name_begin + 1;
+
+ mailbuf = ident.mail_begin;
+ maillen = ident.mail_end - ident.mail_begin;
+ namebuf = ident.name_begin;
+ namelen = ident.name_end - ident.name_begin;
+
+ if (pp->mailmap)
+ map_user(pp->mailmap, &mailbuf, &maillen, &namebuf, &namelen);
+
+ strbuf_init(&mail, 0);
+ strbuf_init(&name, 0);
+
+ strbuf_add(&mail, mailbuf, maillen);
+ strbuf_add(&name, namebuf, namelen);
+
+ namelen = name.len + mail.len + 3; /* ' ' + '<' + '>' */
time = strtoul(ident.date_begin, &date, 10);
tz = strtol(date, NULL, 10);
if (pp->fmt == CMIT_FMT_EMAIL) {
- int display_name_length;
-
- display_name_length = ident.name_end - ident.name_begin;
-
strbuf_addstr(sb, "From: ");
- if (needs_rfc2047_encoding(line, display_name_length, RFC2047_ADDRESS)) {
- add_rfc2047(sb, line, display_name_length,
- encoding, RFC2047_ADDRESS);
+ if (needs_rfc2047_encoding(name.buf, name.len, RFC2047_ADDRESS)) {
+ add_rfc2047(sb, name.buf, name.len,
+ encoding, RFC2047_ADDRESS);
max_length = 76; /* per rfc2047 */
- } else if (needs_rfc822_quoting(line, display_name_length)) {
+ } else if (needs_rfc822_quoting(name.buf, name.len)) {
struct strbuf quoted = STRBUF_INIT;
- add_rfc822_quoted("ed, line, display_name_length);
+ add_rfc822_quoted("ed, name.buf, name.len);
strbuf_add_wrapped_bytes(sb, quoted.buf, quoted.len,
-6, 1, max_length);
strbuf_release("ed);
} else {
- strbuf_add_wrapped_bytes(sb, line, display_name_length,
- -6, 1, max_length);
+ strbuf_add_wrapped_bytes(sb, name.buf, name.len,
+ -6, 1, max_length);
}
- if (namelen - display_name_length + last_line_length(sb) > max_length) {
+ if (namelen - name.len + last_line_length(sb) > max_length)
strbuf_addch(sb, '\n');
- if (!isspace(ident.name_end[0]))
- strbuf_addch(sb, ' ');
- }
- strbuf_add(sb, ident.name_end, namelen - display_name_length);
- strbuf_addch(sb, '\n');
+
+ strbuf_addf(sb, " <%s>\n", mail.buf);
} else {
- strbuf_addf(sb, "%s: %.*s%.*s\n", what,
+ strbuf_addf(sb, "%s: %.*s%s <%s>\n", what,
(pp->fmt == CMIT_FMT_FULLER) ? 4 : 0,
- " ", namelen, line);
+ " ", name.buf, mail.buf);
}
+
+ strbuf_release(&mail);
+ strbuf_release(&name);
+
switch (pp->fmt) {
case CMIT_FMT_MEDIUM:
strbuf_addf(sb, "Date: %s\n", show_date(time, tz, pp->date_mode));
--
1.8.1.304.gf036638
^ permalink raw reply related
* [PATCH v2 09/10] log: grep author/committer using mailmap
From: Junio C Hamano @ 2013-01-08 0:10 UTC (permalink / raw)
To: git; +Cc: Antoine Pelisse
In-Reply-To: <1357603821-8647-1-git-send-email-gitster@pobox.com>
From: Antoine Pelisse <apelisse@gmail.com>
Currently you can use mailmap to display log authors and committers
but you can't use the mailmap to find commits with mapped values.
This commit allows you to run:
git log --use-mailmap --author mapped_name_or_email
git log --use-mailmap --committer mapped_name_or_email
Of course it only works if the --use-mailmap option is used.
The new name and email are copied only when necessary.
Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
revision.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
t/t4203-mailmap.sh | 18 ++++++++++++++++++
2 files changed, 72 insertions(+)
diff --git a/revision.c b/revision.c
index 95d21e6..2cce85a 100644
--- a/revision.c
+++ b/revision.c
@@ -13,6 +13,7 @@
#include "decorate.h"
#include "log-tree.h"
#include "string-list.h"
+#include "mailmap.h"
volatile show_early_output_fn_t show_early_output;
@@ -2219,6 +2220,51 @@ static int rewrite_parents(struct rev_info *revs, struct commit *commit)
return 0;
}
+static int commit_rewrite_person(struct strbuf *buf, const char *what, struct string_list *mailmap)
+{
+ char *person, *endp;
+ size_t len, namelen, maillen;
+ const char *name;
+ const char *mail;
+ struct ident_split ident;
+
+ person = strstr(buf->buf, what);
+ if (!person)
+ return 0;
+
+ person += strlen(what);
+ endp = strchr(person, '\n');
+ if (!endp)
+ return 0;
+
+ len = endp - person;
+
+ if (split_ident_line(&ident, person, len))
+ return 0;
+
+ mail = ident.mail_begin;
+ maillen = ident.mail_end - ident.mail_begin;
+ name = ident.name_begin;
+ namelen = ident.name_end - ident.name_begin;
+
+ if (map_user(mailmap, &mail, &maillen, &name, &namelen)) {
+ struct strbuf namemail = STRBUF_INIT;
+
+ strbuf_addf(&namemail, "%.*s <%.*s>",
+ (int)namelen, name, (int)maillen, mail);
+
+ strbuf_splice(buf, ident.name_begin - buf->buf,
+ ident.mail_end - ident.name_begin + 1,
+ namemail.buf, namemail.len);
+
+ strbuf_release(&namemail);
+
+ return 1;
+ }
+
+ return 0;
+}
+
static int commit_match(struct commit *commit, struct rev_info *opt)
{
int retval;
@@ -2237,6 +2283,14 @@ static int commit_match(struct commit *commit, struct rev_info *opt)
if (buf.len)
strbuf_addstr(&buf, commit->buffer);
+ if (opt->mailmap) {
+ if (!buf.len)
+ strbuf_addstr(&buf, commit->buffer);
+
+ commit_rewrite_person(&buf, "\nauthor ", opt->mailmap);
+ commit_rewrite_person(&buf, "\ncommitter ", opt->mailmap);
+ }
+
/* Append "fake" message parts as needed */
if (opt->show_notes) {
if (!buf.len)
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index db043dc..e16187f 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -248,11 +248,29 @@ Author: Other Author <other@author.xx>
Author: Some Dude <some@dude.xx>
Author: A U Thor <author@example.com>
EOF
+
test_expect_success 'Log output with --use-mailmap' '
git log --use-mailmap | grep Author >actual &&
test_cmp expect actual
'
+cat >expect <<\EOF
+Author: Santa Claus <santa.claus@northpole.xx>
+Author: Santa Claus <santa.claus@northpole.xx>
+EOF
+
+test_expect_success 'Grep author with --use-mailmap' '
+ git log --use-mailmap --author Santa | grep Author >actual &&
+ test_cmp expect actual
+'
+
+>expect
+
+test_expect_success 'Only grep replaced author with --use-mailmap' '
+ git log --use-mailmap --author "<cto@coompany.xx>" >actual &&
+ test_cmp expect actual
+'
+
# git blame
cat >expect <<\EOF
^OBJI (A U Thor DATE 1) one
--
1.8.1.304.gf036638
^ permalink raw reply related
* [PATCH v2 08/10] test: add test for --use-mailmap option
From: Junio C Hamano @ 2013-01-08 0:10 UTC (permalink / raw)
To: git; +Cc: Antoine Pelisse
In-Reply-To: <1357603821-8647-1-git-send-email-gitster@pobox.com>
From: Antoine Pelisse <apelisse@gmail.com>
The new option '--use-mailmap' can be used to make sure that mailmap
file is used to convert name when running log commands.
The test is simple and checks that the Author line
is correctly replaced when running log.
Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t4203-mailmap.sh | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index 1f182f6..db043dc 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -239,6 +239,20 @@ test_expect_success 'Log output (complex mapping)' '
test_cmp expect actual
'
+cat >expect <<\EOF
+Author: CTO <cto@company.xx>
+Author: Santa Claus <santa.claus@northpole.xx>
+Author: Santa Claus <santa.claus@northpole.xx>
+Author: Other Author <other@author.xx>
+Author: Other Author <other@author.xx>
+Author: Some Dude <some@dude.xx>
+Author: A U Thor <author@example.com>
+EOF
+test_expect_success 'Log output with --use-mailmap' '
+ git log --use-mailmap | grep Author >actual &&
+ test_cmp expect actual
+'
+
# git blame
cat >expect <<\EOF
^OBJI (A U Thor DATE 1) one
--
1.8.1.304.gf036638
^ permalink raw reply related
* Re: Version 1.8.1 does not compile on Cygwin 1.7.14
From: Mark Levedahl @ 2013-01-08 3:12 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jason Pyeron, git, Jonathan Nieder, Torsten Bögershausen,
Stephen & Linda Smith, Eric Blake
In-Reply-To: <7v1udxladc.fsf@alter.siamese.dyndns.org>
On 01/07/2013 02:29 AM, Junio C Hamano wrote:
>
> I do not have much stake in this personally, but IIRC, the (l)stat
> workaround was back then found to make Cygwin version from "unusably
> slow" to "slow but torelable", as our POSIX-y codebase assumes that
> lstat is fairly efficient, which Cygwin cannot satisify because it
> has call many win32 calls to collect bits that we do not even look
> at, in order to give faithful emulation. It does place extra
> maintenance burden
The maintenance burden is the only issue here, and I just wanted to
point out the origin. I never enable that run-time option, the small
gain in speed cannot compensate the loss of cross-platform operations
for my uses. Actually, my git usage is mostly on Linux, but it seems 99%
of my maintenance time is on the cygwin side that I almost never use. Sigh.
Mark
^ permalink raw reply
* Re: git.wiki.kernel.org spam ...
From: @ 2013-01-08 3:39 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Theodore Ts'o, git
In-Reply-To: <20130104234759.GC6501@thunk.org>
On 4 January 2013 23:47, Theodore Ts'o <tytso@mit.edu> wrote:
> On Sat, Jan 05, 2013 at 12:27:12AM +0100, Johannes Schindelin wrote:
>> I was. John Hawley trusted me when I asked for admin privileges to keep
>> the spam at bay, but a very vocal voice on the mailing list tried to
>> discredit my work, and in the wake of the ensuing mailing list thread I
>> got the impression that that feeling was universal, so I abided and
>> stopped.
> changed since those early days because LF sysadmins (e.g., John and
> Konstantin) do *not* have time to police the various wikis for
> spam....)
Based on the proliferation of new users listed in some "recent
changes" pages, the account registration step needs a captcha test.
Perhaps some other limits too.. one new account per ip per day or
whatever.. (edu domains excepted perhaps) :)
(the explosion of users seem to have started back in mid 2010 or so.)
No human could keep up with unrestricted automated spamming from the
internet that isn't filtered in any way. Human efforts to remove the
spam will always result in a false positive from time to time and is
simple mistake to make. Automated efforts to remove spam will
DEFINATELY result in false positives too.. but you won't find a
computer that graciously says "sorry about that" when it happens.
IMHO.. a big thanks to Johannes for keeping the beasts at bay, I'm
sure it would have been MUCH worse with nobody cleaning the mess up.
Cheers!
-phil
^ permalink raw reply
* Re: [PATCH 1/4] test: Add target test-lint-shell-syntax
From: Torsten Bögershausen @ 2013-01-08 4:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Torsten Bögershausen, Jeff King, git
In-Reply-To: <7vwqvoj293.fsf@alter.siamese.dyndns.org>
On 07.01.13 19:07, Junio C Hamano wrote:
> Torsten Bögershausen <tboegi@web.de> writes:
>
>> Sorry for late answer, but there is a problem (both linux and Mac OS X) :-(
>> $ make test-lint does not do shel syntax check, neither
>> $ make test-lint-shell-syntax
>
> In which directory?
>
> $ make -C t test-lint-shell-syntax
> ... passes silently ...
> $ ed t/t0000-basic.sh
> /test_expect_success/
> a
> which sh
> .
> w
> q
> $ make -C t test-lint-shell-syntax
> t0000-basic.sh:28: error: which is not portable (please use type): which sh
> make: *** [test-lint-shell-syntax] Error 1
>
> If you edit out '@' (but nothing else) from this line:
>
>> @'$(PERL_PATH_SQ)' check-non-portable-shell.pl $(T)
>
> and run the above again, you would see that it is running this shell
> command:
>
> '/usr/bin/perl' check-non-portable-shell.pl t0000-basic.sh t0001-init.sh ...
>
> If you introduce a Perl syntax error to check-non-portable-shell.pl,
> like this, you will get:
>
> $ make -C t test-lint-shell-syntax
> syntax error at check-non-portable-shell.pl line 11, near "whoa
>
> So... is your shell broken? The above seems to work for dash, bash,
> ksh and zsh.
Thanks for helping me out, and sorry for the noise.
My brain "went off track" while chasing a failure of t7400 on pu under Mac OS :-(
^ permalink raw reply
* Re: [PATCH] git clone depth of 0 not possible.
From: Jonathan Nieder @ 2013-01-08 6:28 UTC (permalink / raw)
To: Stefan Beller; +Cc: schlotter, gitster, Ralf.Wildenhues, git
In-Reply-To: <1357581996-17505-1-git-send-email-stefanbeller@googlemail.com>
Stefan Beller wrote:
> Currently it is not possible to have a shallow depth of
> just 0, i.e. only one commit in that repository after cloning.
> The minimum number of commits is 2, caused by depth=1.
Sounds buggy. Would anything break if we were to make --depth=1 mean
"1 deep, including the tip commit"?
^ permalink raw reply
* Re: [PATCH] git clone depth of 0 not possible.
From: Junio C Hamano @ 2013-01-08 6:54 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Stefan Beller, schlotter, Ralf.Wildenhues, git
In-Reply-To: <20130108062811.GA3131@elie.Belkin>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Stefan Beller wrote:
>
>> Currently it is not possible to have a shallow depth of
>> just 0, i.e. only one commit in that repository after cloning.
>> The minimum number of commits is 2, caused by depth=1.
>
> Sounds buggy. Would anything break if we were to make --depth=1 mean
> "1 deep, including the tip commit"?
As long as we do not change the meaning of the "shallow" count going
over the wire (i.e. the number we receive from the user will be
fudged, so that user's "depth 1" that used to mean "the tip and one
behind it" is expressed as "depth 2" at the end-user level, and we
send over the wire the number that corresponded to the old "depth
1"), I do not think anything will break, and then --depth=0 may
magically start meaning "only the tip; its immediate parents will
not be transferred and recorded as the shallow boundary in the
receiving repository".
I do not mind carrying such a (technially) backward incompatible
change in jn/clone-2.0-depth-off-by-one branch, keep it cooking in
'next' for a while and push it out together with other "2.0" topics
in a future release ;-).
^ permalink raw reply
* Re: [PATCH v2 00/10] reroll of ap/log-mailmap
From: Antoine Pelisse @ 2013-01-08 7:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1357603821-8647-1-git-send-email-gitster@pobox.com>
On Tue, Jan 8, 2013 at 1:10 AM, Junio C Hamano <gitster@pobox.com> wrote:
> This is a reroll of the previous series Antoine posted on Saturday.
Thanks a lot for the reroll Junio
> A new patch "string-list: allow case-insensitive string list"
> teaches the string-list API that some string lists can be sorted
> case insensitively (actually, you can feed any custom two string
> comparison functions).
>
> The string_list_lookup_extended() function introduced by the
> previous series has been discarded. Instead, the third patch
> "mailmap: remove email copy and length limitation" introduces a
> helper function that takes a <char *, size_t> key that is not NUL
> terminated to look for a matching item in a string list, and uses
> that to update map_user() function, together with the fourth
> patch "mailmap: simplify map_user() interface".
>
> All other patches are unmodified from Antoine's series (modulo
> wording tweaks here and there).
>
> Antoine Pelisse (9):
> Use split_ident_line to parse author and committer
> mailmap: remove email copy and length limitation
> mailmap: simplify map_user() interface
> mailmap: add mailmap structure to rev_info and pp
> pretty: use mailmap to display username and email
> log: add --use-mailmap option
> test: add test for --use-mailmap option
> log: grep author/committer using mailmap
> log: add log.mailmap configuration option
>
> Junio C Hamano (1):
> string-list: allow case-insensitive string list
I think this one is missing (and I forgot to reroll it before):
log --use-mailmap: optimize for cases without --author/--committer search
>
> Documentation/config.txt | 4 +
> Documentation/git-log.txt | 5 ++
> builtin/blame.c | 183 ++++++++++++++++++++++------------------------
> builtin/log.c | 16 +++-
> builtin/shortlog.c | 54 ++++----------
> commit.h | 1 +
> log-tree.c | 1 +
> mailmap.c | 94 +++++++++++++++---------
> mailmap.h | 4 +-
> pretty.c | 114 ++++++++++++++++-------------
> revision.c | 54 ++++++++++++++
> revision.h | 1 +
> string-list.c | 17 ++++-
> string-list.h | 4 +
> t/t4203-mailmap.sh | 56 ++++++++++++++
> 15 files changed, 379 insertions(+), 229 deletions(-)
Have you been able to measure a speed increase due to less copies ?
Thanks,
Antoine
^ permalink raw reply
* Re: [PATCH] git clone depth of 0 not possible.
From: Duy Nguyen @ 2013-01-08 7:33 UTC (permalink / raw)
To: Stefan Beller; +Cc: schlotter, gitster, Ralf.Wildenhues, git
In-Reply-To: <1357581996-17505-1-git-send-email-stefanbeller@googlemail.com>
On Tue, Jan 8, 2013 at 1:06 AM, Stefan Beller
<stefanbeller@googlemail.com> wrote:
> Currently it is not possible to have a shallow depth of
> just 0, i.e. only one commit in that repository after cloning.
> The minimum number of commits is 2, caused by depth=1.
>
> I had no good idea how to add this behavior to git clone as
> the depth variable in git_transport_options struct (file transport.h)
> uses value 0 for another meaning, so it would have need changes at
> all places, where the transport options depth is being used
> (e.g. fetch)
>
> So I documented the current behavior, see attached patch.
If we choose not to do the off-by-one topic Junio suggested elsewhere
in the same thread, I think this document patch should be turned into
code instead. Just reject --depth=0 with an explanation. Users who are
hit by this will be caught without the need to read through the
document.
--
Duy
^ permalink raw reply
* Re: [PATCH] git clone depth of 0 not possible.
From: Junio C Hamano @ 2013-01-08 7:36 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Stefan Beller, schlotter, Ralf.Wildenhues, git
In-Reply-To: <7vip78go6b.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> Stefan Beller wrote:
>>
>>> Currently it is not possible to have a shallow depth of
>>> just 0, i.e. only one commit in that repository after cloning.
>>> The minimum number of commits is 2, caused by depth=1.
>>
>> Sounds buggy. Would anything break if we were to make --depth=1 mean
>> "1 deep, including the tip commit"?
>
> As long as we do not change the meaning of the "shallow" count going
> over the wire (i.e. the number we receive from the user will be
> fudged, so that user's "depth 1" that used to mean "the tip and one
> behind it" is expressed as "depth 2" at the end-user level, and we
> send over the wire the number that corresponded to the old "depth
> 1"), I do not think anything will break, and then --depth=0 may
> magically start meaning "only the tip; its immediate parents will
> not be transferred and recorded as the shallow boundary in the
> receiving repository".
>
> I do not mind carrying such a (technially) backward incompatible
> change in jn/clone-2.0-depth-off-by-one branch, keep it cooking in
> 'next' for a while and push it out together with other "2.0" topics
> in a future release ;-).
Speaking of --depth, I think in Git 2.0 we should fix the semantics
of "deepening" done with "git fetch".
Its "--depth" parameter is used to specify the new depth of the
history that you can tangle from the updated tip of remote tracking
branches, and it has a rather unpleasant ramifications.
Suppose you start from "git clone --depth=1 $there". You have the
today's snapshot, and one parent behind it. You keep working happily
with the code and then realize that you want to know a bit more
history behind the snapshot you started from.
(upstream)
---o---o---o---A---B
(you)
A---B
So you do:
$ git fetch --depth=3
(upstream)
---o---o---o---A---B---C---D---E---F---...---W---X---Y---Z
(you)
A---B W---X---Y---Z
But in the meantime, if the upstream accumulated 20+ commits, you
end up getting the commit at the updated tip of the upstream, and 3
generations of parents behind it. There will be a 10+ commit worth
of gap between the bottom of the new shallow history and the old tip
you have been working on, and the history becomes disjoint.
I think we need a protocol update to fix this; instead of sending
"Now I want your tips and N commits behind it, please update my
shallow bottom accordingly", which creates the above by giving you Z
and 3 generations back and updates your cut-off point to W, the
receiving end should be able to ask "I have a shallow history that
cuts off at these commits. I want to get the history leading up to
your tips, and also deepen the history further back from my current
cut-off points by N commits", so that you would instead end up with
something like this:
(you)
o---o---o---A---B---C---D---E---F---...---W---X---Y---Z
That is, truly "deepen my history by 3". We could call that "git
fetch --deepen=3" or something.
^ permalink raw reply
* Re: [PATCH] git clone depth of 0 not possible.
From: Junio C Hamano @ 2013-01-08 7:37 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Stefan Beller, schlotter, Ralf.Wildenhues, git
In-Reply-To: <CACsJy8B0ftDDagTpO4wh-LsBOBy+BhwhV=H-68U246Lq4=Ssfw@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> If we choose not to do the off-by-one topic Junio suggested elsewhere
> in the same thread, I think this document patch should be turned into
> code instead. Just reject --depth=0 with an explanation. Users who are
> hit by this will be caught without the need to read through the
> document.
I thought --depth=0 was a way to explicitly say "I do not want any
shallow history" so far, so we would need to be a bit more careful,
though.
^ permalink raw reply
* Re: [PATCH] git clone depth of 0 not possible.
From: Duy Nguyen @ 2013-01-08 7:38 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, Stefan Beller, schlotter, Ralf.Wildenhues, git
In-Reply-To: <7vip78go6b.fsf@alter.siamese.dyndns.org>
On Tue, Jan 8, 2013 at 1:54 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Sounds buggy. Would anything break if we were to make --depth=1 mean
>> "1 deep, including the tip commit"?
>
> As long as we do not change the meaning of the "shallow" count going
> over the wire (i.e. the number we receive from the user will be
> fudged, so that user's "depth 1" that used to mean "the tip and one
> behind it" is expressed as "depth 2" at the end-user level, and we
> send over the wire the number that corresponded to the old "depth
> 1"), I do not think anything will break, and then --depth=0 may
> magically start meaning "only the tip; its immediate parents will
> not be transferred and recorded as the shallow boundary in the
> receiving repository".
I'd rather we reserve 0 for unlimited fetch, something we haven't done
so far [1]. And because "unlimited clone" with --depth does not make
sense, --depth=0 should be rejected by git-clone.
[1] If we don't want to break the protocol, we could make depth
0xffffffff a special value as "unlimited" for newer git. Older git
works most of the time, until some project exceeds 4G commit depth
history.
--
Duy
^ permalink raw reply
* Re: [PATCH v2 00/10] reroll of ap/log-mailmap
From: Junio C Hamano @ 2013-01-08 7:39 UTC (permalink / raw)
To: Antoine Pelisse; +Cc: git
In-Reply-To: <CALWbr2w8z2iVx2PxM2sn3yDQzB5rTXc4EuZD9GCKSCofHzEzLQ@mail.gmail.com>
Antoine Pelisse <apelisse@gmail.com> writes:
> Have you been able to measure a speed increase due to less copies ?
No.
This topic was not strictly my itch, but I did the rewrite because I
couldn't stand staring at that *_extended() function ;-)
^ permalink raw reply
* Re: [PATCH v2 00/10] reroll of ap/log-mailmap
From: Junio C Hamano @ 2013-01-08 8:02 UTC (permalink / raw)
To: Antoine Pelisse; +Cc: git
In-Reply-To: <7v4nisgm3l.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Antoine Pelisse <apelisse@gmail.com> writes:
>
>> Have you been able to measure a speed increase due to less copies ?
>
> No.
>
> This topic was not strictly my itch, but I did the rewrite because I
> couldn't stand staring at that *_extended() function ;-)
Now I have. The overall numbers are larger primarily because the
kernel history has more commits than back when I did the previous
commit (thanks for reminding me of that one, by the way), but the
basic trend is the same.
The patch with updated numbers in the log is attached.
I am not absolutely sure about the correctness of the third patch in
the series; a review on the logic that uses the insertion point
search to implement the prefix match is very much appreciated.
Thanks.
-- >8 --
Subject: [PATCH] log --use-mailmap: optimize for cases without --author/--committer search
When we taught the commit_match() mechanism to pay attention to the
new --use-mailmap option, we started to unconditionally copy the
commit object to a temporary buffer, just in case we need the author
and committer lines updated via the mailmap mechanism, and rewrite
author and committer using the mailmap.
It turns out that this has a rather unpleasant performance
implications. In the linux kernel repository, running
$ git log --author='Junio C Hamano' --pretty=short >/dev/null
under /usr/bin/time, with and without --use-mailmap (the .mailmap
file is 118 entries long, the particular author does not appear in
it), cost (with warm cache):
[without --use-mailmap]
5.42user 0.26system 0:05.70elapsed 99%CPU (0avgtext+0avgdata 2005936maxresident)k
0inputs+0outputs (0major+137669minor)pagefaults 0swaps
[with --use-mailmap]
6.47user 0.30system 0:06.78elapsed 99%CPU (0avgtext+0avgdata 2006288maxresident)k
0inputs+0outputs (0major+137692minor)pagefaults 0swaps
which incurs about 20% overhead. The command is doing extra work,
so the extra cost may be justified.
But it is inexcusable to pay the cost when we do not need
author/committer match. In the same repository,
$ git log --grep='fix menuconfig on debian lenny' --pretty=short >/dev/null
shows very similar numbers as the above:
[without --use-mailmap]
5.32user 0.30system 0:05.63elapsed 99%CPU (0avgtext+0avgdata 2005984maxresident)k
0inputs+0outputs (0major+137672minor)pagefaults 0swaps
[with --use-mailmap]
6.64user 0.24system 0:06.89elapsed 99%CPU (0avgtext+0avgdata 2006320maxresident)k
0inputs+0outputs (0major+137694minor)pagefaults 0swaps
The latter case is an unnecessary performance regression. We may
want to _show_ the result with mailmap applied, but we do not have
to copy and rewrite the author/committer of all commits we try to
match if we do not query for these fields.
Trivially optimize this performace regression by limiting the
rewrites for only when we are matching with author/committer fields.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
revision.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/revision.c b/revision.c
index 2cce85a..d7562ee 100644
--- a/revision.c
+++ b/revision.c
@@ -2283,7 +2283,7 @@ static int commit_match(struct commit *commit, struct rev_info *opt)
if (buf.len)
strbuf_addstr(&buf, commit->buffer);
- if (opt->mailmap) {
+ if (opt->grep_filter.header_list && opt->mailmap) {
if (!buf.len)
strbuf_addstr(&buf, commit->buffer);
--
1.8.1.304.gf036638
^ permalink raw reply related
* Re: [PATCH] git clone depth of 0 not possible.
From: Junio C Hamano @ 2013-01-08 8:05 UTC (permalink / raw)
To: Duy Nguyen
Cc: Jonathan Nieder, Stefan Beller, schlotter, Ralf.Wildenhues, git
In-Reply-To: <CACsJy8D9+KHT=YfU0+rPCbs+AwxQOpfKzPChDhk8d-MMkRzZug@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> On Tue, Jan 8, 2013 at 1:54 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Sounds buggy. Would anything break if we were to make --depth=1 mean
>>> "1 deep, including the tip commit"?
>>
>> As long as we do not change the meaning of the "shallow" count going
>> over the wire (i.e. the number we receive from the user will be
>> fudged, so that user's "depth 1" that used to mean "the tip and one
>> behind it" is expressed as "depth 2" at the end-user level, and we
>> send over the wire the number that corresponded to the old "depth
>> 1"), I do not think anything will break, and then --depth=0 may
>> magically start meaning "only the tip; its immediate parents will
>> not be transferred and recorded as the shallow boundary in the
>> receiving repository".
>
> I'd rather we reserve 0 for unlimited fetch, something we haven't done
> so far [1]. And because "unlimited clone" with --depth does not make
> sense, --depth=0 should be rejected by git-clone.
I actually was thinking about changing --depth=1 to mean "the tip,
with zero commits behind it" (and that was consistent with my
description of "fudging"), but ended up saying "--depth=0" by
mistake. I too think "--depth=0" or "--depth<0" does not make
sense, so we are in agreement.
Thanks for a sanity check.
^ permalink raw reply
* [PATCH] add warning for depth=0 in git clone.
From: Stefan Beller @ 2013-01-08 8:07 UTC (permalink / raw)
To: Junio C Hamano, Duy Nguyen, Jonathan Nieder, schlotter,
Ralf.Wildenhues, git
Cc: Stefan Beller
---
builtin/clone.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/builtin/clone.c b/builtin/clone.c
index ec2f75b..5e91c1e 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -818,6 +818,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
remote = remote_get(option_origin);
transport = transport_get(remote, remote->url[0]);
+ if (option_depth && transport->smart_options->depth < 1)
+ die(_("--depth less or equal 0 makes no sense; read manpage."));
+
if (!is_local) {
if (!transport->get_refs_list || !transport->fetch)
die(_("Don't know how to clone %s"), transport->url);
--
1.8.1.166.g27cbb96
^ permalink raw reply related
* Re: [PATCH] add warning for depth=0 in git clone.
From: Stefan Beller @ 2013-01-08 8:09 UTC (permalink / raw)
To: Stefan Beller
Cc: Junio C Hamano, Duy Nguyen, Jonathan Nieder, schlotter,
Ralf.Wildenhues, git
In-Reply-To: <1357632422-5686-1-git-send-email-stefanbeller@googlemail.com>
Hi,
I am struggling a little with the development process,
is a sign-off strictly required for git as it is for kernel development?
If so here would be my sign-off:
Signed-off-by: Stefan Beller <stefanbeller@googlemail.com>
This adds a warning and the previous patch adds the documentation.
However I agree to Junio, that the meaning should actually
depth=0 -> just the tip and no (0) other commits before.
depth=n -> the tip and n other commits before.
On 01/08/2013 09:07 AM, Stefan Beller wrote:
> ---
> builtin/clone.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/builtin/clone.c b/builtin/clone.c
> index ec2f75b..5e91c1e 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -818,6 +818,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
> remote = remote_get(option_origin);
> transport = transport_get(remote, remote->url[0]);
>
> + if (option_depth && transport->smart_options->depth < 1)
> + die(_("--depth less or equal 0 makes no sense; read manpage."));
> +
> if (!is_local) {
> if (!transport->get_refs_list || !transport->fetch)
> die(_("Don't know how to clone %s"), transport->url);
>
^ permalink raw reply
* Re: [PATCH] add warning for depth=0 in git clone.
From: Jonathan Nieder @ 2013-01-08 8:12 UTC (permalink / raw)
To: Stefan Beller; +Cc: Junio C Hamano, Duy Nguyen, schlotter, Ralf.Wildenhues, git
In-Reply-To: <1357632422-5686-1-git-send-email-stefanbeller@googlemail.com>
Stefan Beller wrote:
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -818,6 +818,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
> remote = remote_get(option_origin);
> transport = transport_get(remote, remote->url[0]);
>
> + if (option_depth && transport->smart_options->depth < 1)
> + die(_("--depth less or equal 0 makes no sense; read manpage."));
"Makes no sense" is a little harsh. Presumably it made sense to the
user, or she wouldn't have passed that option. The relevant point is
that git was not able to make sense of it.
How about something like
fatal: shallow clone must have positive depth
? The "see manpage" is always implied by die(), and I don't see any
particular reason to point to a specific section here.
Hope that helps,
Jonathan
^ permalink raw reply
* Re: [PATCH] add warning for depth=0 in git clone.
From: Jonathan Nieder @ 2013-01-08 8:14 UTC (permalink / raw)
To: Stefan Beller; +Cc: Junio C Hamano, Duy Nguyen, schlotter, Ralf.Wildenhues, git
In-Reply-To: <50EBD453.9050101@googlemail.com>
Stefan Beller wrote:
> I am struggling a little with the development process,
> is a sign-off strictly required for git as it is for kernel development?
Yes. Documentation/SubmittingPatches has more hints.
> Signed-off-by: Stefan Beller <stefanbeller@googlemail.com>
>
> This adds a warning and the previous patch adds the documentation.
Thanks for your work on this.
^ permalink raw reply
* Re: [PATCH] git clone depth of 0 not possible.
From: Junio C Hamano @ 2013-01-08 8:19 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Stefan Beller, schlotter, Ralf.Wildenhues, git
In-Reply-To: <7vd2xggm8a.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> I think we need a protocol update to fix this; instead of sending
> "Now I want your tips and N commits behind it, please update my
> shallow bottom accordingly", which creates the above by giving you Z
> and 3 generations back and updates your cut-off point to W, the
> receiving end should be able to ask "I have a shallow history that
> cuts off at these commits. I want to get the history leading up to
> your tips, and also deepen the history further back from my current
> cut-off points by N commits", so that you would instead end up with
> something like this:
>
> (you)
> o---o---o---A---B---C---D---E---F---...---W---X---Y---Z
>
> That is, truly "deepen my history by 3". We could call that "git
> fetch --deepen=3" or something.
I take that back. If you start from
> (upstream)
> ---o---o---o---A---B
>
> (you)
> A---B
and you are interested in peeking the history a bit deeper, you
should be able to ask "I have a shallow history that cuts off at
these commits. I want my history deepened by N commits. I do not
care where your current tips are, by the way." with
git fetch --deepen=3
and end up with
> (you)
> o---o---o---A---B
without getting the new history leading to the updated tip at the
upstream. If you want the new history leading to the updated tip,
you can just say:
git fetch
without any --depth nor --deepen option to end up with:
> (you)
> A---B---C---D---E---F---...---W---X---Y---Z
instead.
^ permalink raw reply
* Re: [PATCH 1/8] Use %B for Split Subject/Body
From: greened @ 2013-01-08 10:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Techlive Zheng
In-Reply-To: <7vehi477er.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> The question was about the lossage of the blank line, which does not
> seem to be related to what this patch wants to do.
Ah, missed that.
>>>> -# 25
>>>> +#25
>>>
>>> Why the lossage of a SP?
>>
>> I think this got fixed later in the series.
>
> That is not a good excuse to introduce breakages in the first place, no?
Oh, I agree. I wasn't making excuses. :)
>>> It may make sense to lose these "# num" that will have to be touched
>>> every time somebody inserts new test pieces in the middle, as a
>>> preparatory step before any of these patches, by the way. That will
>>> reduce noise in the patches for real changes.
>>
>> Yeah, I know, but it makes it really easy to find a test when something
>> goes wrong.
>
> That is what "tXXXX-*.sh -i" is for, isn't it?
Oh, I didn't know about that!
-David
^ permalink raw reply
* Re: [PATCH 2/8] Add --unannotate
From: greened @ 2013-01-08 10:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, James Nylen
In-Reply-To: <7va9ss77bu.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> greened@obbligato.org writes:
>
>> In the meantime, will you apply the patch or do you prefer a new design?
>
> The --unannotate option will become a baggage you will have to keep
> working until the end of time, if we applied it. I think it is not
> too uch a baggage, so it probably is OK.
Ok. I think it's worth applying since people find it useful. It's not
very complicated.
-David
^ permalink raw reply
* [PATCH] upload-pack: only accept commits from "shallow" line
From: Nguyễn Thái Ngọc Duy @ 2013-01-08 11:32 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
We only allow cuts at commits, not arbitrary objects. upload-pack will
fail eventually in register_shallow if a non-commit is given with a
generic error "Object %s is a %s, not a commit". Check it early and
give a more accurate error.
This should never show up in an ordinary session. It's for buggy
clients, or when the user manually edits .git/shallow.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
upload-pack.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/upload-pack.c b/upload-pack.c
index 6142421..95d8313 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -603,6 +603,8 @@ static void receive_needs(void)
object = parse_object(sha1);
if (!object)
die("did not find object for %s", line);
+ if (object->type != OBJ_COMMIT)
+ die("invalid shallow object %s", sha1_to_hex(sha1));
object->flags |= CLIENT_SHALLOW;
add_object_array(object, NULL, &shallows);
continue;
--
1.8.0.rc0.19.g7bbb31d
^ permalink raw reply related
* Revised git-subtree Patches
From: David A. Greene @ 2013-01-08 12:09 UTC (permalink / raw)
To: git
Here is the set of revised patches to git-subtree. I think I've
got everything cleaned up now.
^ 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