Re: [PATCH v2 14/19] tree-diff: rework diff_tree interface to be sha1 based
Am 3/27/2014 19:48, schrieb Junio C Hamano: From: Kirill Smelkov k...@mns.spb.ru Date: Mon, 24 Feb 2014 20:21:46 +0400 ... By the way, in general I do not appreciate people lying on the Date: with an in-body header in their patches, either in the original or in rerolls. format-patch is not very cooperative in this aspect. When I prepare a patch series with format-patch, I find myself editing out the Date: line from all patches it produces again and again. :-( -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 03/12] Move lower case functions into wrapper.c
From: Jeff King p...@peff.net Subject: Re: [PATCH v8 03/12] Move lower case functions into wrapper.c Date: Thu, 27 Mar 2014 18:34:06 -0400 On Thu, Mar 27, 2014 at 03:16:48PM -0700, Junio C Hamano wrote: I wasn't looking at the caller (and I haven't). I agree that, if you have to compare case-insensitive user input against known set of tokens, using strcasecmp() would be saner than making a downcased copy and the set of known tokens. I do not know however you want to compare in a case-insensitive way in your application, though. It appears that one place this lowercase is used is to allow rAnDOm casing in the configuration file, e.g. [trailer Signed-off-by] where = AfTEr which I find is totally unnecessary. Do we churn code to accept such a nonsense input in other places? I think we are very inconsistent. All bool config values allow tRuE. Ones that take auto often use strcasecmp (e.g., diff.*.binary). blame.date and help.format choose from a fixed set of tokens, but use strcmp. Command line parameters are of course case-sensitive, and tokens used by them usually are, too (e.g., the date formats for blame.date or also the same ones taken by --date=). In general I do not see any reason _not_ to use strcasecmp for config values that are matching a fixed set. It's friendlier to the user, the extra CPU time is negligible, and the code is no harder to read than a strcmp. I agree with this. I think it would be better to just use strcasecmp() for all the config values matching a fixed set. It is just much easier to explain to users how things work this way. Even if no one ever complained about this on the mailing list, many users complain that Git is very inconsistent. Just looking at the callers in patch 04/12, I think it would be better just used strcasecmp instead of making a lowercase copy. Not because the copy is wasteful (it is, but it almost certainly doesn't matter here), but because avoiding the copy is shorter and easier to follow (you don't have to wonder about memory ownership). Yeah, that's also why I am not very happy to have to change things in this area. Thanks, Christian. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] test-lib: '--run' to run only specific tests
On 3/27/2014 8:36 PM, Eric Sunshine wrote:
On Thu, Mar 27, 2014 at 6:32 AM, Ilya Bobyr ilya.bo...@gmail.com wrote:
Allow better control of the set of tests that will be executed for a
single test suite. Mostly useful while debugging or developing as it
allows to focus on a specific test.
Signed-off-by: Ilya Bobyr ilya.bo...@gmail.com
---
No changes from the previous version.
t/README | 65 ++-
t/t-basic.sh | 233
++
t/test-lib.sh| 85
3 files changed, 379 insertions(+), 4 deletions(-)
diff --git a/t/README b/t/README
index 6b93aca..c911f89 100644
--- a/t/README
+++ b/t/README
@@ -100,6 +100,10 @@ appropriately before running make.
This causes additional long-running tests to be run (where
available), for more exhaustive testing.
+-r,--run=test numbers::
Perhaps test-selection or something similar would be closer to the truth.
I think your naming is better. I will include it in the next version.
+ This causes only specific tests to be included or excluded. See
This is phrased somewhat oddly, as if you had already been talking
about tests being included or excluded, and that this option merely
changes that selection. Perhaps something like:
Run only the subset of tests indicated by test-selection.
Will use that sentence as well :)
+ section Skipping Tests below for test numbers syntax.
+
--valgrind=tool::
Execute all Git binaries under valgrind tool tool and exit
with status 126 on errors (just like regular tests, this will
@@ -187,10 +191,63 @@ and either can match the t[0-9]{4} part to skip the
whole
test, or t[0-9]{4} followed by .$number to say which
particular test to skip.
-Note that some tests in the existing test suite rely on previous
-test item, so you cannot arbitrarily disable one and expect the
-remainder of test to check what the test originally was intended
-to check.
+For an individual test suite --run could be used to specify that
+only some tests should be run or that some tests should be
+excluded from a run.
+
+--run argument is a list of patterns with optional prefixes that
The argument for --run is a list...
I think it could be either. But I am not a native speaker.
So, I will use your version :)
+are matched against test numbers within the current test suite.
+Supported pattern:
+
+ - A number matches a test with that number.
+
+ - sh metacharacters such as '*', '?' and '[]' match as usual in
+ shell.
+
+ - A number prefixed with '', '=', '', or '=' matches all
+ tests 'before', 'before or including', 'after', or 'after or
+ including' the specified one.
I think you want and rather than or: before and including,
after and including.
I was thinking about an analogy to the corresponding mathematical
operations here. In mathematics, '=' is called less than or
equal to[1].
If you are thinking about test numbers you can say that you
include a test if it has a number before or equal to the given
one. The sentence is A number prefixed with = matches all
tests [with numbers] before or including the specified [number].
Maybe if I change one to number it would be a bit less
ambiguous. Or even include all the omitted words.
I would not mind a completely different way to say it, but I am
not yet sure that if I replace or with and it would make it
a lot better.
[1] https://en.wikipedia.org/wiki/Inequality_%28mathematics%29
+Optional prefixes are:
+
+ - '+' or no prefix: test(s) matching the pattern are included in
+ the run.
+
+ - '-' or '!': test(s) matching the pattern are exluded from the
+ run.
I've been playing with --run, and I find that test selection is not
especially intuitive. For instance, =16 !24 !20 is easier to
reason about when written instead with ranges, such as 16-19 21-24,
or perhaps 16-24 !20. Open-ended ranges make sense too: 5- means
tests 5 through the last, and -5 means tests 1 through 5. (Yes, this
conflicts with your use of '-' to mean negation, but you already have
the perfectly serviceable '!' as an alias for negation.)
I completely agree that ranges allow one to express certain
obvious things much easier than just inequalities. I was even
thinking on a possible syntax. But then I realized that I do not
have a real use case for it.
The only use case that I had is described in the cover letter: to
run several setup tests and then the target test. For that even
simple lists were enough and I was using that original version
with an environment variable. After a conversation on the list I
thought that it would be nice to be able to say '', as it would
save typing several extra characters for cases like '1 2 3 4 25'.
While test suits that I've seen so far actually have no more than
two tests that do the setup.
The next use case that I could come up with was running up to a
specific test. I was in a
Re: [PATCH 09/10] t4213: test --function-name option
Am 3/27/2014 19:50, schrieb David A. Dalrymple (and Bhushan G. Lodha):
From: Bhushan G. Lodha David A. Dalrymple dad-...@mit.edu
This test builds a sample C file, adding and removing functions, and
checks that the right commits are filtered by --function-name matching.
This is probably the most important patch in your series as it documents
the expected behavior. Unfortunately, I find its clarity very lacking. :(
This new feature uses the userdiff driver, IIUC. Does it do so in all
respects? In particular, does it also evaluate the negative patterns? For
example, when there is a label in the code, is it not mistaken as the
beginning of a function? A test for this case would be very instructive.
Furthermore, consider a patch for a change at the very beginning of a
function. Then the function name would appear in the pre-context of the
hunk, but the hunk header would show the function before the one with the
change. Would such a change confuse your implementation? I guess not.
Again, a test case would remove any doubts.
Is it possible to search for a change that is before any functions? It
would be useful to enumerate commits that change #include lines.
Signed-off-by: David Dalrymple (on zayin) davi...@alum.mit.edu
---
t/t4213-log-function-name.sh | 73
1 file changed, 73 insertions(+)
create mode 100755 t/t4213-log-function-name.sh
diff --git a/t/t4213-log-function-name.sh b/t/t4213-log-function-name.sh
new file mode 100755
index 000..1243ce5
--- /dev/null
+++ b/t/t4213-log-function-name.sh
@@ -0,0 +1,73 @@
+#!/bin/sh
+
+test_description='log --function-name'
+. ./test-lib.sh
+
+test_expect_success setup '
+ echo * diff=cpp .gitattributes
+
+ file
+ git add file
+ test_tick
+ git commit -m initial
+
+ printf int main(){\n\treturn 0;\n}\n file
+ test_tick
+ git commit -am second
Broken chain here and later as well. Please be careful.
+
+ printf void newfunc(){\n\treturn;\n}\n file
+ test_tick
+ git commit -am third
git commit -am append a function
+
+ printf void newfunc2(){\n\treturn;\n}\n | cat - file temp
+ mv temp file
+ test_tick
+ git commit -am fourth
git commit -am prepend a function
etc. You get the picture.
+
+ printf void newfunc3(){\n\treturn;\n}\n | cat - file temp
+ mv temp file
+ test_tick
+ git commit -am fifth
+
+ sed -i -e s/void newfunc2/void newfunc4/ file
+ test_tick
+ git commit -am sixth
+'
+
+test_expect_success 'log --function-name=main' '
test_expect_success 'log --function-name finds a function with a change' '
+ git log --function-name=main actual
+ git log --grep=second expect
+ test_cmp expect actual
+'
+
+test_expect_success 'log --function-name newfunc\W' '
test_expect_success 'log --function-name with extended regexp' '
etc. You get the picture.
+ git log --function-name newfunc\W actual
+ git log --grep=third expect
+ test_cmp expect actual
+'
+
+test_expect_success 'log --function-name newfunc2' '
+ git log --function-name newfunc2 actual
+ git log -E --grep sixth|fourth expect
+ test_cmp expect actual
+'
+
+test_expect_success 'log --function-name newfunc3' '
+ git log --function-name newfunc3 actual
+ git log --grep=fifth expect
+ test_cmp expect actual
+'
+
+test_expect_success 'log --function-name newfunc4' '
+ git log --function-name newfunc4 actual
+ git log --grep=sixth expect
+ test_cmp expect actual
+'
+
+test_expect_success 'log --function-name newfunc' '
+ git log --function-name newfunc actual
+ git log -E --grep third|fourth|fifth|sixth expect
+ test_cmp expect actual
+'
+
+test_done
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] MSVC: define INLINE=__inline so simple `make MSVC=1` actually works
Without this, xdiff/xutils.c fails to compile. Signed-off-by: Marat Radchenko ma...@slonopotamus.org --- I thought about removing #define inline __inline from compat/msvc.h but: * compat/msvc.h is included based on #if defined(_MSC_VER) and can be enabled even if MSVC != 1 * compat/msvc.h also has #define __inline__ __inline and I don't see a nice way to handle both of them in config.mak.uname config.mak.uname | 1 + 1 file changed, 1 insertion(+) diff --git a/config.mak.uname b/config.mak.uname index 6c7b904..38c60af 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -355,6 +355,7 @@ ifeq ($(uname_S),Windows) NO_POSIX_GOODIES = UnfortunatelyYes NATIVE_CRLF = YesPlease DEFAULT_HELP_FORMAT = html + INLINE = __inline CC = compat/vcbuild/scripts/clink.pl AR = compat/vcbuild/scripts/lib.pl -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/10] t4213: test --function-name option
On Fri, Mar 28, 2014 at 3:25 AM, Johannes Sixt j.s...@viscovery.net wrote:
Am 3/27/2014 19:50, schrieb David A. Dalrymple (and Bhushan G. Lodha):
From: Bhushan G. Lodha David A. Dalrymple dad-...@mit.edu
This test builds a sample C file, adding and removing functions, and
checks that the right commits are filtered by --function-name matching.
This is probably the most important patch in your series as it documents
the expected behavior. Unfortunately, I find its clarity very lacking. :(
This new feature uses the userdiff driver, IIUC. Does it do so in all
respects? In particular, does it also evaluate the negative patterns? For
example, when there is a label in the code, is it not mistaken as the
beginning of a function? A test for this case would be very instructive.
Furthermore, consider a patch for a change at the very beginning of a
function. Then the function name would appear in the pre-context of the
hunk, but the hunk header would show the function before the one with the
change. Would such a change confuse your implementation? I guess not.
Again, a test case would remove any doubts.
Is it possible to search for a change that is before any functions? It
would be useful to enumerate commits that change #include lines.
Signed-off-by: David Dalrymple (on zayin) davi...@alum.mit.edu
---
t/t4213-log-function-name.sh | 73
1 file changed, 73 insertions(+)
create mode 100755 t/t4213-log-function-name.sh
diff --git a/t/t4213-log-function-name.sh b/t/t4213-log-function-name.sh
new file mode 100755
index 000..1243ce5
--- /dev/null
+++ b/t/t4213-log-function-name.sh
@@ -0,0 +1,73 @@
+#!/bin/sh
+
+test_description='log --function-name'
+. ./test-lib.sh
+
+test_expect_success setup '
+ echo * diff=cpp .gitattributes
Broken -chain here, as well.
+
+ file
+ git add file
+ test_tick
+ git commit -m initial
+
+ printf int main(){\n\treturn 0;\n}\n file
+ test_tick
+ git commit -am second
Broken chain here and later as well. Please be careful.
+
+ printf void newfunc(){\n\treturn;\n}\n file
+ test_tick
+ git commit -am third
git commit -am append a function
+
+ printf void newfunc2(){\n\treturn;\n}\n | cat - file temp
+ mv temp file
+ test_tick
+ git commit -am fourth
git commit -am prepend a function
etc. You get the picture.
+
+ printf void newfunc3(){\n\treturn;\n}\n | cat - file temp
+ mv temp file
+ test_tick
+ git commit -am fifth
+
+ sed -i -e s/void newfunc2/void newfunc4/ file
+ test_tick
+ git commit -am sixth
+'
+
+test_expect_success 'log --function-name=main' '
test_expect_success 'log --function-name finds a function with a change' '
+ git log --function-name=main actual
+ git log --grep=second expect
+ test_cmp expect actual
+'
+
+test_expect_success 'log --function-name newfunc\W' '
test_expect_success 'log --function-name with extended regexp' '
etc. You get the picture.
+ git log --function-name newfunc\W actual
+ git log --grep=third expect
+ test_cmp expect actual
+'
+
+test_expect_success 'log --function-name newfunc2' '
+ git log --function-name newfunc2 actual
+ git log -E --grep sixth|fourth expect
+ test_cmp expect actual
+'
+
+test_expect_success 'log --function-name newfunc3' '
+ git log --function-name newfunc3 actual
+ git log --grep=fifth expect
+ test_cmp expect actual
+'
+
+test_expect_success 'log --function-name newfunc4' '
+ git log --function-name newfunc4 actual
+ git log --grep=sixth expect
+ test_cmp expect actual
+'
+
+test_expect_success 'log --function-name newfunc' '
+ git log --function-name newfunc actual
+ git log -E --grep third|fourth|fifth|sixth expect
+ test_cmp expect actual
+'
+
+test_done
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
K1tchen Designer Hertfordshire
K1tchen Designer Hertfordshire. Thirty Ex Display K1tchens To Clear. w.w.w-e.x.d.i.s.p.l.a.y.k.i.t.c.h.e.n.s.1-c.o-u.k. .£ 5.9.5. Each with appliances. -- View this message in context: http://git.661346.n2.nabble.com/K1tchen-Designer-Hertfordshire-tp7606921.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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[BUG] MSVC: error box when interrupting `gitlog` by quitting less
Jeff King peff at peff.net writes: The write_or_die function will always die on an error, including EPIPE. However, it currently treats EPIPE specially by suppressing any error message, and by exiting with exit code 0. This causes error box on Windows in MSVC=1 build: git.exe!_invoke_watson(...) Line 132C++ git.exe!_invalid_parameter(...) Line 85 C++ git.exe!_invalid_parameter_noinfo() Line 97 C++ git.exe!raise(int signum) Line 499 C git.exe!mingw_raise(int sig) Line 1745 C git.exe!check_pipe(int err) Line 9 C git.exe!maybe_flush_or_die(_iobuf * f, const char * desc) Line 48 C git.exe!log_tree_commit(rev_info * opt, commit * commit) Line 820 C git.exe!cmd_log_walk(rev_info * rev) Line 344 C git.exe!cmd_log(int argc, const char * * argv, const char * prefix) Line 637 C git.exe!run_builtin(cmd_struct * p, int argc, const char * * argv) Line 314 C git.exe!handle_builtin(int argc, const char * * argv) Line 487 C git.exe!run_argv(int * argcp, const char * * * argv) Line 536 C git.exe!mingw_main(int argc, char * * av) Line 616 C git.exe!main(int argc, char * * argv) Line 551 C Should never happen, ha-ha. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] MSVC: error box when interrupting `gitlog` by quitting less
Marat Radchenko marat at slonopotamus.org writes: Jeff King peff at peff.net writes: The write_or_die function will always die on an error, including EPIPE. However, it currently treats EPIPE specially by suppressing any error message, and by exiting with exit code 0. This causes error box on Windows in MSVC=1 build: After deeper investigation it turned out that Windows supports much less signals [1] than POSIX and If the argument is not a valid signal as specified above, the invalid parameter handler is invoked. The question is - what is the proper way to fix this? Patch mingw_raise in compat/mingw.c to map unsupported signals into supported ones like SIGPIPE - SIGTERM? [1]: http://msdn.microsoft.com/en-us/library/dwwzkt4c.aspx -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] MSVC: error box when interrupting `gitlog` by quitting less
On Fri, Mar 28, 2014 at 09:14:07AM +, Marat Radchenko wrote: Jeff King peff at peff.net writes: The write_or_die function will always die on an error, including EPIPE. However, it currently treats EPIPE specially by suppressing any error message, and by exiting with exit code 0. This causes error box on Windows in MSVC=1 build: After deeper investigation it turned out that Windows supports much less signals [1] than POSIX and If the argument is not a valid signal as specified above, the invalid parameter handler is invoked. The question is - what is the proper way to fix this? Patch mingw_raise in compat/mingw.c to map unsupported signals into supported ones like SIGPIPE - SIGTERM? [1]: http://msdn.microsoft.com/en-us/library/dwwzkt4c.aspx I'm not sure what an actual SIGPIPE death looks like on Windows. What happens if git is still writing data to the pager and the pager exits? Does it receive a signal of some sort? The point of the code in check_pipe is to simulate that death. So whatever happens to git in that case is what we would want to happen when we call raise(SIGPIPE). A possibly simpler option would be to just have the MSVC build skip the raise() call, and do the exit(141) that comes just after. That is probably close enough simulation of SIGPIPE death. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gitweb: gpg signature status indication for commits
show gpg signature (if any) for commit message in gitweb
in case of valid signature highlight it with green
in case of invalid signature highlight it with red
Signed-off-by: Victor Kartashov victor.kartas...@gmail.com
---
here's new patch
fixed remarks by Eric Sunshine
pop @commit_lines in parse_commit_text() leads to a loss of the last line in
commit message ('sign-off' line, for example), so I search for '\0' before
removing it.
gitweb/gitweb.perl | 36 +---
gitweb/static/gitweb.css | 11 +++
2 files changed, 40 insertions(+), 7 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 79057b7..ccde90f 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3430,8 +3430,9 @@ sub parse_commit_text {
my ($commit_text, $withparents) = @_;
my @commit_lines = split '\n', $commit_text;
my %co;
+ my @signature = ();
- pop @commit_lines; # Remove '\0'
+ pop @commit_lines if ($commit_lines[-1] =~ \0); # Remove '\0'
if (! @commit_lines) {
return;
@@ -3469,6 +3470,9 @@ sub parse_commit_text {
$co{'committer_name'} = $co{'committer'};
}
}
+ elsif ($line =~ /^gpg: /) {
+ push @signature, $line;
+ }
}
if (!defined $co{'tree'}) {
return;
@@ -3508,6 +3512,10 @@ sub parse_commit_text {
foreach my $line (@commit_lines) {
$line =~ s/^//;
}
+ push(@commit_lines, ) if scalar @signature;
+ foreach my $sig (@signature) {
+ push(@commit_lines, $sig);
+ }
$co{'comment'} = \@commit_lines;
my $age = time - $co{'committer_epoch'};
@@ -3530,13 +3538,13 @@ sub parse_commit {
local $/ = \0;
- open my $fd, -|, git_cmd(), rev-list,
- --parents,
- --header,
- --max-count=1,
+ open my $fd, -|, git_cmd(), show,
+ --quiet,
+ --date=raw,
+ --pretty=format:%H %P%ntree %T%nparent %P%nauthor %an %ae
%ad%ncommitter %cn %ce %cd%n%GG%n%s%n%n%b,
$commit_id,
--,
- or die_error(500, Open git-rev-list failed);
+ or die_error(500, Open git-show failed);
%co = parse_commit_text($fd, 1);
close $fd;
@@ -4571,7 +4579,21 @@ sub git_print_log {
# print log
my $skip_blank_line = 0;
foreach my $line (@$log) {
- if ($line =~ m/^\s*([A-Z][-A-Za-z]*-[Bb]y|C[Cc]): /) {
+ if ($line =~ m/^gpg:(.)+Good(.)+/) {
+ if (! $opts{'-remove_signoff'}) {
+ print span class=\good_sign\ .
esc_html($line) . /spanbr/\n;
+ $skip_blank_line = 1;
+ }
+ next;
+ }
+ elsif ($line =~ m/^gpg:(.)+BAD(.)+/) {
+ if (! $opts{'-remove_signoff'}) {
+ print span class=\bad_sign\ .
esc_html($line) . /spanbr/\n;
+ $skip_blank_line = 1;
+ }
+ next;
+ }
+ elsif ($line =~ m/^\s*([A-Z][-A-Za-z]*-[Bb]y|C[Cc]): /) {
if (! $opts{'-remove_signoff'}) {
print span class=\signoff\ .
esc_html($line) . /spanbr/\n;
$skip_blank_line = 1;
diff --git a/gitweb/static/gitweb.css b/gitweb/static/gitweb.css
index 3212601..e99e223 100644
--- a/gitweb/static/gitweb.css
+++ b/gitweb/static/gitweb.css
@@ -136,6 +136,17 @@ span.signoff {
color: #88;
}
+span.good_sign {
+ font-weight: bold;
+ background-color: #aaffaa;
+}
+
+span.bad_sign {
+ font-weight: bold;
+ background-color: #88;
+ color: #ff
+}
+
div.log_link {
padding: 0px 8px;
font-size: 70%;
--
1.8.3.rc0.10.g8974033
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] add `ignore_missing_links` mode to revwalk
From: Vicent Marti tan...@gmail.com
When pack-objects is computing the reachability bitmap to
serve a fetch request, it can erroneously die() if some of
the UNINTERESTING objects are not present. Upload-pack
throws away HAVE lines from the client for objects we do not
have, but we may have a tip object without all of its
ancestors (e.g., if the tip is no longer reachable and was
new enough to survive a `git prune`, but some of its
reachable objects did get pruned).
In the non-bitmap case, we do a revision walk with the HAVE
objects marked as UNINTERESTING. The revision walker
explicitly ignores errors in accessing UNINTERESTING commits
to handle this case (and we do not bother looking at
UNINTERESTING trees or blobs at all).
When we have bitmaps, however, the process is quite
different. The bitmap index for a pack-objects run is
calculated in two separate steps:
First, we perform an extensive walk from all the HAVEs to
find the full set of objects reachable from them. This walk
is usually optimized away because we are expected to hit an
object with a bitmap during the traversal, which allows us
to terminate early.
Secondly, we perform an extensive walk from all the WANTs,
which usually also terminates early because we hit a commit
with an existing bitmap.
Once we have the resulting bitmaps from the two walks, we
AND-NOT them together to obtain the resulting set of objects
we need to pack.
When we are walking the HAVE objects, the revision walker
does not know that we are walking it only to mark the
results as uninteresting. We strip out the UNINTERESTING flag,
because those objects _are_ interesting to us during the
first walk. We want to keep going to get a complete set of
reachable objects if we can.
We need some way to tell the revision walker that it's OK to
silently truncate the HAVE walk, just like it does for the
UNINTERESTING case. This patch introduces a new
`ignore_missing_links` flag to the `rev_info` struct, which
we set only for the HAVE walk.
It also adds tests to cover UNINTERESTING objects missing
from several positions: a missing blob, a missing tree, and
a missing parent commit. The missing blob already worked (as
we do not care about its contents at all), but the other two
cases caused us to die().
Note that there are a few cases we do not need to test:
1. We do not need to test a missing tree, with the blob
still present. Without the tree that refers to it, we
would not know that the blob is relevant to our walk.
2. We do not need to test a tip commit that is missing.
Upload-pack omits these for us (and in fact, we
complain even in the non-bitmap case if it fails to do
so).
Reported-by: Siddharth Agarwal s...@fb.com
Signed-off-by: Vicent Marti tan...@gmail.com
Signed-off-by: Jeff King p...@peff.net
---
I believe this should solve the problem you're seeing, and I think any
solution is going to be along these lines.
This covers all code paths that can be triggered by pack-objects. But
it does not necessarily cover all code paths that a revision walker
might use (e.g., it is still possible to die in try_to_simplify_commit,
but we would never hit that in pack-objects, because we do not do
pathspec limiting).
So it's a tradeoff. On the one hand, leaving it like this creates a flag
in rev_info that may surprise somebody later by not being as generally
useful. On the other hand, covering every die() is extra code churn, and
creates complexity for cases that cannot actually be triggered in
practice (complexity because each site has to decide how to handle a
failure to access the object).
list-objects.c | 5 -
pack-bitmap.c | 2 ++
revision.c | 8 +---
revision.h | 3 ++-
t/t5310-pack-bitmaps.sh | 31 +++
5 files changed, 44 insertions(+), 5 deletions(-)
diff --git a/list-objects.c b/list-objects.c
index 206816f..3595ee7 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -81,8 +81,11 @@ static void process_tree(struct rev_info *revs,
die(bad tree object);
if (obj-flags (UNINTERESTING | SEEN))
return;
- if (parse_tree(tree) 0)
+ if (parse_tree(tree) 0) {
+ if (revs-ignore_missing_links)
+ return;
die(bad tree object %s, sha1_to_hex(obj-sha1));
+ }
obj-flags |= SEEN;
show(obj, path, name, cb_data);
me.up = path;
diff --git a/pack-bitmap.c b/pack-bitmap.c
index ae0b57b..91e4101 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -727,8 +727,10 @@ int prepare_bitmap_walk(struct rev_info *revs)
revs-pending.objects = NULL;
if (haves) {
+ revs-ignore_missing_links = 1;
haves_bitmap = find_objects(revs, haves, NULL);
reset_revision_walk();
+ revs-ignore_missing_links = 0;
if (haves_bitmap == NULL)
die(BUG: failed to
Re: [BUG] MSVC: error box when interrupting `gitlog` by quitting less
Jeff King peff at peff.net writes: I'm not sure what an actual SIGPIPE death looks like on Windows. There is no SIGPIPE death on Windows due to total absence of SIGPIPE. raise(unsupported int) just causes ugly git.exe has stopped working window and possibly ends up as SIGABT (I don't know how to check this). What happens if git is still writing data to the pager and the pager exits? Does it receive a signal of some sort? I'm not sure what you mean, sorry. check_pipe properly detects pager exit. The problem is with the way it tries to die. The point of the code in check_pipe is to simulate that death. So whatever happens to git in that case is what we would want to happen when we call raise(SIGPIPE). That's what I'm talking about. On Windows, you can't raise(SIGPIPE). You can only raise(Windows_supported_signal) where signal is one of: SIGABRT, SIGFPE, SIGILL, SIGINT, SIGSEGV, SIGTERM as MSDN tells us. A possibly simpler option would be to just have the MSVC build skip the raise() call, and do the exit(141) that comes just after. That is probably close enough simulation of SIGPIPE death. Isn't raise(SIGTERM/SIGINT) good enough? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] MSVC: error box when interrupting `gitlog` by quitting less
On Fri, Mar 28, 2014 at 10:07:22AM +, Marat Radchenko wrote: What happens if git is still writing data to the pager and the pager exits? Does it receive a signal of some sort? I'm not sure what you mean, sorry. check_pipe properly detects pager exit. The problem is with the way it tries to die. Right, but check_pipe shouldn't trigger in most cases on Unix because the process will be killed by SIGPIPE automatically. It's only there to catch the case where we have disabled SIGPIPE. On Windows, what happens to yes if you run: yes | (exit 0) On Unix, yes receives SIGPIPE and dies. Does it run forever on Windows? If it dies, what does the death look like (does it have a signal death, or exit with a specific code?). The point of the code in check_pipe is to simulate that death. So whatever happens to git in that case is what we would want to happen when we call raise(SIGPIPE). That's what I'm talking about. On Windows, you can't raise(SIGPIPE). You can only raise(Windows_supported_signal) where signal is one of: SIGABRT, SIGFPE, SIGILL, SIGINT, SIGSEGV, SIGTERM as MSDN tells us. Right, I understand that you don't have SIGPIPE. But we want to emulate whatever happens in the case I described above. A possibly simpler option would be to just have the MSVC build skip the raise() call, and do the exit(141) that comes just after. That is probably close enough simulation of SIGPIPE death. Isn't raise(SIGTERM/SIGINT) good enough? Perhaps. It is a slight lie. We _didn't_ get a SIGTERM, and anybody looking at our exit code to find out why we died would be misled. But the most important thing is that we die and that the exit status is non-zero. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] MSVC: error box when interrupting `gitlog` by quitting less
Please do not cull the Cc list. Am 3/28/2014 11:07, schrieb Marat Radchenko: Jeff King peff at peff.net writes: I'm not sure what an actual SIGPIPE death looks like on Windows. There is no SIGPIPE death on Windows due to total absence of SIGPIPE. raise(unsupported int) just causes ugly git.exe has stopped working window and possibly ends up as SIGABT (I don't know how to check this). This happens only with newer Microsoft C runtime libraries. They do not return EINVAL (because that usually indicates a bug caused by insufficient checks before the function call), but crash the program by default in the way that you observed. What happens if git is still writing data to the pager and the pager exits? Does it receive a signal of some sort? No; the write attempt returns with EPIPE. I'm not sure what you mean, sorry. check_pipe properly detects pager exit. The problem is with the way it tries to die. The point of the code in check_pipe is to simulate that death. So whatever happens to git in that case is what we would want to happen when we call raise(SIGPIPE). That's what I'm talking about. On Windows, you can't raise(SIGPIPE). You can only raise(Windows_supported_signal) where signal is one of: SIGABRT, SIGFPE, SIGILL, SIGINT, SIGSEGV, SIGTERM as MSDN tells us. Correct. All other signal number should return EINVAL. But, as I said, that does not happen by default. The correct solution is to link against invalidcontinue.obj in the MSVC build. This is a compiler-provided object file that changes the default behavior to the expected kind, i.e., C runtime functions return EINVAL when appropriate instead of crashing the application. A possibly simpler option would be to just have the MSVC build skip the raise() call, and do the exit(141) that comes just after. That is probably close enough simulation of SIGPIPE death. Correct. The MinGW build uses an older C runtime library, which does not have the strange default behavior, and we do use that exit(141). And with the fix to the MSVC build suggested above, that version would do likewise. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Problems with git 1.8.5.3 on HP-UX 11.11
In order to set up automated builds and tests of the CMake toolchain (www.cmake.org) on HP-UX 11.11 (hppa) and 11.23 (ia64), I needed to install git on those platforms. The latest binary package available from hpux.connect.org.uk is version 1.8.5.3, which I installed with all of its dependencies. When trying to set up the CMake build, I ran into the first problem: $ git pull origin error: cannot create thread: Function is not available fatal: fetch-pack: unable to fork off sideband demultiplexer So I examined the git source package and found that the author of the HP-UX port forgot to set PTHREAD_CFLAGS=-mt in config.mak.autogen to enable threading. I added this setting and rebuilt git. On 11.23, everything was fine now - no further issues. On 11.11 though, git now crashed with a Bus Error. Some debugging showed that this was due to a multithreading issue - obviously some dependency library has not been built as reentrant code. To fix this, I disabled threading by setting PTHREAD_CFLAGS= NO_PTHREADS=YesPlease in config.mak.autogen and rebuilt git again. After that, git pull and git fetch worked correctly and I could proceed to set up the CMake build and test. Alas, the CMake tests include a test case CTest.UpdateGIT that creates a git repository, creates a submodule, imports some content and attempts to check out a revision. At that point, the command git submodule init fails with the output Assertion failed: err == REG_ESPACE, file compat/regex/regexec.c, line 1096 No submodule mapping found in .gitmodules for path 'module' and the stacktrace of the resulting core dump is #0 0xc020ced0 in kill+0x10 () from /usr/lib/libc.2 #1 0xc01a7f84 in raise+0x24 () from /usr/lib/libc.2 #2 0xc01e9308 in abort_C+0x160 () from /usr/lib/libc.2 #3 0xc01e9364 in abort+0x1c () from /usr/lib/libc.2 #4 0xc0176998 in _assert+0x178 () from /usr/lib/libc.2 #5 0x205fa0 in check_matching+0x290 () #6 0x2053b8 in re_search_internal+0x128 () #7 0x204ac0 in regexec+0xc8 () #8 0x4da40 in collect_config+0x60 () #9 0x108b30 in get_value+0xd8 () #10 0x108efc in git_parse_source+0x1bc () #11 0x10ac70 in do_config_from+0x70 () #12 0x10ad3c in git_config_from_file+0x8c () #13 0x10b274 in git_config_with_options+0x84 () #14 0x4dd6c in get_value+0x224 () #15 0x4eed4 in cmd_config+0x744 () #16 0x17150 in run_builtin+0x110 () #17 0x1739c in handle_internal_command+0xcc () #18 0x174fc in run_argv+0x2c () #19 0x17724 in main+0x194 () Since I'm no git expert (I'm not even a regular git user in fact), there's nothing left for me to do except asking for help... Please CC me (gerhard dot grimm at detec dot com) with any replies since I'm not subscribed to the list. Thank you! Best regards, Gerhard This e-mail message has been scanned and cleared by Postini / Google Message Security and the UNICOM Global security systems. This message is for the named person's use only. If you receive this message in error, please delete it and notify the sender. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Problems with git 1.8.5.3 on HP-UX 11.11
Gerhard Grimm gerhard.grimm at detec.com writes: In order to set up automated builds and tests of the CMake toolchain (www.cmake.org) on HP-UX 11.11 (hppa) and 11.23 (ia64), I needed to install git on those platforms. The latest binary package available from hpux.connect.org.uk is version 1.8.5.3, which I installed with all of its dependencies. Did you try to build the most current version v1.9.1 by using autoconf as described in 'INSTALL'? --- Thomas -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] MSVC: fix t0040-parse-options
Signed-off-by: Marat Radchenko ma...@slonopotamus.org
---
test-parse-options.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/test-parse-options.c b/test-parse-options.c
index 434e8b8..7840493 100644
--- a/test-parse-options.c
+++ b/test-parse-options.c
@@ -11,6 +11,7 @@ static char *string = NULL;
static char *file = NULL;
static int ambiguous;
static struct string_list list;
+static const char *default_string = default;
static int length_callback(const struct option *opt, const char *arg, int
unset)
{
@@ -60,7 +61,7 @@ int main(int argc, char **argv)
OPT_STRING('o', NULL, string, str, get another string),
OPT_NOOP_NOARG(0, obsolete),
OPT_SET_PTR(0, default-string, string,
- set string to default, (unsigned long)default),
+ set string to default, default_string),
OPT_STRING_LIST(0, list, list, str, add str to list),
OPT_GROUP(Magic arguments),
OPT_ARGUMENT(quux, means --quux),
--
1.9.1.501.gfbd1a76.dirty.MSVC
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/3] patch-id-test: test new --stable and --unstable flags
Verify that patch ID is now stable against hunk reordering.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
changes from v1:
Use -\EOF to address comment by Eric Sunshine
t/t4204-patch-id.sh | 68 +
1 file changed, 63 insertions(+), 5 deletions(-)
diff --git a/t/t4204-patch-id.sh b/t/t4204-patch-id.sh
index d2c930d..44dfd33 100755
--- a/t/t4204-patch-id.sh
+++ b/t/t4204-patch-id.sh
@@ -5,12 +5,27 @@ test_description='git patch-id'
. ./test-lib.sh
test_expect_success 'setup' '
- test_commit initial foo a
- test_commit first foo b
+ test_commit initial-foo foo a
+ test_commit initial-bar bar a
+ echo b foo
+ echo b bar
+ git commit -a -m first
git checkout -b same HEAD^
- test_commit same-msg foo b
+ echo b foo
+ echo b bar
+ git commit -a -m same-msg
git checkout -b notsame HEAD^
- test_commit notsame-msg foo c
+ echo c foo
+ echo c bar
+ git commit -a -m notsame-msg
+ cat bar-then-foo -\EOF
+ bar
+ foo
+ EOF
+ cat foo-then-bar -\EOF
+ foo
+ bar
+ EOF
'
test_expect_success 'patch-id output is well-formed' '
@@ -23,11 +38,33 @@ calc_patch_id () {
sed s# .*## patch-id_$1
}
+calc_patch_id_unstable () {
+ git patch-id --unstable |
+ sed s# .*## patch-id_$1
+}
+
+calc_patch_id_stable () {
+ git patch-id --stable |
+ sed s# .*## patch-id_$1
+}
+
+
get_patch_id () {
- git log -p -1 $1 | git patch-id |
+ git log -p -1 $1 -O bar-then-foo -- | git patch-id |
sed s# .*## patch-id_$1
}
+get_patch_id_stable () {
+ git log -p -1 $1 -O bar-then-foo | git patch-id --stable |
+ sed s# .*## patch-id_$1
+}
+
+get_patch_id_unstable () {
+ git log -p -1 $1 -O bar-then-foo | git patch-id --unstable |
+ sed s# .*## patch-id_$1
+}
+
+
test_expect_success 'patch-id detects equality' '
get_patch_id master
get_patch_id same
@@ -56,6 +93,27 @@ test_expect_success 'whitespace is irrelevant in footer' '
test_cmp patch-id_master patch-id_same
'
+test_expect_success 'file order is irrelevant by default' '
+ get_patch_id master
+ git checkout same
+ git format-patch -1 --stdout -O foo-then-bar | calc_patch_id same
+ test_cmp patch-id_master patch-id_same
+'
+
+test_expect_success 'file order is irrelevant with --stable' '
+ get_patch_id_stable master
+ git checkout same
+ git format-patch -1 --stdout -O foo-then-bar | calc_patch_id_stable
same
+ test_cmp patch-id_master patch-id_same
+'
+
+test_expect_success 'file order is relevant with --unstable' '
+ get_patch_id_unstable master
+ git checkout same
+ git format-patch -1 --stdout -O foo-then-bar | calc_patch_id_unstable
notsame
+ ! test_cmp patch-id_master patch-id_notsame
+'
+
test_expect_success 'patch-id supports git-format-patch MIME output' '
get_patch_id master
git checkout same
--
MST
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/3] patch-id: document new behaviour
Clarify that patch ID is now a sum of hashes, not a hash. Document --stable and --unstable flags. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- No change from v1. Documentation/git-patch-id.txt | 21 - 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/Documentation/git-patch-id.txt b/Documentation/git-patch-id.txt index 312c3b1..1bc6d52 100644 --- a/Documentation/git-patch-id.txt +++ b/Documentation/git-patch-id.txt @@ -8,14 +8,14 @@ git-patch-id - Compute unique ID for a patch SYNOPSIS [verse] -'git patch-id' patch +'git patch-id' [--stable | --unstable] patch DESCRIPTION --- -A patch ID is nothing but a SHA-1 of the diff associated with a patch, with -whitespace and line numbers ignored. As such, it's reasonably stable, but at -the same time also reasonably unique, i.e., two patches that have the same patch -ID are almost guaranteed to be the same thing. +A patch ID is nothing but a sum of SHA-1 of the diff hunks associated with a +patch, with whitespace and line numbers ignored. As such, it's reasonably +stable, but at the same time also reasonably unique, i.e., two patches that +have the same patch ID are almost guaranteed to be the same thing. IOW, you can use this thing to look for likely duplicate commits. @@ -27,6 +27,17 @@ This can be used to make a mapping from patch ID to commit ID. OPTIONS --- + +--stable:: + Use a symmetrical sum of hashes, such that order of + hunks in the diff does not affect the ID. + This is the default. + +--unstable:: + Use a non-symmetrical sum of hashes, such that order of + hunks in the diff affects the ID. + This was the default value for git 1.9 and older. + patch:: The diff to create the ID of. -- MST -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/3] patch-id: make it stable against hunk reordering
Patch id changes if you reorder hunks in a diff.
As the result is functionally equivalent, this is surprising to many
people.
In particular, reordering hunks is helpful to make patches
more readable (e.g. API header diff before implementation diff).
In git, it is often done e.g. using the -O orderfile option,
so supporting it better has value.
Hunks within file can be reordered manually provided
the same pathname can appear more than once in the input.
Change patch-id behaviour making it stable against
hunk reodering:
- prepend header to each hunk (if not there)
Note: POSIX requires patch to be robust against hunk reordering
provided each diff hunk has a header:
http://pubs.opengroup.org/onlinepubs/7908799/xcu/patch.html
If the patch file contains more than one patch, patch will
attempt to
apply each of them as if they came from separate patch files.
(In this
case the name of the patch file must be determinable for each
diff
listing.)
- calculate SHA1 hash for each hunk separately
- sum all hashes to get patch id
Add a new flag --unstable to get the historical behaviour.
Add --stable which is a nop, for symmetry.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
Changes from v1: documented motivation for supporting
hunk reordering (and not just file reordering).
No code changes.
Junio, you didn't respond so I'm not sure whether I convinced
you that supporting hunk reordering within file has value.
So I kept this functionality around for now, if
you think I should drop this, please let me know explicitly.
Thanks, and sorry about being dense!
builtin/patch-id.c | 71 ++
1 file changed, 55 insertions(+), 16 deletions(-)
diff --git a/builtin/patch-id.c b/builtin/patch-id.c
index 3cfe02d..253ad87 100644
--- a/builtin/patch-id.c
+++ b/builtin/patch-id.c
@@ -1,17 +1,14 @@
#include builtin.h
-static void flush_current_id(int patchlen, unsigned char *id, git_SHA_CTX *c)
+static void flush_current_id(int patchlen, unsigned char *id, unsigned char
*result)
{
- unsigned char result[20];
char name[50];
if (!patchlen)
return;
- git_SHA1_Final(result, c);
memcpy(name, sha1_to_hex(id), 41);
printf(%s %s\n, sha1_to_hex(result), name);
- git_SHA1_Init(c);
}
static int remove_space(char *line)
@@ -56,10 +53,30 @@ static int scan_hunk_header(const char *p, int *p_before,
int *p_after)
return 1;
}
-static int get_one_patchid(unsigned char *next_sha1, git_SHA_CTX *ctx, struct
strbuf *line_buf)
+static void flush_one_hunk(unsigned char *result, git_SHA_CTX *ctx)
{
- int patchlen = 0, found_next = 0;
+ unsigned char hash[20];
+ unsigned short carry = 0;
+ int i;
+
+ git_SHA1_Final(hash, ctx);
+ git_SHA1_Init(ctx);
+ /* 20-byte sum, with carry */
+ for (i = 0; i 20; ++i) {
+ carry += result[i] + hash[i];
+ result[i] = carry;
+ carry = 8;
+ }
+}
+static int get_one_patchid(unsigned char *next_sha1, unsigned char *result,
+ struct strbuf *line_buf, int stable)
+{
+ int patchlen = 0, found_next = 0, hunks = 0;
int before = -1, after = -1;
+ git_SHA_CTX ctx, header_ctx;
+
+ git_SHA1_Init(ctx);
+ hashclr(result);
while (strbuf_getwholeline(line_buf, stdin, '\n') != EOF) {
char *line = line_buf-buf;
@@ -99,6 +116,18 @@ static int get_one_patchid(unsigned char *next_sha1,
git_SHA_CTX *ctx, struct st
if (!memcmp(line, @@ -, 4)) {
/* Parse next hunk, but ignore line numbers. */
scan_hunk_header(line, before, after);
+ if (stable) {
+ if (hunks) {
+ flush_one_hunk(result, ctx);
+ memcpy(ctx, header_ctx,
+ sizeof ctx);
+ } else {
+ /* Save ctx for next hunk. */
+ memcpy(header_ctx, ctx,
+ sizeof ctx);
+ }
+ }
+ hunks++;
continue;
}
@@ -107,7 +136,10 @@ static int get_one_patchid(unsigned char *next_sha1,
git_SHA_CTX *ctx, struct st
break;
/* Else we're parsing another header. */
+ if (stable hunks)
+ flush_one_hunk(result, ctx);
[PATCH] ls-files: do not trust stat info if lstat() fails
If 'err' is non-zero, lstat() has failed. Consider the entry modified
without passing the (unreliable) stat info to ce_modified() in this
case.
Noticed-by: Eric Sunshine sunsh...@sunshineco.com
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
On Fri, Mar 28, 2014 at 11:04 AM, Eric Sunshine sunsh...@sunshineco.com
wrote:
On Wed, Mar 26, 2014 at 9:48 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com
wrote:
+ err = lstat(ce-name, st);
+ if (show_deleted err) {
+ show_ce_entry(tag_removed, ce);
+ shown = 1;
+ }
+ if (show_modified ce_modified(ce, st, 0)) {
Is it possible for the lstat() to have failed for some reason when we
get here? If so, relying upon 'st' is unsafe, isn't it?
The chance of random stat making ce_modified() return false is pretty
low, but you're right. This code is a copy from the old show_files().
I'll fix it in the git-ls series. Meanwhile a patch for maint to fix
the original function.
builtin/ls-files.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 47c3880..e6bd00e 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -260,7 +260,7 @@ static void show_files(struct dir_struct *dir)
err = lstat(ce-name, st);
if (show_deleted err)
show_ce_entry(tag_removed, ce);
- if (show_modified ce_modified(ce, st, 0))
+ if (show_modified (err || ce_modified(ce, st, 0)))
show_ce_entry(tag_modified, ce);
}
}
--
1.9.1.345.ga1a145c
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] submodule: change submodule.name.branch default from master to HEAD
Am 28.03.2014 04:58, schrieb W. Trevor King: On Thu, Mar 27, 2014 at 08:52:55PM -0700, W. Trevor King wrote: On Thu, Mar 27, 2014 at 11:43:47PM -0400, Eric Sunshine wrote: On Thu, Mar 27, 2014 at 11:36 PM, W. Trevor King wk...@tremily.us wrote: submodule.name.branch:: A remote branch name for tracking updates in the upstream submodule. - If the option is not specified, it defaults to 'master'. See the - `--remote` documentation in linkgit:git-submodule[1] for details. + If the option is not specified, it defaults to the subproject's Did you mean s/subproject/submodule/ ? + HEAD. See the `--remote` documentation in linkgit:git-submodule[1] + for details. No the remote branch is in the upstream subproject. I suppose I meant “the submodule's remote-tracking branch following the upstream subproject's HEAD which we just fetched so it's fairly current” ;). Hmm, maybe we should change the existing “upstream submodule” to “upstream subproject” for consistency? For me it's still an upstream submodule ... -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Documentation/submodule: Fix submodule.name - .path typos
On Fri, Mar 28, 2014 at 05:55:18PM +0100, Jens Lehmann wrote: I just noticed that the two patches Junio added to pu have a reworded commit message I'm perfectly happy with. The revised wording works for me too. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: Borrowing objects from nearby repositories
Andrew Keller and...@kellerfarm.com writes: Okay, so to re-frame my idea, like you said, the goal is to find a user- friendly way for the user to tell git-clone to set up the alternates file (or perhaps just use the --alternates parameter), and run a repack, and disconnect the alternate. And yet, we still want to be able to use --reference on its own, because there are existing use cases for that. Here are a few possible action items that came out of this discussion: 1. Introduce a new --borrow option to git clone. The updates to the SYNOPSIS section may go like this: -'git clone' [--reference repository] ...other options... +'git clone' [[--reference|--borrow] repository] ...other options... The new option can be used instead of --reference and they will be mutually incompatible. The first implementation of the --borrow option would do the following: (1) run the same git clone with the same command line but replacing --borrow with --reference; if this fails, exit with the same failure. (2) in the resulting repository, run git repack -a -d; if this fails, remove the entire directory the first step created, and exit with failure. (3) remove .git/objects/info/alternates from the resulting repository and exit with success. and it may be acceptable as the final implementation as well. 2. Make git repack safer for the users of clone --reference who want to keep sharing objects from the original. - Introduce the repack.local configuration variable that can be set to either true or false. Missing variable defaults to false. - A repack that is run without -l option on the command line will pretend as if it was given -l from the command line if repack.local is set to true. Add repack --no-local option to countermand this configuration variable from the command line. - Teach git clone --reference (but not git clone --borrow) to set repack.local = true in the configuration of the resulting repository. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] submodule: change submodule.name.branch default from master to HEAD
On Fri, Mar 28, 2014 at 05:57:50PM +0100, Jens Lehmann wrote: Am 28.03.2014 04:58, schrieb W. Trevor King: On Thu, Mar 27, 2014 at 08:52:55PM -0700, W. Trevor King wrote: No the remote branch is in the upstream subproject. I suppose I meant “the submodule's remote-tracking branch following the upstream subproject's HEAD which we just fetched so it's fairly current” ;). Hmm, maybe we should change the existing “upstream submodule” to “upstream subproject” for consistency? For me it's still an upstream submodule ... We have a few existing “[upstream] subproject” references though. I prefer subproject, because the submodule's upstream repository is likely a bare repo and not a submodule itself. It's also possible (likely?) that the upstream repository is a stand-alone project, and not designed to always be a submodule. However, “upstream submodule” and “submodule's upstream” are both clear enough, and if they're the consensus phrasing, I'd rather standardize on them than jump back and forth between phrasings in the docs. I can write up a patch that shifts us to consistently use one form, once we decide what that should be (although I'm happy to let someone else write the patch too ;). Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [PATCH] Documentation/submodule: Fix submodule.name - .path typos
I just noticed that the two patches Junio added to pu have a reworded commit message I'm perfectly happy with. Thanks all. Am 28.03.2014 03:06, schrieb W. Trevor King: On Fri, Mar 28, 2014 at 12:15:00AM +0100, Jens Lehmann wrote: Am 27.03.2014 22:06, schrieb W. Trevor King: The transition from submodule.path.* to submodule.name.* happened in 73b0898d (Teach git submodule add the --name option, 2012-09-30), which landed in v1.8.1-rc0 on 2012-12-03. Nope, the distinction between path and name is way older (AFAIK it is there from day one). That was just the point in time where you could choose a different name without editing .gitmodules. And the fact that the name is initialized with the path confused a lot of people. Before 73b0898d, cmd_add used: git config -f .gitmodules submodule.$sm_path.path $sm_path and similar, so I used submodule.path.branch in my initial documentation of this patch (v5 of that series) [1]. By the final v8 (which rebased onto the then-current master with 73b0898d), the surrounding calls were [2]: git config -f .gitmodules submodule.$sm_name.path $sm_path but I missed the update to name in my rebasing. I suppose I could have used name instead of path in my initial v5 patch, but I was one of the folks confused by the old name == path behavior ;). This patch is against master, because 23d25e48 hasn't landed in maint yet. If you want, I can split this into two patches, one against maint fixing the b9289227 typo and another against master fixing the 23d25e48 typo. This fixes the only two usages of 'submodule.path.*' in the Documentation I can see in current master. Right. However, this patch won't apply to the maint branch (where 23d25e48 hasn't landed). I'm just saying that we may want to split this patch in half and push the fix for b9289227 in a maintenance release. On the other hand, we've survived since 2012 with the current docs, so *not* splitting this patch apart works for me too. Cheers, Trevor [1]: http://article.gmane.org/gmane.comp.version-control.git/210763 [2]: http://article.gmane.org/gmane.comp.version-control.git/211832 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git commit vs. ignore-submodules
Am 28.03.2014 00:36, schrieb Ronald Weiss: Hello. As this is my first post to this list, let me first thank all the people involved in Git development - it's really a great tool. Welcome and thanks for the feedback! Now to the point. Since Git 1.8 (I think), git commit command honours the submodules' ignore settings, configured either in .gitmodules, or in .git/config. That's very nice and certainly correct for git commit -a, but it's less clear if one explicitely stages an updated submodule using git add. Git commit will ignore it anyway, if ignore=all is configured in .gitmodules. Maybe that's correct too, I'm not sure about that, but it's inconvenient in our use case, especially combined with the lack of --ignore-submodule parameter to git commit, as git status and git diff have. We use submodules in such a way that normally we don't ever want to see changes in them in output of git diff and git status. So we set ignore=all in .gitmodules for each submodule. But occasionally, we need to add a new submodule, and sometimes also commit changed submodule. This got harder with Git 1.8, we have to git config submodule.name.ignore none before the commit, and git config --unset ... after. I'd like to at least add an --ignore-submodules parameter to git commit. I though about posting a patch, but as I looked into the commit source file, I didn't see any straightforward way to implement it. I don't have enough free time for a deeper analysis of the sources, I'm sorry. So please, let me first know, whether you could possibly accept such patch, and if so, then I'd really appreciate some hints on how to do it. Such a patch would be very much appreciated. You might want to look into other commands that already have the --ignore-submodules option to get an idea how to do that. cmd_status() in builtin/commit.c should be a good starting point. And also, I'd like to know git folks' opinion on whether it's OK that git commit with ignore=all in .gitmodules ignores submodules even when they are explicitely staged with git add. No, they should be visible in status and commit when the user chose to add them. I see if I can hack something up (as I've been bitten myself by that recently ;-). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: SSL_CTX leak?
Jeff King p...@peff.net writes: On Thu, Mar 27, 2014 at 10:37:07AM -0300, Thiago Farina wrote: Do we leak the context we allocate in imap-send.c:280 intentionally? It was never mentioned on the mailing list when the patches came originally, so I suspect is just an omission. Presumably the SSL_CTX is needed by the connection that survives after the function, but my reading of SSL_CTX_free implies that the data is reference-counted, and the library would presumably handle it fine. Yes, I was reading the SSL_new() yesterday and found out that at least in a recent code it increments the reference count on the ctx it is fed. So it would be the right thing to decrement the refcount in the caller that created the context and used to call SSL_new(), but I fully agree with the analysis below (with s/a huge/any/): OTOH, it is probably not causing a huge problem (since we wouldn't end up freeing it until the end of the program anyway), so I would not personally devote to many brain cycles to figuring out how OpenSSL handles it. Heh. So you are saying that I wasted 30 minutes yesterday? ;-) Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t4212: handle systems with post-apocalyptic gmtime
Jeff King p...@peff.net writes: Sat Jan 25 10:46:39 316889355 -0700 9 Wed Sep 6 02:46:39 -1126091476 -0700 99 Thu Oct 24 18:46:39 1623969404 -0700 Thanks. Given the value where it fails, it kind of looks like there is some signed 32-bit value at work (~300 million years is OK, but 10 times that, rather than yielding ~3 billion, gets us -1 billion). Perhaps tm.tm_year is 32-bit. That is what it looks like to me, too. It makes me wonder if some other platforms may have similar breakage using 16-bit signed tm_year and how such a breakage would look like, though ;-) So what do we want to do? I think the options are: 1. Try to guess when we have a bogus timestamp value with an arbitrary cutoff like greater than 1 million years from now (and enforce it via time_t seconds, and avoid gmtime entirely). That is made-up and arbitrary, but it also is sufficiently far that it won't ever matter, and sufficiently close that any gmtime should behave sensibly with it. I think that two assumptions here are that (1) we would be able to find a single insane value (like 3 billion years from now expressed in time_t seconds) the test script would be able to feed and expect it to fail on all platforms we care about, even though how exactly that value causes various platforms fail may be different (glibc may give us a NULL from gmtime, FreeBSD may leave their own sentinel in gmtime we can recognize, and some others may simply wrap around the years); and that (2) the only other class of failure mode we haven't considered bevore Charles's report is tm_year wrapping around 32-bit signed int. Offhand, the three possible failure modes this thread identified sounds to me like the only plausible ones, and I think the best way forward might be to - teach the is the result sane, even though we may have got a non-NULL from gmtime? otherwise let's signal a failure by replacing it with a known sentinel value codepath the new failure mode Charles's report suggests---if we feed a positive timestamp and gmtime gave us back a tm_year+1900 0, that is certainly an overflow; and - Use that 3-billion years timestamp from Charles's report in the test and make sure we convert it to the known sentinel value. perhaps? 2. Accept that we can't guess at every broken gmtime's output, and just loosen the test to make sure we don't segfault. Of course that is a simpler option, but we may have identified all plausible failure modes, in which case we can afford to go with your original plan to validate that we not just avoid segfaulting on one of the three failure modes from gmtime, but also cover the other two failure modes and signal any of them with a sentinel. That way may allow us to identify the fourth failure mode we haven't anticipated. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] submodule: change submodule.name.branch default from master to HEAD
Jens Lehmann jens.lehm...@web.de writes: Am 28.03.2014 04:58, schrieb W. Trevor King: On Thu, Mar 27, 2014 at 08:52:55PM -0700, W. Trevor King wrote: On Thu, Mar 27, 2014 at 11:43:47PM -0400, Eric Sunshine wrote: On Thu, Mar 27, 2014 at 11:36 PM, W. Trevor King wk...@tremily.us wrote: submodule.name.branch:: A remote branch name for tracking updates in the upstream submodule. - If the option is not specified, it defaults to 'master'. See the - `--remote` documentation in linkgit:git-submodule[1] for details. + If the option is not specified, it defaults to the subproject's Did you mean s/subproject/submodule/ ? + HEAD. See the `--remote` documentation in linkgit:git-submodule[1] + for details. No the remote branch is in the upstream subproject. I suppose I meant “the submodule's remote-tracking branch following the upstream subproject's HEAD which we just fetched so it's fairly current” ;). Hmm, maybe we should change the existing “upstream submodule” to “upstream subproject” for consistency? For me it's still an upstream submodule ... I inherited the habit of saying submodule vs superproject from Linus (I think) during the very early days, and there is no such thing as subproject or supermodule in my vocabulary. Just one documentation-reader's opinion, not an edict from the maintainer. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/8] ls-files: support --max-depth
On Fri, Mar 28, 2014 at 9:15 PM, Duy Nguyen pclo...@gmail.com wrote: On Fri, Mar 28, 2014 at 8:52 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Duy Nguyen pclo...@gmail.com writes: I'd rather go with no trailing slash by default and add -F (which seems to be more than just '/') ... and then add a configuration variable to let users enable it by default. For GNU ls, I have alias ls='ls -F --color=auto' in my shell's configuration, but I cannot push the analogy by aliasing git ls because Git doesn't allow aliasing existing commands. I can do that but I want to push for a general solution instead of ls-only. How about config key defaults.cmd, containing a list of arguments, that will be prepended to git-cmd? Only some commands are marked to support this by adding USE_DEFAULTS in the array commands[] in git.c. And git --no-defaults cmd will ignore defaults.cmd (or git -c defaults.cmd= cmd but it's less obvious). GIT_NO_DEFAULTS can also be set, which has the same effect for all commands. Another option is to make git recognize program name gsomething and auto map it to the alias something. For example, the symlink gls will be executed as git ls (and the alias version is preferred over the builtin one). Of course you can't have alias it because git is already taken. It works for many cases, it's faster to type, and it does not break current scripts. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/8] ls-files: support --max-depth
On Fri, Mar 28, 2014 at 8:52 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Duy Nguyen pclo...@gmail.com writes: I'd rather go with no trailing slash by default and add -F (which seems to be more than just '/') ... and then add a configuration variable to let users enable it by default. For GNU ls, I have alias ls='ls -F --color=auto' in my shell's configuration, but I cannot push the analogy by aliasing git ls because Git doesn't allow aliasing existing commands. I can do that but I want to push for a general solution instead of ls-only. How about config key defaults.cmd, containing a list of arguments, that will be prepended to git-cmd? Only some commands are marked to support this by adding USE_DEFAULTS in the array commands[] in git.c. And git --no-defaults cmd will ignore defaults.cmd (or git -c defaults.cmd= cmd but it's less obvious). GIT_NO_DEFAULTS can also be set, which has the same effect for all commands. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 03/12] Move lower case functions into wrapper.c
Jeff King p...@peff.net writes: But I also do not overly care. Literally zero people have complained that [log]date = RFC822 is not accepted, so it is probably not a big deal either way. That is most likely because we do not advertise these enum values spelled in random cases in our documentation and people do not even attempt to upcase the examples they see. So you are right to say that it does not hurt anybody in practice if the code does not strcasecmp() input from them. We do not know if using strcasecmp() there and allowing them to feed the enums spelled in random cases would invite confusions in the longer run, so let's not risk it. There is no upside in using strcasecmp() here. By the way, that is rfc2822---do we want rfc822 as its synonym as well as rfc, I wonder ;-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 14/19] tree-diff: rework diff_tree interface to be sha1 based
Am 28.03.2014 18:06, schrieb Junio C Hamano: Johannes Sixt j.s...@viscovery.net writes: Am 3/27/2014 19:48, schrieb Junio C Hamano: From: Kirill Smelkov k...@mns.spb.ru Date: Mon, 24 Feb 2014 20:21:46 +0400 ... By the way, in general I do not appreciate people lying on the Date: with an in-body header in their patches, either in the original or in rerolls. format-patch is not very cooperative in this aspect. When I prepare a patch series with format-patch, I find myself editing out the Date: line from all patches it produces again and again. :-( I am not sure what you mean. If you are pasting the format-patch output into an editor your MUA is using to receive the body of the message from you, you would remove all the non-body lines, not just Date: but Subject: and From:, no? Correct. So I should add that my gripe is about when I want to send a patch series with git-send-email that was prepared with git-format-patch. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gitweb: gpg signature status indication for commits
Victor Kartashov v.kartas...@npo-echelon.ru writes:
show gpg signature (if any) for commit message in gitweb
in case of valid signature highlight it with green
in case of invalid signature highlight it with red
If that is a single sentence, please write it as such:
Show gpg signature (if any) for commit message in gitweb in case of
valid signature highlight it with green in case of invalid signature
highlight it with red.
But that is almost unparsable ;-)
Teach gitweb to show GPG signature verification status when
showing a commit that is signed. Highlight in green or red to
differentiate valid and invalid signatures.
or something?
Is it a good idea to do this unconditionally in all repositories?
Signed-off-by: Victor Kartashov victor.kartas...@gmail.com
---
gitweb/gitweb.perl | 36 +---
gitweb/static/gitweb.css | 11 +++
2 files changed, 40 insertions(+), 7 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 79057b7..ccde90f 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3430,8 +3430,9 @@ sub parse_commit_text {
my ($commit_text, $withparents) = @_;
my @commit_lines = split '\n', $commit_text;
my %co;
+ my @signature = ();
- pop @commit_lines; # Remove '\0'
+ pop @commit_lines if ($commit_lines[-1] =~ \0); # Remove '\0'
This comment, which only says what it intends to do without saying
why it wants to do so, does not explain why this is a good change.
Does the code even know if $commit_lines[-1] is a non-empty string?
Is it safe to assume if $commit_lines[-1] has a NUL anywhere, it is
always the last line that ends the record, which is not part of the
commit log message?
I am assuming that this $commit_text is read from log -z (or
rev-list -z) output and split at NUL boundary, but if that is the
case, it might be cleaner to remove the trailing NUL from $commit_text
before even attempting to split it into an array at LFs, but that is
an unrelated tangent.
A bigger question is why does the incoming data sometimes ends with
NUL that must be stripped out, and sometimes does not? I see that
you are updating the data producer in the later part of the patch;
wouldn't it be saner if you have the data producer produce the input
to this function in a way that is consistent with the current code,
i.e. always terminate the output with a NUL?
@@ -3469,6 +3470,9 @@ sub parse_commit_text {
$co{'committer_name'} = $co{'committer'};
}
}
+ elsif ($line =~ /^gpg: /) {
I think Eric already pointed out the style issue on this line.
@@ -3508,6 +3512,10 @@ sub parse_commit_text {
foreach my $line (@commit_lines) {
$line =~ s/^//;
}
+ push(@commit_lines, ) if scalar @signature;
+ foreach my $sig (@signature) {
+ push(@commit_lines, $sig);
+ }
Hmm, is it different from doing:
push @commit_lines, @signature;
in some way?
@@ -4571,7 +4579,21 @@ sub git_print_log {
# print log
my $skip_blank_line = 0;
foreach my $line (@$log) {
- if ($line =~ m/^\s*([A-Z][-A-Za-z]*-[Bb]y|C[Cc]): /) {
+ if ($line =~ m/^gpg:(.)+Good(.)+/) {
+ if (! $opts{'-remove_signoff'}) {
Sorry, but I fail to see what the remove-signoff option has to do
with this new feature. The interaction needs to be explained in the
log message.
+ print span class=\good_sign\ .
esc_html($line) . /spanbr/\n;
+ $skip_blank_line = 1;
+ }
+ next;
+ }
+ elsif ($line =~ m/^gpg:(.)+BAD(.)+/) {
+ if (! $opts{'-remove_signoff'}) {
+ print span class=\bad_sign\ .
esc_html($line) . /spanbr/\n;
+ $skip_blank_line = 1;
+ }
+ next;
+ }
+ elsif ($line =~ m/^\s*([A-Z][-A-Za-z]*-[Bb]y|C[Cc]): /) {
if (! $opts{'-remove_signoff'}) {
print span class=\signoff\ .
esc_html($line) . /spanbr/\n;
$skip_blank_line = 1;
diff --git a/gitweb/static/gitweb.css b/gitweb/static/gitweb.css
index 3212601..e99e223 100644
--- a/gitweb/static/gitweb.css
+++ b/gitweb/static/gitweb.css
@@ -136,6 +136,17 @@ span.signoff {
color: #88;
}
+span.good_sign {
+ font-weight: bold;
+ background-color: #aaffaa;
+}
+
+span.bad_sign {
+ font-weight: bold;
+ background-color: #88;
+ color: #ff
+}
+
div.log_link {
padding: 0px 8px;
font-size: 70%;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at
Re: git commit vs. ignore-submodules
Jens Lehmann jens.lehm...@web.de writes: .. but it's less clear if one explicitely stages an updated submodule using git add. Git commit will ignore it anyway, if ignore=all is configured in .gitmodules. Maybe that's correct too That definitely smells like a bug to me. Excluding modified submodules when git add -u is run with ignore=all would be fine and most likely the right thing to do, but once the user actually adds the change to the index, it should not be ignored. ... And also, I'd like to know git folks' opinion on whether it's OK that git commit with ignore=all in .gitmodules ignores submodules even when they are explicitely staged with git add. No, they should be visible in status and commit when the user chose to add them. I see if I can hack something up (as I've been bitten myself by that recently ;-). Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] MSVC: fix t0040-parse-options
Marat Radchenko ma...@slonopotamus.org writes:
Signed-off-by: Marat Radchenko ma...@slonopotamus.org
---
test-parse-options.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/test-parse-options.c b/test-parse-options.c
index 434e8b8..7840493 100644
--- a/test-parse-options.c
+++ b/test-parse-options.c
@@ -11,6 +11,7 @@ static char *string = NULL;
static char *file = NULL;
static int ambiguous;
static struct string_list list;
+static const char *default_string = default;
That wastes 4 or 8 bytes compared to
static const char default_string[] = default;
no?
static int length_callback(const struct option *opt, const char *arg, int
unset)
{
@@ -60,7 +61,7 @@ int main(int argc, char **argv)
OPT_STRING('o', NULL, string, str, get another string),
OPT_NOOP_NOARG(0, obsolete),
OPT_SET_PTR(0, default-string, string,
- set string to default, (unsigned long)default),
+ set string to default, default_string),
OPT_STRING_LIST(0, list, list, str, add str to list),
OPT_GROUP(Magic arguments),
OPT_ARGUMENT(quux, means --quux),
I can see how this patch would not hurt, but at the same time, I
cannot see why this patch is a FIX. A string literal default is
a pointer to constant string, and being able to cast a pointer to
unsigned long is something that is done fairly commonly without
problems [*1*]. It needs to be explained why this change is needed
along the lines of...
We prepare an element in an array of struct option with
OPT_SET_PTR to point a variable to a literal string
default, but MSVC compiler fails to distim the doshes for
such and such reasons.
Work it around by moving the literal string outside the
definition of the struct option, which MSVC can understand
it.
in the log message.
[Footnote]
*1* The cast should actually be intptr_t for it to be kosher. I
also suspect that the cast should happen inside OPT_SET_PTR()
macro defintion, like in the attached patch.
parse-options.h | 2 +-
test-parse-options.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/parse-options.h b/parse-options.h
index d670cb9..7a24d2e 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -129,7 +129,7 @@ struct option {
#define OPT_HIDDEN_BOOL(s, l, v, h) { OPTION_SET_INT, (s), (l), (v), NULL, \
(h), PARSE_OPT_NOARG | PARSE_OPT_HIDDEN,
NULL, 1}
#define OPT_SET_PTR(s, l, v, h, p) { OPTION_SET_PTR, (s), (l), (v), NULL, \
- (h), PARSE_OPT_NOARG, NULL, (p) }
+ (h), PARSE_OPT_NOARG, NULL, (intptr_t)(p)
}
#define OPT_CMDMODE(s, l, v, h, i) { OPTION_CMDMODE, (s), (l), (v), NULL, \
(h), PARSE_OPT_NOARG|PARSE_OPT_NONEG,
NULL, (i) }
#define OPT_INTEGER(s, l, v, h) { OPTION_INTEGER, (s), (l), (v), N_(n),
(h) }
diff --git a/test-parse-options.c b/test-parse-options.c
index 434e8b8..10da63e 100644
--- a/test-parse-options.c
+++ b/test-parse-options.c
@@ -60,7 +60,7 @@ int main(int argc, char **argv)
OPT_STRING('o', NULL, string, str, get another string),
OPT_NOOP_NOARG(0, obsolete),
OPT_SET_PTR(0, default-string, string,
- set string to default, (unsigned long)default),
+ set string to default, default),
OPT_STRING_LIST(0, list, list, str, add str to list),
OPT_GROUP(Magic arguments),
OPT_ARGUMENT(quux, means --quux),
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Borrowing objects from nearby repositories
On Mar 26, 2014, at 1:29 PM, Junio C Hamano gits...@pobox.com wrote: Andrew Keller and...@kellerfarm.com writes: On Mar 25, 2014, at 6:17 PM, Junio C Hamano gits...@pobox.com wrote: ... I think that the standard practice with the existing toolset is to clone with reference and then repack. That is: $ git clone --reference borrowee git://over/there mine $ cd mine $ git repack -a -d And then you can try this: $ mv .git/objects/info/alternates .git/objects/info/alternates.disabled $ git fsck to make sure that you are no longer borrowing anything from the borrowee. Once you are satisfied, you can remove the saved-away alternates.disabled file. Oh, I forgot to say that I am not opposed if somebody wants to teach git clone a new option to copy its objects from two places, (hopefully) the majority from near-by reference repository and the remainder over the network, without permanently relying on the former via the alternates mechanism. The implementation of such a feature could even literally be clone with reference first and then repack at least initially but even in the final version. [Administrivia: please wrap your lines to a reasonable length] That was actually one of my first ideas - adding some sort of '--auto-repack' option to git-clone. It's a relatively small change, and would work. However, keeping in mind my end goal of automating the feature to the point where you could run simply 'git clone url', an '--auto-repack' option is more difficult to undo. You would need a new parameter to disable the automatic adding of reference repositories, and a new parameter to undo '--auto-repack', and you'd have to remember to actually undo both of those settings. In contrast, if the new feature was '--borrow', and the evolution of the feature was a global configuration 'fetch.autoBorrow', then to turn it off temporarily, one only needs a single new parameter '--no-auto-borrow'. I think this is a cleaner approach than the former, although much more work. I think you may have misread me. With the new option, I was hinting that the clone --reference repack rm alternates will be an acceptable internal implementation of the --borrow option that was mentioned in the thread. I am not sure where you got the auto-repack from. Ah, yes - that is better than what I was thinking. I was thinking a bit too low-level, and using two arguments in the place of your one. One of the reasons you may have misread me may be because I made it sound as if this may work and when it works you will be happy, but if it does not work you did not lose very much by mentioning mv fsck. That wasn't what I meant. The repack -a procedure is to make the borrower repository no longer dependent on the borrowee, and it is supposed to always work. In fact, this behaviour was the whole reason why repack later learned its -l option to disable it, because people who cloned with --reference in order to reduce the disk footprint by sharing older and more common objects [*1*] were rightfully surprised to see that the borrowed objects were copied over to their borrower repository when they ran repack [*2*]. Because this is clone, there is nothing complex to undo. Either it succeeds, or you remove the whole new directory if anything fails. I said even in the final version for a simple reason: you cannot cannot do realistically any better than the clone --reference repack -a d rm alternates sequence. Wow, that's very insightful - thanks! So, it sounds like I was right about the general areas of concern when trying to do this during a fetch, but I underestimated just how complicated it would be. Okay, so to re-frame my idea, like you said, the goal is to find a user- friendly way for the user to tell git-clone to set up the alternates file (or perhaps just use the --alternates parameter), and run a repack, and disconnect the alternate. And yet, we still want to be able to use --reference on its own, because there are existing use cases for that. Thanks! - Andrew Keller -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] patch-id: make it stable against hunk reordering
Michael S. Tsirkin m...@redhat.com writes: Patch id changes if you reorder hunks in a diff. Reording files is fine, and as we discussed, having multiple patches that touch the same path is fine, but do not sound as if you are allowing to reorder hunks inside a single patch that touch a single file. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] MSVC: link in invalidcontinue.obj for better POSIX compatibility
Marat Radchenko ma...@slonopotamus.org writes: This patch fixes crashes caused by quitting from PAGER. Can you elaborate a bit more on the underlying cause, summarizing what you learned from this discussion, so that those who read git log output two weeks from now do not have to come back to this thread in the mail archive in order to figure out why we suddenly needs to link with yet another library? Thanks. Signed-off-by: Marat Radchenko ma...@slonopotamus.org --- Please do not cull the Cc list. That was gmane web interface. The correct solution is to link against invalidcontinue.obj in the MSVC build. This is a compiler-provided object file that changes the default behavior to the expected kind, i.e., C runtime functions return EINVAL when appropriate instead of crashing the application. Thanks for a hint. config.mak.uname | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.mak.uname b/config.mak.uname index 38c60af..8e7ec6e 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -366,7 +366,7 @@ ifeq ($(uname_S),Windows) compat/win32/dirent.o COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -Icompat -Icompat/regex -Icompat/win32 -DSTRIP_EXTENSION=\.exe\ BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib - EXTLIBS = user32.lib advapi32.lib shell32.lib wininet.lib ws2_32.lib + EXTLIBS = user32.lib advapi32.lib shell32.lib wininet.lib ws2_32.lib invalidcontinue.obj PTHREAD_LIBS = lib = ifndef DEBUG -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t4212: handle systems with post-apocalyptic gmtime
On Fri, Mar 28, 2014 at 09:41:53AM -0700, Junio C Hamano wrote:
Offhand, the three possible failure modes this thread identified
sounds to me like the only plausible ones, and I think the best way
forward might be to
- teach the is the result sane, even though we may have got a
non-NULL from gmtime? otherwise let's signal a failure by
replacing it with a known sentinel value codepath the new
failure mode Charles's report suggests---if we feed a positive
timestamp and gmtime gave us back a tm_year+1900 0, that is
certainly an overflow; and
I don't think we can analyze the output from gmtime. If it wraps the
year at N, then won't N+2014 look like a valid value?
If we are going to do something trustworthy I think it has to be before
we hand off to gmtime. Like:
diff --git a/date.c b/date.c
index e1a2cee..e0c43c4 100644
--- a/date.c
+++ b/date.c
@@ -57,6 +57,8 @@ static time_t gm_time_t(unsigned long time, int tz)
static struct tm *time_to_tm(unsigned long time, int tz)
{
time_t t = gm_time_t(time, tz);
+ if (t )
+ return NULL;
return gmtime(t);
}
I suspect that would handle the FreeBSD case, as well.
By the way, I have a suspicion that the gm_time_t above can overflow if
you specially craft a value at the edge of what time_t can handle (we
check that our value will not overflow time_t earlier, but now we might
be adding up to 86400 seconds to it). sigh
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 03/12] Move lower case functions into wrapper.c
On Fri, Mar 28, 2014 at 10:12:15AM -0700, Junio C Hamano wrote: By the way, that is rfc2822---do we want rfc822 as its synonym as well as rfc, I wonder ;-) Oops, I wrote that as I was literally looking at the code that said rfc2822 and didn't notice. On the other hand, I have never made the mistake when actually running git, so it is probably not a big deal. Besides which, isn't it 5322 these days? I do not think we need to keep up with the treadmill. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Problems with git 1.8.5.3 on HP-UX 11.11
On Fri, Mar 28, 2014 at 11:09:14AM -, Gerhard Grimm wrote: So I examined the git source package and found that the author of the HP-UX port forgot to set PTHREAD_CFLAGS=-mt in config.mak.autogen to enable threading. You probably want to place such manual settings in config.mak. If you use the ./configure script, it will overwrite config.mak.autogen. git submodule init fails with the output Assertion failed: err == REG_ESPACE, file compat/regex/regexec.c, line 1096 No submodule mapping found in .gitmodules for path 'module' and the stacktrace of the resulting core dump is #0 0xc020ced0 in kill+0x10 () from /usr/lib/libc.2 #1 0xc01a7f84 in raise+0x24 () from /usr/lib/libc.2 #2 0xc01e9308 in abort_C+0x160 () from /usr/lib/libc.2 #3 0xc01e9364 in abort+0x1c () from /usr/lib/libc.2 #4 0xc0176998 in _assert+0x178 () from /usr/lib/libc.2 #5 0x205fa0 in check_matching+0x290 () #6 0x2053b8 in re_search_internal+0x128 () #7 0x204ac0 in regexec+0xc8 () #8 0x4da40 in collect_config+0x60 () #9 0x108b30 in get_value+0xd8 () [...] The regexes we use here are not particularly complicated. So either there is a bug (but nobody else has reported anything on any other platform) or your system regex library has some problem with what we are feeding it. The simplest solution may be to compile with: NO_REGEX=YesPlease which will build and use the glibc implementation in compat/regex instead. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t4212: handle systems with post-apocalyptic gmtime
Jeff King p...@peff.net writes:
On Fri, Mar 28, 2014 at 09:41:53AM -0700, Junio C Hamano wrote:
Offhand, the three possible failure modes this thread identified
sounds to me like the only plausible ones, and I think the best way
forward might be to
- teach the is the result sane, even though we may have got a
non-NULL from gmtime? otherwise let's signal a failure by
replacing it with a known sentinel value codepath the new
failure mode Charles's report suggests---if we feed a positive
timestamp and gmtime gave us back a tm_year+1900 0, that is
certainly an overflow; and
I don't think we can analyze the output from gmtime. If it wraps the
year at N, then won't N+2014 look like a valid value?
Yes, but I was hoping that there are small number of possible N's
;-)
If we are going to do something trustworthy I think it has to be before
we hand off to gmtime. Like:
diff --git a/date.c b/date.c
index e1a2cee..e0c43c4 100644
--- a/date.c
+++ b/date.c
@@ -57,6 +57,8 @@ static time_t gm_time_t(unsigned long time, int tz)
static struct tm *time_to_tm(unsigned long time, int tz)
{
time_t t = gm_time_t(time, tz);
+ if (t )
+ return NULL;
return gmtime(t);
}
I suspect that would handle the FreeBSD case, as well.
By the way, I have a suspicion that the gm_time_t above can overflow if
you specially craft a value at the edge of what time_t can handle (we
check that our value will not overflow time_t earlier, but now we might
be adding up to 86400 seconds to it). sigh
Yuck. Let's not go there.
Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t4212: handle systems with post-apocalyptic gmtime
On Fri, Mar 28, 2014 at 12:02:46PM -0700, Junio C Hamano wrote:
- teach the is the result sane, even though we may have got a
non-NULL from gmtime? otherwise let's signal a failure by
replacing it with a known sentinel value codepath the new
failure mode Charles's report suggests---if we feed a positive
timestamp and gmtime gave us back a tm_year+1900 0, that is
certainly an overflow; and
I don't think we can analyze the output from gmtime. If it wraps the
year at N, then won't N+2014 look like a valid value?
Yes, but I was hoping that there are small number of possible N's
;-)
I'm not sure I understand. Even if we know N, we've lost information
during the truncation done by time_t (we cannot distingiuish true M from
N+M).
diff --git a/date.c b/date.c
index e1a2cee..e0c43c4 100644
--- a/date.c
+++ b/date.c
@@ -57,6 +57,8 @@ static time_t gm_time_t(unsigned long time, int tz)
static struct tm *time_to_tm(unsigned long time, int tz)
{
time_t t = gm_time_t(time, tz);
+ if (t )
+ return NULL;
return gmtime(t);
}
I suspect that would handle the FreeBSD case, as well.
By the way, I have a suspicion that the gm_time_t above can overflow if
you specially craft a value at the edge of what time_t can handle (we
check that our value will not overflow time_t earlier, but now we might
be adding up to 86400 seconds to it). sigh
Yuck. Let's not go there.
Do you mean let's not worry about the absurdly specific overflow case,
or let's not do this gross time_to_tm hack?
This (non-)issue has consumed a lot more brain power than it is probably
worth. I'd like to figure out which patch to go with and be done. :)
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] MSVC: link in invalidcontinue.obj for better POSIX compatibility
Junio C Hamano gits...@pobox.com writes: Marat Radchenko ma...@slonopotamus.org writes: This patch fixes crashes caused by quitting from PAGER. Can you elaborate a bit more on the underlying cause, summarizing what you learned from this discussion, so that those who read git log output two weeks from now do not have to come back to this thread in the mail archive in order to figure out why we suddenly needs to link with yet another library? Thanks. Just to avoid getting misunderstood, I am not asking it to be explained to me in an e-mail. I want to see a patch with its proposed commit log message to explain it to readers of git log. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 14/19] tree-diff: rework diff_tree interface to be sha1 based
Am 28.03.2014 19:36, schrieb Junio C Hamano: Johannes Sixt j...@kdbg.org writes: Am 28.03.2014 18:06, schrieb Junio C Hamano: Johannes Sixt j.s...@viscovery.net writes: Am 3/27/2014 19:48, schrieb Junio C Hamano: From: Kirill Smelkov k...@mns.spb.ru Date: Mon, 24 Feb 2014 20:21:46 +0400 ... By the way, in general I do not appreciate people lying on the Date: with an in-body header in their patches, either in the original or in rerolls. format-patch is not very cooperative in this aspect. When I prepare a patch series with format-patch, I find myself editing out the Date: line from all patches it produces again and again. :-( I am not sure what you mean. If you are pasting the format-patch output into an editor your MUA is using to receive the body of the message from you, you would remove all the non-body lines, not just Date: but Subject: and From:, no? Correct. So I should add that my gripe is about when I want to send a patch series with git-send-email that was prepared with git-format-patch. Hmph. Don't you get fresh timestamps for your messages in such a case, ignoring whatever is at the beginning of the input files? My reading of git-send-email is: * $time = time - scalar $#files prepares the initial timestamp, so that running two git send-email back to back will give timestamps to the series sent out by the first invocation that are older than the ones the second series will get; * sub send_message calls format_2822_time($time++) to send the first message with that initial timestamp, incrementing the timestamps by 1 second intervals (without having to actually wait 1 second in between messages) for each patch. Ah, nice! I didn't know that. I never dared to leave an old author date (or any date) in the patches, and assumed that it would be kept and disrupt the email time line. Thanks for the hint, and sorry for the noise. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gitweb: gpg signature status indication for commits
On 28 March 2014 21:47, Junio C Hamano gits...@pobox.com wrote:
Teach gitweb to show GPG signature verification status when
showing a commit that is signed. Highlight in green or red to
differentiate valid and invalid signatures.
or something?
Yes, kind of :)
Is it a good idea to do this unconditionally in all repositories?
Actually, I don't know. It's a question to discuss. This patch
doesn't affect commits without signature. And if there is a signature,
you robably would like to validate it.
There is an option remove_signoff that, as I thought, would have an
effect on this. But I couldn't find where it's defined.
This comment, which only says what it intends to do without saying
why it wants to do so, does not explain why this is a good change.
Does the code even know if $commit_lines[-1] is a non-empty string?
Is it safe to assume if $commit_lines[-1] has a NUL anywhere, it is
always the last line that ends the record, which is not part of the
commit log message?
I am assuming that this $commit_text is read from log -z (or
rev-list -z) output and split at NUL boundary, but if that is the
case, it might be cleaner to remove the trailing NUL from $commit_text
before even attempting to split it into an array at LFs, but that is
an unrelated tangent.
A bigger question is why does the incoming data sometimes ends with
NUL that must be stripped out, and sometimes does not? I see that
you are updating the data producer in the later part of the patch;
wouldn't it be saner if you have the data producer produce the input
to this function in a way that is consistent with the current code,
i.e. always terminate the output with a NUL?
It seems to be a good idea. I'll try to do that.
@@ -3469,6 +3470,9 @@ sub parse_commit_text {
$co{'committer_name'} = $co{'committer'};
}
}
+ elsif ($line =~ /^gpg: /) {
I think Eric already pointed out the style issue on this line.
@@ -3508,6 +3512,10 @@ sub parse_commit_text {
foreach my $line (@commit_lines) {
$line =~ s/^//;
}
+ push(@commit_lines, ) if scalar @signature;
+ foreach my $sig (@signature) {
+ push(@commit_lines, $sig);
+ }
Hmm, is it different from doing:
push @commit_lines, @signature;
in some way?
@@ -4571,7 +4579,21 @@ sub git_print_log {
# print log
my $skip_blank_line = 0;
foreach my $line (@$log) {
- if ($line =~ m/^\s*([A-Z][-A-Za-z]*-[Bb]y|C[Cc]): /) {
+ if ($line =~ m/^gpg:(.)+Good(.)+/) {
+ if (! $opts{'-remove_signoff'}) {
Sorry, but I fail to see what the remove-signoff option has to do
with this new feature. The interaction needs to be explained in the
log message.
I explained it above. From my point of view, one may want to remove
gpg signature and sign-off inscription together.
Maybe, we should discuss this question, and after that I'll prepare new patch.
+ print span class=\good_sign\ .
esc_html($line) . /spanbr/\n;
+ $skip_blank_line = 1;
+ }
+ next;
+ }
+ elsif ($line =~ m/^gpg:(.)+BAD(.)+/) {
+ if (! $opts{'-remove_signoff'}) {
+ print span class=\bad_sign\ .
esc_html($line) . /spanbr/\n;
+ $skip_blank_line = 1;
+ }
+ next;
+ }
+ elsif ($line =~ m/^\s*([A-Z][-A-Za-z]*-[Bb]y|C[Cc]): /) {
if (! $opts{'-remove_signoff'}) {
print span class=\signoff\ .
esc_html($line) . /spanbr/\n;
$skip_blank_line = 1;
diff --git a/gitweb/static/gitweb.css b/gitweb/static/gitweb.css
index 3212601..e99e223 100644
--- a/gitweb/static/gitweb.css
+++ b/gitweb/static/gitweb.css
@@ -136,6 +136,17 @@ span.signoff {
color: #88;
}
+span.good_sign {
+ font-weight: bold;
+ background-color: #aaffaa;
+}
+
+span.bad_sign {
+ font-weight: bold;
+ background-color: #88;
+ color: #ff
+}
+
div.log_link {
padding: 0px 8px;
font-size: 70%;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] patch-id: make it stable against hunk reordering
Michael S. Tsirkin m...@redhat.com writes:
Patch id changes if you reorder hunks in a diff.
As the result is functionally equivalent, this is surprising to many
people.
In particular, reordering hunks is helpful to make patches
more readable (e.g. API header diff before implementation diff).
In git, it is often done e.g. using the -O orderfile option,
so supporting it better has value.
Hunks within file can be reordered manually provided
the same pathname can appear more than once in the input.
Change patch-id behaviour making it stable against
hunk reodering:
- prepend header to each hunk (if not there)
Note: POSIX requires patch to be robust against hunk reordering
provided each diff hunk has a header:
http://pubs.opengroup.org/onlinepubs/7908799/xcu/patch.html
If the patch file contains more than one patch, patch will
attempt to
apply each of them as if they came from separate patch files.
(In this
case the name of the patch file must be determinable for each
diff
listing.)
- calculate SHA1 hash for each hunk separately
- sum all hashes to get patch id
Add a new flag --unstable to get the historical behaviour.
Add --stable which is a nop, for symmetry.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
Changes from v1: documented motivation for supporting
hunk reordering (and not just file reordering).
No code changes.
Junio, you didn't respond so I'm not sure whether I convinced
you that supporting hunk reordering within file has value.
So I kept this functionality around for now, if
you think I should drop this, please let me know explicitly.
Thanks, and sorry about being dense!
The motivation I read from the exchange was that:
(1) we definitely want to support a mode that is stable with use of
diff -O (i.e. reordering patches per paths);
(2) supporting a patch with swapped hunks does not have any
practical value. When you have a patch to the same file F with
two hunks starting at lines 20 and 40, manually reordering
hunks to create a patch that shows the hunk that starts at line
40 and then the other hunk that starts at line 20 is not a
useful exercise;
(3) but supporting a patch that touches the same path more than
once is in line with what patch and git apply after
7a07841c (git-apply: handle a patch that touches the same path
more than once better, 2008-06-27) do.
In other words, the goal of this change would be to give the same id
for all these three:
(A) straight-forward:
diff --git a/foo.c b/foo.c
--- a/foo.c
+++ b/foo.c
@@ -20,1 +20,1 @@
-foo
+bar
@@ -40,1 +40,1 @@
-frotz
+xyzzy
(B) as two patches concatenated together:
diff --git a/foo.c b/foo.c
--- a/foo.c
+++ b/foo.c
@@ -20,1 +20,1 @@
-foo
+bar
diff --git a/foo.c b/foo.c
--- a/foo.c
+++ b/foo.c
@@ -40,1 +40,1 @@
-frotz
+xyzzy
(C) the same as (B) but with a swapped order:
diff --git a/foo.c b/foo.c
--- a/foo.c
+++ b/foo.c
@@ -40,1 +40,1 @@
-frotz
+xyzzy
diff --git a/foo.c b/foo.c
--- a/foo.c
+++ b/foo.c
@@ -20,1 +20,1 @@
-foo
+bar
Am I reading you correctly?
If that is the case, I think I can buy such a change. It appears to
me that in addition to changing the way the bytes form multiple
hunks are hashed, it would need to hash the file-level headers
together with each hunk when processing input (A) in order to make
the output consistent with the output for the latter two.
Alternatively, you could hash the header for the same path only once
when processing input like (B) or (C) and mix. That would also give
you the same result as processing (A) in a straight-forward way.
builtin/patch-id.c | 71
++
1 file changed, 55 insertions(+), 16 deletions(-)
diff --git a/builtin/patch-id.c b/builtin/patch-id.c
index 3cfe02d..253ad87 100644
--- a/builtin/patch-id.c
+++ b/builtin/patch-id.c
@@ -1,17 +1,14 @@
#include builtin.h
-static void flush_current_id(int patchlen, unsigned char *id, git_SHA_CTX *c)
+static void flush_current_id(int patchlen, unsigned char *id, unsigned char
*result)
{
- unsigned char result[20];
char name[50];
if (!patchlen)
return;
- git_SHA1_Final(result, c);
memcpy(name, sha1_to_hex(id), 41);
printf(%s %s\n, sha1_to_hex(result), name);
- git_SHA1_Init(c);
}
static int remove_space(char *line)
@@ -56,10 +53,30 @@ static int scan_hunk_header(const char *p, int *p_before,
int *p_after)
return 1;
}
-static int get_one_patchid(unsigned char *next_sha1, git_SHA_CTX
Re: [PATCH] t4212: handle systems with post-apocalyptic gmtime
Jeff King p...@peff.net writes: This (non-)issue has consumed a lot more brain power than it is probably worth. I'd like to figure out which patch to go with and be done. :) Let's just deal with a simple known cases (like FreeBSD) in the real code that everybody exercises at runtime, and have the new test only check we do not segfault on a value we used to segfault upon seeing. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 14/19] tree-diff: rework diff_tree interface to be sha1 based
Johannes Sixt j...@kdbg.org writes: My reading of git-send-email is: * $time = time - scalar $#files prepares the initial timestamp, so that running two git send-email back to back will give timestamps to the series sent out by the first invocation that are older than the ones the second series will get; A completely irrelevant tangent, but I was being an idiot here. The -scaler #$files is not about two send-email running back to back. A second invocation that sends out a long series will start its timestamp #$files in the past, that will overlap with the timestamp of the last one in the first invocation. And that is not what the code attempts to address. It wants to merely avoid timestamps from the future. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Problems with git 1.8.5.3 on HP-UX 11.11
On Fri, Mar 28, 2014 at 3:01 PM, Jeff King p...@peff.net wrote: On Fri, Mar 28, 2014 at 11:09:14AM -, Gerhard Grimm wrote: git submodule init fails with the output Assertion failed: err == REG_ESPACE, file compat/regex/regexec.c, line 1096 No submodule mapping found in .gitmodules for path 'module' The regexes we use here are not particularly complicated. So either there is a bug (but nobody else has reported anything on any other platform) or your system regex library has some problem with what we are feeding it. The simplest solution may be to compile with: NO_REGEX=YesPlease which will build and use the glibc implementation in compat/regex instead. Based upon the assertion-failure message, it looks like he's already using compat/regex. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Problems with git 1.8.5.3 on HP-UX 11.11
On Fri, Mar 28, 2014 at 03:43:29PM -0400, Eric Sunshine wrote: On Fri, Mar 28, 2014 at 3:01 PM, Jeff King p...@peff.net wrote: On Fri, Mar 28, 2014 at 11:09:14AM -, Gerhard Grimm wrote: git submodule init fails with the output Assertion failed: err == REG_ESPACE, file compat/regex/regexec.c, line 1096 No submodule mapping found in .gitmodules for path 'module' The regexes we use here are not particularly complicated. So either there is a bug (but nobody else has reported anything on any other platform) or your system regex library has some problem with what we are feeding it. The simplest solution may be to compile with: NO_REGEX=YesPlease which will build and use the glibc implementation in compat/regex instead. Based upon the assertion-failure message, it looks like he's already using compat/regex. Heh, I didn't even notice that. I just looked at all of the libc calls at the top of the backtrace, but of course that is just from assert() on up. So now it seems doubly odd to me, since it is running the same regex library that is used elsewhere. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] MSVC: link in invalidcontinue.obj for better POSIX compatibility
By default, Windows abort()'s instead of setting errno=EINVAL when invalid arguments are passed to standard functions. For example, when PAGER quits and git detects it with errno=EPIPE on write(), check_pipe() in write_or_die.c tries raise(SIGPIPE) but since there is no SIGPIPE on Windows, it is treated as invalid argument, causing abort() and crash report window. Linking in invalidcontinue.obj (provided along with MS compiler) allows raise(SIGPIPE) to return with errno=EINVAL. Signed-off-by: Marat Radchenko ma...@slonopotamus.org --- config.mak.uname | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.mak.uname b/config.mak.uname index 38c60af..8e7ec6e 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -366,7 +366,7 @@ ifeq ($(uname_S),Windows) compat/win32/dirent.o COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -Icompat -Icompat/regex -Icompat/win32 -DSTRIP_EXTENSION=\.exe\ BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib - EXTLIBS = user32.lib advapi32.lib shell32.lib wininet.lib ws2_32.lib + EXTLIBS = user32.lib advapi32.lib shell32.lib wininet.lib ws2_32.lib invalidcontinue.obj PTHREAD_LIBS = lib = ifndef DEBUG -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] MSVC: link in invalidcontinue.obj for better POSIX compatibility
Marat Radchenko ma...@slonopotamus.org writes: By default, Windows abort()'s instead of setting errno=EINVAL when invalid arguments are passed to standard functions. For example, when PAGER quits and git detects it with errno=EPIPE on write(), check_pipe() in write_or_die.c tries raise(SIGPIPE) but since there is no SIGPIPE on Windows, it is treated as invalid argument, causing abort() and crash report window. Linking in invalidcontinue.obj (provided along with MS compiler) allows raise(SIGPIPE) to return with errno=EINVAL. Signed-off-by: Marat Radchenko ma...@slonopotamus.org --- Thanks; will queue. config.mak.uname | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.mak.uname b/config.mak.uname index 38c60af..8e7ec6e 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -366,7 +366,7 @@ ifeq ($(uname_S),Windows) compat/win32/dirent.o COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -Icompat -Icompat/regex -Icompat/win32 -DSTRIP_EXTENSION=\.exe\ BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib - EXTLIBS = user32.lib advapi32.lib shell32.lib wininet.lib ws2_32.lib + EXTLIBS = user32.lib advapi32.lib shell32.lib wininet.lib ws2_32.lib invalidcontinue.obj PTHREAD_LIBS = lib = ifndef DEBUG -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] patch-id: make it stable against hunk reordering
Michael S. Tsirkin m...@redhat.com writes:
+static void flush_one_hunk(unsigned char *result, git_SHA_CTX *ctx)
{
- int patchlen = 0, found_next = 0;
+ unsigned char hash[20];
+ unsigned short carry = 0;
+ int i;
+
+ git_SHA1_Final(hash, ctx);
+ git_SHA1_Init(ctx);
+ /* 20-byte sum, with carry */
+ for (i = 0; i 20; ++i) {
+ carry += result[i] + hash[i];
+ result[i] = carry;
+ carry = 8;
+ }
Was there a reason why bitwise xor is not sufficient for mixing
these two indenendent hashes? If the 20-byte sums do not offer
benefit over that, the code for bitwise xor would be certainly be
simpler, I would imagine?
+}
+static int get_one_patchid(unsigned char *next_sha1, unsigned char *result,
+struct strbuf *line_buf, int stable)
+{
+ int patchlen = 0, found_next = 0, hunks = 0;
int before = -1, after = -1;
+ git_SHA_CTX ctx, header_ctx;
+
+ git_SHA1_Init(ctx);
+ hashclr(result);
while (strbuf_getwholeline(line_buf, stdin, '\n') != EOF) {
char *line = line_buf-buf;
@@ -99,6 +116,18 @@ static int get_one_patchid(unsigned char *next_sha1,
git_SHA_CTX *ctx, struct st
if (!memcmp(line, @@ -, 4)) {
/* Parse next hunk, but ignore line numbers. */
scan_hunk_header(line, before, after);
+ if (stable) {
+ if (hunks) {
+ flush_one_hunk(result, ctx);
+ memcpy(ctx, header_ctx,
+sizeof ctx);
+ } else {
+ /* Save ctx for next hunk. */
+ memcpy(header_ctx, ctx,
+sizeof ctx);
+ }
+ }
+ hunks++;
continue;
}
@@ -107,7 +136,10 @@ static int get_one_patchid(unsigned char *next_sha1,
git_SHA_CTX *ctx, struct st
break;
/* Else we're parsing another header. */
+ if (stable hunks)
+ flush_one_hunk(result, ctx);
before = after = -1;
+ hunks = 0;
}
/* If we get here, we're inside a hunk. */
@@ -119,39 +151,46 @@ static int get_one_patchid(unsigned char *next_sha1,
git_SHA_CTX *ctx, struct st
/* Compute the sha without whitespace */
len = remove_space(line);
patchlen += len;
- git_SHA1_Update(ctx, line, len);
+ git_SHA1_Update(ctx, line, len);
}
if (!found_next)
hashclr(next_sha1);
+ flush_one_hunk(result, ctx);
What I read from these changes is that you do not do anything
special about the per-file header, so two no overlapping patches
with a single hunk each that touches the same path concatenated
together would not result in the same patch-id as a single-patch
that has the same two hunks. Which would break your earlier 'Yes,
reordering only the hunks will not make sense, but before each hunk
you could insert the same diff --git a/... b/... to make them a
concatenation of patches that touch the same file', I would think.
Is that what we want to happen? Or is my reading mistaken?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
What's cooking in git.git (Mar 2014, #07; Fri, 28)
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.
More topics merged to 'master', many of which are fallouts from GSoC
microprojects.
You can find the changes described here in the integration branches
of the repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--
[Graduated to master]
* ah/doc-gitk-config (2014-03-20) 1 commit
(merged to 'next' on 2014-03-20 at d671b60)
+ Documentation/gitk: document the location of the configulation file
* bg/rebase-off-of-previous-branch (2014-03-19) 1 commit
(merged to 'next' on 2014-03-21 at 916b759)
+ rebase: allow - short-hand for the previous branch
git rebase learned to interpret a lone - as @{-1}, the
branch that we were previously on.
* bp/commit-p-editor (2014-03-18) 7 commits
(merged to 'next' on 2014-03-21 at 23b6b06)
+ run-command: mark run_hook_with_custom_index as deprecated
+ merge hook tests: fix and update tests
+ merge: fix GIT_EDITOR override for commit hook
+ commit: fix patch hunk editing with commit -p -m
+ test patch hunk editing with commit -p -m
+ merge hook tests: use 'test_must_fail' instead of '!'
+ merge hook tests: fix missing '' in test
When it is not necessary to edit a commit log message (e.g. git
commit -m is given a message without specifying -e), we used to
disable the spawning of the editor by overriding GIT_EDITOR, but
this means all the uses of the editor, other than to edit the
commit log message, are also affected.
* fr/add-interactive-argv-array (2014-03-18) 1 commit
(merged to 'next' on 2014-03-20 at 9d65f3d)
+ add: use struct argv_array in run_add_interactive()
* jk/pack-bitmap (2014-03-17) 1 commit
(merged to 'next' on 2014-03-20 at bba6246)
+ pack-objects: turn off bitmaps when skipping objects
Instead of dying when asked to (re)pack with the reachability
bitmap when a bitmap cannot be built, just (re)pack without
producing a bitmap in such a case, with a warning.
* jk/pack-bitmap-progress (2014-03-17) 2 commits
(merged to 'next' on 2014-03-20 at c7a83f9)
+ pack-objects: show reused packfile objects in Counting objects
+ pack-objects: show progress for reused packfiles
The progress output while repacking and transferring objects showed
an apparent large silence while writing the objects out of existing
packfiles, when the reachability bitmap was in use.
* jk/subtree-prefix (2014-03-17) 1 commit
(merged to 'next' on 2014-03-20 at 81367fa)
+ subtree: initialize prefix variable
A stray environment variable $prefix could have leaked into and
affected the behaviour of the subtree script.
* ys/fsck-commit-parsing (2014-03-19) 2 commits
(merged to 'next' on 2014-03-21 at 2728983)
+ fsck.c:fsck_commit(): use skip_prefix() to verify and skip constant
+ fsck.c:fsck_ident(): ident points at a const string
--
[New Topics]
* jc/apply-ignore-whitespace (2014-03-26) 1 commit
- apply --ignore-space-change: lines with and without leading whitespaces do
not match
An RFC. --ignore-space-change option of git apply ignored the
spaces at the beginning of line too aggressively, which is
inconsistent with the option of the same name diff and git diff
have.
Will hold.
* jc/rev-parse-argh-dashed-multi-words (2014-03-24) 3 commits
- parse-options: make sure argh string does not have SP or _
- update-index: teach --cacheinfo a new syntax mode,sha1,path
- parse-options: multi-word argh should use dash to separate words
(this branch uses ib/rev-parse-parseopt-argh.)
Make sure that the help text given to describe the param part
of the git cmd --option=param does not contain SP or _,
e.g. --gpg-sign=key-id option for git commit is not spelled
as --gpg-sign=key id.
Will merge to 'next'.
* jk/commit-dates-parsing-fix (2014-03-26) 1 commit
- t4212: loosen far-in-future test for AIX
I think we agreed that a simpler test would be a better way
forward.
* mr/msvc-link-with-invalidcontinue (2014-03-28) 1 commit
- MSVC: link in invalidcontinue.obj for better POSIX compatibility
Will merge to 'next'.
* mr/msvc-link-with-lcurl (2014-03-27) 1 commit
(merged to 'next' on 2014-03-28 at 3281dab)
+ MSVC: allow linking with the cURL library
Will merge to 'master'.
* wt/doc-submodule-name-path-confusion-1 (2014-03-27) 1 commit
(merged to 'next' on 2014-03-28 at 225f241)
+ doc: submodule.* config are keyed by submodule names
Will merge to 'master'.
* wt/doc-submodule-name-path-confusion-2 (2014-03-27) 1 commit
(merged to 'next' on 2014-03-28 at ec5bcf3)
+ doc: submodule.*.branch config is keyed by name
Will merge to 'master'.
* ep/shell-command-substitution (2014-03-25) 2 commits
(merged to 'next' on 2014-03-28 at 99a512a)
+ git-am.sh: use the $(...) construct for command substitution
+ check-builtins.sh: use
Re: [PATCH v2 1/3] patch-id: make it stable against hunk reordering
Junio C Hamano gits...@pobox.com writes:
@@ -99,6 +116,18 @@ static int get_one_patchid(unsigned char *next_sha1,
git_SHA_CTX *ctx, struct st
if (!memcmp(line, @@ -, 4)) {
/* Parse next hunk, but ignore line numbers. */
scan_hunk_header(line, before, after);
+if (stable) {
+if (hunks) {
+flush_one_hunk(result, ctx);
+memcpy(ctx, header_ctx,
+ sizeof ctx);
+} else {
+/* Save ctx for next hunk. */
+memcpy(header_ctx, ctx,
+ sizeof ctx);
+}
+}
+hunks++;
continue;
}
@@ -107,7 +136,10 @@ static int get_one_patchid(unsigned char *next_sha1,
git_SHA_CTX *ctx, struct st
break;
/* Else we're parsing another header. */
+if (stable hunks)
+flush_one_hunk(result, ctx);
before = after = -1;
+hunks = 0;
}
/* If we get here, we're inside a hunk. */
@@ -119,39 +151,46 @@ static int get_one_patchid(unsigned char *next_sha1,
git_SHA_CTX *ctx, struct st
/* Compute the sha without whitespace */
len = remove_space(line);
patchlen += len;
-git_SHA1_Update(ctx, line, len);
+git_SHA1_Update(ctx, line, len);
}
if (!found_next)
hashclr(next_sha1);
+flush_one_hunk(result, ctx);
What I read from these changes is that you do not do anything
special about the per-file header, so two no overlapping patches
with a single hunk each that touches the same path concatenated
together would not result in the same patch-id as a single-patch
that has the same two hunks. Which would break your earlier 'Yes,
reordering only the hunks will not make sense, but before each hunk
you could insert the same diff --git a/... b/... to make them a
concatenation of patches that touch the same file', I would think.
Is that what we want to happen? Or is my reading mistaken?
Heh, I was blind---copying into header_ctx and then restoring that
in preparation for the next round is exactly for duplicating the
per-file header sum to each and every hunk, so you are already doing
that.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Mar 2014, #07; Fri, 28)
On 03/28/2014 11:21 PM, Junio C Hamano wrote: Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. Junio, Have you overlooked my ref-transactions series [1], or just not gotten to it yet? If you would like a version of the series that already addresses Brad King's comments, you can get it from my GitHub fork [2], the ref-transactions branch. I'd be happy to post a v3 to the list if you prefer, but the only changes since v2 were to a commit message and a comment so it seems like overkill. Michael [1] http://thread.gmane.org/gmane.comp.version-control.git/244857 [2] https://github.com/mhagger/git -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] Fix misuses of nor in comments
I feel like I'm splitting hairs, but I think there's a change in meaning if you use that phrasing. The difference being not expecting vs. should not. I don't know which is correct, so I'll defer that to someone else. Okay, changed to + * This shouldn't be be set by the Makefile or by the user (e.g. via + * CFLAGS). My intent is not to get hung up on any of these points, so I'm also happy to punt on this or any hunk. I'll send out new versions of the patches which apply to maint, master, next, and pu in a bit. -Justin On Sat, Mar 22, 2014 at 4:47 PM, Jason St. John jstj...@purdue.edu wrote: On Thu, Mar 20, 2014 at 7:13 PM, Justin Lebar jle...@google.com wrote: Thanks for the quick reply. When I send a new patch, should I fold these changes into the original commit, or should I send them as a separate commit? diff --git a/builtin/apply.c b/builtin/apply.c index b0d0986..6013e19 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -4061,7 +4061,7 @@ static int write_out_one_reject(struct patch *patch) return error(_(cannot open %s: %s), namebuf, strerror(errno)); /* Normal git tools never deal with .rej, so do not pretend -* this is a git patch by saying --git nor give extended +* this is a git patch by saying --git or giving extended * headers. While at it, maybe please kompare that wants * the trailing TAB and some garbage at the end of line ;-). */ I don't think the change from give to giving here is grammatically correct. Is it? I might be misunderstanding the sentence, then. I parse the new sentence as Do not pretend this is a git patch by - saying --git, or - giving extended headers. Giving is definitely awkward, but I'm not sure of a better word. I'm happy to rephrase this, but I'm not sure how. I don't think the original makes much sense, but I'm also happy to leave it. You're right; that makes sense. Disregard my comment about that chunk. How about ``If none of always, never, or auto is specified, then setting layout implies always.``? Sure. To leave nor here, I think you need to replace not with neither. I think it actually works after the change, but unfortunately Garner's doesn't give me a lot of ammunition to back up that feeling. :) How about We don't expect this to be set by the Makefile or by the user (via CFLAGS). I feel like I'm splitting hairs, but I think there's a change in meaning if you use that phrasing. The difference being not expecting vs. should not. I don't know which is correct, so I'll defer that to someone else. This would be better worded as If src_buffer and *src_buffer are not NULL, it should ... Done. -Justin Jason -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
