* Re: What's cooking in git.git (Nov 2012, #06; Mon, 19)
From: Paul Fox @ 2012-11-20 11:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vpq39up0m.fsf@alter.siamese.dyndns.org>
junio c hamano wrote:
> [Stalled]
>
> * pf/editor-ignore-sigint (2012-11-11) 5 commits
> - launch_editor: propagate SIGINT from editor to git
> - run-command: do not warn about child death by SIGINT
> - run-command: drop silent_exec_failure arg from wait_or_whine
> - launch_editor: ignore SIGINT while the editor has control
> - launch_editor: refactor to use start/finish_command
>
> Avoid confusing cases where the user hits Ctrl-C while in the editor
> session, not realizing git will receive the signal. Since most editors
> will take over the terminal and will block SIGINT, this is not likely
> to confuse anyone.
>
> Some people raised issues with emacsclient, which are addressed by this
> re-roll. It should probably also handle SIGQUIT, and there were a
> handful of other review comments.
>
> Expecting a re-roll.
i don't have strong feelings on any of the later review comments
(though i guess the check on finish_command()'s return code needed
attention), and wasn't the last submitter, but would certainly like to
see this move forward.
paul
=---------------------
paul fox, pgf@foxharp.boston.ma.us (arlington, ma, where it's 26.1 degrees)
^ permalink raw reply
* Re: Failure to extra stable@vger.kernel.org addresses
From: Krzysztof Mazur @ 2012-11-20 11:59 UTC (permalink / raw)
To: Felipe Contreras; +Cc: Junio C Hamano, Felipe Balbi, git, Tomi Valkeinen
In-Reply-To: <CAMP44s38gTB_3Ao1rFZgMo2EAuiNb+h88-qRFcQPRMJNxo3CAQ@mail.gmail.com>
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/"
causes small regression - '>' character is no longer allowed
in "phrase" before "<email address>". Maybe the initial version,
that removes everything after last '>' is better? In this case '>'
is not allowed in garbage after email.
Krzysiek
^ permalink raw reply
* Re: RFC: Making submodules "track" branches
From: W. Trevor King @ 2012-11-20 12:04 UTC (permalink / raw)
To: nottrobin; +Cc: git
In-Reply-To: <1353410195055-7571610.post@n2.nabble.com>
[-- Attachment #1: Type: text/plain, Size: 441 bytes --]
On Tue, Nov 20, 2012 at 03:16:35AM -0800, nottrobin wrote:
> Did any of this ever find its way into the submodule core? I'd like
> to have a submodule that tracks a branch.
In progress. See:
http://thread.gmane.org/gmane.comp.version-control.git/208254
Cheers,
Trevor
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option
From: W. Trevor King @ 2012-11-20 12:19 UTC (permalink / raw)
To: Junio C Hamano
Cc: Heiko Voigt, Git, Jeff King, Phil Hord, Shawn Pearce,
Jens Lehmann, Nahor
In-Reply-To: <7v1ufou92h.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 3039 bytes --]
On Mon, Nov 19, 2012 at 09:39:34PM -0800, Junio C Hamano wrote:
> "W. Trevor King" <wking@tremily.us> writes:
>
> > On Mon, Nov 19, 2012 at 04:49:09PM -0800, Junio C Hamano wrote:
> >> "W. Trevor King" <wking@tremily.us> writes:
> >> ...
> >> > I think it's best to have users craft their own commit messages
> >> > explaining why the branch was updated. That said, an auto-generated
> >> > hint (a la "git merge") would probably be a useful extra feature.
> >>
> >> I am not quite sure I agree. When the project says "Use the tip of
> >> 'bar' branch for the submodule 'foo'" at the top-level, does an
> >> individual user who is not working on the submodule 'foo' but merely
> >> is using it have any clue as to why the submodule's 'foo' branch
> >> 'foo' moved, or does he necessarily even care?
> >
> > If he doesn't care, why is he updating the submodule gitlink?
>
> He may not be updating the gitlink with "git add foo" at the
> top-level superproject level. He is just using that submodule as
> part of the larger whole as he is working on either the top-level or
> some other submodule. And checkout of 'foo' is necessary in the
> working tree for him to work in the larger context of the project,
> and 'foo' is set to float at the tip of its 'bar' branch. And that
> checkout results in a commit that is different from the commit the
> gitlink suggests, perhaps because somebody worked in 'foo' submodule
> and advanced the tip of branch 'bar'.
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. The uncaring user should skip the "--pull", and there will be no
superproject changes to worry about.
> Or are you envisioning that such a check-out will and should show a
> local difference at the submodule 'foo' by leaving the index of the
> top-level superproject unchanged,
A plain "git submodule update" will, yes. And this will clobber any
changes that have happened in the submodule directory and its index
(because the user explicitly asked to checkout the
superproject-recorded SHA)
> and the user should refrain from using "git commit -a" to avoid
> having to describe the changes made on the 'bar' branch in the
> meantime in his top-level commit?
What would "git commit -a" be picking up? Nothing in the superproject
has changed?
> That is certainly fine by me (I am no a heavy submodule user to
> begin with), but I am not sure if that is useful and helpful to the
> submodule users.
The benefit is that Ævar's
$ git submodule foreach 'git checkout $(git config --file $toplevel/.gitmodules submodule.$name.branch) && git pull'
becomes
$ git submodule update --pull
Still an explicit pull, but much easier to remember.
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH] git-send-email: don't return undefined value in extract_valid_address()
From: Krzysztof Mazur @ 2012-11-20 12:20 UTC (permalink / raw)
To: git; +Cc: Krzysztof Mazur
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.
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
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.
--
1.8.0.283.gc57d856
^ permalink raw reply related
* Re: [PATCH 2/4] pathspec: do exact comparison on the leading non-wildcard part
From: Nguyen Thai Ngoc Duy @ 2012-11-20 13:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vzk2duw3m.fsf@alter.siamese.dyndns.org>
On Tue, Nov 20, 2012 at 3:54 AM, Junio C Hamano <gitster@pobox.com> wrote:
> How would we protect this optimization from future breakages?
>
> Once we start using FNM_PERIOD, this becomes unsafe, as the simple
> part in "foo/bar*baz" would be "foo/bar" with remainder "*baz".
>
> The pattern "foo/bar*baz" should match "foo/bar.baz" with FNM_PERIOD
> set. But with this optimization, fnmatch is fed pattern="*baz",
> string=".baz" and they would not match.
The first question is how FNM_PERIOD comes in the first place without
breaking the current syntax. I guess we just turn off the optimization
if FNM_PERIOD is used.
My secret "plan" is to convert all fnmatch() calls to git_fnmatch()
then replace fnmatch() with wildmatch() and move these optimization
down to wildmatch(). I think we can handle FNM_PERIOD case better down
there because string is still "foo/bar.baz".
--
Duy
^ permalink raw reply
* 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
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