* [PATCH 14/13] test-wildmatch: avoid Windows path mangling
From: Nguyễn Thái Ngọc Duy @ 2012-11-13 10:06 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Johannes Sixt,
Nguyễn Thái Ngọc Duy
In-Reply-To: <7vvcdco1pf.fsf@alter.siamese.dyndns.org>
On Windows, arguments starting with a forward slash is mangled as if
it were full pathname. This causes the patterns beginning with a slash
not to be passed to test-wildmatch correctly. Avoid mangling by never
accepting patterns starting with a slash. Those arguments must be
rewritten with a leading "XXX" (e.g. "/abc" becomes "XXX/abc"), which
will be removed by test-wildmatch itself before feeding the patterns
to wildmatch() or fnmatch().
Reported-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
On Sun, Nov 11, 2012 at 5:47 PM, Junio C Hamano <gitster@pobox.com> wrote:
> The title taken together with the above explanation makes it sound
> as if wildmatch code does not work with the pattern /foo on Windows
> at all and to avoid the issue (instead of fixing the breakage) this
> patch removes such tests....
OK how about this?
t/t3070-wildmatch.sh | 10 +++++-----
test-wildmatch.c | 8 ++++++++
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/t/t3070-wildmatch.sh b/t/t3070-wildmatch.sh
index e6ad6f4..3155eab 100755
--- a/t/t3070-wildmatch.sh
+++ b/t/t3070-wildmatch.sh
@@ -74,7 +74,7 @@ match 0 0 'foo/bar' 'foo[/]bar'
match 0 0 'foo/bar' 'f[^eiu][^eiu][^eiu][^eiu][^eiu]r'
match 1 1 'foo-bar' 'f[^eiu][^eiu][^eiu][^eiu][^eiu]r'
match 1 0 'foo' '**/foo'
-match 1 x '/foo' '**/foo'
+match 1 x 'XXX/foo' '**/foo'
match 1 0 'bar/baz/foo' '**/foo'
match 0 0 'bar/baz/foo' '*/foo'
match 0 0 'foo/bar/baz' '**/bar*'
@@ -95,8 +95,8 @@ match 0 0 ']' '[!]-]'
match 1 x 'a' '[!]-]'
match 0 0 '' '\'
match 0 x '\' '\'
-match 0 x '/\' '*/\'
-match 1 x '/\' '*/\\'
+match 0 x 'XXX/\' '*/\'
+match 1 x 'XXX/\' '*/\\'
match 1 1 'foo' 'foo'
match 1 1 '@foo' '@foo'
match 0 0 'foo' '@foo'
@@ -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
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"))
--
1.8.0.rc2.23.g1fb49df
^ permalink raw reply related
* Re: [PATCH] send-email: add proper default sender
From: Felipe Contreras @ 2012-11-13 9:06 UTC (permalink / raw)
To: Jeff King; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <20121113074720.GA18746@sigill.intra.peff.net>
On Tue, Nov 13, 2012 at 8:47 AM, Jeff King <peff@peff.net> wrote:
> But I still don't see how that has anything to do with what send-email
> does or should do. That is why I said "strawman" above. You seem to
> think I am saying that send-email should use the system that generated
> those broken names, when I am saying the opposite.
No, I'm saying none should use that system, and that in fact 'git
commit' should be stricter... both should be stricter.
> Those people would also not be using a new version of git-send-email,
> and it will always prompt. I thought we were talking about what
> send-email should do in future versions. Namely, loosening that safety
> valve (the prompt) because it is inconvenient, but tightening the checks
> so that losing the safety valve is not a problem.
Yeah, but all I'm saying is that the issue happens, you seemed to
suggest that it doesn't.
>> I'm not talking about git send-email, I'm talking about your comment
>> 'it has always been the case that you can use git without setting
>> user.*', which has caused issues with wrong author/commmitter names in
>> commits, and will probably continue to do so.
>
> The second half of that sentence that you quoted above is "...instead
> only using the environment." As in, environment variables like
> GIT_AUTHOR_EMAIL, GIT_COMMITTER_EMAIL, and EMAIL. _Not_ implicit
> generation of the email from the username and hostname.
That sentence was *not* about 'git send-email', it was about git in
general, and 'git commit' is perfectly happy with implicit generation
of the email from the username and hostname.
I don't thin 'git commit' should do that, and I don't think 'git
send-email' should do that. I'm criticizing the whole approach.
> I am tempted to fault myself for not communicating well, but I feel like
> I have made that point at least 3 times in this conversation now. Is
> that the source of the confusion?
I think you are the one that is not understanding what I'm saying. But
I don't think it matters.
This is what I'm saying; the current situation with 'git commit' is
not OK, _both_ 'git commit' and 'git send-email' should change.
> And you will survive if upstream git (whether it is me today or Junio
> tomorrow) does not pick up your patch.
Indeed I would, but there's other people that would benefit from this
patch. I'm sure I'm not the only person that doesn't have
sendmail.from configured, but does have user.name/user.email, and is
constantly typing enter.
And the difference is that I'm _real_, the hypothetical user that
sends patches with GIT_AUTHOR_NAME/EMAIL is not. I would be convinced
otherwise if some evidence was presented that such a user is real
though.
> I remember writing you a long
> email recently about how one of the responsibilities of the maintainer
> is to balance features versus regressions. I'll not bother repeating
> myself here.
And to balance you need to *measure*, and that means taking into
consideration who actually uses the features, if there's any. And it
looks to me this is a feature nobody uses.
But listen closely to what you said:
> I actually think it would make more sense to drop the prompt entirely and just die when the user has not given us a usable ident.
Suppose somebody has a full name, and a fully qualified domain name,
and he can receive mails to it directly. Such a user would not need a
git configuration, and would not need $EMAIL, or anything.
Currently 'git send-email' will throw 'Felipe Contreras
<felipec@felipec.org>' which would actually work, but is not explicit.
You are suggesting to break that use-case. You are introducing a
regression. And this case is realistic, unlike the
GIT_AUTHOR_NAME/EMAIL. Isn't it?
I prefer to concentrate on real issues, but that's just me.
> As for whether they exist, what data do you have?
What data do _you_ have?
When there's no evidence either way, the rational response is to don't
believe. That's the default position.
> Are you aware that the
> test suite, for example, relies on setting GIT_AUTHOR_NAME but not
> having any user.* config?
What tests? My patch doesn't seem to break anything there:
% make -C t t9001-send-email.sh
# passed all 96 test(s)
> When somebody comes on the list and asks why
> every git program in the entire system respects GIT_* environment
> variables as an override to user.* configuration _except_ for
> send-email, what should I say?
The same thing you say when somebody comes reporting a bug: "yeah, we
should probably fix that".
But that's not going to happen. And in the unlikely event that it
does, it's not going to be a major issue.
It's all about proportion. Is it possible that we all are going to die
tomorrow because of an asteroid? Sure... but what's the point of
worrying about it if it's not likely?
>> But let's look at the current situation closely:
>>
>> PERL5LIB=~/dev/git/perl ./git-send-email.perl --confirm=always -1
>>
>> 1) No information at all
>>
>> fatal: empty ident name (for <felipec@nysa.(none)>) not allowed
>
> That is dependent on your system. If you have a non-empty name in your
> GECOS field, and if your machine has a FQDN, it will currently work (and
> prompt).
Yes, that's point 2).
>> 2) Full Name + full hostname
>>
>> Who should the emails appear to be from? [Felipe Contreras
>> <felipec@nysa.felipec.org>]
>>
>> That's right, ident doesn't fail, and that's not the mail address I
>> specified, it's *implicit*.
>
> Right. I never said it did. I said it currently rejected obviously bogus
> stuff (like the ".(none)" above) due to IDENT_STRICT, but currently
> allowed implicit definitions. And I also said that if we get rid of the
> prompt, we should disallow implicit definitions like this, because the
> prompt is the safety valve on sending out mails with broken from
> addresses. I even wrote a patch that let you find out whether the ident
> was generated implicitly.
Correct.
>> 3) Full Name + EMAIL
>>
>> Who should the emails appear to be from? [Felipe Contreras
>> <felipe.contreras@gmail.com>]
>
> Which sounds fine to me. EMAIL is considered explicit, and I have not
> seen any evidence that people are putting bogus values in their EMAIL
> variable and complaining that it is git's fault for respecting it.
Agreed.
>> 5) GIT_COMMITTER
>>
>> Who should the emails appear to be from? [Felipe Contreras 2nd
>> <felipe.contreras+2@gmail.com>]
>>
>> Whoa, what happened there?
>>
>> Well:
>>
>> $sender = $repoauthor || $repocommitter || '';
>> ($repoauthor) = Git::ident_person(@repo, 'author');
>> % ./git var GIT_AUTHOR_IDENT
>> Felipe Contreras 2nd <felipe.contreras+2@gmail.com> 1352783223 +0100
>>
>> That's right, AUTHOR_IDENT would fall back to the default email and full name.
>
> Yeah, I find that somewhat questionable in the current behavior, and I'd
> consider it a bug. Typically we prefer the committer ident when given a
> choice (e.g., for writing reflog entries).
Yeah, but clearly the intention of the code was to use the committer
if the author wasn't available, which is the case here.
>> 5.1) GIT_COMMITER without anything else
>>
>> fatal: empty ident name (for <felipec@nysa.(none)>) not allowed
>> var GIT_AUTHOR_IDENT: command returned error: 128
>
> Right. Same bug as above.
No, this is a different bug.
The bug above 5) is here:
$sender = $repoauthor || $repocommitter || '';
$repoauthor will always evaluate to true.
This one 5.1) is there:
($repoauthor) = Git::ident_person(@repo, 'author');
($repocommitter) = Git::ident_person(@repo, 'committer'); <-
>> So $repoauthor || $repocommiter is pointless.
>
> Agreed.
Good.
>> 6) GIT_AUTHOR
>>
>> Who should the emails appear to be from? [Felipe Contreras 4th
>> <felipe.contreras+4@gmail.com>]
>
> Right, that's what I'd expect.
You mean without the input question?
>> What about after my change?
>>
>> 6.1) GIT_AUTHOR without anything else
>>
>> fatal: empty ident name (for <felipec@nysa.(none)>) not allowed
>> var GIT_COMMITTER_IDENT: command returned error: 128
>
> Doesn't that seem like a regression? It used to work.
No, this is *before* my change.
I's the same bug as 5.1):
($repoauthor) = Git::ident_person(@repo, 'author'); <- here
($repocommitter) = Git::ident_person(@repo, 'committer');
>> 4) config user
>>
>> From: Felipe Contreras 2nd <felipe.contreras+2@gmail.com>
>
> OK.
>
>> 5) GIT_COMMITTER
>>
>> From: Felipe Contreras 2nd <felipe.contreras+2@gmail.com>
>
> OK.
>
>> 6) GIT_AUTHOR
>>
>> From: Felipe Contreras 2nd <felipe.contreras+2@gmail.com>
>
> Doesn't that seem like a regression? It used to use a different address,
> and in every other git program, the environment takes precedence over
> config.
Yes, it is a regression (that won't affect anybody).
>> And what about your proposed change?
>
> Let me be clear that I sent you a "something like this" patch to try to
> point you in the right direction. If it has a bug or is incomplete, that
> does not mean the direction is wrong, but only that I did not spend very
> much time on the patch.
It doesn't matter, the idea was to use user_ident_sufficiently_given().
>> 6.1) GIT_AUTHOR without anything else
>>
>> Even if the previous problem was solved:
>>
>> export GIT_AUTHOR_NAME='Felipe Contreras 4th'; export
>> GIT_AUTHOR_EMAIL='felipe.contreras+4@gmail.com'
>> ./git var GIT_EXPLICIT_IDENT
>> 0
>>
>> No explicit ident? This is most certainly not what the user would expect.
>
> Yes, it looks like we do not set up the explicit ident flags when
> parsing the author. So my patch is insufficient.
Indeed.
>> 5.2) GIT_COMMITTER with Full Name and full hostname
>>
>> export GIT_COMMITTER_NAME='Felipe Contreras 3nd'; export
>> GIT_COMMITTER_EMAIL='felipe.contreras+3@gmail.com'
>> ./git var GIT_EXPLICIT_IDENT
>> 1
>>
>> From: Felipe Contreras <felipec@nysa.felipec.org>
>>
>> It is explicit, yeah, but 'git send-email' would not be picking the
>> committer, it would pick the author.
>
> Yep.
>
> The explicitness needs to be tied to the specific ident we grabbed.
> Probably adding a "git var GIT_AUTHOR_EXPLICIT" would be enough, or
> alternatively, adding a flag to "git var" to error out rather than
> return a non-explicit ident (this may need to adjust the error
> handling of the "git var" calls from send-email).
I think strictess should be tied to explicitness, and 'git var' should
error out, and not die.
>> > I tried to help you by pointing you in the right direction and even
>> > providing a sample "git var" patch.
>>
>> Are you 100% sure that was the right direction?
>
> I think that respecting the usual ident lookup but disallowing implicit
> identities (either totally, or causing them to fallback to prompting) is
> the right direction. I agree my patch was not a complete solution. I'm
> sorry if it led you astray in terms of implementation, but I also think
> I've been very clear in my text about what the behavior should be.
I think that is orthogonal to what I'm trying accomplish.
>> I think the right approach is more along these lines:
>
> I think that is moving in the right direction, but...
>
>> --- a/ident.c
>> +++ b/ident.c
>> @@ -291,9 +291,9 @@ const char *fmt_ident(const char *name, const char *email,
>> }
>>
>> if (strict && email == git_default_email.buf &&
>> - strstr(email, "(none)")) {
>> + !(user_ident_explicitly_given & IDENT_MAIL_GIVEN)) {
>> fputs(env_hint, stderr);
>> - die("unable to auto-detect email address (got '%s')", email);
>> + die("no explicit email address");
>
> I think this needs to be optional, otherwise you are breaking callers
> who use IDENT_STRICT but are OK with the implicit ident (e.g.,
> commit, format-patch with threading).
>
> You can argue whether "git commit" should disallow such addresses, but
> that is a separate topic from how send-email should behave.
Yes, that's exactly what I would argue.
>> Not only will this fix 'git send-email', but it will also fix 'git
>> commit' so that we don't end up with authors such as 'Felipe Contreras
>> <felipec@nysa.felipec.org>' ever again.
>
> While simultaneously breaking "git commit" for people who are happily
> using the implicit generation. I can see the appeal of doing so; I was
> tempted to suggest it when I cleaned up IDENT_STRICT a few months back.
> But do we have any data on how many people are currently using that
> feature that would be annoyed by it?
No, but it can also be considered a bug... do we have any data on how
many people are being affected by this? If the '(none)' commits are
any indication of it, probably a lot. At least the ones that do have a
fqdn.
>> > But it is not my itch to scratch.
>>
>> Suit yourself, it's only git users that would get hurt. I can always
>> use my own 'git send-email' (as I am doing right now).
>
> Don't get me wrong. I think the spirit of your patch is correct, and it
> helps some git users. But it also hurts others. And it is not that hard
> to do it right.
And I disagree, I think it hurts nobody, and I think it's hard to do it right.
> It may be something I would work on myself in the future, but I have
> other things to work on at the moment, and since you are interested in
> the topic, I thought you would be a good candidate to polish it enough
> to be suitable upstream. But instead I see a lot of push-back on what I
> considered to be a fairly straightforward technical comment on a
> regression.
I'm just trying to be pragmatic. I don't see the point in wasting my
time for people that don't exist. As I said, I don't think anybody
would be hit by this.
> And now I have wasted a large chunk of the evening responding to you,
> neither accomplishing my other tasks nor polishing this topic. I do not
> mind reviewing patches or responding to discussions, nor do I consider
> them time wasted; they are an important part of the development process.
> But I feel like I am fighting an uphill battle just to convince you that
> regressions are bad, and that I am having to make the same points
> repeatedly. That makes me frustrated and less excited about reviewing
> your patches; and when I say "it is not my itch", that is my most polite
> way of saying "If that is going to be your attitude, then I do not feel
> like dealing with you anymore on this topic".
Fixing a regression that nobody would notice is not my itch either,
yet the patch I sent above does it, and it even fixes 'git commit'
(IMO). But it's also not good enough.
I scratched my itch with the original patch, anything after that is to
help other people.
I think it would be much easier to just remove the question input. The
only "regression" would be the people that have a fqdn and full name
_and_ expect the question. But Junio suggested to just die in those
cases, and trying to send an email that would probably fail is not
that different.
Fixing all the var and ident infrastructure seems way, *way* far from
what I intended to do.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* [PATCH 3/3] fix corner case for relative submodule path calculation
From: Heiko Voigt @ 2012-11-13 8:35 UTC (permalink / raw)
To: Jeff King; +Cc: Jeffrey S. Haemer, Jens Lehmann, Git Issues
In-Reply-To: <20121113083233.GA38188@book.hvoigt.net>
A trailing /. for the superprojects origin is treated as
a full path component. This is wrong. Lets add a test and
fix this.
Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
---
git-submodule.sh | 22 ++++++++++++++++++++++
t/t7400-submodule-basic.sh | 44 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 66 insertions(+)
diff --git a/git-submodule.sh b/git-submodule.sh
index ab6b110..9f61a9c 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -69,6 +69,28 @@ resolve_relative_url ()
;;
esac
+ # strip one dot path components
+ tempurl="$remoteurl"
+ remoteurl=
+ sep=
+ while test -n "$tempurl"
+ do
+ case "$tempurl" in
+ */.)
+ tempurl="${tempurl%/.}"
+ ;;
+ ?*/*)
+ remoteurl="${tempurl##*/}$sep$remoteurl"
+ tempurl="${tempurl%/*}"
+ sep=/
+ ;;
+ *)
+ remoteurl="$tempurl$sep$remoteurl"
+ tempurl=
+ ;;
+ esac
+ done
+
while test -n "$url"
do
case "$url" in
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 3c2afa6..1b4cc00 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -518,6 +518,50 @@ test_expect_success 'subrepo is NOT considered a relative path"' '
)
'
+test_expect_success '../subrepo works with absolute local path - "$submodurl/repo/."' '
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ git config remote.origin.url "$submodurl/repo/." &&
+ git submodule init &&
+ test "$(git config submodule.sub.url)" = "$submodurl/subrepo"
+ )
+'
+
+test_expect_success '../subrepo works with absolute local path - "$submodurl/repo/./"' '
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ git config remote.origin.url "$submodurl/repo/./" &&
+ git submodule init &&
+ test "$(git config submodule.sub.url)" = "$submodurl/subrepo"
+ )
+'
+
+test_expect_success '../subrepo works with absolute local path - "$submodurl/./repo/."' '
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ git config remote.origin.url "$submodurl/./repo/." &&
+ git submodule init &&
+ test "$(git config submodule.sub.url)" = "$submodurl/subrepo"
+ )
+'
+
+test_expect_success '../subrepo works with absolute local path - "$submodurl/././repo/."' '
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ git config remote.origin.url "$submodurl/././repo/." &&
+ git submodule init &&
+ test "$(git config submodule.sub.url)" = "$submodurl/subrepo"
+ )
+'
+
test_expect_success '../subrepo works with URL - ssh://hostname/repo' '
(
cd reltest &&
--
1.8.0.3.gaed4666
^ permalink raw reply related
* [PATCH 2/3] ensure that relative submodule url needs ./ or ../
From: Heiko Voigt @ 2012-11-13 8:35 UTC (permalink / raw)
To: Jeff King; +Cc: Jeffrey S. Haemer, Jens Lehmann, Git Issues
In-Reply-To: <20121113083233.GA38188@book.hvoigt.net>
Even though a relative path can be without them the
documentation explicitely talks about them. Lets ensure
that behavior with a test.
Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
---
t/t7400-submodule-basic.sh | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 5397037..3c2afa6 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -506,6 +506,18 @@ test_expect_success 'set up for relative path tests' '
)
'
+test_expect_success 'subrepo is NOT considered a relative path"' '
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ git config -f .gitmodules submodule.sub.url "subrepo" &&
+ git config remote.origin.url "$submodurl" &&
+ git submodule init &&
+ test "$(git config submodule.sub.url)" = subrepo
+ )
+'
+
test_expect_success '../subrepo works with URL - ssh://hostname/repo' '
(
cd reltest &&
--
1.8.0.3.gaed4666
^ permalink raw reply related
* [PATCH 1/3] Fix relative submodule setup of submodule tests
From: Heiko Voigt @ 2012-11-13 8:34 UTC (permalink / raw)
To: Jeff King; +Cc: Jeffrey S. Haemer, Jens Lehmann, Git Issues
In-Reply-To: <20121113083233.GA38188@book.hvoigt.net>
If a remote is configured in a superproject relative submodule urls
should be relative to that remote. Since we have a bug in relative
path calculation for superproject paths that contain a "/." using
../submodule was accepted here. We are going to fix this behavior so
we first need to correct these tests.
Later tests expect the submodules origin to be in a directory underneath
the tests root. Lets remove the origin from super (which points directly
at the tests root directory) to keep these tests expectations.
Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
---
t/t7403-submodule-sync.sh | 2 ++
t/t7406-submodule-update.sh | 2 ++
t/t7407-submodule-foreach.sh | 2 ++
t/t7506-status-submodule.sh | 2 ++
4 files changed, 8 insertions(+)
diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh
index 524d5c1..b310a58 100755
--- a/t/t7403-submodule-sync.sh
+++ b/t/t7403-submodule-sync.sh
@@ -18,6 +18,8 @@ test_expect_success setup '
git clone . super &&
git clone super submodule &&
(cd super &&
+ # relative submodule urls relate to this folder not the remotes
+ git remote rm origin &&
git submodule add ../submodule submodule &&
test_tick &&
git commit -m "submodule"
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 1542653..f3628c9 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -32,6 +32,8 @@ test_expect_success 'setup a submodule tree' '
git clone super merging &&
git clone super none &&
(cd super &&
+ # relative submodule urls relate to this folder not the remotes
+ git remote rm origin &&
git submodule add ../submodule submodule &&
test_tick &&
git commit -m "submodule" &&
diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
index 9b69fe2..99956a6 100755
--- a/t/t7407-submodule-foreach.sh
+++ b/t/t7407-submodule-foreach.sh
@@ -21,6 +21,8 @@ test_expect_success 'setup a submodule tree' '
git clone super submodule &&
(
cd super &&
+ # relative submodule urls relate to this folder not the remotes
+ git remote rm origin &&
git submodule add ../submodule sub1 &&
git submodule add ../submodule sub2 &&
git submodule add ../submodule sub3 &&
diff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh
index d31b34d..9021b1a 100755
--- a/t/t7506-status-submodule.sh
+++ b/t/t7506-status-submodule.sh
@@ -203,6 +203,8 @@ test_expect_success 'status with merge conflict in .gitmodules' '
test_create_repo_with_commit sub2 &&
(
cd super &&
+ # relative submodule urls relate to this folder not the remotes
+ git remote rm origin &&
prev=$(git rev-parse HEAD) &&
git checkout -b add_sub1 &&
git submodule add ../sub1 &&
--
1.8.0.3.gaed4666
^ permalink raw reply related
* [PATCH 0/3] fix cloning superprojects from "."
From: Heiko Voigt @ 2012-11-13 8:32 UTC (permalink / raw)
To: Jeff King; +Cc: Jeffrey S. Haemer, Jens Lehmann, Git Issues
In-Reply-To: <20121109184225.GA1190@book.hvoigt.net>
Hi,
On Fri, Nov 09, 2012 at 07:42:26PM +0100, Heiko Voigt wrote:
> Since this is a change in behaviour I would like to further think about
> the implications this brings if we fix this. Not sure how many people
> clone from ".". The correct behavior (as documented) is the one you
> introduce with your patch. If we decide to fix this we should also correct
> the path calculation in git-submodule.sh.
Ok I think this corner case is not that commonly used since most people
work with remote remotes which you can not cd into to clone from ".".
Here is a patch series to clean this handling up.
Cheers Heiko
Heiko Voigt (3):
Fix relative submodule setup of submodule tests
ensure that relative submodule url needs ./ or ../
fix corner case for relative submodule path calculation
git-submodule.sh | 22 +++++++++++++++++
t/t7400-submodule-basic.sh | 56 ++++++++++++++++++++++++++++++++++++++++++++
t/t7403-submodule-sync.sh | 2 ++
t/t7406-submodule-update.sh | 2 ++
t/t7407-submodule-foreach.sh | 2 ++
t/t7506-status-submodule.sh | 2 ++
6 files changed, 86 insertions(+)
--
1.8.0.3.gaed4666
^ permalink raw reply
* RE: [BUG] gitweb: XSS vulnerability of RSS feed
From: Pyeron, Jason J CTR (US) @ 2012-11-13 8:31 UTC (permalink / raw)
To: git@vger.kernel.org
In-Reply-To: <CAM9Z-n=6xsC7yiKJ+NU-CxNPxEXWmJzvXLUocgZgWPQnuK6G4Q@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1644 bytes --]
> -----Original Message-----
> From: Drew Northup
> Sent: Monday, November 12, 2012 1:56 PM
>
> On Sun, Nov 11, 2012 at 6:28 PM, glpk xypron <xypron.glpk@gmx.de>
> wrote:
> > Gitweb can be used to generate an RSS feed.
> >
> > Arbitrary tags can be inserted into the XML document describing
> > the RSS feed by careful construction of the URL.
> >
> > Example
> >
> http://server/?p=project.git&a=rss&f=</title><script>alert(document.coo
> kie)</script><title>
> >
> > The generated XML contains
> > <script>alert(document.cookie)</script>
This is just an example.
> >
> > Depending on the system used to render the XML this might lead
> > to the execution of javascript in the security context of the
> > gitweb server pages.
> >
> > Please, escape all URL parameters.
We should look for the general entry points, not the script tag.
> >
> > Version tested:
> > gitweb v.1.8.0.dirty with git 1.7.2.5
> >
> > Best regards
> >> Heinrich Schuchardt
>
> Something like this may be useful to defuse the "file" parameter, but
> I presume a more definitive fix is in order...
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 10ed9e5..af93e65 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1447,6 +1447,10 @@ sub validate_pathname {
> if ($input =~ m!\0!) {
> return undef;
> }
> + # No XSS <script></script> inclusions
### not real perl...
foreach $xml in ( <, >, &, ...)
{
$input=~s/$xml/xmlescape{$xml}/g;
}
### "<" => "<"
> + if ($input =~ m!(<script>)(.*)(</script>)!){
> + return undef;
> + }
> return $input;
> }
>
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5615 bytes --]
^ permalink raw reply
* Re: [PATCH] send-email: add proper default sender
From: Jeff King @ 2012-11-13 7:47 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <CAMP44s1w3oZhEUM-cnO=kECH2bhdOTGVuKy8JS4uhWFbA_oi3w@mail.gmail.com>
On Tue, Nov 13, 2012 at 07:42:58AM +0100, Felipe Contreras wrote:
> >> > No, it's not. Those broken names do not come from the environment, but
> >> > from our last-resort guess of the hostname.
> >>
> >> That depends how you define environment, but fine, the point is that
> >> it happens.
> >
> > If you have a strawman definition that does not have anything to do with
> > what I said in my original email, then yes, it could happen.
>
> It happens, I've seen commits with (none) not that long ago.
There was a bug that caused the check to fail in some cases. I fixed it
in f20f387 (this July).
But I still don't see how that has anything to do with what send-email
does or should do. That is why I said "strawman" above. You seem to
think I am saying that send-email should use the system that generated
those broken names, when I am saying the opposite.
> > But as I said already, "git var" uses IDENT_STRICT and will not
> > allow such broken names.
>
> Since 1.7.11, sure. But not everyone is using such a recent version of
> git, and people with fully qualified domains would still get unwanted
> behavior.
Those people would also not be using a new version of git-send-email,
and it will always prompt. I thought we were talking about what
send-email should do in future versions. Namely, loosening that safety
valve (the prompt) because it is inconvenient, but tightening the checks
so that losing the safety valve is not a problem.
> > Did you read my email? I explicitly proposed that we would _not_ allow
> > send-email to use implicit email addresses constructed in that way.
>
> I'm not talking about git send-email, I'm talking about your comment
> 'it has always been the case that you can use git without setting
> user.*', which has caused issues with wrong author/commmitter names in
> commits, and will probably continue to do so.
The second half of that sentence that you quoted above is "...instead
only using the environment." As in, environment variables like
GIT_AUTHOR_EMAIL, GIT_COMMITTER_EMAIL, and EMAIL. _Not_ implicit
generation of the email from the username and hostname.
I am tempted to fault myself for not communicating well, but I feel like
I have made that point at least 3 times in this conversation now. Is
that the source of the confusion?
> > Sorry, but that is not how things work on this project. You do not get
> > to cause regressions because you are too lazy to implement the feature
> > _you_ want in a way that does not break other people.
>
> That doesn't change the fact that they would survive, and the fact
> that those users don't actually exist.
And you will survive if upstream git (whether it is me today or Junio
tomorrow) does not pick up your patch. I remember writing you a long
email recently about how one of the responsibilities of the maintainer
is to balance features versus regressions. I'll not bother repeating
myself here.
As for whether they exist, what data do you have? Are you aware that the
test suite, for example, relies on setting GIT_AUTHOR_NAME but not
having any user.* config? When somebody comes on the list and asks why
every git program in the entire system respects GIT_* environment
variables as an override to user.* configuration _except_ for
send-email, what should I say?
> But let's look at the current situation closely:
>
> PERL5LIB=~/dev/git/perl ./git-send-email.perl --confirm=always -1
>
> 1) No information at all
>
> fatal: empty ident name (for <felipec@nysa.(none)>) not allowed
That is dependent on your system. If you have a non-empty name in your
GECOS field, and if your machine has a FQDN, it will currently work (and
prompt).
> 2) Full Name + full hostname
>
> Who should the emails appear to be from? [Felipe Contreras
> <felipec@nysa.felipec.org>]
>
> That's right, ident doesn't fail, and that's not the mail address I
> specified, it's *implicit*.
Right. I never said it did. I said it currently rejected obviously bogus
stuff (like the ".(none)" above) due to IDENT_STRICT, but currently
allowed implicit definitions. And I also said that if we get rid of the
prompt, we should disallow implicit definitions like this, because the
prompt is the safety valve on sending out mails with broken from
addresses. I even wrote a patch that let you find out whether the ident
was generated implicitly.
> 3) Full Name + EMAIL
>
> Who should the emails appear to be from? [Felipe Contreras
> <felipe.contreras@gmail.com>]
Which sounds fine to me. EMAIL is considered explicit, and I have not
seen any evidence that people are putting bogus values in their EMAIL
variable and complaining that it is git's fault for respecting it.
> 4) config user
>
> Who should the emails appear to be from? [Felipe Contreras 2nd
> <felipe.contreras+2@gmail.com>]
OK.
> 5) GIT_COMMITTER
>
> Who should the emails appear to be from? [Felipe Contreras 2nd
> <felipe.contreras+2@gmail.com>]
>
> Whoa, what happened there?
>
> Well:
>
> $sender = $repoauthor || $repocommitter || '';
> ($repoauthor) = Git::ident_person(@repo, 'author');
> % ./git var GIT_AUTHOR_IDENT
> Felipe Contreras 2nd <felipe.contreras+2@gmail.com> 1352783223 +0100
>
> That's right, AUTHOR_IDENT would fall back to the default email and full name.
Yeah, I find that somewhat questionable in the current behavior, and I'd
consider it a bug. Typically we prefer the committer ident when given a
choice (e.g., for writing reflog entries).
> 5.1) GIT_COMMITER without anything else
>
> fatal: empty ident name (for <felipec@nysa.(none)>) not allowed
> var GIT_AUTHOR_IDENT: command returned error: 128
Right. Same bug as above.
> So $repoauthor || $repocommiter is pointless.
Agreed.
> 6) GIT_AUTHOR
>
> Who should the emails appear to be from? [Felipe Contreras 4th
> <felipe.contreras+4@gmail.com>]
Right, that's what I'd expect.
> What about after my change?
>
> 6.1) GIT_AUTHOR without anything else
>
> fatal: empty ident name (for <felipec@nysa.(none)>) not allowed
> var GIT_COMMITTER_IDENT: command returned error: 128
Doesn't that seem like a regression? It used to work.
> 4) config user
>
> From: Felipe Contreras 2nd <felipe.contreras+2@gmail.com>
OK.
> 5) GIT_COMMITTER
>
> From: Felipe Contreras 2nd <felipe.contreras+2@gmail.com>
OK.
> 6) GIT_AUTHOR
>
> From: Felipe Contreras 2nd <felipe.contreras+2@gmail.com>
Doesn't that seem like a regression? It used to use a different address,
and in every other git program, the environment takes precedence over
config.
> And what about your proposed change?
Let me be clear that I sent you a "something like this" patch to try to
point you in the right direction. If it has a bug or is incomplete, that
does not mean the direction is wrong, but only that I did not spend very
much time on the patch.
> 6.1) GIT_AUTHOR without anything else
>
> Even if the previous problem was solved:
>
> export GIT_AUTHOR_NAME='Felipe Contreras 4th'; export
> GIT_AUTHOR_EMAIL='felipe.contreras+4@gmail.com'
> ./git var GIT_EXPLICIT_IDENT
> 0
>
> No explicit ident? This is most certainly not what the user would expect.
Yes, it looks like we do not set up the explicit ident flags when
parsing the author. So my patch is insufficient.
> 5.2) GIT_COMMITTER with Full Name and full hostname
>
> export GIT_COMMITTER_NAME='Felipe Contreras 3nd'; export
> GIT_COMMITTER_EMAIL='felipe.contreras+3@gmail.com'
> ./git var GIT_EXPLICIT_IDENT
> 1
>
> From: Felipe Contreras <felipec@nysa.felipec.org>
>
> It is explicit, yeah, but 'git send-email' would not be picking the
> committer, it would pick the author.
Yep.
The explicitness needs to be tied to the specific ident we grabbed.
Probably adding a "git var GIT_AUTHOR_EXPLICIT" would be enough, or
alternatively, adding a flag to "git var" to error out rather than
return a non-explicit ident (this may need to adjust the error
handling of the "git var" calls from send-email).
> > I tried to help you by pointing you in the right direction and even
> > providing a sample "git var" patch.
>
> Are you 100% sure that was the right direction?
I think that respecting the usual ident lookup but disallowing implicit
identities (either totally, or causing them to fallback to prompting) is
the right direction. I agree my patch was not a complete solution. I'm
sorry if it led you astray in terms of implementation, but I also think
I've been very clear in my text about what the behavior should be.
> I think the right approach is more along these lines:
I think that is moving in the right direction, but...
> --- a/ident.c
> +++ b/ident.c
> @@ -291,9 +291,9 @@ const char *fmt_ident(const char *name, const char *email,
> }
>
> if (strict && email == git_default_email.buf &&
> - strstr(email, "(none)")) {
> + !(user_ident_explicitly_given & IDENT_MAIL_GIVEN)) {
> fputs(env_hint, stderr);
> - die("unable to auto-detect email address (got '%s')", email);
> + die("no explicit email address");
I think this needs to be optional, otherwise you are breaking callers
who use IDENT_STRICT but are OK with the implicit ident (e.g.,
commit, format-patch with threading).
You can argue whether "git commit" should disallow such addresses, but
that is a separate topic from how send-email should behave.
> Not only will this fix 'git send-email', but it will also fix 'git
> commit' so that we don't end up with authors such as 'Felipe Contreras
> <felipec@nysa.felipec.org>' ever again.
While simultaneously breaking "git commit" for people who are happily
using the implicit generation. I can see the appeal of doing so; I was
tempted to suggest it when I cleaned up IDENT_STRICT a few months back.
But do we have any data on how many people are currently using that
feature that would be annoyed by it?
> > But it is not my itch to scratch.
>
> Suit yourself, it's only git users that would get hurt. I can always
> use my own 'git send-email' (as I am doing right now).
Don't get me wrong. I think the spirit of your patch is correct, and it
helps some git users. But it also hurts others. And it is not that hard
to do it right.
It may be something I would work on myself in the future, but I have
other things to work on at the moment, and since you are interested in
the topic, I thought you would be a good candidate to polish it enough
to be suitable upstream. But instead I see a lot of push-back on what I
considered to be a fairly straightforward technical comment on a
regression.
And now I have wasted a large chunk of the evening responding to you,
neither accomplishing my other tasks nor polishing this topic. I do not
mind reviewing patches or responding to discussions, nor do I consider
them time wasted; they are an important part of the development process.
But I feel like I am fighting an uphill battle just to convince you that
regressions are bad, and that I am having to make the same points
repeatedly. That makes me frustrated and less excited about reviewing
your patches; and when I say "it is not my itch", that is my most polite
way of saying "If that is going to be your attitude, then I do not feel
like dealing with you anymore on this topic".
-Peff
^ permalink raw reply
* Re: [PATCH] send-email: add proper default sender
From: Felipe Contreras @ 2012-11-13 7:18 UTC (permalink / raw)
To: Jeff King; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <CAMP44s1w3oZhEUM-cnO=kECH2bhdOTGVuKy8JS4uhWFbA_oi3w@mail.gmail.com>
On Tue, Nov 13, 2012 at 7:42 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> 6) GIT_AUTHOR
>
> Who should the emails appear to be from? [Felipe Contreras 4th
> <felipe.contreras+4@gmail.com>]
>
> What about after my change?
>
> 6.1) GIT_AUTHOR without anything else
>
> fatal: empty ident name (for <felipec@nysa.(none)>) not allowed
> var GIT_COMMITTER_IDENT: command returned error: 128
This was supposed to be above (before my change).
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 1/4] remote-hg: add missing config for basic tests
From: Felipe Contreras @ 2012-11-13 7:12 UTC (permalink / raw)
To: Jeff King; +Cc: git, Ramkumar Ramachandra
In-Reply-To: <20121113054826.GC10995@sigill.intra.peff.net>
On Tue, Nov 13, 2012 at 6:48 AM, Jeff King <peff@peff.net> wrote:
> Any objection to me marking it up as I apply?
Nope.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 2/4] remote-hg: fix compatibility with older versions of hg
From: Felipe Contreras @ 2012-11-13 7:11 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: git
In-Reply-To: <CALkWK0k9Ta-SOo43Knz9kMV0ePKf7wjK-3Zm6ocDQpe7VzFz+A@mail.gmail.com>
On Tue, Nov 13, 2012 at 6:25 AM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> This patch fixes my original problem. So,
> Reported-by: Ramkumar Ramachandra <artagnon@gmail.com>
> Tested-by: Ramkumar Ramachandra <artagnon@gmail.com>
>
> However, test 4 in test-hg.sh still fails for me:
>
> --- expected 2012-11-13 05:22:57.946637384 +0000
> +++ actual 2012-11-13 05:22:57.946637384 +0000
> @@ -1,2 +1,2 @@
> -zero
> -refs/heads/master
> +feature-a
> +refs/heads/feature-a
>
> What is going on?
Probably a bug in mercurial, but can be worked around:
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -429,10 +429,18 @@ def get_branch_tip(repo, branch):
return heads[0]
+def get_current_bookmark(repo):
+ head = bookmarks.readcurrent(repo)
+ if not head:
+ return None
+ if repo[head] != repo['.']:
+ return None
+ return head
+
def list_head(repo, cur):
global g_head, bmarks
- head = bookmarks.readcurrent(repo)
+ head = get_current_bookmark(repo)
if head:
node = repo[head]
else:
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH] send-email: add proper default sender
From: Felipe Contreras @ 2012-11-13 6:42 UTC (permalink / raw)
To: Jeff King; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <20121113040104.GA9361@sigill.intra.peff.net>
On Tue, Nov 13, 2012 at 5:01 AM, Jeff King <peff@peff.net> wrote:
> On Tue, Nov 13, 2012 at 04:55:25AM +0100, Felipe Contreras wrote:
>
>> > No, it's not. Those broken names do not come from the environment, but
>> > from our last-resort guess of the hostname.
>>
>> That depends how you define environment, but fine, the point is that
>> it happens.
>
> If you have a strawman definition that does not have anything to do with
> what I said in my original email, then yes, it could happen.
It happens, I've seen commits with (none) not that long ago.
> But as I
> said already, "git var" uses IDENT_STRICT and will not allow such broken
> names.
Since 1.7.11, sure. But not everyone is using such a recent version of
git, and people with fully qualified domains would still get unwanted
behavior.
>> > We long ago switched to
>> > printing the name as a warning when we have made such a guess (bb1ae3f),
>> > then more recently started rejecting them outright (8c5b1ae).
>>
>> Right, but these would still happen:
>>
>> michael <michael@michael-laptop.michael.org>
>
> Did you read my email? I explicitly proposed that we would _not_ allow
> send-email to use implicit email addresses constructed in that way.
I'm not talking about git send-email, I'm talking about your comment
'it has always been the case that you can use git without setting
user.*', which has caused issues with wrong author/commmitter names in
commits, and will probably continue to do so.
>> > But in the meantime you are causing a regression for anybody who expects
>> > GIT_AUTHOR_NAME to override user.email when running git-send-email (and
>> > you have taken away the prompt that they could have used to notice and
>> > correct it).
>>
>> I think they can survive. If anybody like this exists.
>
> Sorry, but that is not how things work on this project. You do not get
> to cause regressions because you are too lazy to implement the feature
> _you_ want in a way that does not break other people.
That doesn't change the fact that they would survive, and the fact
that those users don't actually exist.
But let's look at the current situation closely:
PERL5LIB=~/dev/git/perl ./git-send-email.perl --confirm=always -1
1) No information at all
fatal: empty ident name (for <felipec@nysa.(none)>) not allowed
2) Full Name + full hostname
Who should the emails appear to be from? [Felipe Contreras
<felipec@nysa.felipec.org>]
That's right, ident doesn't fail, and that's not the mail address I
specified, it's *implicit*.
3) Full Name + EMAIL
Who should the emails appear to be from? [Felipe Contreras
<felipe.contreras@gmail.com>]
4) config user
Who should the emails appear to be from? [Felipe Contreras 2nd
<felipe.contreras+2@gmail.com>]
5) GIT_COMMITTER
Who should the emails appear to be from? [Felipe Contreras 2nd
<felipe.contreras+2@gmail.com>]
Whoa, what happened there?
Well:
$sender = $repoauthor || $repocommitter || '';
($repoauthor) = Git::ident_person(@repo, 'author');
% ./git var GIT_AUTHOR_IDENT
Felipe Contreras 2nd <felipe.contreras+2@gmail.com> 1352783223 +0100
That's right, AUTHOR_IDENT would fall back to the default email and full name.
Hmm, I wonder...
5.1) GIT_COMMITER without anything else
fatal: empty ident name (for <felipec@nysa.(none)>) not allowed
var GIT_AUTHOR_IDENT: command returned error: 128
Why? Because:
% PERL5LIB=~/dev/git/perl perl -e 'use Git; printf("%s\n",
Git::ident_person(@repo, 'author'));'
fatal: empty ident name (for <felipec@nysa.(none)>) not allowed
($repoauthor) = Git::ident_person(@repo, 'author');
($repocommitter) = Git::ident_person(@repo, 'committer');
So $repoauthor || $repocommiter is pointless.
6) GIT_AUTHOR
Who should the emails appear to be from? [Felipe Contreras 4th
<felipe.contreras+4@gmail.com>]
What about after my change?
6.1) GIT_AUTHOR without anything else
fatal: empty ident name (for <felipec@nysa.(none)>) not allowed
var GIT_COMMITTER_IDENT: command returned error: 128
4) config user
From: Felipe Contreras 2nd <felipe.contreras+2@gmail.com>
5) GIT_COMMITTER
From: Felipe Contreras 2nd <felipe.contreras+2@gmail.com>
6) GIT_AUTHOR
From: Felipe Contreras 2nd <felipe.contreras+2@gmail.com>
And what about your proposed change?
2) Full Name + full hostname
./git var GIT_EXPLICIT_IDENT
0
6.1) GIT_AUTHOR without anything else
Even if the previous problem was solved:
export GIT_AUTHOR_NAME='Felipe Contreras 4th'; export
GIT_AUTHOR_EMAIL='felipe.contreras+4@gmail.com'
./git var GIT_EXPLICIT_IDENT
0
No explicit ident? This is most certainly not what the user would expect.
And then:
5.2) GIT_COMMITTER with Full Name and full hostname
export GIT_COMMITTER_NAME='Felipe Contreras 3nd'; export
GIT_COMMITTER_EMAIL='felipe.contreras+3@gmail.com'
./git var GIT_EXPLICIT_IDENT
1
From: Felipe Contreras <felipec@nysa.felipec.org>
It is explicit, yeah, but 'git send-email' would not be picking the
committer, it would pick the author.
> I tried to help you by pointing you in the right direction and even
> providing a sample "git var" patch.
Are you 100% sure that was the right direction?
I think the right approach is more along these lines:
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -748,16 +748,11 @@ if (!$force) {
}
}
-my $prompting = 0;
if (!defined $sender) {
$sender = $repoauthor || $repocommitter || '';
- $sender = ask("Who should the emails appear to be from? [$sender] ",
- default => $sender,
- valid_re => qr/\@.*\./, confirm_only => 1);
- print "Emails will be sent from: ", $sender, "\n";
- $prompting++;
}
+my $prompting = 0;
if (!@initial_to && !defined $to_cmd) {
my $to = ask("Who should the emails be sent to (if any)? ",
default => "",
diff --git a/ident.c b/ident.c
index a4bf206..c73ba82 100644
--- a/ident.c
+++ b/ident.c
@@ -291,9 +291,9 @@ const char *fmt_ident(const char *name, const char *email,
}
if (strict && email == git_default_email.buf &&
- strstr(email, "(none)")) {
+ !(user_ident_explicitly_given & IDENT_MAIL_GIVEN)) {
fputs(env_hint, stderr);
- die("unable to auto-detect email address (got '%s')", email);
+ die("no explicit email address");
}
if (want_date) {
With that we get:
2) Full Name + full hostname
fatal: no explicit email address
3) Full Name + EMAIL
From: Felipe Contreras <felipe.contreras@gmail.com>
4) config user
From: Felipe Contreras 2nd <felipe.contreras+2@gmail.com>
5) GIT_COMMITTER
From: Felipe Contreras 2nd <felipe.contreras+2@gmail.com>
(as buggy as before)
6) GIT_AUTHOR
From: Felipe Contreras 4th <felipe.contreras+4@gmail.com>
Not only will this fix 'git send-email', but it will also fix 'git
commit' so that we don't end up with authors such as 'Felipe Contreras
<felipec@nysa.felipec.org>' ever again.
> But it is not my itch to scratch.
Suit yourself, it's only git users that would get hurt. I can always
use my own 'git send-email' (as I am doing right now).
Cheers.
--
Felipe Contreras
^ permalink raw reply related
* Re: Re: [PATCH] git tag --contains : avoid stack overflow
From: Jeff King @ 2012-11-13 6:08 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, Jean-Jacques Lafay, René Scharfe,
msysgit, Git List, Philip Oakley
In-Reply-To: <7vwqxqf6li.fsf@alter.siamese.dyndns.org>
On Mon, Nov 12, 2012 at 08:51:37PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > Yeah. We tolerate a certain amount of skew (24 hours for --name-rev, and
> > 5 broken commits in a row for --since). But the big ones are usually
> > software bugs (the big kernel ones were from broken "guilt", I think) or
> > broken imports (when I published a bunch of skew statistics last year,
> > the interesting ones were all imports; I don't know if they were
> > software bugs, or just garbage in, garbage out).
>
> I was hoping that 2e6bdd3 (test-generation: compute generation
> numbers and clock skews, 2012-09-04) may be a good first step to
> come up with a practical and cheap solution on top of it.
>
> The traversal can be fooled by clock skews when it sees a commit
> that has a timestamp that is older than it should, causing it to
> give up, incorrectly thinking that there won't be newer commits that
> it is interested in behind the problematic commit.
I wrote a similar skew-finding tool last year, though some of the
numbers it came up with were different (I remember having many fewer
skewed commits in the kernel repo).
One problem is that it identifies commits which behave badly with
certain algorithms, but it does not identify commits which are wrong.
If I skew backwards, it will find my commit. But if I skew forwards, it
will label my children as wrong.
> The logic implemented by the change is to identify these problematic
> commits, and we could record these commits with the value of the
> timestamps they should have had (e.g. the timestamp of the newest
> ancestor for each of these commits) in a notes tree. Then the
> traversal logic (commit-list-insert-by-date) could be updated use
> that "corrected" timestamp instead not to be fooled by the clock
> skew.
>
> Such a notes tree can be built once and updated by only "appending",
> as a commit will never acquire more ancestors in its parents chain
> once it is made.
>
> Is it too simplistic, or too costly? In git.git we have three such
> commits whose timestamp need to be corrected, while in the Linux
> kernel there were 2.2k skewed commits when I counted them a few
> months ago.
This came up in the big generations discussion last summer, and I think
I even implemented a proof of concept. I couldn't find the actual code,
though but only that I got "pleasing performance results using a notes
tree to store a list of commits with bogus timestamps":
http://article.gmane.org/gmane.comp.version-control.git/161101
It is a little wasteful in space if you have a lot of skewed commits
(the notes tree stores a 160-bit hash pointing to a blob object storing
a 32-bit integer).
My personal preference at this point would be:
1. introduce an auxiliary metadata file that would live alongside the
pack index and contain generation numbers
2. generate the metadata file during pack indexing.
3. If we have a generation metadata file, but a particular object is
not in it, compute the generation; this should be quick because we
will hit a file with a stored generation eventually
4. If we do not have any generation metadata files, or if grafts or
replace objects are in use, do not use cutoffs in algorithms. Be
safe but slow.
On the other hand, just switching to doing a single traversal instead of
one merge-base computation per tag already got rid of the really awful
performance cases. Nobody has complained since that went in, so maybe
nobody cares about shaving a few seconds per operation down to a few
tens of milliseconds. The real win was shaving tens of seconds down to a
few seconds.
-Peff
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
^ permalink raw reply
* Re: [PATCH 1/4] remote-hg: add missing config for basic tests
From: Jeff King @ 2012-11-13 5:48 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Ramkumar Ramachandra
In-Reply-To: <CAMP44s2GzX_+6+eNU5n6kGue_-MWvU9qNXAr3Caw3A7nWhkawA@mail.gmail.com>
On Tue, Nov 13, 2012 at 04:46:36AM +0100, Felipe Contreras wrote:
> >> +setup () {
> >> + (
> >> + echo "[ui]"
> >> + echo "username = A U Thor <author@example.com>"
> >> + ) >> "$HOME"/.hgrc
> >> +}
> >
> > This makes sense, but I wonder if we should use something different from
> > the git author ident set up by the test scripts, just to double check
> > that we do not have any bugs in confusing the two during the import.
>
> I don't know, but these tests would not check for any of those issues.
> When such tests are added I would prefer the author to use to be
> explicitly defined, but lets see.
It's OK if we do not add more explicit tests at this point. I'd just
rather set a safer precedent on the off chance that it might catch
something in a later test, just as we use separate GIT_AUTHOR_* and
GIT_COMMITTER_* in the rest of the test suite. If the choice were not
completely arbitrary and had some maintenance cost, I might be more
concerned, but as far as I can tell, one name is as good as another at
this point.
Any objection to me marking it up as I apply?
-Peff
^ permalink raw reply
* Re: [PATCH 3/3] submodule: display summary header in bold
From: Jeff King @ 2012-11-13 5:44 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Jens Lehmann
In-Reply-To: <1352653146-3932-4-git-send-email-artagnon@gmail.com>
On Sun, Nov 11, 2012 at 10:29:06PM +0530, Ramkumar Ramachandra wrote:
> Currently, 'git diff --submodule' displays output with a bold diff
> header for non-submodules. So this part is in bold:
>
> diff --git a/file1 b/file1
> index 30b2f6c..2638038 100644
> --- a/file1
> +++ b/file1
>
> For submodules, the header looks like this:
>
> Submodule submodule1 012b072..248d0fd:
>
> Unfortunately, it's easy to miss in the output because it's not bold.
> Change this.
Reading this, I was at first concerned that you were using "bold" and
not DIFF_METAINFO. But:
> diff --git a/diff.c b/diff.c
> index b486070..51c0d6c 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -2267,7 +2267,7 @@ static void builtin_diff(const char *name_a,
> const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
> show_submodule_summary(o->file, one ? one->path : two->path,
> one->sha1, two->sha1, two->dirty_submodule,
> - del, add, reset);
> + set, del, add, reset);
This is correctly passing the DIFF_METAINFO color, which is in the "set"
variable. Good. I looked at the output on a sample repository, and it is
much easier to read (though I use magenta for my metainfo).
"set" is a terrible name for that variable (not your fault, of course).
When you re-roll (and I think you need to because of the diff_ui thing
in patch 2), would you mind throwing this cleanup in the middle?
-- >8 --
Subject: builtin_diff: rename "set" variable
Once upon a time the builtin_diff function used one color, and the color
variables were called "set" and "reset". Nowadays it is a much longer
function and we use several colors (e.g., "add", "del"). Rename "set" to
"meta" to show that it is the color for showing diff meta-info (it still
does not indicate that it is a "color", but at least it matches the
scheme of the other color variables).
Signed-off-by: Jeff King <peff@peff.net>
---
diff.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/diff.c b/diff.c
index e89a201..978e1af 100644
--- a/diff.c
+++ b/diff.c
@@ -2222,7 +2222,7 @@ static void builtin_diff(const char *name_a,
mmfile_t mf1, mf2;
const char *lbl[2];
char *a_one, *b_two;
- const char *set = diff_get_color_opt(o, DIFF_METAINFO);
+ const char *meta = diff_get_color_opt(o, DIFF_METAINFO);
const char *reset = diff_get_color_opt(o, DIFF_RESET);
const char *a_prefix, *b_prefix;
struct userdiff_driver *textconv_one = NULL;
@@ -2269,24 +2269,24 @@ static void builtin_diff(const char *name_a,
b_two = quote_two(b_prefix, name_b + (*name_b == '/'));
lbl[0] = DIFF_FILE_VALID(one) ? a_one : "/dev/null";
lbl[1] = DIFF_FILE_VALID(two) ? b_two : "/dev/null";
- strbuf_addf(&header, "%s%sdiff --git %s %s%s\n", line_prefix, set, a_one, b_two, reset);
+ strbuf_addf(&header, "%s%sdiff --git %s %s%s\n", line_prefix, meta, a_one, b_two, reset);
if (lbl[0][0] == '/') {
/* /dev/null */
- strbuf_addf(&header, "%s%snew file mode %06o%s\n", line_prefix, set, two->mode, reset);
+ strbuf_addf(&header, "%s%snew file mode %06o%s\n", line_prefix, meta, two->mode, reset);
if (xfrm_msg)
strbuf_addstr(&header, xfrm_msg);
must_show_header = 1;
}
else if (lbl[1][0] == '/') {
- strbuf_addf(&header, "%s%sdeleted file mode %06o%s\n", line_prefix, set, one->mode, reset);
+ strbuf_addf(&header, "%s%sdeleted file mode %06o%s\n", line_prefix, meta, one->mode, reset);
if (xfrm_msg)
strbuf_addstr(&header, xfrm_msg);
must_show_header = 1;
}
else {
if (one->mode != two->mode) {
- strbuf_addf(&header, "%s%sold mode %06o%s\n", line_prefix, set, one->mode, reset);
- strbuf_addf(&header, "%s%snew mode %06o%s\n", line_prefix, set, two->mode, reset);
+ strbuf_addf(&header, "%s%sold mode %06o%s\n", line_prefix, meta, one->mode, reset);
+ strbuf_addf(&header, "%s%snew mode %06o%s\n", line_prefix, meta, two->mode, reset);
must_show_header = 1;
}
if (xfrm_msg)
^ permalink raw reply related
* Re: [PATCH 2/3] diff: introduce diff.submodule configuration variable
From: Jeff King @ 2012-11-13 5:33 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Jens Lehmann
In-Reply-To: <1352653146-3932-3-git-send-email-artagnon@gmail.com>
On Sun, Nov 11, 2012 at 10:29:05PM +0530, Ramkumar Ramachandra wrote:
> +static int parse_submodule_params(struct diff_options *options, const char *value,
> + struct strbuf *errmsg)
> +{
> + if (!strcmp(value, "log"))
> + DIFF_OPT_SET(options, SUBMODULE_LOG);
> + else if (!strcmp(value, "short"))
> + DIFF_OPT_CLR(options, SUBMODULE_LOG);
> + else {
> + strbuf_addf(errmsg, _("'%s'"), value);
> + return 1;
> + }
> + return 0;
> +}
I think "-1" would be the more normal error return.
> @@ -223,6 +238,15 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
> return 0;
> }
>
> + if (!strcmp(var, "diff.submodule")) {
Shouldn't this be in git_diff_ui_config so it does not affect scripts
calling plumbing?
> + struct strbuf errmsg = STRBUF_INIT;
> + if (parse_submodule_params(&default_diff_options, value, &errmsg))
> + warning(_("Unknown value for 'diff.submodule' config variable: %s"),
> + errmsg.buf);
> + strbuf_release(&errmsg);
> + return 0;
> + }
Hmm. This strbuf error handling strikes me as very clunky, considering
that it does not pass any useful information out of the parse function
(it always just adds '$value' to the error string). Wouldn't it be
simpler to just have parse_submodule_params return -1, and then let the
caller warn or generate an error as appropriate?
-Peff
^ permalink raw reply
* Re: Test failures in contrib/remote-helpers
From: Ramkumar Ramachandra @ 2012-11-13 5:29 UTC (permalink / raw)
To: Felipe Contreras; +Cc: Git List
In-Reply-To: <CAMP44s1E7qKCPTH1MMuOwJkW8opmD+Q6bzD6j1-KM039g-Uq2g@mail.gmail.com>
Felipe Contreras wrote:
> And here's the one for bzr:
>
> --- a/contrib/remote-helpers/git-remote-bzr
> +++ b/contrib/remote-helpers/git-remote-bzr
> @@ -646,12 +646,12 @@ def get_repo(url, alias):
> global dirname, peer
>
> clone_path = os.path.join(dirname, 'clone')
> - origin = bzrlib.controldir.ControlDir.open(url)
> + origin = bzrlib.bzrdir.BzrDir.open(url)
> remote_branch = origin.open_branch()
>
> if os.path.exists(clone_path):
> # pull
> - d = bzrlib.controldir.ControlDir.open(clone_path)
> + d = bzrlib.bzrdir.BzrDir.open(clone_path)
> branch = d.open_branch()
> result = branch.pull(remote_branch, [], None, False)
> else:
>
> I have other fixes to run up to 2.0.
Works for me. All four tests pass now.
Reported-by: Ramkumar Ramachandra <artagnon@gmail.com>
Tested-by: Ramkumar Ramachandra <artagnon@gmail.com>
Ram
^ permalink raw reply
* Re: [PATCH 2/4] remote-hg: fix compatibility with older versions of hg
From: Ramkumar Ramachandra @ 2012-11-13 5:25 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git
In-Reply-To: <1352742068-15346-3-git-send-email-felipe.contreras@gmail.com>
This patch fixes my original problem. So,
Reported-by: Ramkumar Ramachandra <artagnon@gmail.com>
Tested-by: Ramkumar Ramachandra <artagnon@gmail.com>
However, test 4 in test-hg.sh still fails for me:
--- expected 2012-11-13 05:22:57.946637384 +0000
+++ actual 2012-11-13 05:22:57.946637384 +0000
@@ -1,2 +1,2 @@
-zero
-refs/heads/master
+feature-a
+refs/heads/feature-a
What is going on?
Ram
^ permalink raw reply
* Re: [PATCH v3 0/3] Introduce diff.submodule
From: Ramkumar Ramachandra @ 2012-11-13 5:17 UTC (permalink / raw)
To: Git List; +Cc: Jens Lehmann, Jeff King, Junio C Hamano
In-Reply-To: <1352653146-3932-1-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra wrote:
> v1 is here: http://mid.gmane.org/1349196670-2844-1-git-send-email-artagnon@gmail.com
> v2 is here: http://mid.gmane.org/1351766630-4837-1-git-send-email-artagnon@gmail.com
>
> This version was prepared in response to Peff's review of v2. As
> suggested, I've created a separate function which both '--submodule'
> and 'diff.submodule' use to set/ unset SUBMODULE_OPT.
Junio, can we pick this up?
Ram
^ permalink raw reply
* Re: Re: [PATCH] git tag --contains : avoid stack overflow
From: Johannes Schindelin @ 2012-11-13 4:52 UTC (permalink / raw)
To: Jeff King
Cc: Jean-Jacques Lafay, René Scharfe, msysgit, Git List,
Philip Oakley
In-Reply-To: <20121113040541.GA9439@sigill.intra.peff.net>
Hi Peff,
On Mon, 12 Nov 2012, Jeff King wrote:
> On Tue, Nov 13, 2012 at 04:01:11AM +0000, Johannes Schindelin wrote:
>
> > > Note that name-rev will produce wrong answers in the face of clock skew.
> > > And I think that you even wrote that code. :)
> >
> > IIRC the cute code to short-circuit using the date is not from me. If it
> > is, I am very ashamed.
>
> Sorry, but it was:
>
> $ git blame -L'/commit->date < cutoff/',+1 builtin/name-rev.c
> bd321bcc name-rev.c (Johannes Schindelin 2005-10-26 15:10:20 +0200 32)
> if (commit->date < cutoff)
>
> But it is never too late to fix it. :)
I will now go and find a hole to hide in. Or alternatively finally go to
sleep.
Ciao,
Johannes
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
^ permalink raw reply
* Re: Re: [PATCH] git tag --contains : avoid stack overflow
From: Junio C Hamano @ 2012-11-13 4:51 UTC (permalink / raw)
To: Jeff King
Cc: Johannes Schindelin, Jean-Jacques Lafay, René Scharfe,
msysgit, Git List, Philip Oakley
In-Reply-To: <20121113034605.GB8387@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Yeah. We tolerate a certain amount of skew (24 hours for --name-rev, and
> 5 broken commits in a row for --since). But the big ones are usually
> software bugs (the big kernel ones were from broken "guilt", I think) or
> broken imports (when I published a bunch of skew statistics last year,
> the interesting ones were all imports; I don't know if they were
> software bugs, or just garbage in, garbage out).
I was hoping that 2e6bdd3 (test-generation: compute generation
numbers and clock skews, 2012-09-04) may be a good first step to
come up with a practical and cheap solution on top of it.
The traversal can be fooled by clock skews when it sees a commit
that has a timestamp that is older than it should, causing it to
give up, incorrectly thinking that there won't be newer commits that
it is interested in behind the problematic commit.
The logic implemented by the change is to identify these problematic
commits, and we could record these commits with the value of the
timestamps they should have had (e.g. the timestamp of the newest
ancestor for each of these commits) in a notes tree. Then the
traversal logic (commit-list-insert-by-date) could be updated use
that "corrected" timestamp instead not to be fooled by the clock
skew.
Such a notes tree can be built once and updated by only "appending",
as a commit will never acquire more ancestors in its parents chain
once it is made.
Is it too simplistic, or too costly? In git.git we have three such
commits whose timestamp need to be corrected, while in the Linux
kernel there were 2.2k skewed commits when I counted them a few
months ago.
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
^ permalink raw reply
* Re: Re: [PATCH] git tag --contains : avoid stack overflow
From: Jeff King @ 2012-11-13 4:05 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Jean-Jacques Lafay, René Scharfe, msysgit, Git List,
Philip Oakley
In-Reply-To: <alpine.DEB.1.00.1211130400030.13573@bonsai2>
On Tue, Nov 13, 2012 at 04:01:11AM +0000, Johannes Schindelin wrote:
> > Note that name-rev will produce wrong answers in the face of clock skew.
> > And I think that you even wrote that code. :)
>
> IIRC the cute code to short-circuit using the date is not from me. If it
> is, I am very ashamed.
Sorry, but it was:
$ git blame -L'/commit->date < cutoff/',+1 builtin/name-rev.c
bd321bcc name-rev.c (Johannes Schindelin 2005-10-26 15:10:20 +0200 32)
if (commit->date < cutoff)
But it is never too late to fix it. :)
-Peff
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
^ permalink raw reply
* Re: Re: [PATCH] git tag --contains : avoid stack overflow
From: Johannes Schindelin @ 2012-11-13 4:01 UTC (permalink / raw)
To: Jeff King
Cc: Jean-Jacques Lafay, René Scharfe, msysgit, Git List,
Philip Oakley
In-Reply-To: <20121113034605.GB8387@sigill.intra.peff.net>
Hi Peff,
On Mon, 12 Nov 2012, Jeff King wrote:
> On Tue, Nov 13, 2012 at 01:16:01AM +0000, Johannes Schindelin wrote:
>
> > > We can do much better than O(number of commits), though, if we stop
> > > traversing down a path when its timestamp shows that it is too old to
> > > contain the commits we are searching for. The problem is that the
> > > timestamps cannot always be trusted, because they are generated on
> > > machines with wrong clocks, or by buggy software. This could be solved
> > > by calculating and caching a "generation" number, but last time it was
> > > discussed there was a lot of arguing and nothing got done.
> >
> > Sadly, not only machines with skewed clocks, but in particular buggy
> > 3rd-party SCMs make this more than just problematic. In a git-svn clone
> > that was used as base for heavy Git development, I encountered quite a lot
> > of Jan 1, 1970 commits.
>
> Yeah. We tolerate a certain amount of skew (24 hours for --name-rev, and
> 5 broken commits in a row for --since). But the big ones are usually
> software bugs (the big kernel ones were from broken "guilt", I think) or
> broken imports (when I published a bunch of skew statistics last year,
> the interesting ones were all imports; I don't know if they were
> software bugs, or just garbage in, garbage out).
>
> > It just cannot be helped, we must distrust timestamps completely.
>
> Note that name-rev will produce wrong answers in the face of clock skew.
> And I think that you even wrote that code. :)
IIRC the cute code to short-circuit using the date is not from me. If it
is, I am very ashamed.
Ciao,
Johannes
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
^ permalink raw reply
* Re: [PATCH] send-email: add proper default sender
From: Jeff King @ 2012-11-13 4:01 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <CAMP44s0SDHVzSd-8Rq7Z1sbiQ6m0pxX+2pgx16_DoWnHeyNsNQ@mail.gmail.com>
On Tue, Nov 13, 2012 at 04:55:25AM +0100, Felipe Contreras wrote:
> > No, it's not. Those broken names do not come from the environment, but
> > from our last-resort guess of the hostname.
>
> That depends how you define environment, but fine, the point is that
> it happens.
If you have a strawman definition that does not have anything to do with
what I said in my original email, then yes, it could happen. But as I
said already, "git var" uses IDENT_STRICT and will not allow such broken
names.
> > We long ago switched to
> > printing the name as a warning when we have made such a guess (bb1ae3f),
> > then more recently started rejecting them outright (8c5b1ae).
>
> Right, but these would still happen:
>
> michael <michael@michael-laptop.michael.org>
Did you read my email? I explicitly proposed that we would _not_ allow
send-email to use implicit email addresses constructed in that way.
> > But in the meantime you are causing a regression for anybody who expects
> > GIT_AUTHOR_NAME to override user.email when running git-send-email (and
> > you have taken away the prompt that they could have used to notice and
> > correct it).
>
> I think they can survive. If anybody like this exists.
Sorry, but that is not how things work on this project. You do not get
to cause regressions because you are too lazy to implement the feature
_you_ want in a way that does not break other people.
I tried to help you by pointing you in the right direction and even
providing a sample "git var" patch. But it is not my itch to scratch.
-Peff
^ permalink raw reply
* Re: [PATCH] send-email: add proper default sender
From: Felipe Contreras @ 2012-11-13 3:55 UTC (permalink / raw)
To: Jeff King; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <20121113032727.GA8387@sigill.intra.peff.net>
On Tue, Nov 13, 2012 at 4:27 AM, Jeff King <peff@peff.net> wrote:
> On Tue, Nov 13, 2012 at 01:54:59AM +0100, Felipe Contreras wrote:
>
>> > But we use the environment to default the field, so the distinction
>> > doesn't make much sense to me. Plus, it has always been the case that
>> > you can use git without setting user.*, but instead only using the
>> > environment. I don't see any reason not to follow that principle here,
>> > too.
>>
>> And that's why a lot of commits end up like michael
>> <michael@michael-laptop.(none)>.
>
> No, it's not. Those broken names do not come from the environment, but
> from our last-resort guess of the hostname.
That depends how you define environment, but fine, the point is that it happens.
> We long ago switched to
> printing the name as a warning when we have made such a guess (bb1ae3f),
> then more recently started rejecting them outright (8c5b1ae).
Right, but these would still happen:
michael <michael@michael-laptop.michael.org>
>> Probably. But what I really want is to stop 'git send-email' from
>> asking. I think the one next step further can be done later.
>
> But in the meantime you are causing a regression for anybody who expects
> GIT_AUTHOR_NAME to override user.email when running git-send-email (and
> you have taken away the prompt that they could have used to notice and
> correct it).
I think they can survive. If anybody like this exists.
--
Felipe Contreras
^ permalink raw reply
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