* Re: [PATCH 3/4] pathspec: apply "*.c" optimization from exclude
From: Nguyen Thai Ngoc Duy @ 2012-11-20 13:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vtxsluw3k.fsf@alter.siamese.dyndns.org>
On Tue, Nov 20, 2012 at 4:20 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> $ time git rev-list --quiet HEAD -- '*.c'
>>
>> real 0m40.770s
>> user 0m40.290s
>> sys 0m0.256s
>>
>> With the patch
>>
>> $ time ~/w/git/git rev-list --quiet HEAD -- '*.c'
>>
>> real 0m34.288s
>> user 0m33.997s
>> sys 0m0.205s
>>
>> The above command is not supposed to be widely popular.
>
> Hrm, perhaps. I use "git grep <pattern> -- \*.c" quite a lot, but
> haven't seen use case for \*.c in the context of the "log" family.
"git diff *.c" is also helpful (maybe "git diff *.[ch]" is more often
used). But I suspect I/O dominates in both grep and diff cases. I just
try to make sure matching code won't show up in profile.
>> +#define PSF_ONESTAR 1
>
> Together with the GF_ prefix in the previous, PSF_ prefix needs a
> bit of in-code explanation. Is it just an RC3L (random combination
> of 3 letters?)
I'm pretty sure "PS" stands for pathspec. "F" is probably from
fnmatch. Any suggestions?
>> @@ -46,6 +46,12 @@ inline int git_fnmatch(const char *pattern, const char *string,
>> pattern += prefix;
>> string += prefix;
>> }
>> + if (flags & GF_ONESTAR) {
>> + int pattern_len = strlen(++pattern);
>> + int string_len = strlen(string);
>> + return strcmp(pattern,
>> + string + string_len - pattern_len);
>> + }
>
> What happens when pattern="foo*oob" and string="foob"?
>
> The prefix match before this code determines that the literal prefix
> in the pattern matches with the early part of the string, and makes
> pattern="*oob" and string="b".
>
> When you come to strcmp(), you see that string_len is 1, pattern_len
> is 3, and pattern is "oob". string+string_len-pattern_len = "oob",
> one past the beginning of the original string "foob". They match.
>
> Oops?
Oops indead. I'll need to check exclude code too :(
> return (string_len < pattern_len) ||
> strcmp(pattern, string + string_len - pattern_len);
>
> perhaps?
Yeah.
--
Duy
^ permalink raw reply
* Re: [PATCH] tcsh-completion re-using git-completion.bash
From: Marc Khouzam @ 2012-11-20 14:58 UTC (permalink / raw)
To: Felipe Contreras; +Cc: Junio C Hamano, SZEDER Gábor, git
In-Reply-To: <CAMP44s30wYnkQdq8yup3z-t=FEf1R+k8OC-o7-uY=19z9VHDPg@mail.gmail.com>
On Sat, Nov 17, 2012 at 1:01 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>
>> I gather that using a wrapper for zsh causes concerns about
>> backwards-compatibility.
>
> I don't see any concerns.
>
>> if [[ -n ${ZSH_VERSION-} ]]; then
>> # replace below by zsh completion commands calling `bash
>> ${HOME}/.git-completion.bash`
>
>> complete git 'p/*/`bash ${HOME}/.git-completion.bash ${COMMAND_LINE}`/'
>> complete gitk 'p/*/`bash ${HOME}/.git-completion.bash ${COMMAND_LINE}`/'
>
> That doesn't work in zsh. It might be possible to do something
> similar, but it would probably require many more lines.
Hi,
since there doesn't seem to be an agreement that the approach to achieve tcsh
git-completion would be useful for zsh (the other possible shell that could use
it is ksh, but I haven't looked into that), maybe the simplest thing
is to keep the
tcsh solution contained in a tcsh-only script. This is the latest solution as
proposed here:
[1] http://www.mail-archive.com/git@vger.kernel.org/msg12192.html
For reference, the more general solution was proposed here:
[2] http://www.mail-archive.com/git@vger.kernel.org/msg12122.html
If there is interest in merging [1], please let me know and I'll post another
version which adds a check to make sure that the user properly copied
git-completion.bash to be used by the new git-completion.tcsh.
Thanks for your input.
Marc
^ permalink raw reply
* Re: [PATCH] tcsh-completion re-using git-completion.bash
From: Felipe Contreras @ 2012-11-20 15:15 UTC (permalink / raw)
To: Marc Khouzam; +Cc: Junio C Hamano, SZEDER Gábor, git
In-Reply-To: <CAFj1UpHs08seVH8Kb3CuoNTaF+x6vA+ybVTEu0TyLX8NYuuidQ@mail.gmail.com>
On Tue, Nov 20, 2012 at 3:58 PM, Marc Khouzam <marc.khouzam@gmail.com> wrote:
> Hi,
>
> since there doesn't seem to be an agreement that the approach to achieve tcsh
> git-completion would be useful for zsh (the other possible shell that could use
> it is ksh, but I haven't looked into that), maybe the simplest thing
> is to keep the
> tcsh solution contained in a tcsh-only script. This is the latest solution as
> proposed here:
>
> [1] http://www.mail-archive.com/git@vger.kernel.org/msg12192.html
This one is already merged to 'next'.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: git merge commits are non-deterministic? what changed?
From: Ulrich Spörlein @ 2012-11-20 16:22 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Jeff King, Matthieu Moy, Andreas Schwab, git
In-Reply-To: <50A0DD23.4040800@drmicha.warpmail.net>
On Mon, 2012-11-12 at 12:27:31 +0100, Michael J Gruber wrote:
> Ulrich Spörlein venit, vidit, dixit 09.11.2012 19:27:
> > On Fri, 2012-11-09 at 11:16:47 -0500, Jeff King wrote:
> >> On Fri, Nov 09, 2012 at 04:52:48PM +0100, Matthieu Moy wrote:
> >>
> >>> Ulrich Spörlein <uqs@spoerlein.net> writes:
> >>>
> >>>>>> 2. Why the hell is the commit hash dependent on the ordering of the
> >>>>>> parent commits? IMHO it should sort the set of parents before
> >>>>>> calculating the hash ...
> >>>>>
> >>>>> What would be the sort key?
> >>>>
> >>>> Trivially, the hash of the parents itself. So you'd always get
> >>>>
> >>>> ...
> >>>> parent 0000
> >>>> parent 1111
> >>>> parent aaaa
> >>>> parent ffff
> >>>
> >>> That would change the behavior of --first-parent. Or you'd need to
> >>> compute the sha1 of the sorted list, but keep the unsorted one in the
> >>> commit. Possible, but weird ;-).
> >>
> >> Right. The reason that merge parents are stored in the order given on
> >> the command line is not random or because it was not considered. It
> >> encodes a valuable piece of information: did the user merge "foo" into
> >> "bar", or did they merge "bar" into "foo"?
> >>
> >> So I think this discussion is going in the wrong direction; git should
> >> never sort the parents, because the order is meaningful. The original
> >> complaint was that a run of svn2git produced different results on two
> >> different git versions. The important question to me is: did svn2git
> >> feed the parents to git in the same order?
> >>
> >> If it did, and git produced different results, then that is a serious
> >> bug.
> >>
> >> If it did not, then the issue needs to be resolved in svn2git (which
> >> _may_ want to sort the parents that it feeds to git, but it would depend
> >> on whether the order it is currently presenting is meaningful).
> >
> > Yeah, thanks, looks like I have some more work to do. I don't quite get
> > how it could come up with a different order, seeing that it is using svn
> > as the base.
> >
> > Will run some more experiments, thanks for the info so far.
>
> There was a change in the order in which "git cherry-pick A B C" applies
> the commits. It's the only odering affecting change in 1.8.0 that I can
> think of right now.
Just to wrap this up, it was of course a "feature" of the converter,
that resulted in this unrepeatable behavior. The SVN API makes use of
apr_hashes, which were traversed in arbitrary order, hence SVN commits
spanning multiple git-branches would be handled in a non-deterministic
order, leading to randomly ordered parent objects for later git merge
commits.
It it still debatable, whether a merge commit should have a
list-of-parents or a set-of-parents. Changing it to a set-of-parents
(with a well-defined hash function), would have made this problem go
away.
But this will never be changed, it would break the fundamental git
storage model as it is in place now.
Cheers,
Uli
^ permalink raw reply
* Re: [PATCH] tcsh-completion re-using git-completion.bash
From: Marc Khouzam @ 2012-11-20 18:20 UTC (permalink / raw)
To: Felipe Contreras; +Cc: Junio C Hamano, SZEDER Gábor, git
In-Reply-To: <CAMP44s1i59VtX9xMmM-j3Gzcufg6jtKy34MMuwrfenmSw3oLAg@mail.gmail.com>
On Tue, Nov 20, 2012 at 10:15 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Tue, Nov 20, 2012 at 3:58 PM, Marc Khouzam <marc.khouzam@gmail.com> wrote:
>
>> Hi,
>>
>> since there doesn't seem to be an agreement that the approach to achieve tcsh
>> git-completion would be useful for zsh (the other possible shell that could use
>> it is ksh, but I haven't looked into that), maybe the simplest thing
>> is to keep the
>> tcsh solution contained in a tcsh-only script. This is the latest solution as
>> proposed here:
>>
>> [1] http://www.mail-archive.com/git@vger.kernel.org/msg12192.html
>
> This one is already merged to 'next'.
Awesome! I didn't notice.
If I want to suggest an improvement (like checking if the bash script
is available),
do I just post a patch here?
Thanks a lot for moving forward with this so quickly!
Marc
^ permalink raw reply
* Re: [PATCH 0/8] fix git-config with duplicate variable entries
From: Junio C Hamano @ 2012-11-20 19:08 UTC (permalink / raw)
To: Jeff King; +Cc: Ævar Arnfjörð Bjarmason, Git Mailing List
In-Reply-To: <20121023223502.GA23194@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Tue, Oct 23, 2012 at 04:13:44PM +0200, Ævar Arnfjörð Bjarmason wrote:
>
>> > It fails a few tests in t1300, but it looks like those tests are testing
>> > for the behavior we have identified as wrong, and should be fixed.
>>
>> I think this patch looks good.
>
> Thanks. It had a few minor flaws (like a memory leak). I fixed those,
> updated the tests, and split it out into a few more readable commits. In
> the process, I managed to uncover and fix a few other memory leaks in
> the area. I think this version is much more readable, and writing the
> rationale for patch 7 convinced me that it's the right thing to do.
> Another round of review would be appreciated.
>
> [1/8]: t1300: style updates
> [2/8]: t1300: remove redundant test
> [3/8]: t1300: test "git config --get-all" more thoroughly
> [4/8]: git-config: remove memory leak of key regexp
> [5/8]: git-config: fix regexp memory leaks on error conditions
> [6/8]: git-config: collect values instead of immediately printing
> [7/8]: git-config: do not complain about duplicate entries
> [8/8]: git-config: use git_config_with_options
>
> For those just joining us, the interesting bit is patch 7, which fixes
> some inconsistencies between the "git-config" tool and how the internal
> config callbacks work.
The way for the Porcelain scripts to mimick the internal "last one
wins" has been to grab values out of --get-all and do the "last one
wins" themselves, and I agree that a mode that frees them from
having to worry about it is a good *addition*. I would even say
that if we were designing "git config" plumbing without existing
users, it would be the only sensible behaviour for "--get".
But "git config --get" users have been promised to get errors on
duplicate values so far, so I have to say this needs to come with a
big red sign about API breakage.
I would feel safer to introduce --get-one or something now, and
worry about migration as a separate step.
^ permalink raw reply
* Re: [PATCH 3/4] pathspec: apply "*.c" optimization from exclude
From: Junio C Hamano @ 2012-11-20 19:47 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <CACsJy8C=vdvHYgMr8Qa7M+Mq=QO6p6dsOFm_ZW=Nf2VQrPwaMg@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>> When you come to strcmp(), you see that string_len is 1, pattern_len
>> is 3, and pattern is "oob". string+string_len-pattern_len = "oob",
>> one past the beginning of the original string "foob". They match.
>>
>> Oops?
>
> Oops indead. I'll need to check exclude code too :(
Ok, the one queued on 'pu' has been locally fixed when I sent the
message you are responding to.
^ permalink raw reply
* Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option
From: Junio C Hamano @ 2012-11-20 19:52 UTC (permalink / raw)
To: W. Trevor King
Cc: Heiko Voigt, Git, Jeff King, Phil Hord, Shawn Pearce,
Jens Lehmann, Nahor
In-Reply-To: <20121120121912.GC7096@odin.tremily.us>
"W. Trevor King" <wking@tremily.us> writes:
> The superproject gitlink should only be updated after
>
> $ git submodule update --pull
>
> A plain
>
> $ git submodule update
>
> would still checkout the previously-recorded SHA, not the new upstream
> tip.
Hrm, doesn't it make the "float at the tip of a branch" mode
useless, though?
^ permalink raw reply
* Re: Failure to extra stable@vger.kernel.org addresses
From: Andreas Schwab @ 2012-11-20 19:58 UTC (permalink / raw)
To: Krzysztof Mazur
Cc: Felipe Contreras, Junio C Hamano, Felipe Balbi, git,
Tomi Valkeinen
In-Reply-To: <20121120115942.GA6132@shrek.podlesie.net>
Krzysztof Mazur <krzysiek@podlesie.net> writes:
> On Tue, Nov 20, 2012 at 11:28:39AM +0100, Felipe Contreras wrote:
>> On Tue, Nov 20, 2012 at 8:56 AM, Krzysztof Mazur <krzysiek@podlesie.net> wrote:
>>
>> > --- a/git-send-email.perl
>> > +++ b/git-send-email.perl
>> > @@ -925,8 +925,11 @@ sub quote_subject {
>> > sub sanitize_address {
>> > my ($recipient) = @_;
>> >
>> > + my $local_part_regexp = qr/[^<>"\s@]+/;
>> > + my $domain_regexp = qr/[^.<>"\s@]+(?:\.[^.<>"\s@]+)+/;
>> > +
>> > # remove garbage after email address
>> > - $recipient =~ s/(.*>).*$/$1/;
>> > + $recipient =~ s/^(.*?<$local_part_regexp\@$domain_regexp>).*/$1/;
>>
>> I don't think all that extra complexity is warranted, to me
>> s/(.*?>)(.*)$/$1/ is just fine.
>>
>
> Yeah, it's a little bit too complex, but "s/(.*?>)(.*)$/$1/"
How about "s/(.*?<[^>]*>).*$/$1/"? That will still fail on "<foo@bar>"
<foo@bar>, but you'll need a full rfc822 parser to handle the general
case anyway.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Re: [PATCH 14/13] test-wildmatch: avoid Windows path mangling
From: Junio C Hamano @ 2012-11-20 20:11 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <50AB2B0F.8090808@viscovery.net>
Johannes Sixt <j.sixt@viscovery.net> writes:
> From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>
> The MSYS bash mangles arguments that begin with a forward slash
> when they are passed to test-wildmatch. This causes tests to fail.
> Avoid mangling by prepending "XXX", which is removed by
> test-wildmatch before further processing.
>
> [J6t: reworded commit message]
>
> Reported-by: Johannes Sixt <j6t@kdbg.org>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
> Works well, and I'm fine with this work-around.
>
> -- Hannes
Thanks, but you need to fix your format-patch somehow.
> @@ -187,8 +187,8 @@ match 0 0 '-' '[[-\]]'
> match 1 1 '-adobe-courier-bold-o-normal--12-120-75-75-m-70-iso8859-1' '-*-*-*-*-*-*-12-*-*-*-m-*-*-*'
> match 0 0 '-adobe-courier-bold-o-normal--12-120-75-75-X-70-iso8859-1' '-*-*-*-*-*-*-12-*-*-*-m-*-*-*'
> match 0 0 '-adobe-courier-bold-o-normal--12-120-75-75-/-70-iso8859-1' '-*-*-*-*-*-*-12-*-*-*-m-*-*-*'
> -match 1 1 '/adobe/courier/bold/o/normal//12/120/75/75/m/70/iso8859/1' '/*/*/*/*/*/*/12/*/*/*/m/*/*/*'
> -match 0 0 '/adobe/courier/bold/o/normal//12/120/75/75/X/70/iso8859/1' '/*/*/*/*/*/*/12/*/*/*/m/*/*/*'
> +match 1 1 'XXX/adobe/courier/bold/o/normal//12/120/75/75/m/70/iso8859/1' 'XXX/*/*/*/*/*/*/12/*/*/*/m/*/*/*'
> +match 0 0 'XXX/adobe/courier/bold/o/normal//12/120/75/75/X/70/iso8859/1' 'XXX/*/*/*/*/*/*/12/*/*/*/m/*/*/*'
> match 1 0 'abcd/abcdefg/abcdefghijk/abcdefghijklmnop.txt' '**/*a*b*g*n*t'
> match 0 0 'abcd/abcdefg/abcdefghijk/abcdefghijklmnop.txtz' '**/*a*b*g*n*t'
> diff --git a/test-wildmatch.c b/test-wildmatch.c
This hunk claims that there are 8 lines in preimage and postimage,
but it is not the case. It has only 7 lines each.
You also have the first line of the next patch "diff --git" somehow
indented. How did this happen?
> index 74c0864..e384c8e 100644
> --- a/test-wildmatch.c
> +++ b/test-wildmatch.c
> @@ -3,6 +3,14 @@
> int main(int argc, char **argv)
> {
> + int i;
> + for (i = 2; i < argc; i++) {
> + if (argv[i][0] == '/')
> + die("Forward slash is not allowed at the beginning of the\n"
> + "pattern because Windows does not like it. Use `XXX/' instead.");
> + else if (!strncmp(argv[i], "XXX/", 4))
> + argv[i] += 3;
> + }
> if (!strcmp(argv[1], "wildmatch"))
> return !!wildmatch(argv[3], argv[2], 0);
> else if (!strcmp(argv[1], "iwildmatch"))
And again this claims that the preimage has 6 lines while the
postimage has 14. Somebody is overcounting, or perhaps you removed
the first pre-context by hand without adjusting the line number?
^ permalink raw reply
* Re: [PATCH] git-send-email: don't return undefined value in extract_valid_address()
From: Junio C Hamano @ 2012-11-20 20:27 UTC (permalink / raw)
To: Krzysztof Mazur; +Cc: git
In-Reply-To: <1353414053-25261-1-git-send-email-krzysiek@podlesie.net>
Krzysztof Mazur <krzysiek@podlesie.net> writes:
> In the fallback check, when Email::Valid is not available, the
> extract_valid_address() does not check for success of matching regex,
> and $1, which can be undefined, is always returned. Now if match
> fails an empty string is returned.
That much we can read from the code, but a bigger question is why
would it be a good thing for the callers? Wouldn't they want to
be able to distinguish a failure from an empty string?
> Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>
> ---
> This fixes following warnings:
> Use of uninitialized value in string eq at ./git-send-email.perl line 1017.
> Use of uninitialized value in quotemeta at ./git-send-email.perl line 1017.
> W: unable to extract a valid address from: x a.patch
>
> when invalid email address was added by --cc-cmd,
> ./git-send-email.perl --dry-run --to a@podlesie.net --cc-cmd=echo x a.patch
In other words, would we want to *hide* (not "fix") the warning?
Shouldn't we be barfing loudly and possibly erroring it out until
the user fixes her --cc-cmd?
> git-send-email.perl | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 5a7c29d..045f25f 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -831,12 +831,12 @@ sub extract_valid_address {
> $address =~ s/^\s*<(.*)>\s*$/$1/;
> if ($have_email_valid) {
> return scalar Email::Valid->address($address);
> - } else {
> - # less robust/correct than the monster regexp in Email::Valid,
> - # but still does a 99% job, and one less dependency
> - $address =~ /($local_part_regexp\@$domain_regexp)/;
> - return $1;
> }
> +
> + # less robust/correct than the monster regexp in Email::Valid,
> + # but still does a 99% job, and one less dependency
> + return $1 if $address =~ /($local_part_regexp\@$domain_regexp)/;
> + return "";
> }
>
> # Usually don't need to change anything below here.
^ permalink raw reply
* Re: git merge commits are non-deterministic? what changed?
From: Junio C Hamano @ 2012-11-20 20:39 UTC (permalink / raw)
To: Ulrich Spörlein
Cc: Michael J Gruber, Jeff King, Matthieu Moy, Andreas Schwab, git
In-Reply-To: <20121120162226.GK69724@acme.spoerlein.net>
Ulrich Spörlein <uqs@spoerlein.net> writes:
> But this will never be changed, it would break the fundamental git
> storage model as it is in place now.
It doesn't just break "storage model", but more importantly, it
breaks the semantics.
Imagine that things started breaking after merging your topic branch
'foo' to the integration branch 'master', and how people would
perceive the situation. Everybody would say your topic 'foo' broke
the build. Nobody except you would say, even if the tip of your
topic 'foo' alone works perfectly, merging the 'master' to your
topic 'foo' broke that topic. The topic should have been adjusted
to the updated baseline, that is the 'master' branch before this
merge since your topic 'foo' forked off of it, before or during the
merge.
To express what was merged into what, the order of parents in the
commit is fundamentally a part of what a commit is.
^ permalink raw reply
* Re: [PATCH] git-send-email: don't return undefined value in extract_valid_address()
From: Krzysztof Mazur @ 2012-11-20 20:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8v9wrpdz.fsf@alter.siamese.dyndns.org>
On Tue, Nov 20, 2012 at 12:27:36PM -0800, Junio C Hamano wrote:
> Krzysztof Mazur <krzysiek@podlesie.net> writes:
>
> > In the fallback check, when Email::Valid is not available, the
> > extract_valid_address() does not check for success of matching regex,
> > and $1, which can be undefined, is always returned. Now if match
> > fails an empty string is returned.
Maybe the last line of comment should be changed to:
fails an empty string is returned to indicate failure.
>
> That much we can read from the code, but a bigger question is why
> would it be a good thing for the callers? Wouldn't they want to
> be able to distinguish a failure from an empty string?
In this case returning empty string does not make sense, so it's
really used to indicate failure.
>
> > Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>
> > ---
> > This fixes following warnings:
> > Use of uninitialized value in string eq at ./git-send-email.perl line 1017.
> > Use of uninitialized value in quotemeta at ./git-send-email.perl line 1017.
> > W: unable to extract a valid address from: x a.patch
> >
> > when invalid email address was added by --cc-cmd,
> > ./git-send-email.perl --dry-run --to a@podlesie.net --cc-cmd=echo x a.patch
>
> In other words, would we want to *hide* (not "fix") the warning?
> Shouldn't we be barfing loudly and possibly erroring it out until
> the user fixes her --cc-cmd?
>
Yes, it's just to hide the warning, the error (warning in this case) it's
already correctly generated:
W: unable to extract a valid address from: x a.patch
Maybe we should change it to an error?
Krzysiek
^ permalink raw reply
* Re: [PATCH] tcsh-completion re-using git-completion.bash
From: Junio C Hamano @ 2012-11-20 21:07 UTC (permalink / raw)
To: Marc Khouzam; +Cc: Felipe Contreras, SZEDER Gábor, git
In-Reply-To: <CAFj1UpFTu7GnpKSvs6qGH6XjAT16RAk4rmdX0sPFOo9ABg8BKg@mail.gmail.com>
Marc Khouzam <marc.khouzam@gmail.com> writes:
>> This one is already merged to 'next'.
>
> Awesome! I didn't notice.
>
> If I want to suggest an improvement (like checking if the bash
> script is available), do I just post a patch here?
Yes, as a follow-up patch (or two).
Thanks.
^ permalink raw reply
* Re: Failure to extra stable@vger.kernel.org addresses
From: Krzysztof Mazur @ 2012-11-20 21:21 UTC (permalink / raw)
To: Andreas Schwab
Cc: Felipe Contreras, Junio C Hamano, Felipe Balbi, git,
Tomi Valkeinen
In-Reply-To: <m2lidw11yb.fsf@igel.home>
On Tue, Nov 20, 2012 at 08:58:20PM +0100, Andreas Schwab wrote:
> How about "s/(.*?<[^>]*>).*$/$1/"? That will still fail on "<foo@bar>"
> <foo@bar>, but you'll need a full rfc822 parser to handle the general
> case anyway.
That will fail also on "<something>" <foo@bar>.
I think it's good compromise between complexity and correctness.
Felipe, may you check, it again? This time the change is trivial.
Andreas, may I add you in Thanks-to?
Thanks,
Krzysiek
-- >8 --
Subject: [PATCH] git-send-email: remove garbage after email address
In some cases it's very useful to add some additional information
after email in Cc-list, for instance:
"Cc: Stable kernel <stable@vger.kernel.org> #v3.4 v3.5 v3.6"
Currently the git refuses to add such invalid email to Cc-list,
when the Email::Valid perl module is available or just uses whole line
as the email address.
Now in sanitize_address() everything after the email address is
removed, so the resulting line is correct email address and Email::Valid
validates it correctly.
To avoid unnecessary complexity this code assumes that in phrase before
email address '<something>' never exists.
Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>
---
git-send-email.perl | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/git-send-email.perl b/git-send-email.perl
index 5a7c29d..157eabc 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -924,6 +924,10 @@ sub quote_subject {
# use the simplest quoting being able to handle the recipient
sub sanitize_address {
my ($recipient) = @_;
+
+ # remove garbage after email address
+ $recipient =~ s/(.*?<[^>]*>).*$/$1/;
+
my ($recipient_name, $recipient_addr) = ($recipient =~ /^(.*?)\s*(<.*)/);
if (not $recipient_name) {
--
1.8.0.283.gc57d856
^ permalink raw reply related
* Re: Failure to extra stable@vger.kernel.org addresses
From: Felipe Contreras @ 2012-11-20 22:06 UTC (permalink / raw)
To: Krzysztof Mazur
Cc: Andreas Schwab, Junio C Hamano, Felipe Balbi, git, Tomi Valkeinen
In-Reply-To: <20121120212126.GA12656@shrek.podlesie.net>
On Tue, Nov 20, 2012 at 10:21 PM, Krzysztof Mazur <krzysiek@podlesie.net> wrote:
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -924,6 +924,10 @@ sub quote_subject {
> # use the simplest quoting being able to handle the recipient
> sub sanitize_address {
> my ($recipient) = @_;
> +
> + # remove garbage after email address
> + $recipient =~ s/(.*?<[^>]*>).*$/$1/;
That won't work for 'foo@bar.com # test'. I think we should abandon
hopes of properly parsing an email address and just do:
$recipient =~ s/(.*?) #.*$/$1/;
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH] git-send-email: don't return undefined value in extract_valid_address()
From: Junio C Hamano @ 2012-11-20 22:14 UTC (permalink / raw)
To: Krzysztof Mazur; +Cc: git
In-Reply-To: <20121120204736.GA7039@shrek.podlesie.net>
Krzysztof Mazur <krzysiek@podlesie.net> writes:
> Yes, it's just to hide the warning, the error (warning in this case) it's
> already correctly generated:
>
> W: unable to extract a valid address from: x a.patch
But it is of no use if the message is sent out without the intended
recipient, no? It is too late when you notice it.
> Maybe we should change it to an error?
At least, when we are not giving the "final sanity check [Y/n]?"
prompt, I think the code should error out.
^ permalink raw reply
* Re* [PATCH] gitweb: make remote_heads config setting work.
From: Junio C Hamano @ 2012-11-20 22:21 UTC (permalink / raw)
To: Jeff King; +Cc: Phil Pennock, git
In-Reply-To: <20121109163710.GD19725@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Thu, Nov 08, 2012 at 08:40:11PM -0800, Junio C Hamano wrote:
>
>> Looking at the code before this part:
>>
>> if (my ($hi, $mi, $lo) = ($key =~ /^([^.]*)\.(.*)\.([^.]*)$/)) {
>> $key = join(".", lc($hi), $mi, lc($lo));
>> } else {
>> $key = lc($key);
>> }
>> $key =~ s/^gitweb\.//;
>> return if ($key =~ m/\W/);
>>
>> the new code is munding the $hi and $mi parts, while the mistaken
>> configuration this patch is trying to correct is about the $lo part,
>> and possibly the $hi part, but never the $mi part.
>
> Good catch. I think the "return" in the existing code suffers from the
> same problem: it will bail on non-word characters in the $mi part, but
> that part should allow arbitrary characters.
I am tired of keeping the "expecting reroll" entries and having to
worry about them, so let's do this
-- >8 --
Subject: [squash] gitweb: make remote_heads config setting work
Only the top-level and bottom-level names are case insensitive and
spelled without "_". Protect future support of subsection names
from name mangling.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
gitweb/gitweb.perl | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git c/gitweb/gitweb.perl w/gitweb/gitweb.perl
index f2144c8..c421fa4 100755
--- c/gitweb/gitweb.perl
+++ w/gitweb/gitweb.perl
@@ -2697,13 +2697,15 @@ sub git_get_project_config {
# only subsection, if exists, is case sensitive,
# and not lowercased by 'git config -z -l'
if (my ($hi, $mi, $lo) = ($key =~ /^([^.]*)\.(.*)\.([^.]*)$/)) {
+ $lo =~ s/_//g;
$key = join(".", lc($hi), $mi, lc($lo));
+ return if ($lo =~ /\W/ || $hi =~ /\W/);
} else {
$key = lc($key);
+ $key =~ s/_//g;
+ return if ($key =~ /\W/);
}
$key =~ s/^gitweb\.//;
- $key =~ s/_//g;
- return if ($key =~ m/\W/);
# type sanity check
if (defined $type) {
^ permalink raw reply related
* git-fetch does not work from .git subdirectory
From: Timur Tabi @ 2012-11-20 22:24 UTC (permalink / raw)
To: git
I was under the impression that git commands which affect repository (as
opposed to the local file system) work from any subdirectory inside the
repository. For example:
[b04825@efes linux.cq-test]$ git log -1
commit f35d179fde24be5e1675b1df9f7a49b8d95561b2
Author: Timur Tabi <timur@freescale.com>
Date: Wed Oct 31 15:56:20 2012 +0200
...
[b04825@efes linux.cq-test]$ cd .git
[b04825@efes .git]$ git log -1
commit f35d179fde24be5e1675b1df9f7a49b8d95561b2
Author: Timur Tabi <timur@freescale.com>
Date: Wed Oct 31 15:56:20 2012 +0200
...
It appears, however, that git-fetch does not work this way:
[b04825@efes linux.cq-test]$ git fetch upstream master
From ../linux-2.6
* branch master -> FETCH_HEAD
[b04825@efes linux.cq-test]$ cd .git
[b04825@efes .git]$ git fetch upstream master
fatal: '../linux-2.6.git' does not appear to be a git repository
fatal: The remote end hung up unexpectedly
This makes it complicated because git hooks run from the .git directory on
normal repositories, but they run from the top-level directory on bare
repositories. Apparently, you need to be in the top-level directory for
git-fetch to run in any kind of repository.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: Failure to extra stable@vger.kernel.org addresses
From: Junio C Hamano @ 2012-11-20 22:30 UTC (permalink / raw)
To: Felipe Contreras
Cc: Krzysztof Mazur, Andreas Schwab, Felipe Balbi, git,
Tomi Valkeinen
In-Reply-To: <CAMP44s3+vnKfhhh=qqU2vuKvWwhii4CQ7=YAuhFiceX1EDaVKQ@mail.gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> On Tue, Nov 20, 2012 at 10:21 PM, Krzysztof Mazur <krzysiek@podlesie.net> wrote:
>
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -924,6 +924,10 @@ sub quote_subject {
>> # use the simplest quoting being able to handle the recipient
>> sub sanitize_address {
>> my ($recipient) = @_;
>> +
>> + # remove garbage after email address
>> + $recipient =~ s/(.*?<[^>]*>).*$/$1/;
>
> That won't work for 'foo@bar.com # test'. I think we should abandon
> hopes of properly parsing an email address and just do:
>
> $recipient =~ s/(.*?) #.*$/$1/;
We should probably fix the tools that generate these bogus
non-addresses first. What's wrong with
Cc: stable kernel (v3.5 v3.6 v3.7) <stable@vger.kernel.org>
which should be OK?
Also I suspect that this should be also deemed valid:
Cc: stable@vger.kernel.org (Stable kernel - v3.5 v3.6 v3.7)
^ permalink raw reply
* Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs
From: Junio C Hamano @ 2012-11-20 22:43 UTC (permalink / raw)
To: Felipe Contreras
Cc: git, Johannes Schindelin, Max Horn, Jeff King, Sverre Rabbelier,
Brandon Casey, Brandon Casey, Jonathan Nieder, Ilari Liusvaara,
Pete Wyckoff, Ben Walton, Matthieu Moy, Julian Phillips
In-Reply-To: <CAMP44s0WH-P7WY4UqhMX3WdrrSCYXUR9yCgsUV+mzLOCK5LkHQ@mail.gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> Of course, transport-helper shouldn't even be specifying the negative
> (^) refs, but that's another story.
Hrm, I am not sure I understand what you mean by this.
How should it be telling the fast-export up to what commit the
receiving end should already have the history for (hence they do not
need to be sent)? Or are you advocating to re-send the entire
history down to the root commit every time?
^ permalink raw reply
* Re: Failure to extra stable@vger.kernel.org addresses
From: Krzysztof Mazur @ 2012-11-20 23:09 UTC (permalink / raw)
To: Junio C Hamano
Cc: Felipe Contreras, Andreas Schwab, Felipe Balbi, git,
Tomi Valkeinen
In-Reply-To: <7vhaojrjpx.fsf@alter.siamese.dyndns.org>
On Tue, Nov 20, 2012 at 02:30:02PM -0800, Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
> > On Tue, Nov 20, 2012 at 10:21 PM, Krzysztof Mazur <krzysiek@podlesie.net> wrote:
> >
> >> --- a/git-send-email.perl
> >> +++ b/git-send-email.perl
> >> @@ -924,6 +924,10 @@ sub quote_subject {
> >> # use the simplest quoting being able to handle the recipient
> >> sub sanitize_address {
> >> my ($recipient) = @_;
> >> +
> >> + # remove garbage after email address
> >> + $recipient =~ s/(.*?<[^>]*>).*$/$1/;
> >
> > That won't work for 'foo@bar.com # test'. I think we should abandon
> > hopes of properly parsing an email address and just do:
> >
> > $recipient =~ s/(.*?) #.*$/$1/;
>
> We should probably fix the tools that generate these bogus
> non-addresses first. What's wrong with
>
> Cc: stable kernel (v3.5 v3.6 v3.7) <stable@vger.kernel.org>
>
> which should be OK?
>
> Also I suspect that this should be also deemed valid:
>
> Cc: stable@vger.kernel.org (Stable kernel - v3.5 v3.6 v3.7)
So maybe we should just use the original regex:
$recipient =~ s/(.*>).*$/$1/
which does not add regression for valid addresses, and just fails
in some rare cases when '>' is used in garbage. It was sufficient
for original issue reported by, and tested by Felipe.
The problem with '>' would be fixed in separate patch. The same
problem exits for invalid address generated by --cc-cmd
(see [PATCH] git-send-email: don't return undefined value in
extract_valid_address()). We would report an error in both cases,
as suggested by Junio.
Krzysiek
^ permalink raw reply
* Re: Failure to extra stable@vger.kernel.org addresses
From: Junio C Hamano @ 2012-11-20 23:43 UTC (permalink / raw)
To: Krzysztof Mazur
Cc: Felipe Contreras, Andreas Schwab, Felipe Balbi, git,
Tomi Valkeinen
In-Reply-To: <20121120230955.GA9686@shrek.podlesie.net>
Krzysztof Mazur <krzysiek@podlesie.net> writes:
> On Tue, Nov 20, 2012 at 02:30:02PM -0800, Junio C Hamano wrote:
>
>> We should probably fix the tools that generate these bogus
>> non-addresses first. What's wrong with
>>
>> Cc: stable kernel (v3.5 v3.6 v3.7) <stable@vger.kernel.org>
>>
>> which should be OK?
>>
>> Also I suspect that this should be also deemed valid:
>>
>> Cc: stable@vger.kernel.org (Stable kernel - v3.5 v3.6 v3.7)
>
> So maybe we should just use the original regex:
>
> $recipient =~ s/(.*>).*$/$1/
>
> which does not add regression for valid addresses, and just fails
> in some rare cases when '>' is used in garbage. It was sufficient
> for original issue reported by, and tested by Felipe.
>
> The problem with '>' would be fixed in separate patch. The same
> problem exits for invalid address generated by --cc-cmd
> (see [PATCH] git-send-email: don't return undefined value in
> extract_valid_address()). We would report an error in both cases,
> as suggested by Junio.
OK, sounds like a plan.
^ permalink raw reply
* [wishlist] support git flow-like view
From: Lisandro Damián Nicanor Pérez Meyer @ 2012-11-20 23:42 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: Text/Plain, Size: 865 bytes --]
Hi! I am not suscribed to the list, so please CC-me.
I think this may have been proposed before, but I could not find anything in
the web, so I better try myself :)
The idea would be to gitk to show a "git flow-like"[0] view when it detects
git flow (or the user ask for it or...)
Basiccaly, you can show the main two branches: master and develop. Of course,
having the possibility to show feature/release/hotfixes branches would be
ideal.
Kinds regards, Lisandro.
[0] <http://nvie.com/posts/a-successful-git-branching-model/>
--
Esperando confirmación de ingredientes necesarios
que serán expuestos a la radiación...
Manera geek de expresar que se espera la compra
de carne para un típico asado argentino.
Silvio Rikemberg.
Lisandro Damián Nicanor Pérez Meyer
http://perezmeyer.com.ar/
http://perezmeyer.blogspot.com/
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2012, #06; Mon, 19)
From: Junio C Hamano @ 2012-11-20 23:50 UTC (permalink / raw)
To: git
In-Reply-To: <7vpq39up0m.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Here are the topics that have been cooking. Commits prefixed with
> '-' are only in 'pu' (proposed updates) while commits prefixed with
> '+' are in 'next'.
>
> Bunch of topics have been merged to 'next'.
>
> We are at the beginning of the 5th week of this release cycle
> (cf. http://tinyurl.com/gitcal), and I've moved many topics to the
> Stalled category, which will be discarded without prejudice soonish
> unless there are some updates. I am still a bit behind on some
> topics and already posted rerolls may have to be pulled in.
It feels a bit too busy/loud to issue two "What's cooking" in a row,
so here is an informal update.
- The following have graduated to 'master'.
cn/config-missing-path
jk/checkout-out-of-unborn
jk/maint-gitweb-xss
jk/maint-http-half-auth-fetch
jl/submodule-rm
kb/preload-index-more
mg/replace-resolve-delete
mh/alt-odb-string-list-cleanup
ml/cygwin-mingw-headers
pw/maint-p4-rcs-expansion-newline
rh/maint-gitweb-highlight-ext
ta/doc-cleanup
- Many topics have been merged to 'maint' in preparation for 1.8.0.1.
mg/maint-pull-suggest-upstream-to
mm/maint-doc-commit-edit
as/maint-doc-fix-no-post-rewrite
rs/lock-correct-ref-during-delete
rf/maint-mailmap-off-by-one
jk/maint-diff-grep-textconv
js/format-2047
sz/maint-curl-multi-timeout
po/maint-refs-replace-docs
ph/pull-rebase-detached
mm/maint-doc-remote-tracking
rs/branch-del-symref
nd/grep-true-path
jc/grep-pcre-loose-ends (early part)
da/mergetools-p4
jc/test-say-color-avoid-echo-escape
bw/config-lift-variable-name-length-limit
- A few topics have been resurrected from the stalled category to
cooking:
pp/gitweb-config-underscore
jc/apply-trailing-blank-removal
^ 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