* Re: [PATCH v3 0/2] test: tests for the "double > from mailmap" bug
From: Jonathan Nieder @ 2012-02-14 21:15 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Jeff King, Junio C Hamano
In-Reply-To: <CAMP44s3YRHgMPX2Hzydm_TLB27OABWETjABMcwrHmDk-=pN2hw@mail.gmail.com>
Felipe Contreras wrote:
> On Tue, Feb 14, 2012 at 10:34 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> To summarize the previous discussion[1]: some people had comments, and
>> you seem to have found value in exactly none of them. OK. CC-ing
>> Peff, since he at least probably has looked over this code before.
>
> Just because you have comments doesn't mean I *must* address them. We
> have a difference of opinion, nothing wrong with that.
I said "OK", didn't I?
[...]
>> In fact it seems to be intended to test the case addressed by f026358e
>> (name changing, email not) in various mailmap callers: "git shortlog -e",
>> "git log --pretty", "git blame".
>
> No. As the summary says, it's intended to add a simple name
> translation test, which is missing from all the tests that spawn from
> the repository generated in 'Shortlog output (complex mapping)' test.
> This is the most minimal patch that can be generated if you add a
> commit to this repository, and any further tests that are related to
> it would look the same.
>
> As Junio pointed out what is missing from the explanation is that this
> simple name translation test is targeted toward the 'git blame'
> commands, because such translation is not tested for them currently.
Um. So this has nothing to do with f026358e at all? Mentioning that
commit and that this test does not pass with an older codebase is not
useful to the humans that will look over this test later?
Adding explanation and rearranging things so people encountering this
later have to spend _less_ time to understand it is something I
consider useful. It means people are less likely to randomly break
things. I don't actually understand where the difference of opinion
comes from here.
^ permalink raw reply
* Re: [PATCH v3 0/2] test: tests for the "double > from mailmap" bug
From: Jeff King @ 2012-02-14 21:14 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Felipe Contreras, git
In-Reply-To: <20120214203431.GB13210@burratino>
On Tue, Feb 14, 2012 at 02:34:31PM -0600, Jonathan Nieder wrote:
> Felipe Contreras wrote:
>
> > I sent both the fix and the tests. Another fix was applied, but we are still
> > missing the tests.
> >
> > These are good before, and after the fix.
>
> To summarize the previous discussion[1]: some people had comments, and
> you seem to have found value in exactly none of them. OK. CC-ing
> Peff, since he at least probably has looked over this code before.
The short answer is that both patches look OK to me.
Here's a longer digression that you may feel free to ignore.
When I investigated this problem, I looked only at the code (and I think
Junio's fix is the right one). However, having just looked at your two
patches without having previously looked at those tests, I found them
quite painless to review.
Having participated in the bug-hunt, I knew they were probably related
to testing that bug, but it was very unclear to me from the original
series why there were two patches, and not one. Or why, when we have
mailmap tests, they did not catch this bug. And that is _now_, with the
context of having just participated in the discussion; six months from
now I would have even less hope of understanding the context.
My general review process is (in this order):
1. Figure out why a change is needed. This should be explained in the
commit message. And no, just adding tests is not assumed to be
needed. Why did the old tests not cover this case? The answer can
be a simple "nobody bothered to write them", and that's OK. But
some description of the current state can help reviewers understand
the rationale (e.g., "we tested with shortlog, but not mailmap, and
certain code paths are only exposed through mailmap").
2. Figure out what the change should be doing. This should also be in
the commit message, though at a high level. In the case of tests,
obviously we're adding new tests. But are we properly covering all
of the new cases? Does the "what" match the "why" from (1)?
3. Look at the patch. Do the changes match what the commit message
claimed in (2)?
4. Look at the patch for style, implementation, etc. Is the code
quality good enough to be included?
The steps are dependent on each other, and I usually quit if I can't get
past one step. Sometimes I will look at the patch to try to figure out
(1) or (2), especially if the patch is trivial. But in my ideal world,
every patch lets me just walk through those steps.
I won't claim that these steps create error-free reviews. Based on those
steps, your patches look good to me, and it seems from Felipe's response
that there might be some inaccuracies in your description. But it at
least gives us a starting point for discussion.
-Peff
^ permalink raw reply
* Re: [PATCH v3 0/2] test: tests for the "double > from mailmap" bug
From: Felipe Contreras @ 2012-02-14 21:06 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Jeff King
In-Reply-To: <20120214203431.GB13210@burratino>
Hi,
On Tue, Feb 14, 2012 at 10:34 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Felipe Contreras wrote:
>
>> I sent both the fix and the tests. Another fix was applied, but we are still
>> missing the tests.
>>
>> These are good before, and after the fix.
>
> To summarize the previous discussion[1]: some people had comments, and
> you seem to have found value in exactly none of them. OK. CC-ing
> Peff, since he at least probably has looked over this code before.
Just because you have comments doesn't mean I *must* address them. We
have a difference of opinion, nothing wrong with that.
> By the way, the address you are using for Marius is out of date.
>
>> Felipe Contreras (2):
>> t: mailmap: add 'git blame -e' tests
>
> So that people don't destroy this test in later refactorings, I would
> like to collect statements that we want the test to ensure remain
> true.
>
> Apparently the fix in f026358e ("mailmap: always return a plain mail
> address from map_user()", 2012-02-05) was for the case of the name
> changing and email address not changing due to mailmap mapping. Most
> callers use a separate buffer for the email address so there is room
> to modify the name in place, but "git blame" keeps angle brackets in
> the same buffer for no obvious reason I can see. (Callers should
> be able to add the brackets themselves instead of relying on
> ci.author_mail to contain them, but that's a story for another day.)
>
> Anyway, the existing tests for the returned email missed this since
> it does not affect "git shortlog -e" but only "git blame -e".
> Therefore this patch reuses the test data for shortlog -e and lets us
> use it for blame, too. It is easier to understand after the other
> patch, IMHO.
This is not related to f026358e. The test added by this patch would
pass before and after f026358e.
This is what I mean by reading too much into the patch. There's
nothing to it; "add 'git blame -e' tests" just adds tests for 'git
blame -e', it doesn't catch any issues related to f026358e; it just
makes sure there are no further regressions with the tests that are
already passing.
Again, some tests for 'git blame -e' > no tests for 'git blame -e'.
> This is _not_ meant as a more general test for the "git blame -e"
> format (which would belong somewhere near t8008) as far as I can tell.
> It is just checking that mailmap doesn't screw up.
Thus the 't: mailmap: ' prefix.
>> t: mailmap: add simple name translation test
>
> Before, I thought this might be a straightforward test for the bug
> fixed by f026358e. That didn't justify the patch that touches
> several different test assertions.
>
> In fact it seems to be intended to test the case addressed by f026358e
> (name changing, email not) in various mailmap callers: "git shortlog -e",
> "git log --pretty", "git blame".
No. As the summary says, it's intended to add a simple name
translation test, which is missing from all the tests that spawn from
the repository generated in 'Shortlog output (complex mapping)' test.
This is the most minimal patch that can be generated if you add a
commit to this repository, and any further tests that are related to
it would look the same.
As Junio pointed out what is missing from the explanation is that this
simple name translation test is targeted toward the 'git blame'
commands, because such translation is not tested for them currently.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 6/8] gitweb: Highlight interesting parts of diff
From: Jeff King @ 2012-02-14 20:38 UTC (permalink / raw)
To: Michał Kiedrowicz; +Cc: Jakub Narebski, git
In-Reply-To: <20120214210453.16c6bd2e@gmail.com>
On Tue, Feb 14, 2012 at 09:04:53PM +0100, Michał Kiedrowicz wrote:
> Sure, now it's a simple algorithm, but if we add more code, we will
> have problems with making it consistent in both gitweb and
> diff-highlight (which is nice-to-have IMO).
True. I was thinking they would stay simple enough that porting features
wouldn't be too painful. But that might be overly optimistic.
> Note that my patches to gitweb already support combined diffs (in
> obvious cases) while diff-highlight will fail on them badly (see for
> example 09bb4eb4f14c). I haven't done it in diff-highlight because I
> noticed that problem while working on patches for gitweb.
Hmm. 09bb4eb4f14c looks fine to me, because all of the combined lines
are from the left-hand side. A better example is 4802997, where the
first hunk properly highlights, but the second does not (because we
interpret the lines as context lines).
Even worse is "git show 5de89d3 -- notes-cache.c", where we match a "- "
line with a "++" line, and erroneously think the changes begin in the
second character (even though it is simply a combined-diff marker).
These are reasonably rare, so I don't consider them critical bugs, but
yeah, it would be nice to fix.
-Peff
^ permalink raw reply
* [PATCH 2/2] test: check that "git blame -e" uses mailmap correctly
From: Jonathan Nieder @ 2012-02-14 20:36 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Jeff King
In-Reply-To: <20120214203431.GB13210@burratino>
From: Felipe Contreras <felipe.contreras@gmail.com>
Until f026358e ("mailmap: always return a plain mail address from
map_user()", 2012-02-05), git blame -e would add a spurious '>' after
the unchanged email address with brackets it passed to the mailmap
machinery, resulting in lines with a doubled '>' like this:
620456e6 (<committer@example.com>> 2005-04-07 15:20:13 -0700 8) eight
Add a test to make sure it doesn't happen again. This reuses the test
data for the existing "shortlog -e" test so it also tests other kinds
of mail mapping.
[jn: fixed some cut+paste cruft, added a patch description]
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
t/t4203-mailmap.sh | 16 ++++++++++++++++
1 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index 45526395..ef900d84 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -267,4 +267,20 @@ test_expect_success 'Blame output (complex mapping)' '
test_cmp expect actual.fuzz
'
+cat >expect <<\EOF
+^OBJI (<author@example.com> DATE 1) one
+OBJID (<some@dude.xx> DATE 2) two
+OBJID (<other@author.xx> DATE 3) three
+OBJID (<other@author.xx> DATE 4) four
+OBJID (<santa.claus@northpole.xx> DATE 5) five
+OBJID (<santa.claus@northpole.xx> DATE 6) six
+OBJID (<cto@company.xx> DATE 7) seven
+OBJID (<committer@example.com> DATE 8) eight
+EOF
+test_expect_success 'Blame -e output' '
+ git blame -e one >actual &&
+ fuzz_blame actual >actual.fuzz &&
+ test_cmp expect actual.fuzz
+'
+
test_done
--
1.7.9
^ permalink raw reply related
* [PATCH 1/2] test: mailmap can change author name without changing email
From: Jonathan Nieder @ 2012-02-14 20:35 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Jeff King
In-Reply-To: <20120214203431.GB13210@burratino>
From: Felipe Contreras <felipe.contreras@gmail.com>
A mailmap entry of the format 'A U Thor <email@example.com>' has two
effects: (1) 'A U Thor' becomes the canonical author name for commits
with author address 'email@example.com', and (2) 'email@example.com'
becomes the canonical author email for commits with author name 'A U
Thor'.
We already have tests for the effect (1) in the committer name, but
not in the author name, so the tests do not cover the shortlog and
blame codepaths as they should. Fix that.
[jn: simplified by reusing Committer mailmap entry, clarified
description]
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
t/t4203-mailmap.sh | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index 1f182f61..45526395 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -157,6 +157,9 @@ A U Thor <author@example.com> (1):
CTO <cto@company.xx> (1):
seventh
+Committed <committer@example.com> (1):
+ eighth
+
Other Author <other@author.xx> (2):
third
fourth
@@ -196,6 +199,11 @@ test_expect_success 'Shortlog output (complex mapping)' '
test_tick &&
git commit --author "CTO <cto@coompany.xx>" -m seventh &&
+ echo eight >>one &&
+ git add one &&
+ test_tick &&
+ git commit --author "C O Mitter <committer@example.com>" -m eighth &&
+
mkdir -p internal_mailmap &&
echo "Committed <committer@example.com>" > internal_mailmap/.mailmap &&
echo "<cto@company.xx> <cto@coompany.xx>" >> internal_mailmap/.mailmap &&
@@ -212,6 +220,9 @@ test_expect_success 'Shortlog output (complex mapping)' '
# git log with --pretty format which uses the name and email mailmap placemarkers
cat >expect <<\EOF
+Author C O Mitter <committer@example.com> maps to Committed <committer@example.com>
+Committer C O Mitter <committer@example.com> maps to Committed <committer@example.com>
+
Author CTO <cto@coompany.xx> maps to CTO <cto@company.xx>
Committer C O Mitter <committer@example.com> maps to Committed <committer@example.com>
@@ -248,6 +259,7 @@ OBJID (Other Author DATE 4) four
OBJID (Santa Claus DATE 5) five
OBJID (Santa Claus DATE 6) six
OBJID (CTO DATE 7) seven
+OBJID (Committed DATE 8) eight
EOF
test_expect_success 'Blame output (complex mapping)' '
git blame one >actual &&
--
1.7.9
^ permalink raw reply related
* [PATCH v3 0/2] test: tests for the "double > from mailmap" bug
From: Jonathan Nieder @ 2012-02-14 20:34 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Jeff King
In-Reply-To: <1329235894-20581-1-git-send-email-felipe.contreras@gmail.com>
Hi again,
Felipe Contreras wrote:
> I sent both the fix and the tests. Another fix was applied, but we are still
> missing the tests.
>
> These are good before, and after the fix.
To summarize the previous discussion[1]: some people had comments, and
you seem to have found value in exactly none of them. OK. CC-ing
Peff, since he at least probably has looked over this code before.
By the way, the address you are using for Marius is out of date.
> Felipe Contreras (2):
> t: mailmap: add 'git blame -e' tests
So that people don't destroy this test in later refactorings, I would
like to collect statements that we want the test to ensure remain
true.
Apparently the fix in f026358e ("mailmap: always return a plain mail
address from map_user()", 2012-02-05) was for the case of the name
changing and email address not changing due to mailmap mapping. Most
callers use a separate buffer for the email address so there is room
to modify the name in place, but "git blame" keeps angle brackets in
the same buffer for no obvious reason I can see. (Callers should
be able to add the brackets themselves instead of relying on
ci.author_mail to contain them, but that's a story for another day.)
Anyway, the existing tests for the returned email missed this since
it does not affect "git shortlog -e" but only "git blame -e".
Therefore this patch reuses the test data for shortlog -e and lets us
use it for blame, too. It is easier to understand after the other
patch, IMHO.
This is _not_ meant as a more general test for the "git blame -e"
format (which would belong somewhere near t8008) as far as I can tell.
It is just checking that mailmap doesn't screw up.
> t: mailmap: add simple name translation test
Before, I thought this might be a straightforward test for the bug
fixed by f026358e. That didn't justify the patch that touches
several different test assertions.
In fact it seems to be intended to test the case addressed by f026358e
(name changing, email not) in various mailmap callers: "git shortlog -e",
"git log --pretty", "git blame".
Here's a reroll.
Enjoy,
Jonathan
Felipe Contreras (2):
test: mailmap can change author name without changing email
test: check that "git blame -e" uses mailmap correctly
t/t4203-mailmap.sh | 28 ++++++++++++++++++++++++++++
1 files changed, 28 insertions(+), 0 deletions(-)
[1] http://thread.gmane.org/gmane.comp.version-control.git/189896
^ permalink raw reply
* Re: [PATCH v2 2/2] submodules: always use a relative path from gitdir to work tree
From: Junio C Hamano @ 2012-02-14 20:34 UTC (permalink / raw)
To: Jens Lehmann; +Cc: Git Mailing List, Antony Male, Phil Hord
In-Reply-To: <7v4nut6u4s.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
>
>> After adding a comment, using test instead of [], testing both $a and
>> $b and assigning each variable on it's own line I get the following
>> interdiff. Does that make more sense?
>
> My earlier request for comment was to say
>
> # $a is always longer than $b for such and such reasons
>
> to explain why testing $b without testing $a was sufficient.
Heh, after I follow the entire module_clone, $gitdir is defined in earlier
parts of the function to be "$(rev-parse --git-dir)/modules/$path", so it
is clear that it is longer than $path. Unless "cd $there && pwd" does not
result in a funny situation (such as $something/modules is a symbolic link
to another place that is much closer to the root of the filesystem), that
is.
And in such a case, the prefix part of $a and $b would be different from
the very beginning hopefully.
> It is obvious (at least to me) that the loop continues as long as $a and
> $b begin with the same string before their first '/' and removes that
> common segment from both of them, so I do not think the new comment is
> absolutely necessary, but it would not hurt to have it, especially it is
> short enough and to the point.
>
> Thanks.
>
>> diff --git a/git-submodule.sh b/git-submodule.sh
>> index 3463d6d..ed76ce2 100755
>> --- a/git-submodule.sh
>> +++ b/git-submodule.sh
>> @@ -172,9 +172,11 @@ module_clone()
>>
>> a=$(cd "$gitdir" && pwd)
>> b=$(cd "$path" && pwd)
>> - while [ "$b" ] && [ "${a%%/*}" = "${b%%/*}" ]
>> + # Remove all common leading directories
>> + while test -n "$a" && test -n "$b" && test "${a%%/*}" = "${b%%/*}"
>> do
>> - a=${a#*/} b=${b#*/};
>> + a=${a#*/}
>> + b=${b#*/}
>> done
>> rel=$(echo $a | sed -e 's|[^/]*|..|g')
Perhaps aseert that $a never becomes empty before this line (or set it
explicitly to "." when $a is empty), as otherwise
>> (clear_local_git_env; cd "$path" && git config core.worktree "$rel/$b")
this will refer to "/$b" from the root?
^ permalink raw reply
* Re: diff --stat
From: Jeff King @ 2012-02-14 20:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vfwed6uws.fsf@alter.siamese.dyndns.org>
On Tue, Feb 14, 2012 at 12:07:31PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > Hmm. Looking at scale_linear, the formula is:
> >
> > return ((it - 1) * (width - 1) + max_change - 1) / (max_change - 1);
> >
> > I don't see how that can be accurate, since the magnitude of the "-1"
> > tweak will vary based on the value of "it". This code is due to
> > 3ed74e6, but I don't quite follow the logic in the commit message.
>
> Doesn't it need +1 at the end, I wonder? We want to map:
>
> - zero to zero
> - any number to at least 1
>
> so scaling a non-zero "it" so that maximum maps to (width-1) and then
> adding 1 would be the right way for the latter case.
Yeah, that makes more sense to me. I think you could also get by with
simply rounding the above properly to the nearest integer (so a little
bit of error that makes 23.9 into 24.0 would be OK, because we would end
up rounding 30.9 to 31.0, too). This seems like the most obvious
solution to me:
static int divide_and_round(int a, int b)
{
return (2 * a + b) / (2 * b);
}
/*
* We want non-zero changes to have at least 1 marker, so special-case
* zero, then scale to width-1, and add back in 1.
*/
static int scale_linear(int it, int width, int max_change)
{
if (!it)
return 0;
return 1 + divide_and_round(it * (width-1), max_change);
}
Any "must show at least 1" scheme is going to have some error in the
scaling (because we are rounding up all of the low end). I have a
feeling that the scheme from 3ed74e6 was trying to distribute that error
more evenly throughout the scale, and the scheme above is lumping all of
it at the start (i.e., the difference between what constitutes one
marker and two markers is much greater than the difference between two
and three).
But that's just a vague feeling. For some reason my brain is not doing
well with math today, so I'll forego writing a proof.
> Of course, an easy way out without worrying about the correct math is to
> scale the total and the smaller one and then declare that the scaled
> larger one is the difference between the two. That way, both of these two
> files have 109 in total so the length of the entire graph would be the
> same ;-).
It looks like we actually did that, pre-3ed74e6. I think it's a valid
strategy. It is just pushing the error around, but I don't know that
people are counting the +/- markers on the line and comparing them
exactly. A little error there is less of a big deal than error between
two lines which are supposed to have the same number of changes (you'll
note that we don't give the per-file added/deleted numbers exactly, so
they are a good place to hide error. :) ).
-Peff
^ permalink raw reply
* Re: git-latexdiff: Git and Latexdiff working together
From: Tim Haga @ 2012-02-14 20:19 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git
In-Reply-To: <vpq7gzph7mi.fsf@bauges.imag.fr>
Hi,
your tool seems very useful for LaTeX users, especially in scientific
work. I could not test it in detail until now, but i had two or
three runs with an article draft and it seems to work.
Long story short, i would appreciate it if your tool would be
integrated in git.
T.
Am Tue, 14 Feb 2012 14:22:45 +0100
schrieb Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>:
> Hi,
>
> You may know latexdiff, a neat tool to visualize differences between
> LaTeX files (it annotates your .tex file with colors for removed/added
> parts, producing another compilable .tex file).
>
> I wrote a little shell-script that allows one to use latexdiff on files
> versionned by Git, with e.g.
>
> git latexdiff HEAD^ --main foo.tex --output foo.pdf
>
> Essentially, it does a checkout of the old and new revisions, and calls
> latexdiff + pdflatex for you.
>
> The result is attached in case anyone is interested.
>
> It may be relevant to add this to contrib/ in git.git. If anyone's
> interested, let me know, and I'll resend the code in the form of a
> patch doing that.
>
> Regards,
>
^ permalink raw reply
* Re: [PATCH 2/2] t: mailmap: add simple name translation test
From: Felipe Contreras @ 2012-02-14 20:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Marius Storm-Olsen, Jim Meyering, Jonathan Nieder
In-Reply-To: <7v8vk56us3.fsf@alter.siamese.dyndns.org>
On Tue, Feb 14, 2012 at 10:10 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>
> It was clear that we didn't have any test for "blame -e" hence it was no
> brainer to judge that the patch 1/2 is good without any description.
>
> But I am scratching my head, deciphering what this patch adds.
>
> It appears to me that the existing tests that map author@example.com from
> the original "A U Thor" to "Repo Guy" and inspect names and mails in
> various output already cover this "Wrong with <right@company.xx> can be
> corrected to Mr. Right" case this patch adds.
Yes, but in the first tests they don't check for 'git blame', and much
less 'git blame -e', and the second tests only check complex mappings.
> What am I missing? Instead of explaining it to me, can it be explained in
> the log message?
If an explanations along the lines of the above make sense, I can resend.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH v2 2/2] submodules: always use a relative path from gitdir to work tree
From: Junio C Hamano @ 2012-02-14 20:24 UTC (permalink / raw)
To: Jens Lehmann; +Cc: Git Mailing List, Antony Male, Phil Hord
In-Reply-To: <4F3A9B98.6040908@web.de>
Jens Lehmann <Jens.Lehmann@web.de> writes:
> After adding a comment, using test instead of [], testing both $a and
> $b and assigning each variable on it's own line I get the following
> interdiff. Does that make more sense?
My earlier request for comment was to say
# $a is always longer than $b for such and such reasons
to explain why testing $b without testing $a was sufficient.
It is obvious (at least to me) that the loop continues as long as $a and
$b begin with the same string before their first '/' and removes that
common segment from both of them, so I do not think the new comment is
absolutely necessary, but it would not hurt to have it, especially it is
short enough and to the point.
Thanks.
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 3463d6d..ed76ce2 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -172,9 +172,11 @@ module_clone()
>
> a=$(cd "$gitdir" && pwd)
> b=$(cd "$path" && pwd)
> - while [ "$b" ] && [ "${a%%/*}" = "${b%%/*}" ]
> + # Remove all common leading directories
> + while test -n "$a" && test -n "$b" && test "${a%%/*}" = "${b%%/*}"
> do
> - a=${a#*/} b=${b#*/};
> + a=${a#*/}
> + b=${b#*/}
> done
> rel=$(echo $a | sed -e 's|[^/]*|..|g')
> (clear_local_git_env; cd "$path" && git config core.worktree "$rel/$b")
^ permalink raw reply
* Re: cvs2git migration
From: Holger Hellmuth @ 2012-02-14 20:17 UTC (permalink / raw)
To: supadhyay; +Cc: git
In-Reply-To: <1329220866066-7283631.post@n2.nabble.com>
On 14.02.2012 13:01, supadhyay wrote:
> Hi All,
>
> I want to migrate my version control CVS to GIT. I have few links which
> mentioned about how to migrate CVS repositories to GIT but there is no
> description about how to migrate existing CVS users to GIT?
As far as I know CVS uses its own server called pserver. And the usual
username and password combo for authentication.
if you want to use git with a central repository, you should use
gitolite. And gitolite uses RSA or DSA keys that are not compatible with
CVS passwords. You can't simply migrate the users to gitolite.
Every user has to generate and send the public half of that key
(generated with ssh-keygen) to the administrator of that repository.
If you have problems authenticating all those users sending you public
keys, let them check in the public keys into a specific path in one of
the old CVS repositories. Then the administrator of the git repository
can fetch them from the CVS (and check that the right user added the
patch for his new key).
^ permalink raw reply
* Re: [PATCH v2 2/2] submodules: always use a relative path from gitdir to work tree
From: Jens Lehmann @ 2012-02-14 17:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, Antony Male, Phil Hord
In-Reply-To: <7vlio6ec7q.fsf@alter.siamese.dyndns.org>
Am 13.02.2012 20:59, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
>> + a=$(cd "$gitdir" && pwd)
>> + b=$(cd "$path" && pwd)
>> + while [ "$b" ] && [ "${a%%/*}" = "${b%%/*}" ]
>> + do
>> + a=${a#*/} b=${b#*/};
>> + done
>> + rel=$(echo $a | sed -e 's|[^/]*|..|g')
>> + (clear_local_git_env; cd "$path" && git config core.worktree "$rel/$b")
>> }
>
>
> The style ([ "$b" ] vs "test -n "$b") aside, it strikes me odd that you
> only check $b; it is unclear what guarantees that "$a" is always longer.
> Maybe there is a reason but then a one-line comment here would not hurt?
I just copied that while loop from a few lines up, but you are right
about the style and logic issues (Will send a cleanup patch for the
other loop too when we agree on how to write this one).
After adding a comment, using test instead of [], testing both $a and
$b and assigning each variable on it's own line I get the following
interdiff. Does that make more sense?
diff --git a/git-submodule.sh b/git-submodule.sh
index 3463d6d..ed76ce2 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -172,9 +172,11 @@ module_clone()
a=$(cd "$gitdir" && pwd)
b=$(cd "$path" && pwd)
- while [ "$b" ] && [ "${a%%/*}" = "${b%%/*}" ]
+ # Remove all common leading directories
+ while test -n "$a" && test -n "$b" && test "${a%%/*}" = "${b%%/*}"
do
- a=${a#*/} b=${b#*/};
+ a=${a#*/}
+ b=${b#*/}
done
rel=$(echo $a | sed -e 's|[^/]*|..|g')
(clear_local_git_env; cd "$path" && git config core.worktree "$rel/$b")
^ permalink raw reply related
* Re: [PATCH 2/2] t: mailmap: add simple name translation test
From: Junio C Hamano @ 2012-02-14 20:10 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Marius Storm-Olsen, Jim Meyering, Jonathan Nieder
In-Reply-To: <1329235894-20581-3-git-send-email-felipe.contreras@gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
It was clear that we didn't have any test for "blame -e" hence it was no
brainer to judge that the patch 1/2 is good without any description.
But I am scratching my head, deciphering what this patch adds.
It appears to me that the existing tests that map author@example.com from
the original "A U Thor" to "Repo Guy" and inspect names and mails in
various output already cover this "Wrong with <right@company.xx> can be
corrected to Mr. Right" case this patch adds.
What am I missing? Instead of explaining it to me, can it be explained in
the log message?
Thanks.
> t/t4203-mailmap.sh | 14 ++++++++++++++
> 1 files changed, 14 insertions(+), 0 deletions(-)
>
> diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
> index db12265..fc3855a 100755
> --- a/t/t4203-mailmap.sh
> +++ b/t/t4203-mailmap.sh
> @@ -157,6 +157,9 @@ A U Thor <author@example.com> (1):
> CTO <cto@company.xx> (1):
> seventh
>
> +Mr. Right <right@company.xx> (1):
> + eight
> +
> Other Author <other@author.xx> (2):
> third
> fourth
> @@ -196,6 +199,11 @@ test_expect_success 'Shortlog output (complex mapping)' '
> test_tick &&
> git commit --author "CTO <cto@coompany.xx>" -m seventh &&
>
> + echo eight >>one &&
> + git add one &&
> + test_tick &&
> + git commit --author "Wrong <right@company.xx>" -m eight &&
> +
> mkdir -p internal_mailmap &&
> echo "Committed <committer@example.com>" > internal_mailmap/.mailmap &&
> echo "<cto@company.xx> <cto@coompany.xx>" >> internal_mailmap/.mailmap &&
> @@ -204,6 +212,7 @@ test_expect_success 'Shortlog output (complex mapping)' '
> echo "Other Author <other@author.xx> <nick2@company.xx>" >> internal_mailmap/.mailmap &&
> echo "Santa Claus <santa.claus@northpole.xx> <me@company.xx>" >> internal_mailmap/.mailmap &&
> echo "Santa Claus <santa.claus@northpole.xx> <me@company.xx>" >> internal_mailmap/.mailmap &&
> + echo "Mr. Right <right@company.xx>" >> internal_mailmap/.mailmap &&
>
> git shortlog -e HEAD >actual &&
> test_cmp expect actual
> @@ -212,6 +221,9 @@ test_expect_success 'Shortlog output (complex mapping)' '
>
> # git log with --pretty format which uses the name and email mailmap placemarkers
> cat >expect <<\EOF
> +Author Wrong <right@company.xx> maps to Mr. Right <right@company.xx>
> +Committer C O Mitter <committer@example.com> maps to Committed <committer@example.com>
> +
> Author CTO <cto@coompany.xx> maps to CTO <cto@company.xx>
> Committer C O Mitter <committer@example.com> maps to Committed <committer@example.com>
>
> @@ -248,6 +260,7 @@ OBJID (Other Author DATE 4) four
> OBJID (Santa Claus DATE 5) five
> OBJID (Santa Claus DATE 6) six
> OBJID (CTO DATE 7) seven
> +OBJID (Mr. Right DATE 8) eight
> EOF
> test_expect_success 'Blame output (complex mapping)' '
> git blame one >actual &&
> @@ -264,6 +277,7 @@ OBJID (<other@author.xx> DATE 4) four
> OBJID (<santa.claus@northpole.xx> DATE 5) five
> OBJID (<santa.claus@northpole.xx> DATE 6) six
> OBJID (<cto@company.xx> DATE 7) seven
> +OBJID (<right@company.xx> DATE 8) eight
> EOF
> test_expect_success 'Blame output (complex mapping)' '
> git blame -e one >actual &&
^ permalink raw reply
* Re: diff --stat
From: Junio C Hamano @ 2012-02-14 20:07 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20120214195036.GD12072@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Hmm. Looking at scale_linear, the formula is:
>
> return ((it - 1) * (width - 1) + max_change - 1) / (max_change - 1);
>
> I don't see how that can be accurate, since the magnitude of the "-1"
> tweak will vary based on the value of "it". This code is due to
> 3ed74e6, but I don't quite follow the logic in the commit message.
Doesn't it need +1 at the end, I wonder? We want to map:
- zero to zero
- any number to at least 1
so scaling a non-zero "it" so that maximum maps to (width-1) and then
adding 1 would be the right way for the latter case.
Of course, an easy way out without worrying about the correct math is to
scale the total and the smaller one and then declare that the scaled
larger one is the difference between the two. That way, both of these two
files have 109 in total so the length of the entire graph would be the
same ;-).
^ permalink raw reply
* Re: [PATCH 6/8] gitweb: Highlight interesting parts of diff
From: Michał Kiedrowicz @ 2012-02-14 20:04 UTC (permalink / raw)
To: Jeff King; +Cc: Jakub Narebski, git
In-Reply-To: <20120214185233.GA12072@sigill.intra.peff.net>
Jeff King <peff@peff.net> wrote:
> On Tue, Feb 14, 2012 at 07:23:40PM +0100, Michał Kiedrowicz wrote:
>
> > > > > BTW GitHub is closed source, but we can check what algorithm does Trac
> > > > > use for diff refinement highlighting (highlighting changed portions of
> > > > > diff).
> > > > >
> > > > I think it's
> > > > http://trac.edgewall.org/browser/trunk/trac/versioncontrol/diff.py
> > > > (see markup intraline_changes()).
> > [...]
> > [...]
> > > The markup_intraline_changes() function compares lines from preimage and
> > > from postimage pairwise, requiring that number of lines matches, the same
> > > like in your algorithm.
> >
> > So using Jeff's diff-highlight we remain quite consistent with Trac
> > output. There's nothing we can "steal" from it.
>
> Neat. When I originally wrote diff-highlight, I was inspired by seeing
> what Trac and GitHub did, but came up with the algorithm on my own.
> Learning that Trac does the multiline thing (as we started doing with
> recent patches) makes me feel even better that it's a good strategy.
>
> As an aside, I looked at what GitHub does. It turns on highlighting only
> for the single-line case, and does the same prefix/suffix thing.
>
> It's really not very complex code; most of the hassle in diff-highlight
> is ignoring (but preserving) embedded colors, so that we build on top of
> existing colorization. A web tool will be doing the line coloring itself
> anyway, so it can be much simpler. Elsewhere, I suggested lib-ifying
> diff-highlight for gitweb to use. But once you remove the embedded color
> handling, there really is not that much code, and it's probably simpler
> to just rewrite it.
>
> -Peff
Sure, now it's a simple algorithm, but if we add more code, we will
have problems with making it consistent in both gitweb and
diff-highlight (which is nice-to-have IMO). Note that my patches to
gitweb already support combined diffs (in obvious cases) while
diff-highlight will fail on them badly (see for example 09bb4eb4f14c).
I haven't done it in diff-highlight because I noticed that problem
while working on patches for gitweb.
Anyway, thanks for looking at Trac/GitHub code and sharing your opinion.
^ permalink raw reply
* Re: [RFC PATCH 0/3] git-p4: move to toplevel
From: Clemens Buchacher @ 2012-02-14 19:49 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Pete Wyckoff, git, Luke Diamand, Vitor Antunes
In-Reply-To: <20120214190515.GB12072@sigill.intra.peff.net>
On Tue, Feb 14, 2012 at 02:05:15PM -0500, Jeff King wrote:
>
> 2. We explicitly run "sh" in such situations, not SHELL_PATH. Even if
> "sh" is bash, I believe it should look at its own argv[0] and put
> itself into a more POSIX-y mode (but I didn't test).
Indeed it does.
^ permalink raw reply
* Re: diff --stat
From: Jeff King @ 2012-02-14 19:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v4nuub7el.fsf@alter.siamese.dyndns.org>
On Mon, Feb 13, 2012 at 04:11:46PM -0800, Junio C Hamano wrote:
> Hrm, what is wrong with this picture?
>
> $ git -c diff.color.old=red show --format='%s' --stat
> Merge branch 'jk/diff-highlight' into pu
>
> contrib/diff-highlight/README | 109 ++++++++++++++++++++++++++++++--
> contrib/diff-highlight/diff-highlight | 109 ++++++++++++++++++++++++---------
> 2 files changed, 181 insertions(+), 37 deletions(-)
>
> They both have 109 lines changed but the end of the graph lines do not
> coincide...
I think it is rounding error. The first one is +102/-7, and the second
one is +79/-30. When we get to scale_linear, we try to scale 109 change
markers into a 33-character width. So the right scaling factor is ~.303.
So our "true" scaled widths should be:
README, added: 30.9
README, deleted: 2.1
highlight, added: 23.9
highlight, deleted: 9.1
However, we're dealing with integer numbers of characters, so we need to
round. In this case, it seems that our rounding produces (30, 2) in the
first instance and (24, 9) in the second. Which is odd. You'd think
we'd either always round to the nearest integer, or always round down.
But we end up rounding 30.9 down and 23.9 up. So it may be a subtle
loss-of-precision error in scale_linear.
Hmm. Looking at scale_linear, the formula is:
return ((it - 1) * (width - 1) + max_change - 1) / (max_change - 1);
I don't see how that can be accurate, since the magnitude of the "-1"
tweak will vary based on the value of "it". This code is due to
3ed74e6, but I don't quite follow the logic in the commit message.
-Peff
^ permalink raw reply
* Re: cvs2git migration
From: Kevin @ 2012-02-14 19:50 UTC (permalink / raw)
To: supadhyay; +Cc: git
In-Reply-To: <1329244386868-7285021.post@n2.nabble.com>
I see that you have already asked this question before on the mailing list
See http://git.661346.n2.nabble.com/User-authentication-in-GIT-td7261349.html
On Tue, Feb 14, 2012 at 7:33 PM, supadhyay <supadhyay@imany.com> wrote:
> Hi All,
>
> Can any one please update me what would be the method/steps to migrate
> existing CVS users to GIT in CVS2GIT migration? User migration is possible
> or not?
>
> Thanks in advance...
>
> --
> View this message in context: http://git.661346.n2.nabble.com/cvs2git-migration-tp7283631p7285021.html
> Sent from the git mailing list archive at Nabble.com.
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: cvs2git migration
From: Kevin @ 2012-02-14 19:45 UTC (permalink / raw)
To: supadhyay; +Cc: git
In-Reply-To: <1329244386868-7285021.post@n2.nabble.com>
Hi,
Because git is distributed, it has no user management (which is
impossible to do). Every user adds his name and e-mail address to his
global .gitconfig file.
For access control, you can check gitolite [1].
[1]: http://github.com/sitaramc/gitolite
On Tue, Feb 14, 2012 at 7:33 PM, supadhyay <supadhyay@imany.com> wrote:
> Hi All,
>
> Can any one please update me what would be the method/steps to migrate
> existing CVS users to GIT in CVS2GIT migration? User migration is possible
> or not?
>
> Thanks in advance...
>
> --
> View this message in context: http://git.661346.n2.nabble.com/cvs2git-migration-tp7283631p7285021.html
> Sent from the git mailing list archive at Nabble.com.
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [ANNOUNCE] Git 1.7.9.1
From: Junio C Hamano @ 2012-02-14 19:28 UTC (permalink / raw)
To: git; +Cc: Linux Kernel
The latest maintenance release Git 1.7.9.1 is now available at the usual
places. This is primarily to fix various user experience kinks in the new
feature added in 1.7.9 release, so that there no longer is an excuse for
users to hold on to older releases.
The release tarballs are found at:
http://code.google.com/p/git-core/downloads/list
and their SHA-1 checksums are:
bd85327627f96c4e98071a4d1d32c30f210aa54a git-1.7.9.1.tar.gz
de5ad73499cfdb08e261bc481c84a75f11b7ff0f git-htmldocs-1.7.9.1.tar.gz
8c6ee031b39da5c5e53f927952838796e0959ce9 git-manpages-1.7.9.1.tar.gz
Also the following public repositories all have a copy of the v1.7.9.1
tag and the maint branch that the tag points at:
url = git://repo.or.cz/alt-git.git
url = https://code.google.com/p/git-core/
url = git://git.sourceforge.jp/gitroot/git-core/git.git
url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
url = https://github.com/gitster/git
Have fun.
----------------------------------------------------------------
Git v1.7.9.1 Release Notes
==========================
Fixes since v1.7.9
------------------
* The makefile allowed environment variable X seep into it result in
command names suffixed with unnecessary strings.
* The set of included header files in compat/inet-{ntop,pton}
wrappers was updated for Windows some time ago, but in a way that
broke Solaris build.
* rpmbuild noticed an unpackaged but installed *.mo file and failed.
* Subprocesses spawned from various git programs were often left running
to completion even when the top-level process was killed.
* "git add -e" learned not to show a diff for an otherwise unmodified
submodule that only has uncommitted local changes in the patch
prepared by for the user to edit.
* Typo in "git branch --edit-description my-tpoic" was not diagnosed.
* Using "git grep -l/-L" together with options -W or --break may not
make much sense as the output is to only count the number of hits
and there is no place for file breaks, but the latter options made
"-l/-L" to miscount the hits.
* "git log --first-parent $pathspec" did not stay on the first parent
chain and veered into side branch from which the whole change to the
specified paths came.
* "git merge --no-edit $tag" failed to honor the --no-edit option.
* "git merge --ff-only $tag" failed because it cannot record the
required mergetag without creating a merge, but this is so common
operation for branch that is used _only_ to follow the upstream, so
it was changed to allow fast-forwarding without recording the mergetag.
* "git mergetool" now gives an empty file as the common base version
to the backend when dealing with the "both sides added, differently"
case.
* "git push -q" was not sufficiently quiet.
* When "git push" fails to update any refs, the client side did not
report an error correctly to the end user.
* "rebase" and "commit --amend" failed to work on commits with ancient
timestamps near year 1970.
* When asking for a tag to be pulled, "request-pull" did not show the
name of the tag prefixed with "tags/", which would have helped older
clients.
* "git submodule add $path" forgot to recompute the name to be stored
in .gitmodules when the submodule at $path was once added to the
superproject and already initialized.
* Many small corner case bugs on "git tag -n" was corrected.
Also contains minor fixes and documentation updates.
----------------------------------------------------------------
Changes since v1.7.9 are as follows:
Adrian Weimann (1):
completion: --edit and --no-edit for git-merge
Albert Yale (1):
grep: fix -l/-L interaction with decoration lines
Ben Walton (1):
Drop system includes from inet_pton/inet_ntop compatibility wrappers
Clemens Buchacher (2):
fix push --quiet: add 'quiet' capability to receive-pack
dashed externals: kill children on exit
David Aguilar (1):
mergetool: Provide an empty file when needed
Jakub Narebski (1):
git.spec: Workaround localized messages not put in any RPM
Jeff King (4):
run-command: optionally kill children on exit
imap-send: remove dead code
tag: fix output of "tag -n" when errors occur
tag: die when listing missing or corrupt objects
Jens Lehmann (1):
submodule add: fix breakage when re-adding a deep submodule
Johannes Schindelin (1):
add -e: do not show difference in a submodule that is merely dirty
Junio C Hamano (15):
server_supports(): parse feature list more carefully
Making pathspec limited log play nicer with --first-parent
request-pull: explicitly ask tags/$name to be pulled
tests: add write_script helper function
parse_date(): allow ancient git-timestamp
parse_date(): '@' prefix forces git-timestamp
merge: do not create a signed tag merge under --ff-only option
branch --edit-description: protect against mistyped branch name
Git 1.7.6.6
Prepare for 1.7.9.1
tag: do not show non-tag contents with "-n"
merge: do not launch an editor on "--no-edit $tag"
Update draft release notes to 1.7.9.1
Update draft release notes to 1.7.9.1
Git 1.7.9.1
Michael J Gruber (1):
t5541: avoid TAP test miscounting
Michael Palimaka (1):
Explicitly set X to avoid potential build breakage
Shawn O. Pearce (1):
remote-curl: Fix push status report when all branches fail
Ævar Arnfjörð Bjarmason (3):
Makefile: Change the default compiler from "gcc" to "cc"
Remove Git's support for smoke testing
t: use sane_unset instead of unset
^ permalink raw reply
* Re: [RFC PATCH 0/3] git-p4: move to toplevel
From: Junio C Hamano @ 2012-02-14 19:25 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Clemens Buchacher, Pete Wyckoff, git,
Luke Diamand, Vitor Antunes
In-Reply-To: <20120214190515.GB12072@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Tue, Feb 14, 2012 at 10:40:04AM -0800, Junio C Hamano wrote:
>
>> I do not think tilde in $PATH is expanded by exec*p family to begin with,
>> so it is not "dash" but POSIX, I would think. It is bash that is harming
>> other programs, by trying to be more helpful, encouraging this user
>> mistake to add ~ literally on the PATH.
>
> Hmm. There is an interesting ramification for run-command's "use_shell"
> optimization....
> But if your shell handles PATH entries differently, then there is a
> difference.
Yeah, I agree; that is exactly what I meant by bash harming users by
trying to be more helpful.
^ permalink raw reply
* Re: [PATCH 12/10] support pager.* for external commands
From: Jeff King @ 2012-02-14 19:13 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Junio C Hamano, Steffen Daode Nurpmeso, Ingo Brückl
In-Reply-To: <CACBZZX596wnk2KE9QzUPMc=A6Mt8HbUs7F4rnAZbw1_RrcKHnw@mail.gmail.com>
On Sun, Feb 12, 2012 at 01:46:34AM +0100, Ævar Arnfjörð Bjarmason wrote:
> On Fri, Aug 19, 2011 at 00:01, Jeff King <peff@peff.net> wrote:
>
> > +test_expect_success TTY 'command-specific pager works for external commands' '
> > + sane_unset PAGER GIT_PAGER &&
> > + echo "foo:initial" >expect &&
> > + >actual &&
> > + test_config pager.external "sed s/^/foo:/ >actual" &&
> > + test_terminal git --exec-path="`pwd`" external log --format=%s -1 &&
> > + test_cmp expect actual
>
> For reasons that I haven't looked into using sed like that breaks
> under /usr/bin/ksh on Solaris. Just using:
>
> sed -e \"s/^/foo:/\"
>
> Instead fixes it, it's not broken with /usr/xpg4/bin/sh, so it's some
> ksh peculiarity.
>
> The error it gives is:
>
> sed s/^/foo:/ >actual: Not found
>
> Indicating that for some reason it's considering that whole "sed
> s/^/foo:/ >actual" string to be a single command.
Hrm. Is the problem on the git-executing side, or is it on the setting
up the config side?
Sadly (or perhaps not) I no longer have any Solaris machines to test on.
Can you confirm that "git config pager.external" looks OK inside that
test? Can you confirm via GIT_TRACE=1 what is being sent to the shell?
Also, it looks like we actually run commands internally from git using
"sh -c". So if it is the executing side that is wrong, I don't see how
/usr/bin/ksh would be involved at all (it would either be /bin/sh, or
/usr/xpg4/bin/sh if you have your PATH set).
-Peff
^ permalink raw reply
* Re: [PATCH 2/5] do not override receive-pack errors
From: Junio C Hamano @ 2012-02-14 19:06 UTC (permalink / raw)
To: Clemens Buchacher; +Cc: git
In-Reply-To: <20120214083324.GA1762@ecki>
Clemens Buchacher <drizzd@aon.at> writes:
> Yes, it really is. For example, in t5504 rev-list --verify-objects (it
> was turned on for me if called from there) detects the corrupt object.
> But the error string is later overwritten with the return value of
> update, which is NULL in this case.
> ...
> Actually, check_alias_update searches for aliases of cmd in ref_list,
> which is a list of refs from all commands, irrespective of their error
> status. So this change is correct.
Ok, thanks for clarificatin on both counts.
^ 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