Re: [PATCH v8 03/12] Move lower case functions into wrapper.c
From: Junio C Hamano gits...@pobox.com
Christian Couder chrisc...@tuxfamily.org writes:
diff --git a/wrapper.c b/wrapper.c
index 0cc5636..c46026a 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -455,3 +455,17 @@ struct passwd *xgetpwuid_self(void)
errno ? strerror(errno) : _(no such user));
return pw;
}
+
+void lowercase(char *p)
+{
+for (; *p; p++)
+*p = tolower(*p);
+}
+
+char *xstrdup_tolower(const char *str)
+{
+char *dup = xstrdup(str);
+lowercase(dup);
+return dup;
+}
+
As a pure code-movement step, this may be OK, but I am not sure if
both of them want to be public functions in this shape.
Perhaps
char *downcase_copy(const char *str)
{
char *copy = xmalloc(strlen(str) + 1);
int i;
for (i = 0; str[i]; i++)
copy[i] = tolower(str[i]);
copy[i] = '\0';
return copy;
}
may avoid having to copy things twice.
Yeah, but it seems a bit wasteful to allocate memory for a new string,
then downcase it, then compare it with strcmp() and then free it,
instead of just using strcasecmp() on the original string.
Do you need the other
function exposed?
No, with the change you suggest, I don't.
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 v8 00/12] Add interpret-trailers builtin
From: Junio C Hamano gits...@pobox.com Christian Couder chrisc...@tuxfamily.org writes: Until now git commit has only supported the well known Signed-off-by: trailer, that is used by many projects like the Linux kernel and Git. It is better to implement features for these trailers first in a new command rather than in builtin/commit.c, because this way the prepare-commit-msg and commit-msg hooks can reuse this command. The first is somewhat questionable. It is better to keep builtin/commit.c uncontaminated by any more hard-wired logic, like what we have for the signed-off-by line. Any new things can and should be doable in hooks, and this filter would help writing these hooks. And that is why the design goal of the filter is to make it at least as powerful as the built-in logic we have for signed-off-by lines; that would allow us to later eject the hard-wired logic for signed-off-by line from the main codepath, if/when we wanted to. Alternatively, we could build a library-ish API around this filter code and replace the hard-wired logic for signed-off-by line with a call into that API, if/when we wanted to, but that requires (in addition to the at least as powerful as the built-in logic) that the implementation of this stand-alone filter can be cleanly made into a reusable library, so that is a bit higher bar to cross than everything can be doable with hooks alternative. Ok, I will try to improve this part of the Rationale section. 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 v8 01/12] Add data structures and basic functions for commit trailers
From: Junio C Hamano gits...@pobox.com Subject: Re: [PATCH v8 01/12] Add data structures and basic functions for commit trailers Date: Wed, 26 Mar 2014 16:06:35 -0700 Christian Couder chrisc...@tuxfamily.org writes: Subject: Re: [PATCH v8 01/12] Add data structures and basic functions for commit trailers As pointed out many times for GSoC microprojects students, limit the scope with area: prefix for the commit title, e.g. Subject: trailers: add data structures and basic functions Ok, I will fix that. Please also refer to what has already been queued on 'pu' to avoid wasting review bandwidth and mark patches that are unchanged as such (but do send them to the list for review, so that people who haven't seen the previous round can also comment). Yeah, I forgot to do that for this version of the series, sorry. As far as I can tell, this is the same as 8d1c70e5 (trailers: add data structures and basic functions, 2014-03-06), so I'll queue the remainder on top of that commit already on 'pu', which incidentally will preserve the original author timestamp from the previous incarnation. Ok. 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
[PATCH] MSVC: allow enabling CURL
Signed-off-by: Marat Radchenko ma...@slonopotamus.org
---
compat/vcbuild/scripts/clink.pl | 2 ++
config.mak.uname| 1 -
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/compat/vcbuild/scripts/clink.pl b/compat/vcbuild/scripts/clink.pl
index 4374771..a87d0da 100755
--- a/compat/vcbuild/scripts/clink.pl
+++ b/compat/vcbuild/scripts/clink.pl
@@ -33,6 +33,8 @@ while (@ARGV) {
push(@args, libeay32.lib);
} elsif ($arg eq -lssl) {
push(@args, ssleay32.lib);
+ } elsif ($arg eq -lcurl) {
+ push(@args, libcurl.lib);
} elsif ($arg =~ /^-L/ $arg ne -LTCG) {
$arg =~ s/^-L/-LIBPATH:/;
push(@args, $arg);
diff --git a/config.mak.uname b/config.mak.uname
index 6069a44..cfc2a93 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -340,7 +340,6 @@ ifeq ($(uname_S),Windows)
UNRELIABLE_FSTAT = UnfortunatelyYes
OBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo
NO_REGEX = YesPlease
- NO_CURL = YesPlease
NO_GETTEXT = YesPlease
NO_PYTHON = YesPlease
BLK_SHA1 = YesPlease
--
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
[PATCH] MSVC: added missing include so `make INLINE=__inline` is no longer required
Signed-off-by: Marat Radchenko ma...@slonopotamus.org --- xdiff/xutils.c | 1 + 1 file changed, 1 insertion(+) diff --git a/xdiff/xutils.c b/xdiff/xutils.c index 62cb23d..a21a835 100644 --- a/xdiff/xutils.c +++ b/xdiff/xutils.c @@ -23,6 +23,7 @@ #include limits.h #include assert.h #include xinclude.h +#include git-compat-util.h -- 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
[PATCH 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
---
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..75f77ef 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 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 --- 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 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).
Change patch-id behaviour making it stable against
hunk reodering:
- prepend header to each hunk (if not there)
- 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
---
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);
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);
+
return patchlen;
}
-static void generate_id_list(void)
+static void generate_id_list(int stable)
{
- unsigned char sha1[20], n[20];
- git_SHA_CTX ctx;
+ unsigned char sha1[20], n[20], result[20];
int patchlen;
struct strbuf line_buf = STRBUF_INIT;
- git_SHA1_Init(ctx);
hashclr(sha1);
while (!feof(stdin)) {
- patchlen = get_one_patchid(n, ctx, line_buf);
- flush_current_id(patchlen,
Re: [PATCH 1/3] test-lib: Document short options in t/README
On 3/25/2014 10:23 AM, Junio C Hamano wrote:
Ilya Bobyr ilya.bo...@gmail.com writes:
On 3/24/2014 4:39 AM, Ramsay Jones wrote:
On 24/03/14 08:49, Ilya Bobyr wrote:
[...]
[...]
---valgrind=tool::
+-v,--valgrind=tool::
The -v short option is taken, above ... :-P
Right %)
Thanks :)
This one starts only with --va, will fix it.
Please don't.
In general, when option names can be shortened by taking a unique
prefix, it is better not to give short form in the documentation at
all. There is no guarantee that the short form you happen to pick
when you document it will continue to be unique forever. When we
add another --vasomething option, --va will become ambiguous and one
of these two things must happen:
(1) --valgrind and --vasomething are equally useful and often used.
Neither will get --va and either --val or --vas needs to be
given.
(2) Because we documented --va as --valgrind, people feel that they
are entitled to expect --va will stay forever to be a shorthand
for --valgrind and nothing else. The shortened forms will be
between --va (or longer prefix of --valgrind) and --vas (or
longer prefix of --vasomething).
We would rather want to see (1), as people new to the system do not
have to learn that --valgrind can be spelled --va merely by being
the first to appear, and --vasomething must be spelled --vas because
it happened to come later. Longer term, nobody should care how the
system evolved into the current shape, but (2) will require that to
understand and remember why one is --va and the other has to be --vas.
We already have this suboptimal (2) situation between --valgrind
and --verbose options, but a shorter form v that is used for
verbose is so widely understood and used that I think it is an
acceptable exception. So
--verbose::
+-v::
Give verbose output from the test
is OK, but --valgrind can be shortened to --va is not.
Sure, this is exactly what I meant, but I guess, I was too short
so it created ambiguity =)
I was going to just remove the '-v' from '--valgrind'.
Shortening is a separate issue. I did not look at it.
I can see that it is also not documented. At the same time
shortening is entirely consistent at the moment, and does not
work for options that take arguments.
My main intent was to document '-r' :)
As no other short form were documented, I had to fix that issue
first.
If there is decision on how shortening should work for all the
options, maybe I could add a paragraph on that and make existing
options more consistent.
I guess the questions would be, should it possible to use short
forms for options that take arguments?
If so, '--valgrind' becomes impossible to shorten because there
is '--valgrind-only' that is a separate option. Same for
'--verbose' and '--verbose-only'.
For convenience here is the relevant switch in the way it is
right now:
case $1 in
-d|--d|--de|--deb|--debu|--debug)
debug=t; shift ;;
-i|--i|--im|--imm|--imme|--immed|--immedi|--immedia|--immediat|--immediate)
immediate=t; shift ;;
-l|--l|--lo|--lon|--long|--long-|--long-t|--long-te|--long-tes|--long-test|--long-tests)
GIT_TEST_LONG=t; export GIT_TEST_LONG; shift ;;
-r)
shift; test $# -ne 0 || {
echo 'error: -r requires an argument' 2;
exit 1;
}
run_list=$1; shift ;;
--run=*)
run_list=$(expr z$1 : 'z[^=]*=\(.*\)'); shift ;;
-h|--h|--he|--hel|--help)
help=t; shift ;;
-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
verbose=t; shift ;;
--verbose-only=*)
verbose_only=$(expr z$1 : 'z[^=]*=\(.*\)')
shift ;;
-q|--q|--qu|--qui|--quie|--quiet)
# Ignore --quiet under a TAP::Harness. Saying how many tests
# passed without the ok/not ok details is always an error.
test -z $HARNESS_ACTIVE quiet=t; shift ;;
--with-dashes)
with_dashes=t; shift ;;
--no-color)
color=; shift ;;
--va|--val|--valg|--valgr|--valgri|--valgrin|--valgrind)
valgrind=memcheck
shift ;;
--valgrind=*)
valgrind=$(expr z$1 : 'z[^=]*=\(.*\)')
shift ;;
--valgrind-only=*)
valgrind_only=$(expr z$1 : 'z[^=]*=\(.*\)')
shift ;;
--tee)
shift ;; # was handled already
--root=*)
root=$(expr z$1 : 'z[^=]*=\(.*\)')
shift ;;
*)
echo error: unknown test option '$1' 2; exit 1 ;;
esac
P.S. Sorry it takes me this long to reply. I will try to be
more responsive, should there will be a discussion :)
--
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/PATCH] Better control of the tests run by a test suite
On 3/24/2014 9:58 PM, Junio C Hamano wrote:
Jeff King p...@peff.net writes:
On Mon, Mar 24, 2014 at 01:49:44AM -0700, Ilya Bobyr wrote:
Here are some examples of how functionality added by the patch
could be used. In order to run setup tests and then only a
specific test (use case 1) one can do:
$ ./t-init.sh --run='1 2 25'
or:
$ ./t-init.sh --run='3 25'
('=' is also supported, as well as '' and '=').
I don't have anything against this in principle, but I suspect it will
end up being a big pain to figure out which of the early tests are
required to set up the state, and which are not. Having makes
specifying it easier, but you still have to read the test script to
figure out which tests need to be run.
Likewise.
The idea is that you will use that option when you know what
setup the test need. And the case that I was targeting is when
you are the author of the test, because you are also writing the
relevant functionality or you are really familiar with the test
because you are, again, working on something in that area.
It does not mean you actually have to do it. It is just a
possibility.
And as you mentioned, helps in another case - when you do not
know enough about the test, but want to run it. For example when
you are just starting with a failed test.
My experience, thought quite limited, is that it is very simple
to understand what the test needs and where it is prepared, if
you are actually adding new test to a test suite. Or if you
spent some time figuring how specific test works. I think this
is mostly because all the tests are rather simple. Which is
definitely a good thing.
This is not for cases when you treat test suites as black boxes.
For example, when you are just checking someone else code.
I wonder if it would make sense to auto-select tests that match a
regex like set.?up|create? A while ago, Jonathan made a claim that
this would cover most tests that are dependencies of other tests. I did
not believe him, but looking into it, I recall that we did seem to have
quite a few matching that pattern. If there were a good feature like
this that gave us a reason to follow that pattern, I think people might
fix the remainder
This may be worth experimenting with, I would think.
I was also thinking about it a bit. I do not have that much
knowledge on how all the tests are organized. But I did see some
cases where this rule would fail.
One example would be t\t-basic.sh. It could probably be
considered a very special test suite, but if you skip one of
these tests:
- test runs if prerequisite is satisfied
- unmet prerequisite causes test to be skipped
the test suite would just exit in the middle. There is a number
of other tests that you do not want to skip for the same reason.
Also, in the same test suite showing tree with git ls-tree -r
is a setup test for the next one git ls-tree -r output for a
known tree. And the same pattern is repeated for some other
tests.
I've also looked at t5601-clone.sh. There is indeed a test
called setup at the very beginning. But somewhere in the
middle there is a test called clone from .git file that creates
a folder used in two subsequent tests.
In t0001-init.sh, re-init on .git file creates a folder that
is used in the next test called re-init to update git link.
Maybe these are just some outliers, I do not know for sure.
These were the only test suites I've looked at so far.
I think that if there is a desire to support automatic setup for
tests maybe a rule could be introduced that a test must succeed,
if there is no breakage, if all the tests that match regex
'^(setup|cleanup)\' before it have been run. It should not be
too complicated to create a target that would automate checking
of this rule.
I am not 100% sure that this kind of change is worth the trouble.
People who run individual tests should probably know why they are
doing it. And as such that might know the prerequisites.
Otherwise I can not come up with a reason to run an individual
test.
On the other hand, the rule may add a bit more structure to the
tests and automated checking could enforce 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
[RFC/PATCH v2] Better control of the tests run by a test suite
This is an update verson of the patches I've posted here:
[RFC/PATCH] Better control of the tests run by a test suite
http://www.mail-archive.com/git@vger.kernel.org/msg46419.html
Chanes are only in the first patch, according to
http://www.mail-archive.com/git@vger.kernel.org/msg46423.html
Ramsay Jones
and
http://www.mail-archive.com/git@vger.kernel.org/msg46512.html
Eric Sunshine
The description below is identical to the previous one, but here
it is for convenience if someone would want to quote it in a
comment.
---
Hello,
This is a second attempt on a functionality I proposed in
[PATCH 2/2] test-lib: GIT_TEST_ONLY to run only specific tests
http://www.mail-archive.com/git%40vger.kernel.org/msg44828.html
except that the implementation is quite different now.
I hope that I have accounted for the comments that were voiced so
far. Let's see
The idea behind the change is that sometimes it is convenient to
run only certain tests from a test suite. Specifically I am
thinking about the following two use cases:
1. You are developing new functionality. You add a test that
fails and then you add and modify code to make it pass.
2. You have a failed test and you need to understand what is
wrong.
In the first case you when you run the test suite, you probably
want to run some setup tests and then only one test that you are
focused on.
For code written in C time between you make a change and see a
test result is considerably increased by the compilation. But
for script code turn around time is mostly due to the run time of
the test suite itself. [1]
For the second case you actually want the test suite to stop
after the failing test, so that you can examine the trash
directory without any modifications from the subsequent tests.
You probably do not care about them.
In the previous patch I've added an environment variable to
control tests to be run in a test suite. I thought that it would
be similar to an already existing GIT_SKIP_TESTS. As I did not
provide a cover letter that caused some misunderstanding I think.
This patch adds new command line argument '--run' that accepts a
list of patterns and restrictions on the test numbers that would
be included or excluded from this run of the test suite.
During discussion of the previous patch there were some talks
about extending GIT_SKIP_TESTS syntax. In particular here:
That is
GIT_SKIP_TESTS='t9??? !t91??'
would skip nine-thousand series, but would run 91xx series, and all
the others are not excluded.
Simple rules to consider:
- If the list consists of _only_ negated patterns, pretend that
there is unless otherwise specified with negatives, skip all
tests, i.e. treat GIT_SKIP_TESTS='!t91??' just the same way you
would treat GIT_SKIP_TESTS='* !t91??'.
- The orders should not matter for simplicity of the semantics;
before running each test, check if it matches any negative (and
run it if it matches, without looking at any positives), and
otherwise check if it matches any positive (and skip it if it
does not).
Hmm?
http://www.mail-archive.com/git%40vger.kernel.org/msg44922.html
I've used that as a basis, but the end result is somewhat
different. Plus I've added boundary checks as in 123.
Here are some examples of how functionality added by the patch
could be used. In order to run setup tests and then only a
specific test (use case 1) one can do:
$ ./t-init.sh --run='1 2 25'
or:
$ ./t-init.sh --run='3 25'
('=' is also supported, as well as '' and '=').
In order to run up to a specific test (use case 2) one can do:
$ ./t-init.sh --run='=34'
or:
$ ./t-init.sh --run='!34'
Simple semantics described above is easy to implement, but at the
same time is probably not that convenient. Rules implemented by
the patch are as follows:
- Order does matter. Whatever is specified on the right has
higher precedence.
- When the first pattern is positive the initial set of the
tests to be run is empty. You are adding to an empty set as
in '1 2 25'.
When the first pattern is negative the initial set of the
tests to run contains all the tests. You are subtracting
from that set as in '!34'.
It seems that for simple cases that gives simple syntax and is
almost unbiased if you think about preferring inclusion over
exclusion. While it is unlikely it also allows for complicated
expressions. And the implementation is quite simple as well.
Main functionality is in the third patch. First two are just
minor fixes in related parts of the code.
P.S. I did not reply to the previous patch thread as this one is
quite different.
[1] Here are some times I see on Cygin:
$ touch builtin/rev-parse.c
$ time make
...
real0m10.382s
user0m3.829s
sys 0m5.269s
Running the t-init.sh test suite is like this:
$ time ./t0001-init.sh
[...]
1..36
[PATCH 3/3] test-lib: '--run' to run only specific tests
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::
+ This causes only specific tests to be included or excluded. See
+ 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
+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.
+
+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.
+
+If --run starts with '+' or unprefixed pattern the initial set of
+tests to run is empty. If the first pattern starts with '-' or
+'!' all the tests are added to the initial set. After initial
+set is determined every pattern, test number or range is added or
+excluded from the set one by one, from left to right.
+
+For example, common case is to run several setup tests and then a
+specific test that relies on that setup:
+
+$ sh ./t9200-git-cvsexport-commit.sh --run='1 2 3 21'
+
+or:
+
+$ sh ./t9200-git-cvsexport-commit.sh --run='4 21'
+
+To run only tests up to a specific test one could do this:
+
+$ sh ./t9200-git-cvsexport-commit.sh --run='!=21'
+
+As noted above test set is build going though patterns left to
+right, so this:
+
+$ sh ./t9200-git-cvsexport-commit.sh --run='5 !3'
+
+will run tests 1, 2, and 4.
+
+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.
+--run is mostly useful when you want to focus on a specific test
+and know what you are doing. Or when you want to run up to a
+certain test.
Naming Tests
diff --git a/t/t-basic.sh b/t/t-basic.sh
index ae8874e..4da527f 100755
--- a/t/t-basic.sh
+++ b/t/t-basic.sh
@@ -333,6 +333,239 @@ test_expect_success 'GIT_SKIP_TESTS sh pattern'
EOF
+test_expect_success '--run basic'
+ run_sub_test_lib_test run-basic \
+ '--run basic' --run='1 3 5' -\\EOF
+ for i in 1 2 3 4 5 6
+ do
+ test_expect_success \passing test #\$i\ 'true'
+ done
+ test_done
+ EOF
+ check_sub_test_lib_test run-basic -\\EOF
+ok 1 - passing test #1
+ok 2 # skip passing test #2 (--run)
+ok 3 - passing test #3
+ok 4 # skip passing test #4 (--run)
+ok 5 - passing test #5
+ok 6 # skip passing test #6 (--run)
+# passed all 6 test(s)
+1..6
+ EOF
+
+
+test_expect_success '--run with '
+ run_sub_test_lib_test run-lt \
+ '--run with ' --run='4' -\\EOF
+ for i in 1 2 3 4 5 6
+ do
+ test_expect_success \passing test #\$i\ 'true'
+ done
+ test_done
+ EOF
+ check_sub_test_lib_test run-lt -\\EOF
+ok 1 - passing test #1
+ok 2 - passing test #2
+ok 3 - passing test #3
+ok 4 # skip passing test #4 (--run)
+ok 5 # skip passing test #5 (--run)
+ok 6 # skip passing test #6 (--run)
+# passed all 6 test(s)
+1..6
+ EOF
+
+
+test_expect_success '--run with ='
+ run_sub_test_lib_test run-le \
+ '--run with =' --run='=4' -\\EOF
+ for
[PATCH 2/3] test-lib: tests skipped by GIT_SKIP_TESTS say so
We used to show (missing ) next to tests skipped because they are
specified in GIT_SKIP_TESTS. Use (GIT_SKIP_TESTS) instead.
Plus tests that check basic GIT_SKIP_TESTS functions.
Signed-off-by: Ilya Bobyr ilya.bo...@gmail.com
---
No changes from the previous version.
t/t-basic.sh | 63 ++
t/test-lib.sh| 13 ++
2 files changed, 71 insertions(+), 5 deletions(-)
diff --git a/t/t-basic.sh b/t/t-basic.sh
index a2bb63c..ae8874e 100755
--- a/t/t-basic.sh
+++ b/t/t-basic.sh
@@ -270,6 +270,69 @@ test_expect_success 'test --verbose-only' '
EOF
'
+test_expect_success 'GIT_SKIP_TESTS'
+ GIT_SKIP_TESTS='git.2' \
+ run_sub_test_lib_test git-skip-tests-basic \
+ 'GIT_SKIP_TESTS' -\\EOF
+ for i in 1 2 3
+ do
+ test_expect_success \passing test #\$i\ 'true'
+ done
+ test_done
+ EOF
+ check_sub_test_lib_test git-skip-tests-basic -\\EOF
+ok 1 - passing test #1
+ok 2 # skip passing test #2 (GIT_SKIP_TESTS)
+ok 3 - passing test #3
+# passed all 3 test(s)
+1..3
+ EOF
+
+
+test_expect_success 'GIT_SKIP_TESTS several tests'
+ GIT_SKIP_TESTS='git.2 git.5' \
+ run_sub_test_lib_test git-skip-tests-several \
+ 'GIT_SKIP_TESTS several tests' -\\EOF
+ for i in 1 2 3 4 5 6
+ do
+ test_expect_success \passing test #\$i\ 'true'
+ done
+ test_done
+ EOF
+ check_sub_test_lib_test git-skip-tests-several -\\EOF
+ok 1 - passing test #1
+ok 2 # skip passing test #2 (GIT_SKIP_TESTS)
+ok 3 - passing test #3
+ok 4 - passing test #4
+ok 5 # skip passing test #5 (GIT_SKIP_TESTS)
+ok 6 - passing test #6
+# passed all 6 test(s)
+1..6
+ EOF
+
+
+test_expect_success 'GIT_SKIP_TESTS sh pattern'
+ GIT_SKIP_TESTS='git.[2-5]' \
+ run_sub_test_lib_test git-skip-tests-sh-pattern \
+ 'GIT_SKIP_TESTS sh pattern' -\\EOF
+ for i in 1 2 3 4 5 6
+ do
+ test_expect_success \passing test #\$i\ 'true'
+ done
+ test_done
+ EOF
+ check_sub_test_lib_test git-skip-tests-sh-pattern -\\EOF
+ok 1 - passing test #1
+ok 2 # skip passing test #2 (GIT_SKIP_TESTS)
+ok 3 # skip passing test #3 (GIT_SKIP_TESTS)
+ok 4 # skip passing test #4 (GIT_SKIP_TESTS)
+ok 5 # skip passing test #5 (GIT_SKIP_TESTS)
+ok 6 - passing test #6
+# passed all 6 test(s)
+1..6
+ EOF
+
+
test_set_prereq HAVEIT
haveit=no
test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 569b52d..e035f36 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -452,25 +452,28 @@ test_finish_ () {
test_skip () {
to_skip=
+ skipped_reason=
if match_pattern_list $this_test.$test_count $GIT_SKIP_TESTS
then
to_skip=t
+ skipped_reason=GIT_SKIP_TESTS
fi
if test -z $to_skip test -n $test_prereq
! test_have_prereq $test_prereq
then
to_skip=t
- fi
- case $to_skip in
- t)
+
of_prereq=
if test $missing_prereq != $test_prereq
then
of_prereq= of $test_prereq
fi
-
+ skipped_reason=missing $missing_prereq${of_prereq}
+ fi
+ case $to_skip in
+ t)
say_color skip 3 skipping test: $@
- say_color skip ok $test_count # skip $1 (missing
$missing_prereq${of_prereq})
+ say_color skip ok $test_count # skip $1 ($skipped_reason)
: true
;;
*)
--
1.7.9
--
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 1/3] test-lib: Document short options in t/README
Most arguments that could be provided to a test have short forms. Unless documented, the only way to learn them is to read the code. Signed-off-by: Ilya Bobyr ilya.bo...@gmail.com --- Minor changes according to comments in http://www.mail-archive.com/git@vger.kernel.org/msg46423.html Ramsay Jones and http://www.mail-archive.com/git@vger.kernel.org/msg46512.html Eric Sunshine t/README |8 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/t/README b/t/README index caeeb9d..6b93aca 100644 --- a/t/README +++ b/t/README @@ -71,7 +71,7 @@ You can pass --verbose (or -v), --debug (or -d), and --immediate (or -i) command line argument to the test, or by setting GIT_TEST_OPTS appropriately before running make. ---verbose:: +-v,--verbose:: This makes the test more verbose. Specifically, the command being run and their output if any are also output. @@ -81,7 +81,7 @@ appropriately before running make. numbers matching pattern. The number matched against is simply the running count of the test within the file. ---debug:: +-d,--debug:: This may help the person who is developing a new test. It causes the command defined with test_debug to run. The trash directory (used to store all temporary data @@ -89,14 +89,14 @@ appropriately before running make. failed tests so that you can inspect its contents after the test finished. ---immediate:: +-i,--immediate:: This causes the test to immediately exit upon the first failed test. Cleanup commands requested with test_when_finished are not executed if the test failed, in order to keep the state for inspection by the tester to diagnose the bug. ---long-tests:: +-l,--long-tests:: This causes additional long-running tests to be run (where available), for more exhaustive testing. -- 1.7.9 -- 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
Hello.
Greetings, I, Liliane send you this email. You can read about me on: fr.wikipedia.org/wiki/Liliane_Bettencourt I write to you because I intend to give to you some portion of my Bank net-worth which I have been putting away for a long time. I want to cede it to you for charity purpose. If ready,reply back With love, Liliane Bettencourt -- 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
SSL_CTX leak?
Hi, Do we leak the context we allocate in imap-send.c:280 intentionally? Regards, -- Thiago Farina -- 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 17/19] Portable alloca for Git
On Mon, Mar 24, 2014 at 02:47:24PM -0700, Junio C Hamano wrote:
Kirill Smelkov k...@mns.spb.ru writes:
On Fri, Feb 28, 2014 at 06:19:58PM +0100, Erik Faye-Lund wrote:
On Fri, Feb 28, 2014 at 6:00 PM, Kirill Smelkov k...@mns.spb.ru wrote:
...
In fact that would be maybe preferred, for maintainers to enable alloca
with knowledge and testing, as one person can't have them all at hand.
Yeah, you're probably right.
Erik, the patch has been merged into pu today. Would you please
follow-up with tested MINGW change?
Sooo I lost track but this discussion seems to have petered out
around here. I think the copy we have had for a while on 'pu' is
basically sound, and can easily built on by platform folks by adding
or removing the -DHAVE_ALLOCA_H from the Makefile.
Yes, that is all correct - that version works and we can improve it in
the future with platform-specific follow-up patches, if needed.
Please pick up the patch with ack from Thomas Schwinge.
Thanks,
Kirill
(please keep author email)
8
From: Kirill Smelkov k...@mns.spb.ru
Date: Mon, 24 Feb 2014 20:21:49 +0400
Subject: [PATCH v1a] Portable alloca for Git
In the next patch we'll have to use alloca() for performance reasons,
but since alloca is non-standardized and is not portable, let's have a
trick with compatibility wrappers:
1. at configure time, determine, do we have working alloca() through
alloca.h, and define
#define HAVE_ALLOCA_H
if yes.
2. in code
#ifdef HAVE_ALLOCA_H
# include alloca.h
# define xalloca(size) (alloca(size))
# define xalloca_free(p)do {} while(0)
#else
# define xalloca(size) (xmalloc(size))
# define xalloca_free(p)(free(p))
#endif
and use it like
func() {
p = xalloca(size);
...
xalloca_free(p);
}
This way, for systems, where alloca is available, we'll have optimal
on-stack allocations with fast executions. On the other hand, on
systems, where alloca is not available, this gracefully fallbacks to
xmalloc/free.
Both autoconf and config.mak.uname configurations were updated. For
autoconf, we are not bothering considering cases, when no alloca.h is
available, but alloca() works some other way - its simply alloca.h is
available and works or not, everything else is deep legacy.
For config.mak.uname, I've tried to make my almost-sure guess for where
alloca() is available, but since I only have access to Linux it is the
only change I can be sure about myself, with relevant to other changed
systems people Cc'ed.
NOTE
SunOS and Windows had explicit -DHAVE_ALLOCA_H in their configurations.
I've changed that to now-common HAVE_ALLOCA_H=YesPlease which should be
correct.
Cc: Brandon Casey draf...@gmail.com
Cc: Marius Storm-Olsen msto...@gmail.com
Cc: Johannes Sixt j...@kdbg.org
Cc: Johannes Schindelin johannes.schinde...@gmx.de
Cc: Ramsay Jones ram...@ramsay1.demon.co.uk
Cc: Gerrit Pape p...@smarden.org
Cc: Petr Salinger petr.salin...@seznam.cz
Cc: Jonathan Nieder jrnie...@gmail.com
Cc: Thomas Schwinge tschwi...@gnu.org
Acked-by: Thomas Schwinge tho...@codesourcery.com (GNU Hurd changes)
Signed-off-by: Kirill Smelkov k...@mns.spb.ru
Signed-off-by: Junio C Hamano gits...@pobox.com
---
Changes since v1:
- added ack for GNU/Hurd.
Makefile | 6 ++
config.mak.uname | 10 --
configure.ac | 8
git-compat-util.h | 8
4 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index dddaf4f..0334806 100644
--- a/Makefile
+++ b/Makefile
@@ -30,6 +30,8 @@ all::
# Define LIBPCREDIR=/foo/bar if your libpcre header and library files are in
# /foo/bar/include and /foo/bar/lib directories.
#
+# Define HAVE_ALLOCA_H if you have working alloca(3) defined in that header.
+#
# Define NO_CURL if you do not have libcurl installed. git-http-fetch and
# git-http-push are not built, and you cannot use http:// and https://
# transports (neither smart nor dumb).
@@ -1099,6 +1101,10 @@ ifdef USE_LIBPCRE
EXTLIBS += -lpcre
endif
+ifdef HAVE_ALLOCA_H
+ BASIC_CFLAGS += -DHAVE_ALLOCA_H
+endif
+
ifdef NO_CURL
BASIC_CFLAGS += -DNO_CURL
REMOTE_CURL_PRIMARY =
diff --git a/config.mak.uname b/config.mak.uname
index 7d31fad..71602ee 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -28,6 +28,7 @@ ifeq ($(uname_S),OSF1)
NO_NSEC = YesPlease
endif
ifeq ($(uname_S),Linux)
+ HAVE_ALLOCA_H = YesPlease
NO_STRLCPY = YesPlease
NO_MKSTEMPS = YesPlease
HAVE_PATHS_H = YesPlease
@@ -35,6 +36,7 @@ ifeq ($(uname_S),Linux)
HAVE_DEV_TTY = YesPlease
endif
ifeq ($(uname_S),GNU/kFreeBSD)
+ HAVE_ALLOCA_H = YesPlease
NO_STRLCPY = YesPlease
NO_MKSTEMPS = YesPlease
HAVE_PATHS_H = YesPlease
@@ -103,6 +105,7 @@ ifeq ($(uname_S),SunOS)
NEEDS_NSL = YesPlease
SHELL_PATH = /bin/bash
SANE_TOOL_PATH = /usr/xpg6/bin:/usr/xpg4/bin
+
Re: [PATCH/RFC 0/6] reuse deltas found by bitmaps
On 03/26/2014 03:40 PM, Siddharth Agarwal wrote: On 03/26/2014 12:22 AM, Jeff King wrote: [tl;dr the patch is the same as before, but there is a script to measure its effects; please try it out on your repos] Here are the numbers from another, much larger repo: Test origin HEAD 5311.3: server (1 days)11.72(14.02+1.44) 6.33(5.87+0.75) -46.0% 5311.4: size (1 days) 19.4M 15.3M -21.3% 5311.5: client (1 days)6.99(8.06+1.50) 10.60(10.34+1.83) +51.6% 5311.7: server (2 days)39.82(40.56+3.05) 33.94(31.21+3.10) -14.8% 5311.8: size (2 days) 26.5M 22.8M -14.1% 5311.9: client (2 days)15.81(16.48+4.29) 19.20(18.20+4.19) +21.4% 5311.11: server (4 days) 37.21(39.75+3.73) 28.01(26.97+1.75) -24.7% 5311.12: size (4 days) 37.5M 34.1M -9.0% 5311.13: client (4 days) 24.24(26.43+6.76) 31.14(30.75+5.96) +28.5% 5311.15: server (8 days) 33.74(40.47+3.39) 22.42(22.25+1.51) -33.6% 5311.16: size (8 days) 81.9M 78.4M -4.2% 5311.17: client (8 days) 81.96(91.07+22.35) 88.03(89.45+17.11) +7.4% 5311.19: server (16 days) 30.87(34.57+2.78) 27.03(25.93+2.73) -12.4% 5311.20: size(16 days) 153.2M150.9M -1.5% 5311.21: client (16 days) 169.99(183.57+46.96) 177.12(177.17+37.93) +4.2% 5311.23: server (32 days) 51.00(75.49+4.62) 19.52(19.28+1.93) -61.7% 5311.24: size(32 days) 279.4M283.0M +1.3% 5311.25: client (32 days) 272.43(296.17+76.48) 284.75(285.98+63.19) +4.5% 5311.27: server (64 days) 51.73(97.88+6.40) 37.32(32.63+5.05) -27.9% 5311.28: size(64 days) 1.7G 1.8G +5.0% 5311.29: client (64 days) 2600.42(2751.56+718.10) 2429.06(2501.29+651.56) -6.6% 5311.31: server (128 days) 51.33(95.33+6.91) 37.73(32.98+5.09) -26.5% 5311.32: size (128 days) 1.7G 1.8G +5.0% 5311.33: client (128 days) 2595.68(2739.45+729.07) 2386.99(2524.54+583.07) -8.0% -- 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 18/19] tree-diff: rework diff_tree() to generate diffs for multiparent cases as well
On Mon, Feb 24, 2014 at 08:21:50PM +0400, Kirill Smelkov wrote: [...] not changed: - low-level helpers are still named with __ prefix as, imho, that is the best convention to name such helpers, without sacrificing signal/noise ratio. All of them are now static though. Please find attached corrected version of this patch with __diff_tree_sha1() renamed to ll_diff_tree_sha1() and other identifiers corrected similarly for consistency with Git codebase style. Thanks, Kirill (please keep author email) 8 From: Kirill Smelkov k...@mns.spb.ru Date: Mon, 24 Feb 2014 20:21:50 +0400 Subject: [PATCH v3] tree-diff: rework diff_tree() to generate diffs for multiparent cases as well Previously diff_tree(), which is now named ll_diff_tree_sha1(), was generating diff_filepair(s) for two trees t1 and t2, and that was usually used for a commit as t1=HEAD~, and t2=HEAD - i.e. to see changes a commit introduces. In Git, however, we have fundamentally built flexibility in that a commit can have many parents - 1 for a plain commit, 2 for a simple merge, but also more than 2 for merging several heads at once. For merges there is a so called combine-diff, which shows diff, a merge introduces by itself, omitting changes done by any parent. That works through first finding paths, that are different to all parents, and then showing generalized diff, with separate columns for +/- for each parent. The code lives in combine-diff.c . There is an impedance mismatch, however, in that a commit could generally have any number of parents, and that while diffing trees, we divide cases for 2-tree diffs and more-than-2-tree diffs. I mean there is no special casing for multiple parents commits in e.g. revision-walker . That impedance mismatch *hurts* *performance* *badly* for generating combined diffs - in combine-diff: optimize combine_diff_path sets intersection I've already removed some slowness from it, but from the timings provided there, it could be seen, that combined diffs still cost more than an order of magnitude more cpu time, compared to diff for usual commits, and that would only be an optimistic estimate, if we take into account that for e.g. linux.git there is only one merge for several dozens of plain commits. That slowness comes from the fact that currently, while generating combined diff, a lot of time is spent computing diff(commit,commit^2) just to only then intersect that huge diff to almost small set of files from diff(commit,commit^1). That's because at present, to compute combine-diff, for first finding paths, that every parent touches, we use the following combine-diff property/definition: D(A,P1...Pn) = D(A,P1) ^ ... ^ D(A,Pn) (w.r.t. paths) where D(A,P1...Pn) is combined diff between commit A, and parents Pi and D(A,Pi) is usual two-tree diff Pi..A So if any of that D(A,Pi) is huge, tracting 1 n-parent combine-diff as n 1-parent diffs and intersecting results will be slow. And usually, for linux.git and other topic-based workflows, that D(A,P2) is huge, because, if merge-base of A and P2, is several dozens of merges (from A, via first parent) below, that D(A,P2) will be diffing sum of merges from several subsystems to 1 subsystem. The solution is to avoid computing n 1-parent diffs, and to find changed-to-all-parents paths via scanning A's and all Pi's trees simultaneously, at each step comparing their entries, and based on that comparison, populate paths result, and deduce we could *skip* *recursing* into subdirectories, if at least for 1 parent, sha1 of that dir tree is the same as in A. That would save us from doing significant amount of needless work. Such approach is very similar to what diff_tree() does, only there we deal with scanning only 2 trees simultaneously, and for n+1 tree, the logic is a bit more complex: D(A,X1...Xn) calculation scheme --- D(A,X1...Xn) = D(A,X1) ^ ... ^ D(A,Xn) (regarding resulting paths set) D(A,Xj) - diff between A..Xj D(A,X1...Xn)- combined diff from A to parents X1,...,Xn We start from all trees, which are sorted, and compare their entries in lock-step: A X1 Xn - -- |a| |x1| |xn| |-| |--| ... |--| i = argmin(x1...xn) | | | | | | |-| |--| |--| |.| |. | |. | . .. . .. at any time there could be 3 cases: 1) a xi; 2) a xi; 3) a = xi. Schematic deduction of what every case means, and what to do, follows: 1) a xi - ∀j a ∉ Xj - +a ∈ D(A,Xj) - D += +a; a↓ 2) a xi 2.1) ∃j: xj xi - -xi ∉ D(A,Xj) - D += ø; ∀ xk=xi xk↓ 2.2) ∀j xj = xi - xj ∉ A - -xj ∈ D(A,Xj) - D += -xi; ∀j xj↓ 3) a = xi 3.1) ∃j: xj xi - +a ∈ D(A,Xj) - only xk=xi remains to investigate 3.2) xj = xi - investigate δ(a,xj) | | v 3.1+3.2) looking at δ(a,xk) ∀k: xk=xi - if all != ø -
Possible regression in master? (submodules without a master branch)
Hi, I just found a failure to checkout a project with submodules where there is no explicit submodule branch configuration, and the submodules happen to not have a master branch: git clone git://gitorious.org/qt/qt5.git qt5 cd qt5 git submodule init qtbase git submodule update In current master, the last command fails with the following output: Cloning into 'qtbase'... remote: Counting objects: 267400, done. remote: Compressing objects: 100% (61070/61070), done. remote: Total 267400 (delta 210431), reused 258876 (delta 202642) Receiving objects: 100% (267400/267400), 136.23 MiB | 6.73 MiB/s, done. Resolving deltas: 100% (210431/210431), done. Checking connectivity... done. error: pathspec 'origin/master' did not match any file(s) known to git. Unable to setup cloned submodule 'qtbase' Bisection points to 23d25e48f5ead73c9ce233986f90791abec9f1e8 (W. Trevor King: submodule: explicit local branch creation in module_clone). Looking at the patch, it seems to introduce an implicit assumption on the submodule origin having a master branch. Is this an intended change in behaviour? ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- 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 16/19] tree-diff: reuse base str(buf) memory on sub-tree recursion
On Tue, Mar 25, 2014 at 01:23:20PM +0400, Kirill Smelkov wrote:
On Mon, Mar 24, 2014 at 02:43:36PM -0700, Junio C Hamano wrote:
Kirill Smelkov k...@mns.spb.ru writes:
instead of allocating it all the time for every subtree in
__diff_tree_sha1, let's allocate it once in diff_tree_sha1, and then all
callee just use it in stacking style, without memory allocations.
This should be faster, and for me this change gives the following
slight speedups for
git log --raw --no-abbrev --no-renames --format='%H'
navy.gitlinux.git v3.10..v3.11
before 0.618s 1.903s
after 0.611s 1.889s
speedup 1.1%0.7%
Signed-off-by: Kirill Smelkov k...@mns.spb.ru
---
Changes since v1:
- don't need to touch diff.h, as the function we are changing became
static.
tree-diff.c | 36 ++--
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/tree-diff.c b/tree-diff.c
index aea0297..c76821d 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -115,7 +115,7 @@ static void show_path(struct strbuf *base, struct
diff_options *opt,
if (recurse) {
strbuf_addch(base, '/');
__diff_tree_sha1(t1 ? t1-entry.sha1 : NULL,
- t2 ? t2-entry.sha1 : NULL, base-buf, opt);
+ t2 ? t2-entry.sha1 : NULL, base, opt);
}
strbuf_setlen(base, old_baselen);
I was scratching my head for a while, after seeing that there does
not seem to be any *new* code added by this patch in order to
store-away the original length and restore the singleton base buffer
to the original length after using addch/addstr to extend it.
But I see that the code has already been prepared to do this
conversion. I wonder why we didn't do this earlier ;-)
The conversion to reusing memory started in 48932677 diff-tree: convert
base+baselen to writable strbuf which allowed to avoid quite a bit of
malloc() and memcpy(), but for this to work allocation at diff_tree()
entry had to be there.
In particular it had to be there, because diff_tree() accepted base as C
string, not strbuf, and since diff_tree() was calling itself
recursively - oops - new allocation on every subtree.
I've opened the door for avoiding allocations via splitting diff_tree
into high-level and low-level parts. The high-level part still accepts
`char *base`, but low-level function operates on strbuf and recurses
into low-level self.
The high-level diff_tree_sha1() still allocates memory for every
diff(tree1,tree2), but that is significantly lower compared to
allocating memory on every subtree...
The lesson here is: better use strbuf for api unless there is a reason
not to.
Looks good. Thanks.
Thanks.
Thanks again. Here it goes adjusted to __diff_tree_sha1 - ll_diff_tree_sha1
renaming:
(please keep author email)
8
From: Kirill Smelkov k...@mns.spb.ru
Date: Mon, 24 Feb 2014 20:21:48 +0400
Subject: [PATCH v3] tree-diff: reuse base str(buf) memory on sub-tree recursion
instead of allocating it all the time for every subtree in
ll_diff_tree_sha1, let's allocate it once in diff_tree_sha1, and then all
callee just use it in stacking style, without memory allocations.
This should be faster, and for me this change gives the following
slight speedups for
git log --raw --no-abbrev --no-renames --format='%H'
navy.gitlinux.git v3.10..v3.11
before 0.618s 1.903s
after 0.611s 1.889s
speedup 1.1%0.7%
Signed-off-by: Kirill Smelkov k...@mns.spb.ru
---
Changes since v2:
- adjust to __diff_tree_sha1 - ll_diff_tree_sha1 renaming.
Changes since v1:
- don't need to touch diff.h, as the function we are changing became
static.
tree-diff.c | 38 +++---
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/tree-diff.c b/tree-diff.c
index 7fbb022..8c8bde6 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -8,7 +8,7 @@
static int ll_diff_tree_sha1(const unsigned char *old, const unsigned char
*new,
-const char *base_str, struct diff_options *opt);
+struct strbuf *base, struct diff_options *opt);
/*
* Compare two tree entries, taking into account only path/S_ISDIR(mode),
@@ -123,7 +123,7 @@ static void show_path(struct strbuf *base, struct
diff_options *opt,
if (recurse) {
strbuf_addch(base, '/');
ll_diff_tree_sha1(t1 ? t1-entry.sha1 : NULL,
- t2 ? t2-entry.sha1 : NULL, base-buf, opt);
+ t2 ? t2-entry.sha1 : NULL, base, opt);
}
strbuf_setlen(base, old_baselen);
@@ -146,12 +146,10 @@ static void skip_uninteresting(struct tree_desc *t,
struct strbuf *base,
}
static int
Re: [PATCH 15/19] tree-diff: no need to call full diff_tree_sha1 from show_path()
On Mon, Feb 24, 2014 at 08:21:47PM +0400, Kirill Smelkov wrote:
As described in previous commit, when recursing into sub-trees, we can
use lower-level tree walker, since its interface is now sha1 based.
The change is ok, because diff_tree_sha1() only invokes
__diff_tree_sha1(), and also, if base is empty, try_to_follow_renames().
But base is not empty here, as we have added a path and '/' before
recursing.
Signed-off-by: Kirill Smelkov k...@mns.spb.ru
Signed-off-by: Junio C Hamano gits...@pobox.com
---
( re-posting without change )
tree-diff.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tree-diff.c b/tree-diff.c
index f90acf5..aea0297 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -114,8 +114,8 @@ static void show_path(struct strbuf *base, struct
diff_options *opt,
if (recurse) {
strbuf_addch(base, '/');
- diff_tree_sha1(t1 ? t1-entry.sha1 : NULL,
-t2 ? t2-entry.sha1 : NULL, base-buf, opt);
+ __diff_tree_sha1(t1 ? t1-entry.sha1 : NULL,
+ t2 ? t2-entry.sha1 : NULL, base-buf, opt);
}
strbuf_setlen(base, old_baselen);
I've found this does not compile as I've forgot to add __diff_tree_sha1
prototype, and also we are changing naming for __diff_tree_sha1() to
ll_diff_tree_sha1() to follow Git coding style for consistency and
corrections to previous patch, so here goes v2:
(please keep author email)
8
From: Kirill Smelkov k...@mns.spb.ru
Date: Mon, 24 Feb 2014 20:21:47 +0400
Subject: [PATCH v2] tree-diff: no need to call full diff_tree_sha1 from
show_path()
As described in previous commit, when recursing into sub-trees, we can
use lower-level tree walker, since its interface is now sha1 based.
The change is ok, because diff_tree_sha1() only invokes
ll_diff_tree_sha1(), and also, if base is empty, try_to_follow_renames().
But base is not empty here, as we have added a path and '/' before
recursing.
Signed-off-by: Kirill Smelkov k...@mns.spb.ru
---
Changes since v1:
- adjust to renaming __diff_tree_sha1 - ll_diff_tree_sha1;
- added ll_diff_tree_sha1 prototype as the function is defined below
here-introduced call-site.
tree-diff.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tree-diff.c b/tree-diff.c
index 1d02e43..7fbb022 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -6,6 +6,10 @@
#include diffcore.h
#include tree.h
+
+static int ll_diff_tree_sha1(const unsigned char *old, const unsigned char
*new,
+const char *base_str, struct diff_options *opt);
+
/*
* Compare two tree entries, taking into account only path/S_ISDIR(mode),
* but not their sha1's.
@@ -118,8 +122,8 @@ static void show_path(struct strbuf *base, struct
diff_options *opt,
if (recurse) {
strbuf_addch(base, '/');
- diff_tree_sha1(t1 ? t1-entry.sha1 : NULL,
- t2 ? t2-entry.sha1 : NULL, base-buf, opt);
+ ll_diff_tree_sha1(t1 ? t1-entry.sha1 : NULL,
+ t2 ? t2-entry.sha1 : NULL, base-buf, opt);
}
strbuf_setlen(base, old_baselen);
--
1.9.rc0.143.g6fd479e
--
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 Tue, Mar 25, 2014 at 6:15 PM, Duy Nguyen pclo...@gmail.com wrote: On Tue, Mar 25, 2014 at 3:55 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: - Original Message - The use case in mind is --max-depth=0 to stop recursion. With this we can do git config --global alias.ls 'ls-files --column --color --max-depth=0' and have git ls with an output very similar to GNU ls. One big difference though: your git ls does not show directories. I understand that this is easier to implement, but from the user point of view it resulted in a wtf from me running git ls in a repository containing essentially directories, and seeing just a README file in the output. I was hoping you didn't notice :) It'll be more difficult but not impossible. Ideally (for me), directories should be shown with a trailing / like ls -F does. I'd rather go with no trailing slash by default and add -F (which seems to be more than just '/') And we need a new indicator for submodules when -F is used. I think it should be different than '/'. I randomly picked '' for now. Any suggestions welcome. -- 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 v2 14/19] tree-diff: rework diff_tree interface to be sha1 based
On Wed, Mar 26, 2014 at 02:34:24PM -0700, Junio C Hamano wrote:
Kirill Smelkov k...@navytux.spb.ru writes:
On Tue, Mar 25, 2014 at 10:46:32AM -0700, Junio C Hamano wrote:
Kirill Smelkov k...@navytux.spb.ru writes:
What are the downsides of __ prefix by the way?
Aren't these names reserved for compiler/runtime implementations?
Yes, but there are precedents when people don't obey it widely and
in practice everything works :)
I think you are alluding to the practice in the Linux kernel, but
their requirement is vastly different---their product do not even
link with libc and they always compile with specific selected
versions of gcc, no?
Yes, that is correct. Only __ was so visually appealing that there was
a temptation to break the rules, but...
Let it be something portable anyway -
how about diff_tree_sha1_low() ?
Sure.
As this is a file-scope static, I do not think the exact naming
matters that much. Just FYI, we seem to use ll_ prefix (standing
for low-level) in some places.
... let's then use this ll_ prefix scheme for consistency.
Corrected patch is below, and I've sent corrections to follow-up
patches as well.
Thanks,
Kirill
(please keep author email)
8
From: Kirill Smelkov k...@mns.spb.ru
Date: Mon, 24 Feb 2014 20:21:46 +0400
Subject: [PATCH v3a] tree-diff: rework diff_tree interface to be sha1 based
In the next commit this will allow to reduce intermediate calls, when
recursing into subtrees - at that stage we know only subtree sha1, and
it is natural for tree walker to start from that phase. For now we do
diff_tree
show_path
diff_tree_sha1
diff_tree
...
and the change will allow to reduce it to
diff_tree
show_path
diff_tree
Also, it will allow to omit allocating strbuf for each subtree, and just
reuse the common strbuf via playing with its len.
The above-mentioned improvements go in the next 2 patches.
The downside is that try_to_follow_renames(), if active, we cause
re-reading of 2 initial trees, which was negligible based on my timings,
and which is outweighed cogently by the upsides.
NOTE To keep with the current interface and semantics, I needed to
rename the function from diff_tree() to diff_tree_sha1(). As
diff_tree_sha1() was already used, and the function we are talking here
is its more low-level helper, let's use convention for prefixing
such helpers with ll_. So the final renaming is
diff_tree() - ll_diff_tree_sha1()
Signed-off-by: Kirill Smelkov k...@mns.spb.ru
---
Changes since v3:
- further rename diff_tree_sha1_low() - ll_diff_tree_sha1() to follow Git
style for naming low-level helpers.
Changes since v2:
- renamed __diff_tree_sha1() - diff_tree_sha1_low() as the former
overlaps with reserved-for-implementation identifiers namespace.
Changes since v1:
- don't need to touch diff.h, as diff_tree() became static.
tree-diff.c | 60
1 file changed, 28 insertions(+), 32 deletions(-)
diff --git a/tree-diff.c b/tree-diff.c
index f137f39..1d02e43 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -141,12 +141,17 @@ static void skip_uninteresting(struct tree_desc *t,
struct strbuf *base,
}
}
-static int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
-const char *base_str, struct diff_options *opt)
+static int ll_diff_tree_sha1(const unsigned char *old, const unsigned char
*new,
+const char *base_str, struct diff_options *opt)
{
+ struct tree_desc t1, t2;
+ void *t1tree, *t2tree;
struct strbuf base;
int baselen = strlen(base_str);
+ t1tree = fill_tree_descriptor(t1, old);
+ t2tree = fill_tree_descriptor(t2, new);
+
/* Enable recursion indefinitely */
opt-pathspec.recursive = DIFF_OPT_TST(opt, RECURSIVE);
@@ -159,39 +164,41 @@ static int diff_tree(struct tree_desc *t1, struct
tree_desc *t2,
if (diff_can_quit_early(opt))
break;
if (opt-pathspec.nr) {
- skip_uninteresting(t1, base, opt);
- skip_uninteresting(t2, base, opt);
+ skip_uninteresting(t1, base, opt);
+ skip_uninteresting(t2, base, opt);
}
- if (!t1-size !t2-size)
+ if (!t1.size !t2.size)
break;
- cmp = tree_entry_pathcmp(t1, t2);
+ cmp = tree_entry_pathcmp(t1, t2);
/* t1 = t2 */
if (cmp == 0) {
if (DIFF_OPT_TST(opt, FIND_COPIES_HARDER) ||
- hashcmp(t1-entry.sha1, t2-entry.sha1) ||
- (t1-entry.mode != t2-entry.mode))
- show_path(base, opt, t1, t2);
+ hashcmp(t1.entry.sha1,
Bug report: Git 1.8 on Ubuntu 13.10 refs not valid
Hi, I'm running: Ubuntu 13.10 64 bit and git version git:amd64/saucy 1:1.8.3.2-1 uptodate my remote repository is on a Chiliprojekt server (a fork of Redmine). cloning the repo over http results in following error: sneher@sneher-XPS:~/Dokumente/test$ git clone http://sne...@git.projects.gwdg.de/xrd-csd.git Klone nach 'xrd-csd'... Password for 'http://sne...@git.projects.gwdg.de': fatal: http://sne...@git.projects.gwdg.de/xrd-csd.git/info/refs not valid: is this a git repository? the content of ../info/refs looks like this e49ae34096fd6fff3d1e7b8e7b6e78ae29bad913refs/heads/0.2.2 3d375b2f7eeeb7bc12b24cc8181aa085f471ba10refs/heads/master f7a69735c1e2cb8363be849afa9e9bfdf2db61c6refs/heads/new_lab 879ccace941ea6dc83876b1dcfcc099e5c7e5b42refs/heads/testing 2f9504da3febcdafb9cb92806e7e178144fec0c9refs/remotes/origin/HEAD 2f9504da3febcdafb9cb92806e7e178144fec0c9refs/remotes/origin/master f7a69735c1e2cb8363be849afa9e9bfdf2db61c6refs/remotes/origin/new_lab 58fe57f5a2a0c8e8096c62f8ab8be2077c592e53refs/remotes/origin/testing 4b64a990dc1534abcccfb7f8c22f0cc5388e9db8refs/tags/0.1.0 a90ce817ca3bde41ce6c88cf22a9993bd7560f55refs/tags/0.1.1 9a25635e866979b044b83f914cfd993a7031a9carefs/tags/0.1.2 5a94e698b1042b34a25c87ced98f5f42d40e2578refs/tags/0.2.0 7cb00e325c1fb9a4112700744237f873bd5bae16refs/tags/0.2.1 I use to have the same problem on a different Ubuntu version (12.10). There the problem occurede with the git 1.8 update. I just switcht back to the older version. Problem is, there is no older version for saucy. Thanks for your help! and, in case this do to my inability, sorry for bugging you! Siggi -- 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] gitweb: gpg signature status indication for commits
shows gpg signature (if any) for commit message in gitweb
in case of successfully verifying the signature highlights it with green
Signed-off-by: Victor Kartashov victor.kartas...@gmail.com
---
gitweb/gitweb.perl | 33 ++---
gitweb/static/gitweb.css | 5 +
2 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 79057b7..0b41392 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] eq \0); # Remove '\0'
if (! @commit_lines) {
return;
@@ -3469,6 +3470,10 @@ sub parse_commit_text {
$co{'committer_name'} = $co{'committer'};
}
}
+ elsif ($line =~ /^gpg: /)
+ {
+ push @signature, $line;
+ }
}
if (!defined $co{'tree'}) {
return;
@@ -3508,6 +3513,11 @@ sub parse_commit_text {
foreach my $line (@commit_lines) {
$line =~ s/^//;
}
+ push(@commit_lines, ) if(scalar(@signature) 0);
+ foreach my $sig (@signature)
+ {
+ push(@commit_lines, $sig);
+ }
$co{'comment'} = \@commit_lines;
my $age = time - $co{'committer_epoch'};
@@ -3530,13 +3540,15 @@ 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 +4583,14 @@ 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/^\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..0b7479c 100644
--- a/gitweb/static/gitweb.css
+++ b/gitweb/static/gitweb.css
@@ -136,6 +136,11 @@ span.signoff {
color: #88;
}
+span.good_sign {
+ font-weight: bold;
+ background-color: #aaffaa;
+}
+
div.log_link {
padding: 0px 8px;
font-size: 70%;
--
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: Possible regression in master? (submodules without a master branch)
On Thu, Mar 27, 2014 at 03:21:49PM +0100, Johan Herland wrote: I just found a failure to checkout a project with submodules where there is no explicit submodule branch configuration, and the submodules happen to not have a master branch: The docs say [1]: A remote branch name for tracking updates in the upstream submodule. If the option is not specified, it defaults to 'master'. which is what we do now. Working around that to default to the upstream submodule's HEAD is possible (you can just use --branch HEAD), but I think it's easier to just explicitly specify your preferred branch. Cheers, Trevor [1]: submodule.name.branch in gitmodules(5) http://git-scm.com/docs/gitmodules.html -- 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: Possible regression in master? (submodules without a master branch)
On Thu, Mar 27, 2014 at 08:52:08AM -0700, W. Trevor King wrote: Working around that to default to the upstream submodule's HEAD is possible (you can just use --branch HEAD) Actually, this is probably not a good idea. The initial submodule addition works: $ git submodule add -b HEAD /tmp/submod.git submod Cloning into 'submod'... done. But subsequent log calls (from the superproject) do not: $ git log fatal: bad default revision 'HEAD' $ echo $? 128 and status calls (from the superproject) also have trouble: $ git status warning: refname 'HEAD' is ambiguous warning: refname 'HEAD' is ambiguous. On branch master … So it's better to just specify your preferred upstream branch directly (e.g. --branch next). 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
Git feature request: Option to force Git to abort a checkout if working directory is dirty (i.e. disregarding the check for conflicts)
Hi Git developers, This is my first Git feature request, I hope it wont get me hanged on the gallows ;o) *Git feature request:* Add an option to Git config to configure the criteria for when a git checkout should abort. *Name proposal and options:* checkout.clean false default checkout.clean true *False behavior:* As is: When doing a checkout then Git will check if your working directory is dirty, and if so check if the checkout will result in any conflicts, and if so abort the checkout with a message: $ git checkout some_branch error: Your local changes to the following files would be overwritten by checkout: some_file Please, commit your changes or stash them before you can switch branches. Aborting If no conflicts then: $ git checkout some_branch M some_file M some_other_file Switched to branch 'some_branch' I.e. it will only abort if there are conflicts. *True behavior:* When doing a checkout then Git will check if your working directory is dirty (checking for both modified and added untracked files), and if so abort the checkout with a message: $ git checkout some_branch error: Your working directory is not clean. Please, commit your changes or stash them before you can switch branches. Aborting I.e. it will abort if working directory is dirty (checking for both modified and added untracked files). I.e. you can only do checkout if you get nothing to commit, working directory clean when running git status (ignoring ignored files though). *Usecase in short:* If you use an IDE (like e.g. Eclipse) and do a checkout of 'some_branch' with a dirty working directory which will not result in any conflicts, then you will not be nicely notified (as you would in Git Bash) that the changes you were working on in 'previous_branch' are still present in your working directory after changing to 'some_branch'. I.e. when you compile your code your uncommitted changes from 'previous_branch' are still present in your working directory on 'some_branch'. As I see it Git is extremely strong in context switching (i.e. working on multiple issues on multiple branches), and I could see a use for a setting which setup a strict check for if working directory is not clean (disregarding the check for conflicts). This would mean that your changes created while on branch #1 will not be carried over when changing to branch #2, i.e. you will work strictly context based always. *Usecase also described here:* http://stackoverflow.com/questions/22609566/how-to-force-git-to-abort-a-chec kout-if-working-directory-is-not-clean-i-e-dis Br, Jonas Bang Christensen -- 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 1/3] test-lib: Document short options in t/README
Ilya Bobyr ilya.bo...@gmail.com writes:
If there is decision on how shortening should work for all the
options, maybe I could add a paragraph on that and make existing
options more consistent.
We should strive to make the following from gitcli.txt apply
throughout the system:
* many commands allow a long option `--option` to be abbreviated
only to their unique prefix (e.g. if there is no other option
whose name begins with `opt`, you may be able to spell `--opt` to
invoke the `--option` flag), but you should fully spell them out
when writing your scripts; later versions of Git may introduce a
new option whose name shares the same prefix, e.g. `--optimize`,
to make a short prefix that used to be unique no longer unique.
If so, '--valgrind' becomes impossible to shorten because there
is '--valgrind-only' that is a separate option. Same for
'--verbose' and '--verbose-only'.
Correct. If you really cared, --valgrind={yes,no,only} would be (or
have been) a better possibility, though.
--
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/RFC 0/6] reuse deltas found by bitmaps
Jeff King p...@peff.net writes: But for a small fetch... 5311.3: server (1 days)0.20(0.17+0.03) 4.39(4.03+6.59) +2095.0% 5311.4: size (1 days) 57.2K 59.5K +4.1% 5311.5: client (1 days)0.08(0.08+0.00) 0.08(0.08+0.00) +0.0% Nice ;-) So this is a dead end, but I think it was good to double-check that. 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 v8 03/12] Move lower case functions into wrapper.c
Christian Couder chrisc...@tuxfamily.org writes: Yeah, but it seems a bit wasteful to allocate memory for a new string, then downcase it, then compare it with strcmp() and then free it, instead of just using strcasecmp() on the original string. 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. -- 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: added missing include so `make INLINE=__inline` is no longer required
Marat Radchenko ma...@slonopotamus.org writes: Signed-off-by: Marat Radchenko ma...@slonopotamus.org --- xdiff/xutils.c | 1 + 1 file changed, 1 insertion(+) diff --git a/xdiff/xutils.c b/xdiff/xutils.c index 62cb23d..a21a835 100644 --- a/xdiff/xutils.c +++ b/xdiff/xutils.c @@ -23,6 +23,7 @@ #include limits.h #include assert.h #include xinclude.h +#include git-compat-util.h This is unfortunate for a few reasons: - xdiff/* is a borrowed code; we do not want to have (or add more) dependencies on the rest of Git, including compat-util. - When a piece of our code needs our compatibility support, compat-util must be the first header file included (either directly, or indirectly by including another header file that includes it at the top). My gut feeling is that adding a mechanism to add -DINLINE=__inline only on MSVC to the top-level Makefile, without touching this file, may be a much more palatable. I dunno. -- 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 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. If you reorder hunks, the patch should no longer apply [*1*], so a feature to make patch-id stable across such move would have no practical use ;-), but I am guessing you meant something else. Perhaps this is about using -O orderfile option, even though you happened to have implemented the id mixing at per-hunk level? [Footnote] *1* It has been a long time since I looked at the code, and I do not know offhand if git apply has such a bug not to diagnose a hunk for a file for an earlier part that comes later in its input stream after seeing another hunk for the same file as a bug. If it does not, perhaps we should. -- 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: Possible regression in master? (submodules without a master branch)
Johan Herland jo...@herland.net writes: I just found a failure to checkout a project with submodules where there is no explicit submodule branch configuration, and the submodules happen to not have a master branch: git clone git://gitorious.org/qt/qt5.git qt5 cd qt5 git submodule init qtbase git submodule update In current master, the last command fails with the following output: ... and with a bug-free system, what does it do instead? Just clone 'qtbase' and make a detached-head checkout at the commit recorded in the superproject's tree, or something else? Cloning into 'qtbase'... remote: Counting objects: 267400, done. remote: Compressing objects: 100% (61070/61070), done. remote: Total 267400 (delta 210431), reused 258876 (delta 202642) Receiving objects: 100% (267400/267400), 136.23 MiB | 6.73 MiB/s, done. Resolving deltas: 100% (210431/210431), done. Checking connectivity... done. error: pathspec 'origin/master' did not match any file(s) known to git. Unable to setup cloned submodule 'qtbase' Bisection points to 23d25e48f5ead73c9ce233986f90791abec9f1e8 (W. Trevor King: submodule: explicit local branch creation in module_clone). Looking at the patch, it seems to introduce an implicit assumption on the submodule origin having a master branch. Is this an intended change in behaviour? If an existing set-up that was working in a sensible way is broken by a change that assumes something that should not be assumed, then that is a serious regression, I would have to say. -- 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: Possible regression in master? (submodules without a master branch)
Am 27.03.2014 16:52, schrieb W. Trevor King: On Thu, Mar 27, 2014 at 03:21:49PM +0100, Johan Herland wrote: I just found a failure to checkout a project with submodules where there is no explicit submodule branch configuration, and the submodules happen to not have a master branch: The docs say [1]: A remote branch name for tracking updates in the upstream submodule. If the option is not specified, it defaults to 'master'. But the branch setting isn't configured for Qt, the .gitmodules file contains only this: [submodule qtbase] path = qtbase url = ../qtbase.git ... which is what we do now. Working around that to default to the upstream submodule's HEAD is possible (you can just use --branch HEAD), but I think it's easier to just explicitly specify your preferred branch. That is *not* easier, as Johan did not have to do that before. I think your patch 23d25e48f5ead73c9ce233986f90791abec9f1e8 does not do what the commit message promised: With this change, folks cloning submodules for the first time via: $ git submodule update ... will get a local branch instead of a detached HEAD, unless they are using the default checkout-mode updates. And Qt uses the default checkout-mode updates and doesn't have branch configured either. So we are facing a serious regression here. Cheers, Trevor [1]: submodule.name.branch in gitmodules(5) http://git-scm.com/docs/gitmodules.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
Re: Possible regression in master? (submodules without a master branch)
Am 27.03.2014 18:16, schrieb Junio C Hamano: Johan Herland jo...@herland.net writes: I just found a failure to checkout a project with submodules where there is no explicit submodule branch configuration, and the submodules happen to not have a master branch: git clone git://gitorious.org/qt/qt5.git qt5 cd qt5 git submodule init qtbase git submodule update In current master, the last command fails with the following output: ... and with a bug-free system, what does it do instead? Just clone 'qtbase' and make a detached-head checkout at the commit recorded in the superproject's tree, or something else? After reverting 23d25e48f5ead73 on current master it clones 'qtbase' nicely with a detached HEAD. Cloning into 'qtbase'... remote: Counting objects: 267400, done. remote: Compressing objects: 100% (61070/61070), done. remote: Total 267400 (delta 210431), reused 258876 (delta 202642) Receiving objects: 100% (267400/267400), 136.23 MiB | 6.73 MiB/s, done. Resolving deltas: 100% (210431/210431), done. Checking connectivity... done. error: pathspec 'origin/master' did not match any file(s) known to git. Unable to setup cloned submodule 'qtbase' Bisection points to 23d25e48f5ead73c9ce233986f90791abec9f1e8 (W. Trevor King: submodule: explicit local branch creation in module_clone). Looking at the patch, it seems to introduce an implicit assumption on the submodule origin having a master branch. Is this an intended change in behaviour? If an existing set-up that was working in a sensible way is broken by a change that assumes something that should not be assumed, then that is a serious regression, I would have to say. Yes, especially as it promised to not change this use case. -- 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 1/3] patch-id: make it stable against hunk reordering
On Thu, Mar 27, 2014 at 09:58:41AM -0700, Junio C Hamano wrote: Michael S. Tsirkin m...@redhat.com writes: Patch id changes if you reorder hunks in a diff. If you reorder hunks, the patch should no longer apply [*1*], so a feature to make patch-id stable across such move would have no practical use ;-), but I am guessing you meant something else. Perhaps this is about using -O orderfile option, even though you happened to have implemented the id mixing at per-hunk level? Yes. [Footnote] *1* It has been a long time since I looked at the code, and I do not know offhand if git apply has such a bug not to diagnose a hunk for a file for an earlier part that comes later in its input stream after seeing another hunk for the same file as a bug. If it does not, perhaps we should. Hmm you are right. For some reason I thought that it does work. I'll drop this part then, less code this way. Thanks! Any more comments before I spin v2? -- 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
Re: [PATCH] MSVC: added missing include so `makeINLINE=__inline` is no longer required
Junio C Hamano gitster at pobox.com writes: My gut feeling is that adding a mechanism to add -DINLINE=__inline only on MSVC to the top-level Makefile, without touching this file, may be a much more palatable. Okay, I'll think more about this one. Maybe *moving* inline=__inline from compat-headers into Makefile (actually, config.mak.uname) will work better. Hope it doesn't prevent first patch from being integrated -- joining them in a single thread was unintentional misuse of `git send-email` flags. -- 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 feature request: Option to force Git to abort a checkout if working directory is dirty (i.e. disregarding the check for conflicts)
Jonas Bang em...@jonasbang.dk writes: Hi Git developers, This is my first Git feature request, I hope it won’t get me hanged on the gallows ;o) *Git feature request:* Add an option to Git config to configure the criteria for when a git checkout should abort. *Name proposal and options:* checkout.clean false default checkout.clean true A configuration variable without command line override will make the system unusable. When thinking about a new feature, please make it a habit to first add a command line option and then if that option turns out to be useful in the real world (not in the imaginary world in which you had such a feature, where even you haven't lived in yet), then think about also adding configuration variables to control the default. Also, a useful definition of clean-ness may have to change over time as we gain experience with the feature. And also as a user personally gains experience with using Git. It is somewhat implausible that a boolean true/false may remain sufficient. *False behavior:* What is false? Ah, when the configuration is set to false, which will be the default? As is: When doing a checkout then Git will check if your working directory is dirty, and if so check if the checkout will result in any conflicts, and if so abort the checkout with a message: $ git checkout some_branch error: Your local changes to the following files would be overwritten by checkout: some_file Please, commit your changes or stash them before you can switch branches. Aborting If no conflicts then: $ git checkout some_branch M some_file M some_other_file Switched to branch 'some_branch' I.e. it will only abort if there are conflicts. Sensible. This is the behaviour that is very often depended upon by people who use Git with multiple branches. Are you thinking about changing it in any way when the new configuration is set to false, or is the above just a summary of what happens in the current system? *True behavior:* When doing a checkout then Git will check if your working directory is dirty (checking for both modified and added untracked files), and if so abort the checkout with a message: $ git checkout some_branch error: Your working directory is not clean. Please, commit your changes or stash them before you can switch branches. Aborting I.e. it will abort if working directory is dirty (checking for both modified and added untracked files). I.e. you can only do checkout if you get nothing to commit, working directory clean when running git status (ignoring ignored files though). The above two say very different things. For some people, having many throw away untracked files is a norm that they do not consider it is even worth their time to list them in .gitignore and they do not want to be reminded in git status output, and the latter definition of checkout.clean=true will kill checkout when status says there are some things that could be committed would suit them, while the former definition checkout.clean=true will kill checkout when there is any untracked files would be totally useless. So I can understand the latter, but I do not see how the former could be a useful addition. For some people it is also a norm to keep files that have been modified from HEAD and/or index without committing for a long time (e.g. earlier, Linus said that the version in Makefile is updated and kept modified in the working tree long before a new release is committed with that change). The default behaviour would cover their use case so your proposal would not hurt them, but I wonder if there are things you could do to help them as well, perhaps by allowing this new configuration to express something like local changes in these paths are except from this new check. I dunno. -- 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 1/3] patch-id: make it stable against hunk reordering
On Thu, Mar 27, 2014 at 09:58:41AM -0700, Junio C Hamano wrote: Michael S. Tsirkin m...@redhat.com writes: Patch id changes if you reorder hunks in a diff. If you reorder hunks, the patch should no longer apply [*1*], so a feature to make patch-id stable across such move would have no practical use ;-), but I am guessing you meant something else. Perhaps this is about using -O orderfile option, even though you happened to have implemented the id mixing at per-hunk level? [Footnote] *1* It has been a long time since I looked at the code, and I do not know offhand if git apply has such a bug not to diagnose a hunk for a file for an earlier part that comes later in its input stream after seeing another hunk for the same file as a bug. If it does not, perhaps we should. I started to remove that code, but then I recalled why I did it like this. There is a good reason. Yes, you can't simply reorder hunks just like this. But you can get the same effect by prefixing the header: --- x.txt 2014-03-27 19:31:18.166115449 +0200 +++ y.txt 2014-03-27 19:31:46.731116998 +0200 @@ -30,8 +31,6 @@ a a a a -a -b b b b @@ -60,6 +59,7 @@ b b b b +Y b b b --- x.txt 2014-03-27 19:31:18.166115449 +0200 +++ y.txt 2014-03-27 19:31:46.731116998 +0200 @@ -11,6 +11,7 @@ a a a a +X a a a Is equivalent to --- x.txt 2014-03-27 19:31:18.166115449 +0200 +++ y.txt 2014-03-27 19:31:46.731116998 +0200 @@ -30,8 +31,6 @@ a a a a -a -b b b b @@ -60,6 +59,7 @@ b b b b +Y b b b --- x.txt 2014-03-27 19:31:18.166115449 +0200 +++ y.txt 2014-03-27 19:31:46.731116998 +0200 @@ -11,6 +11,7 @@ a a a a +X a a a And this works fine with regular tools like patch so I think it should work for git too, anything else would be surprising. -- 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
Re: [PATCH 1/3] patch-id: make it stable against hunk reordering
Michael S. Tsirkin m...@redhat.com writes: I started to remove that code, but then I recalled why I did it like this. There is a good reason. Yes, you can't simply reorder hunks just like this. But you can get the same effect by prefixing the header: Yes, that is one of the things I personally have on the chopping block. Having to deal with more than occurrences of the same pathname in the input made things in builtin/apply.c unnecessarily complex and I do not see a real gain for being able to concatenate two patches and feed it into a single git apply invocation. -- 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: Possible regression in master? (submodules without a master branch)
Jens Lehmann jens.lehm...@web.de writes: Am 27.03.2014 16:52, schrieb W. Trevor King: On Thu, Mar 27, 2014 at 03:21:49PM +0100, Johan Herland wrote: I just found a failure to checkout a project with submodules where there is no explicit submodule branch configuration, and the submodules happen to not have a master branch: The docs say [1]: A remote branch name for tracking updates in the upstream submodule. If the option is not specified, it defaults to 'master'. But the branch setting isn't configured for Qt, the .gitmodules file contains only this: [submodule qtbase] path = qtbase url = ../qtbase.git ... which is what we do now. Working around that to default to the upstream submodule's HEAD is possible (you can just use --branch HEAD), but I think it's easier to just explicitly specify your preferred branch. That is *not* easier, as Johan did not have to do that before. I think your patch 23d25e48f5ead73c9ce233986f90791abec9f1e8 does not do what the commit message promised: With this change, folks cloning submodules for the first time via: $ git submodule update ... will get a local branch instead of a detached HEAD, unless they are using the default checkout-mode updates. And Qt uses the default checkout-mode updates and doesn't have branch configured either. So we are facing a serious regression here. There are two potential issues (and a half) then: - When cloning with the default checkout-mode updates, the new feature to avoid detaching the HEAD should not kick in at all. - For a repository that does not have that branch thing configured, the doc says that it will default to 'master'. I do not think this was brought up during the review, but is it a sensible default if the project does not even have that branch? What are viable alternatives? - use 'master' and fail just the way Johan saw? - use any random branch that happens to be at the same commit as what is being checked out? - use the branch clone for the submodule repository saw the upstream was pointing at with its HEAD? - something else? - Johan's set-up was apparently not covered in the addition to t/ in 23d25e48 (submodule: explicit local branch creation in module_clone, 2014-01-26)---otherwise we would have caught this regression. Are there other conditions that are not covered? -- 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 1/3] patch-id: make it stable against hunk reordering
On Thu, Mar 27, 2014 at 11:03:46AM -0700, Junio C Hamano wrote: Michael S. Tsirkin m...@redhat.com writes: I started to remove that code, but then I recalled why I did it like this. There is a good reason. Yes, you can't simply reorder hunks just like this. But you can get the same effect by prefixing the header: Yes, that is one of the things I personally have on the chopping block. Having to deal with more than occurrences of the same pathname in the input made things in builtin/apply.c unnecessarily complex and I do not see a real gain for being able to concatenate two patches and feed it into a single git apply invocation. Well - I expect that this will surprise some people: gnu patch accepts this, and it's a natural assumption that it works. There could be tools producing such diffs, too. Anyway - we can drop this from patch-id and git apply at the same time? -- 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
Re: [PATCH 1/3] patch-id: make it stable against hunk reordering
On Thu, Mar 27, 2014 at 08:39:17PM +0200, Michael S. Tsirkin wrote: On Thu, Mar 27, 2014 at 11:03:46AM -0700, Junio C Hamano wrote: Michael S. Tsirkin m...@redhat.com writes: I started to remove that code, but then I recalled why I did it like this. There is a good reason. Yes, you can't simply reorder hunks just like this. But you can get the same effect by prefixing the header: Yes, that is one of the things I personally have on the chopping block. Having to deal with more than occurrences of the same pathname in the input made things in builtin/apply.c unnecessarily complex and I do not see a real gain for being able to concatenate two patches and feed it into a single git apply invocation. Well - I expect that this will surprise some people: gnu patch accepts this, and it's a natural assumption that it works. There could be tools producing such diffs, too. This behaviour also seems to be explicitly required by POSIX: 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.) It's better to stick to standards even if it does require a bit more code, isn't it? Anyway - we can drop this from patch-id and git apply at the same time? -- 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
Re: [PATCH v2 14/19] tree-diff: rework diff_tree interface to be sha1 based
Kirill Smelkov k...@navytux.spb.ru writes: (please keep author email) 8 From: Kirill Smelkov k...@mns.spb.ru Date: Mon, 24 Feb 2014 20:21:46 +0400 Subject: [PATCH v3a] tree-diff: rework diff_tree interface to be sha1 based git am -c will discard everything above the scissors and then start parsing the in-body headers from there, so the above From: will be used. But you have a few entries in .mailmap; do you want to update them as well? 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. 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: Bug report: Git 1.8 on Ubuntu 13.10 refs not valid
On Thu, Mar 27, 2014 at 03:45:34PM +0100, Siggi wrote: and git version git:amd64/saucy 1:1.8.3.2-1 uptodate my remote repository is on a Chiliprojekt server (a fork of Redmine). cloning the repo over http results in following error: sneher@sneher-XPS:~/Dokumente/test$ git clone http://sne...@git.projects.gwdg.de/xrd-csd.git Klone nach 'xrd-csd'... Password for 'http://sne...@git.projects.gwdg.de': fatal: http://sne...@git.projects.gwdg.de/xrd-csd.git/info/refs not valid: is this a git repository? Hmm. The only way to trigger that message is if the dumb info/refs output from the server is not valid. In particular, it is looking for the tab character between the sha1 and the refs, and making sure that the sha1 is exactly 40 characters. I noticed other people having the problem, too: https://github.com/kubitron/redmine_git_hosting/issues/106 so I think it is related to the output that the redmine plugin is producing. But the interesting thing is that the plugin is supposed to enable git's smart-http protocol. But the error message you are seeing indicates that the client thinks it is doing a dumb http fetch. The parsing code did not change in the v1.8.x series, but the logic to determine whether we are using smart/dumb http did change. For example, we now actually check the content-type returned by the server (which should be application/x-git-upload-pack-advertisement). Can you try running your clone with GIT_CURL_VERBOSE=1 in the environment? That should show the headers (including content-type). Do be careful when sharing the output; I believe it will contain Authorization lines that have your base64-encoded password on them. Also, I would be curious to see the output of: curl http://sne...@git.projects.gwdg.de/xrd-csd.git/info/refs | cat -A My suspicion is that it is really smart-http output, but the client is parsing it as dumb-http output (and probably because of the content-type mentioned above). -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 00/10] [RFC] pickaxe for function names
This series introduces a --function-name=pattern option for git-log, intended to search for commits which touch a function matching a certain pattern (a feature we've seen requested and are interested in using ourselves). This is our first attempt to patch git; we've tried to observe and follow the community standards, but we would greatly appreciate feedback. We've been working on this for a few weeks, and I just noticed that René Scharfe has done conflicing (and better) refactoring work in diffcore-pickaxe.c a few days ago. We'd be happy to rebase our changes and resolve the conflicts once René's patches are committed to master, but we thought we may as well ask for comments on the version we have working now. Thanks for your time! .gitattributes: specify the language used diffcore-pickaxe.c: refactor regex compilation diffcore-pickaxe.c: Refactor pickaxe_fn signature diff.c/diff.h: expose userdiff_funcname diffcore-pickaxe.c: set up funcname pattern log: --function-name pickaxe xdiff: add XDL_EMIT_MOREFUNCNAMES to try harder xdiff: add XDL_EMIT_MOREHUNKHEADS to split hunks t4213: test --function-name option Documentation: Document --function-name usage .gitattributes | 2 +- Documentation/diff-options.txt | 9 +++ Documentation/gitdiffcore.txt | 17 - builtin/log.c | 2 +- diff.c | 13 +++- diff.h | 3 + diffcore-pickaxe.c | 162 +++--- revision.c | 3 +- t/t4213-log-function-name.sh | 73 + xdiff/xdiff.h | 2 + xdiff/xdiffi.c | 2 +- xdiff/xemit.c | 99 ++-- xdiff/xemit.h | 4 +- 13 files changed, 323 insertions(+), 68 deletions(-) -- 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 09/10] t4213: test --function-name option
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.
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
+
+ printf void newfunc(){\n\treturn;\n}\n file
+ test_tick
+ git commit -am third
+
+ printf void newfunc2(){\n\treturn;\n}\n | cat - file temp
+ mv temp file
+ test_tick
+ git commit -am fourth
+
+ 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' '
+ git log --function-name=main actual
+ git log --grep=second expect
+ test_cmp expect actual
+'
+
+test_expect_success 'log --function-name newfunc\W' '
+ 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
--
1.7.12.4 (Apple Git-37)
--
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 05/10] diffcore-pickaxe.c: set up funcname pattern
From: Bhushan G. Lodha David A. Dalrymple dad-...@mit.edu
We use userdiff_funcname to make the filetype-dependent function name
pattern available to pickaxe functions.
Signed-off-by: David Dalrymple (on zayin) davi...@alum.mit.edu
---
diffcore-pickaxe.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index 7e65095..103fe6c 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -7,10 +7,12 @@
#include diffcore.h
#include xdiff-interface.h
#include kwset.h
+#include userdiff.h
struct fn_options {
regex_t *regex;
kwset_t kws;
+ const struct userdiff_funcname *funcname_pattern;
};
typedef int (*pickaxe_fn)(mmfile_t *one, mmfile_t *two,
@@ -224,6 +226,13 @@ static int pickaxe_match(struct diff_filepair *p, struct
diff_options *o,
if (textconv_one == textconv_two diff_unmodified_pair(p))
return 0;
+ const struct userdiff_funcname *funcname_pattern;
+ funcname_pattern = diff_funcname_pattern(p-one);
+ if (!funcname_pattern)
+ funcname_pattern = diff_funcname_pattern(p-two);
+
+ fno-funcname_pattern = funcname_pattern;
+
mf1.size = fill_textconv(textconv_one, p-one, mf1.ptr);
mf2.size = fill_textconv(textconv_two, p-two, mf2.ptr);
--
1.7.12.4 (Apple Git-37)
--
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 06/10] log: --function-name pickaxe
From: Bhushan G. Lodha David A. Dalrymple dad-...@mit.edu
This is similar to the pickaxe grep option (-G), but applies the
provided regex only to diff hunk headers, thereby showing only those
commits which affect a function with a definition line matching the
pattern. These are functions in the same sense as with
--function-context, i.e., they may be classes, structs, etc. depending
on the programming-language-specific pattern specified by the diff
attribute in .gitattributes.
builtin/log.c:
as with pickaxe, set always_show_header when using --function-name
diff.c:
parse option; as with pickaxe, always set the RECURSIVE option
for --function-name
diff.h:
include funcname field in struct diff_options
diffcore-pickaxe.c:
implementation of --function-name filtering (diffcore_funcname), like
the existing diffcore_pickaxe_grep and diffcore_pickaxe_count
revision.c:
as with pickaxe, set revs-diff to always generate diffs when
using --function-name
Signed-off-by: David Dalrymple (on zayin) davi...@alum.mit.edu
---
builtin/log.c | 2 +-
diff.c | 8 +--
diff.h | 1 +
diffcore-pickaxe.c | 69 --
revision.c | 3 ++-
5 files changed, 77 insertions(+), 6 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index b97373d..78694de 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -158,7 +158,7 @@ static void cmd_log_init_finish(int argc, const char
**argv, const char *prefix,
if (rev-show_notes)
init_display_notes(rev-notes_opt);
- if (rev-diffopt.pickaxe || rev-diffopt.filter)
+ if (rev-diffopt.pickaxe || rev-diffopt.filter ||
rev-diffopt.funcname)
rev-always_show_header = 0;
if (DIFF_OPT_TST(rev-diffopt, FOLLOW_RENAMES)) {
rev-always_show_header = 0;
diff --git a/diff.c b/diff.c
index f978ee7..2f6dbc1 100644
--- a/diff.c
+++ b/diff.c
@@ -3298,7 +3298,7 @@ void diff_setup_done(struct diff_options *options)
/*
* Also pickaxe would not work very well if you do not say recursive
*/
- if (options-pickaxe)
+ if (options-pickaxe || options-funcname)
DIFF_OPT_SET(options, RECURSIVE);
/*
* When patches are generated, submodules diffed against the work tree
@@ -3821,6 +3821,10 @@ int diff_opt_parse(struct diff_options *options, const
char **av, int ac)
options-orderfile = optarg;
return argcount;
}
+ else if ((argcount = parse_long_opt(function-name, av, optarg))) {
+ options-funcname = optarg;
+ return argcount;
+ }
else if ((argcount = parse_long_opt(diff-filter, av, optarg))) {
int offending = parse_diff_filter_opt(optarg, options);
if (offending)
@@ -4768,7 +4772,7 @@ void diffcore_std(struct diff_options *options)
if (options-break_opt != -1)
diffcore_merge_broken();
}
- if (options-pickaxe)
+ if (options-pickaxe || options-funcname)
diffcore_pickaxe(options);
if (options-orderfile)
diffcore_order(options-orderfile);
diff --git a/diff.h b/diff.h
index 9e96fc9..0fd5f1d 100644
--- a/diff.h
+++ b/diff.h
@@ -107,6 +107,7 @@ enum diff_words_type {
struct diff_options {
const char *orderfile;
const char *pickaxe;
+ const char *funcname;
const char *single_follow;
const char *a_prefix, *b_prefix;
unsigned flags;
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index 103fe6c..259a8fa 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -67,6 +67,12 @@ struct diffgrep_cb {
int hit;
};
+struct funcname_cb {
+ struct userdiff_funcname *pattern;
+ regex_t *regex;
+ int hit;
+};
+
static void diffgrep_consume(void *priv, char *line, unsigned long len)
{
struct diffgrep_cb *data = priv;
@@ -88,6 +94,20 @@ static void diffgrep_consume(void *priv, char *line,
unsigned long len)
line[len] = hold;
}
+static void match_funcname(void *priv, char *line, unsigned long len)
+{
+ regmatch_t regmatch;
+ int hold;
+ struct funcname_cb *data = priv;
+ hold = line[len];
+ line[len] = '\0';
+
+ if (line[0] == '@' line[1] == '@')
+ if (!regexec(data-regex, line, 1, regmatch, 0))
+ data-hit = 1;
+ line[len] = hold;
+}
+
static int diff_grep(mmfile_t *one, mmfile_t *two,
struct diff_options *o,
struct fn_options *fno)
@@ -117,6 +137,38 @@ static int diff_grep(mmfile_t *one, mmfile_t *two,
return ecbdata.hit;
}
+static int diff_funcname_filter(mmfile_t *one, mmfile_t *two,
+ struct diff_options *o,
+ struct fn_options *fno)
+{
[PATCH 04/10] diff.c/diff.h: expose userdiff_funcname
From: Bhushan G. Lodha David A. Dalrymple dad-...@mit.edu
The functionality of userdiff_funcname (determining the language in use
for a given file and setting up patterns to match function names in
that language) is useful outside of diff.c, so here we remove its static
specifier and declare it in diff.h.
Signed-off-by: David Dalrymple (on zayin) davi...@alum.mit.edu
---
diff.c | 2 +-
diff.h | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/diff.c b/diff.c
index e343191..f978ee7 100644
--- a/diff.c
+++ b/diff.c
@@ -2203,7 +2203,7 @@ int diff_filespec_is_binary(struct diff_filespec *one)
return one-is_binary;
}
-static const struct userdiff_funcname *diff_funcname_pattern(struct
diff_filespec *one)
+const struct userdiff_funcname *diff_funcname_pattern(struct diff_filespec
*one)
{
diff_filespec_load_driver(one);
return one-driver-funcname.pattern ? one-driver-funcname : NULL;
diff --git a/diff.h b/diff.h
index a24a767..9e96fc9 100644
--- a/diff.h
+++ b/diff.h
@@ -349,4 +349,6 @@ extern int print_stat_summary(FILE *fp, int files,
int insertions, int deletions);
extern void setup_diff_pager(struct diff_options *);
+const struct userdiff_funcname *diff_funcname_pattern(struct diff_filespec *);
+
#endif /* DIFF_H */
--
1.7.12.4 (Apple Git-37)
--
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 02/10] diffcore-pickaxe.c: refactor regex compilation
From: Bhushan G. Lodha David A. Dalrymple dad-...@mit.edu
In this file, two functions use identical blocks of code to call the
POSIX regex compiling function and handle a possible error. Here we
factor that block into its own function, in anticipation of using the
same code a third time.
Signed-off-by: David Dalrymple (on zayin) davi...@alum.mit.edu
---
diffcore-pickaxe.c | 33 -
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index 401eb72..0d36a3c 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -12,6 +12,8 @@ typedef int (*pickaxe_fn)(mmfile_t *one, mmfile_t *two,
struct diff_options *o,
regex_t *regexp, kwset_t kws);
+static void compile_regex(regex_t *r, const char *s, int cflags);
+
static int pickaxe_match(struct diff_filepair *p, struct diff_options *o,
regex_t *regexp, kwset_t kws, pickaxe_fn fn);
@@ -110,20 +112,13 @@ static int diff_grep(mmfile_t *one, mmfile_t *two,
static void diffcore_pickaxe_grep(struct diff_options *o)
{
- int err;
regex_t regex;
int cflags = REG_EXTENDED | REG_NEWLINE;
if (DIFF_OPT_TST(o, PICKAXE_IGNORE_CASE))
cflags |= REG_ICASE;
- err = regcomp(regex, o-pickaxe, cflags);
- if (err) {
- char errbuf[1024];
- regerror(err, regex, errbuf, 1024);
- regfree(regex);
- die(invalid regex: %s, errbuf);
- }
+ compile_regex(regex, o-pickaxe, cflags);
pickaxe(diff_queued_diff, o, regex, NULL, diff_grep);
@@ -180,6 +175,18 @@ static int has_changes(mmfile_t *one, mmfile_t *two,
return one_contains != two_contains;
}
+static void compile_regex(regex_t *r, const char *s, int cflags)
+{
+ int err;
+ err = regcomp(r, s, cflags);
+ if (err) {
+ char errbuf[1024];
+ regerror(err, r, errbuf, 1024);
+ regfree(r);
+ die(invalid regex: %s, errbuf);
+ }
+}
+
static int pickaxe_match(struct diff_filepair *p, struct diff_options *o,
regex_t *regexp, kwset_t kws, pickaxe_fn fn)
{
@@ -236,15 +243,7 @@ static void diffcore_pickaxe_count(struct diff_options *o)
kwset_t kws = NULL;
if (opts DIFF_PICKAXE_REGEX) {
- int err;
- err = regcomp(regex, needle, REG_EXTENDED | REG_NEWLINE);
- if (err) {
- /* The POSIX.2 people are surely sick */
- char errbuf[1024];
- regerror(err, regex, errbuf, 1024);
- regfree(regex);
- die(invalid regex: %s, errbuf);
- }
+ compile_regex(regex, needle, REG_EXTENDED | REG_NEWLINE);
regexp = regex;
} else {
kws = kwsalloc(DIFF_OPT_TST(o, PICKAXE_IGNORE_CASE)
--
1.7.12.4 (Apple Git-37)
--
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 03/10] diffcore-pickaxe.c: Refactor pickaxe_fn signature
From: Bhushan G. Lodha David A. Dalrymple dad-...@mit.edu
This function type previously accepted separate regex_t and kwset_t
parameters, which conceptually go together. Here we create a struct to
encapsulate them, in anticipation of adding a third field that
pickaxe_fn's may require.
This parallels the existing diffgrep_cb structure for passing possibly
relevant values through to the callbacks invoked by xdi_diff_outf.
Signed-off-by: David Dalrymple (on zayin) davi...@alum.mit.edu
---
diffcore-pickaxe.c | 50 ++
1 file changed, 30 insertions(+), 20 deletions(-)
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index 0d36a3c..7e65095 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -8,17 +8,22 @@
#include xdiff-interface.h
#include kwset.h
+struct fn_options {
+ regex_t *regex;
+ kwset_t kws;
+};
+
typedef int (*pickaxe_fn)(mmfile_t *one, mmfile_t *two,
struct diff_options *o,
- regex_t *regexp, kwset_t kws);
+ struct fn_options *fno);
static void compile_regex(regex_t *r, const char *s, int cflags);
static int pickaxe_match(struct diff_filepair *p, struct diff_options *o,
-regex_t *regexp, kwset_t kws, pickaxe_fn fn);
+pickaxe_fn fn, struct fn_options *fno);
static void pickaxe(struct diff_queue_struct *q, struct diff_options *o,
- regex_t *regexp, kwset_t kws, pickaxe_fn fn)
+ pickaxe_fn fn, struct fn_options *fno)
{
int i;
struct diff_queue_struct outq;
@@ -29,7 +34,7 @@ static void pickaxe(struct diff_queue_struct *q, struct
diff_options *o,
/* Showing the whole changeset if needle exists */
for (i = 0; i q-nr; i++) {
struct diff_filepair *p = q-queue[i];
- if (pickaxe_match(p, o, regexp, kws, fn))
+ if (pickaxe_match(p, o, fn, fno))
return; /* do not munge the queue */
}
@@ -44,7 +49,7 @@ static void pickaxe(struct diff_queue_struct *q, struct
diff_options *o,
/* Showing only the filepairs that has the needle */
for (i = 0; i q-nr; i++) {
struct diff_filepair *p = q-queue[i];
- if (pickaxe_match(p, o, regexp, kws, fn))
+ if (pickaxe_match(p, o, fn, fno))
diff_q(outq, p);
else
diff_free_filepair(p);
@@ -83,7 +88,7 @@ static void diffgrep_consume(void *priv, char *line, unsigned
long len)
static int diff_grep(mmfile_t *one, mmfile_t *two,
struct diff_options *o,
-regex_t *regexp, kwset_t kws)
+struct fn_options *fno)
{
regmatch_t regmatch;
struct diffgrep_cb ecbdata;
@@ -91,9 +96,9 @@ static int diff_grep(mmfile_t *one, mmfile_t *two,
xdemitconf_t xecfg;
if (!one)
- return !regexec(regexp, two-ptr, 1, regmatch, 0);
+ return !regexec(fno-regex, two-ptr, 1, regmatch, 0);
if (!two)
- return !regexec(regexp, one-ptr, 1, regmatch, 0);
+ return !regexec(fno-regex, one-ptr, 1, regmatch, 0);
/*
* We have both sides; need to run textual diff and see if
@@ -101,7 +106,7 @@ static int diff_grep(mmfile_t *one, mmfile_t *two,
*/
memset(xpp, 0, sizeof(xpp));
memset(xecfg, 0, sizeof(xecfg));
- ecbdata.regexp = regexp;
+ ecbdata.regexp = fno-regex;
ecbdata.hit = 0;
xecfg.ctxlen = o-context;
xecfg.interhunkctxlen = o-interhunkcontext;
@@ -113,6 +118,7 @@ static int diff_grep(mmfile_t *one, mmfile_t *two,
static void diffcore_pickaxe_grep(struct diff_options *o)
{
regex_t regex;
+ struct fn_options fno;
int cflags = REG_EXTENDED | REG_NEWLINE;
if (DIFF_OPT_TST(o, PICKAXE_IGNORE_CASE))
@@ -120,13 +126,14 @@ static void diffcore_pickaxe_grep(struct diff_options *o)
compile_regex(regex, o-pickaxe, cflags);
- pickaxe(diff_queued_diff, o, regex, NULL, diff_grep);
+ fno.regex = regex;
+ pickaxe(diff_queued_diff, o, diff_grep, fno);
regfree(regex);
return;
}
-static unsigned int contains(mmfile_t *mf, regex_t *regexp, kwset_t kws)
+static unsigned int contains(mmfile_t *mf, struct fn_options *fno)
{
unsigned int cnt;
unsigned long sz;
@@ -136,12 +143,12 @@ static unsigned int contains(mmfile_t *mf, regex_t
*regexp, kwset_t kws)
data = mf-ptr;
cnt = 0;
- if (regexp) {
+ if (fno-regex) {
regmatch_t regmatch;
int flags = 0;
assert(data[sz] == '\0');
- while (*data !regexec(regexp, data, 1,
[PATCH 10/10] Documentation: Document --function-name usage
From: Bhushan Lodha David A. Dalrymple dad-...@mit.edu Signed-off-by: David Dalrymple (on zayin) davi...@alum.mit.edu --- Documentation/diff-options.txt | 9 + Documentation/gitdiffcore.txt | 17 ++--- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index 9b37b2a..a778dff 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -427,6 +427,15 @@ information. --pickaxe-regex:: Treat the string given to `-S` as an extended POSIX regular expression to match. + +--function-nameregex:: + Look for differences whose patch text contains hunk headers that match + regex. This can be useful to locate changes to a particular function + or other semantic element in a program, since hunk headers are intended + to indicate the function context (in the sense of + `--function-context`) in which the particular change occurs. The + function context is determined by the diff driver corresponding to the + file in question; see linkgit:gitattributes[7] for details. endif::git-format-patch[] -Oorderfile:: diff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt index c8b3e51..b8477ce 100644 --- a/Documentation/gitdiffcore.txt +++ b/Documentation/gitdiffcore.txt @@ -222,10 +222,11 @@ version prefixed with '+'. diffcore-pickaxe: For Detecting Addition/Deletion of Specified String - -This transformation limits the set of filepairs to those that change +This transformation limits the set of filepairs to those that involve specified strings between the preimage and the postimage in a certain -way. -Sblock of text and -Gregular expression options are used to -specify different ways these strings are sought. +way. -Sblock of text, -Gregular expression, and +--function-nameregular expression options are used to specify +different ways these strings are sought. -Sblock of text detects filepairs whose preimage and postimage have different number of occurrences of the specified block of text. @@ -251,6 +252,16 @@ criterion in a changeset, the entire changeset is kept. This behavior is designed to make reviewing changes in the context of the whole changeset easier. +--function-nameregular expression detects filepairs whose textual +diff contains a hunk header that matches the given regular expression. +The hunk header is generated via the diff driver specified in +`.gitattributes`, and is intended to reflect the function context +(in the sense of `--function-context`) in which the change occurs, +with programming-language-dependent heuristics. As of now, the +programming language is not auto-detected in any way. Also note that +hunks whose headers do not match the regular expression are not +currently filtered out; this is only a filepair filter. + diffcore-order: For Sorting the Output Based on Filenames - -- 1.7.12.4 (Apple Git-37) -- 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 01/10] .gitattributes: specify the language used
From: Bhushan G. Lodha David A. Dalrymple dad-...@mit.edu Since git can intelligently emit diff hunk headers based on the programming language of each file, assuming that the language is specified in .gitattributes, it makes sense to specify our own language (cpp) in our own .gitattributes file. Signed-off-by: David Dalrymple (on zayin) davi...@alum.mit.edu --- .gitattributes | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitattributes b/.gitattributes index 5e98806..320e33c 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,3 +1,3 @@ * whitespace=!indent,trail,space -*.[ch] whitespace=indent,trail,space +*.[ch] whitespace=indent,trail,space diff=cpp *.sh whitespace=indent,trail,space -- 1.7.12.4 (Apple Git-37) -- 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 07/10] xdiff: add XDL_EMIT_MOREFUNCNAMES
From: Bhushan G. Lodha David A. Dalrymple dad-...@mit.edu
For filtering commits by function name, it's useful to identify the
function name in cases such as adding a new function to a file (where
the default functionality will not emit a function name in the hunk
header, because it isn't part of the context).
This adds a flag asking xdiff to be more aggressive in finding function
names to emit, and turns the flag on when the --function-name option is
in use.
Signed-off-by: David Dalrymple (on zayin) davi...@alum.mit.edu
---
diff.c | 2 ++
diffcore-pickaxe.c | 2 +-
xdiff/xdiff.h | 1 +
xdiff/xemit.c | 39 +++
4 files changed, 27 insertions(+), 17 deletions(-)
diff --git a/diff.c b/diff.c
index 2f6dbc1..914b4a2 100644
--- a/diff.c
+++ b/diff.c
@@ -2380,6 +2380,8 @@ static void builtin_diff(const char *name_a,
xecfg.ctxlen = o-context;
xecfg.interhunkctxlen = o-interhunkcontext;
xecfg.flags = XDL_EMIT_FUNCNAMES;
+ if (o-funcname)
+ xecfg.flags |= XDL_EMIT_MOREFUNCNAMES;
if (DIFF_OPT_TST(o, FUNCCONTEXT))
xecfg.flags |= XDL_EMIT_FUNCCONTEXT;
if (pe)
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index 259a8fa..ab31c18 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -164,7 +164,7 @@ static int diff_funcname_filter(mmfile_t *one, mmfile_t
*two,
xecfg.interhunkctxlen = o-interhunkcontext;
if (!(one two))
xecfg.flags = XDL_EMIT_FUNCCONTEXT;
- xecfg.flags |= XDL_EMIT_FUNCNAMES;
+ xecfg.flags |= XDL_EMIT_FUNCNAMES | XDL_EMIT_MOREFUNCNAMES;
xdi_diff_outf(one, two, match_funcname, ecbdata, xpp, xecfg);
return ecbdata.hit;
}
diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
index c033991..469bded 100644
--- a/xdiff/xdiff.h
+++ b/xdiff/xdiff.h
@@ -44,6 +44,7 @@
#define XDL_EMIT_FUNCNAMES (1 0)
#define XDL_EMIT_COMMON (1 1)
#define XDL_EMIT_FUNCCONTEXT (1 2)
+#define XDL_EMIT_MOREFUNCNAMES (1 3)
#define XDL_MMB_READONLY (1 0)
diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index 4266ada..0ddb094 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -23,6 +23,10 @@
#include xinclude.h
+struct func_line {
+ long len;
+ char buf[80];
+};
static long xdl_get_rec(xdfile_t *xdf, long ri, char const **rec);
@@ -135,12 +139,7 @@ static int xdl_emit_common(xdfenv_t *xe, xdchange_t *xscr,
xdemitcb_t *ecb,
return 0;
}
-struct func_line {
- long len;
- char buf[80];
-};
-
-static long get_func_line(xdfenv_t *xe, xdemitconf_t const *xecfg,
+static long get_func_line(xdfile_t *xdf, xdemitconf_t const *xecfg,
struct func_line *func_line, long start, long limit)
{
find_func_t ff = xecfg-find_func ? xecfg-find_func : def_ff;
@@ -150,9 +149,9 @@ static long get_func_line(xdfenv_t *xe, xdemitconf_t const
*xecfg,
buf = func_line ? func_line-buf : dummy;
size = func_line ? sizeof(func_line-buf) : sizeof(dummy);
- for (l = start; l != limit 0 = l l xe-xdf1.nrec; l += step) {
+ for (l = start; l != limit 0 = l l xdf-nrec; l += step) {
const char *rec;
- long reclen = xdl_get_rec(xe-xdf1, l, rec);
+ long reclen = xdl_get_rec(xdf, l, rec);
long len = ff(rec, reclen, buf, size, xecfg-find_func_priv);
if (len = 0) {
if (func_line)
@@ -167,7 +166,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr,
xdemitcb_t *ecb,
xdemitconf_t const *xecfg) {
long s1, s2, e1, e2, lctx;
xdchange_t *xch, *xche;
- long funclineprev = -1;
+ long funclineprev1 = -1, funclineprev2 = -1;
struct func_line func_line = { 0 };
if (xecfg-flags XDL_EMIT_COMMON)
@@ -182,7 +181,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr,
xdemitcb_t *ecb,
s2 = XDL_MAX(xch-i2 - xecfg-ctxlen, 0);
if (xecfg-flags XDL_EMIT_FUNCCONTEXT) {
- long fs1 = get_func_line(xe, xecfg, NULL, xch-i1, -1);
+ long fs1 = get_func_line(xe-xdf1, xecfg, NULL,
xch-i1, -1);
if (fs1 0)
fs1 = 0;
if (fs1 s1) {
@@ -200,7 +199,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr,
xdemitcb_t *ecb,
e2 = xche-i2 + xche-chg2 + lctx;
if (xecfg-flags XDL_EMIT_FUNCCONTEXT) {
- long fe1 = get_func_line(xe, xecfg, NULL,
+ long fe1 = get_func_line(xe-xdf1, xecfg, NULL,
xche-i1 + xche-chg1,
xe-xdf1.nrec);
if (fe1 0)
@@ -218,7 +217,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr,
xdemitcb_t *ecb,
[PATCH 08/10] xdiff: add XDL_EMIT_MOREHUNKHEADS
From: Bhushan G. Lodha David A. Dalrymple dad-...@mit.edu
For filtering by function names, it's useful to split hunks whenever a
function line is encountered, so that each function name being deleted
or inserted gets its own hunk header (which then can be easily detected
by the filter).
This adds a flag, XDL_EMIT_MOREHUNKHEADS, which triggers this
nonstandard behavior, and enables it only in case the --function-name
option is being used.
Signed-off-by: David Dalrymple (on zayin) davi...@alum.mit.edu
---
diff.c | 3 ++-
diffcore-pickaxe.c | 3 ++-
xdiff/xdiff.h | 1 +
xdiff/xdiffi.c | 2 +-
xdiff/xemit.c | 60 --
xdiff/xemit.h | 4 +++-
6 files changed, 67 insertions(+), 6 deletions(-)
diff --git a/diff.c b/diff.c
index 914b4a2..a86206c 100644
--- a/diff.c
+++ b/diff.c
@@ -2381,7 +2381,8 @@ static void builtin_diff(const char *name_a,
xecfg.interhunkctxlen = o-interhunkcontext;
xecfg.flags = XDL_EMIT_FUNCNAMES;
if (o-funcname)
- xecfg.flags |= XDL_EMIT_MOREFUNCNAMES;
+ xecfg.flags |= XDL_EMIT_MOREFUNCNAMES
+ | XDL_EMIT_MOREHUNKHEADS;
if (DIFF_OPT_TST(o, FUNCCONTEXT))
xecfg.flags |= XDL_EMIT_FUNCCONTEXT;
if (pe)
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index ab31c18..d9f4c30 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -164,7 +164,8 @@ static int diff_funcname_filter(mmfile_t *one, mmfile_t
*two,
xecfg.interhunkctxlen = o-interhunkcontext;
if (!(one two))
xecfg.flags = XDL_EMIT_FUNCCONTEXT;
- xecfg.flags |= XDL_EMIT_FUNCNAMES | XDL_EMIT_MOREFUNCNAMES;
+ xecfg.flags |= XDL_EMIT_FUNCNAMES | XDL_EMIT_MOREFUNCNAMES
+ | XDL_EMIT_MOREHUNKHEADS;
xdi_diff_outf(one, two, match_funcname, ecbdata, xpp, xecfg);
return ecbdata.hit;
}
diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
index 469bded..787c376 100644
--- a/xdiff/xdiff.h
+++ b/xdiff/xdiff.h
@@ -45,6 +45,7 @@
#define XDL_EMIT_COMMON (1 1)
#define XDL_EMIT_FUNCCONTEXT (1 2)
#define XDL_EMIT_MOREFUNCNAMES (1 3)
+#define XDL_EMIT_MOREHUNKHEADS (1 4)
#define XDL_MMB_READONLY (1 0)
diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c
index 2358a2d..c29804e 100644
--- a/xdiff/xdiffi.c
+++ b/xdiff/xdiffi.c
@@ -545,7 +545,7 @@ static int xdl_call_hunk_func(xdfenv_t *xe, xdchange_t
*xscr, xdemitcb_t *ecb,
xdchange_t *xch, *xche;
for (xch = xscr; xch; xch = xche-next) {
- xche = xdl_get_hunk(xch, xecfg);
+ xche = xdl_get_hunk(xe, xch, xecfg);
if (!xch)
break;
if (xecfg-hunk_func(xch-i1, xche-i1 + xche-chg1 - xch-i1,
diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index 0ddb094..f49eaaf 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -29,6 +29,9 @@ struct func_line {
};
+static long get_func_line(xdfile_t *xdf, xdemitconf_t const *xecfg,
+ struct func_line *func_line, long start, long limit);
+
static long xdl_get_rec(xdfile_t *xdf, long ri, char const **rec);
static int xdl_emit_record(xdfile_t *xdf, long ri, char const *pre, xdemitcb_t
*ecb);
@@ -62,7 +65,7 @@ static int xdl_emit_record(xdfile_t *xdf, long ri, char const
*pre, xdemitcb_t *
* inside the differential hunk according to the specified configuration.
* Also advance xscr if the first changes must be discarded.
*/
-xdchange_t *xdl_get_hunk(xdchange_t **xscr, xdemitconf_t const *xecfg)
+xdchange_t *xdl_get_hunk(xdfenv_t *xe, xdchange_t **xscr, xdemitconf_t const
*xecfg)
{
xdchange_t *xch, *xchp, *lxch;
long max_common = 2 * xecfg-ctxlen + xecfg-interhunkctxlen;
@@ -83,6 +86,59 @@ xdchange_t *xdl_get_hunk(xdchange_t **xscr, xdemitconf_t
const *xecfg)
lxch = *xscr;
+ if (xecfg-flags XDL_EMIT_MOREHUNKHEADS)
+ for (xch = *xscr; xch; xch=xch-next) {
+ /*
+* If a current change contains a func_line, end this
+* hunk immediately before and create a new hunk
+* starting from that line.
+*/
+ long fl_in_xch1 = get_func_line(xe-xdf1, xecfg, NULL,
+ xch-i1, xch-i1+xch-chg1);
+ long fl_in_xch2 = get_func_line(xe-xdf2, xecfg, NULL,
+ xch-i2, xch-i2+xch-chg2);
+ if (fl_in_xch1 = xch-i1 fl_in_xch2 = xch-i2) {
+ xdchange_t *new_next =
+ (xdchange_t
*)xdl_malloc(sizeof(xdchange_t));
+ new_next-i1 = xch-i1+xch-chg1;
+ new_next-chg1 = 0;
+
Re: [git] Re: Possible regression in master? (submodules without a master branch)
On Thu, Mar 27, 2014 at 06:31:27PM +0100, Jens Lehmann wrote:
Am 27.03.2014 18:16, schrieb Junio C Hamano:
Johan Herland jo...@herland.net writes:
I just found a failure to checkout a project with submodules where
there is no explicit submodule branch configuration, and the
submodules happen to not have a master branch:
git clone git://gitorious.org/qt/qt5.git qt5
cd qt5
git submodule init qtbase
git submodule update
In current master, the last command fails with the following output:
... and with a bug-free system, what does it do instead? Just clone
'qtbase' and make a detached-head checkout at the commit recorded in
the superproject's tree, or something else?
After reverting 23d25e48f5ead73 on current master it clones 'qtbase'
nicely with a detached HEAD.
Fixing this for initial update clone is pretty easy, we just need to
unset start_point before calling module_clone if
submodule.name.branch is not set. However, that's just going to
push remote branch ambiguity problems back to the --remote update
functionality. What should happen when submodule.name.branch is not
set and you run a --remote update, which has used:
git rev-parse ${remote_name}/${branch}
since the submodule.name.branch setting was introduced in 06b1abb
(submodule update: add --remote for submodule's upstream changes,
2012-12-19)?
gitmodules(5) is pretty clear that 'submodule.name.branch' defaults
to master (and not upstream's HEAD), do we want to adjust this at the
same time? For folks using non-checkout updates, should the preferred
local branch name still be master, or should it match upstream's HEAD?
If upstream's HEAD changes, should we update the local branch name to
stay in sync? If we don't rename the local branch, do we keep
integrating remote changes from upstream's original branch or keep
integrating HEAD?
I think this would all be simpler if we just made the
superproject-branch-to-submodule-local-branch binding explicit and
pushed this submodule-to-upstream-subproject binding down into the
submodule itself [1]. Then the usual single-project commands would
handle the tricky remote-tracking cases (with explicit
branch.name.merge entries, etc.), and a dumb syncing mechanism would
pull those clever choices back up into the superproject for
distribution.
If an existing set-up that was working in a sensible way is broken
by a change that assumes something that should not be assumed,
then that is a serious regression, I would have to say.
Yes, especially as it promised to not change this use case.
Sorry. A side effect of relying too much on our existing
documentation and not enough on testing actual use cases. I can work
up some non-master submodule tests to go with the fix.
Cheers,
Trevor
[1]: http://thread.gmane.org/gmane.comp.version-control.git/239955/focus=240336
--
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 00/10] [RFC] pickaxe for function names
On Thu, Mar 27, 2014 at 02:50:46PM -0400, David A. Dalrymple (and Bhushan G. Lodha) wrote: This series introduces a --function-name=pattern option for git-log, intended to search for commits which touch a function matching a certain pattern (a feature we've seen requested and are interested in using ourselves). How does your feature compare with the line-level history viewer? E.g.: git log -L:myfunc:foo.c I guess by being part of pickaxe, it can be used to generally select commits (whereas -L is about drilling down a particular set of lines, so something like --full-diff would not work). But -L can do many things that pickaxe can't. It is not just about finding lines touched within a pattern, but uses the pattern to determine an initial set of lines, and then recursively blames those lines going back through history. So how you specify the pattern is more flexible (you can give any line range, for example), and it should be able to cross boundaries like function renames. -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: Possible regression in master? (submodules without a master branch)
W. Trevor King wk...@tremily.us writes:
On Thu, Mar 27, 2014 at 06:31:27PM +0100, Jens Lehmann wrote:
Am 27.03.2014 18:16, schrieb Junio C Hamano:
Johan Herland jo...@herland.net writes:
I just found a failure to checkout a project with submodules where
there is no explicit submodule branch configuration, and the
submodules happen to not have a master branch:
git clone git://gitorious.org/qt/qt5.git qt5
cd qt5
git submodule init qtbase
git submodule update
In current master, the last command fails with the following output:
... and with a bug-free system, what does it do instead? Just clone
'qtbase' and make a detached-head checkout at the commit recorded in
the superproject's tree, or something else?
After reverting 23d25e48f5ead73 on current master it clones 'qtbase'
nicely with a detached HEAD.
Fixing this for initial update clone is pretty easy, we just need to
unset start_point before calling module_clone if
submodule.name.branch is not set.
There is this bit for update in git-submodule.txt:
For updates that clone missing submodules, checkout-mode updates
will create submodules with detached HEADs; all other modes will
create submodules with a local branch named after
submodule.path.branch.
[side note] Isn't that a typo of submodule.name.branch?
So the proposed change is to make the part before semicolon true?
If we are not newly cloning (because we already have it), if the
submodule.name.branch is not set *OR* refers to a branch that does
not even exist, shouldn't we either (1) abort as an error, or (2) do
the same and detach?
However, that's just going to
push remote branch ambiguity problems back to the --remote update
functionality. What should happen when submodule.name.branch is not
set and you run a --remote update, which has used:
git rev-parse ${remote_name}/${branch}
since the submodule.name.branch setting was introduced in 06b1abb
(submodule update: add --remote for submodule's upstream changes,
2012-12-19)?
Isn't --remote about following one specific branch the user who
issues that command has in mind? If you as the end user did not
give any indication which branch you meant, e.g. by leaving the
submodule.name.branch empty, shouldn't that be diagnosed as an
error?
gitmodules(5) is pretty clear that 'submodule.name.branch' defaults
to master (and not upstream's HEAD), do we want to adjust this at the
same time?
That may be likely. If the value set to a configuration variable
causes an established behaviour of a program change a lot, silently
defaulting that variable to something many people are expected to
have (e.g. 'master') would likely to cause a usability regression.
If an existing set-up that was working in a sensible way is broken
by a change that assumes something that should not be assumed,
then that is a serious regression, I would have to say.
Yes, especially as it promised to not change this use case.
Sorry. A side effect of relying too much on our existing
documentation and not enough on testing actual use cases. I can work
up some non-master submodule tests to go with the fix.
I was wondering if we need to revert the merge with that
branch out of 'master', or submodule folks can work on a set of
fixes to apply on top.
Will wait to see how it goes. 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
+stefanbeller On Thu, Mar 27, 2014 at 11:48:11AM -0700, Junio C Hamano wrote: Kirill Smelkov k...@navytux.spb.ru writes: (please keep author email) 8 From: Kirill Smelkov k...@mns.spb.ru Date: Mon, 24 Feb 2014 20:21:46 +0400 Subject: [PATCH v3a] tree-diff: rework diff_tree interface to be sha1 based git am -c will discard everything above the scissors and then start parsing the in-body headers from there, so the above From: will be used. Thanks. But you have a few entries in .mailmap; do you want to update them as well? When Stefan Beller was contacting me on emails, if I recall correctly, I told him all those kirr@... entries are mine, but the one this patch is authored with indicates that something was done at work, and I'd prefer to acknowledge that. So maybe 8 From: Kirill Smelkov k...@navytux.spb.ru Date: Thu, 27 Mar 2014 23:32:14 +0400 Subject: [PATCH] .mailmap: Separate Kirill Smelkov personal and work addresses The address k...@mns.spb.ru indicates that a patch was done at work and I'd like to acknowledge that. The address k...@navytux.spb.ru is my personal email and indicates that a contribution is done completely on my own time and resources. k...@landau.phys.spbu.ru is old university account which no longer works (sigh, to much spam because of me on the server) and maps to k...@navytux.spb.ru which should be considered as primary. Signed-off-by: Kirill Smelkov k...@navytux.spb.ru --- .mailmap | 1 - 1 file changed, 1 deletion(-) diff --git a/.mailmap b/.mailmap index 11057cb..0be5e02 100644 --- a/.mailmap +++ b/.mailmap @@ -117,7 +117,6 @@ Keith Cascio ke...@cs.ucla.edu ke...@cs.ucla.edu Kent Engstrom k...@lysator.liu.se Kevin Leung kevin...@gmail.com Kirill Smelkov k...@navytux.spb.ru k...@landau.phys.spbu.ru -Kirill Smelkov k...@navytux.spb.ru k...@mns.spb.ru Knut Franke knut.fra...@gmx.de k.fra...@science-computing.de Lars Doelle lars.doelle@on-line ! de Lars Doelle lars.doe...@on-line.de -- 1.9.rc0.143.g6fd479e 8 On the other hand, it is still all me, and the main address (navytux) is indicated correctly, so I dunno... 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. Thanks. I see. Somehow it is pity that the date of original work is lost via this approach, as now we are only changing cosmetics etc, and the bulk of the work was done earlier. Anyway, we can drop the date, but please keep the email, as it is used for the acknowledgment. Thanks, Kirill -- 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 Thu, Mar 27, 2014 at 10:56 AM, Victor Kartashov
victor.kartas...@gmail.com wrote:
shows gpg signature (if any) for commit message in gitweb
in case of successfully verifying the signature highlights it with green
Write in imperative mood: Show gpg ... highlight it...
As a corollary, would it be meaningful to highlight a bad signature with red?
Signed-off-by: Victor Kartashov victor.kartas...@gmail.com
---
gitweb/gitweb.perl | 33 ++---
gitweb/static/gitweb.css | 5 +
2 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 79057b7..0b41392 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] eq \0); # Remove '\0'
What is this change about? Is it related to your gpg change or something else?
if (! @commit_lines) {
return;
@@ -3469,6 +3470,10 @@ sub parse_commit_text {
$co{'committer_name'} = $co{'committer'};
}
}
+ elsif ($line =~ /^gpg: /)
Inconsistent 'elsif' placement. (Cuddle it with the close-brace.)
+ {
Inconsistent open-brace placement.
+ push @signature, $line;
+ }
}
if (!defined $co{'tree'}) {
return;
@@ -3508,6 +3513,11 @@ sub parse_commit_text {
foreach my $line (@commit_lines) {
$line =~ s/^//;
}
+ push(@commit_lines, ) if(scalar(@signature) 0);
Missing space after 'if'.
In this Perl file, it would be more consistent to drop the ' 0' and
say merely 'if scalar @signature'.
+ foreach my $sig (@signature)
+ {
Brace placement.
+ push(@commit_lines, $sig);
+ }
$co{'comment'} = \@commit_lines;
my $age = time - $co{'committer_epoch'};
@@ -3530,13 +3540,15 @@ sub parse_commit {
local $/ = \0;
- open my $fd, -|, git_cmd(), rev-list,
- --parents,
- --header,
- --max-count=1,
+
+
Unnecessary two extra blank lines.
+ 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 +4583,14 @@ 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/^\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..0b7479c 100644
--- a/gitweb/static/gitweb.css
+++ b/gitweb/static/gitweb.css
@@ -136,6 +136,11 @@ span.signoff {
color: #88;
}
+span.good_sign {
+ font-weight: bold;
+ background-color: #aaffaa;
+}
+
div.log_link {
padding: 0px 8px;
font-size: 70%;
--
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: Re: Possible regression in master? (submodules without a master branch)
On Thu, Mar 27, 2014 at 12:39:03PM -0700, Junio C Hamano wrote:
W. Trevor King wk...@tremily.us writes:
On Thu, Mar 27, 2014 at 06:31:27PM +0100, Jens Lehmann wrote:
Am 27.03.2014 18:16, schrieb Junio C Hamano:
Johan Herland jo...@herland.net writes:
I just found a failure to checkout a project with submodules where
there is no explicit submodule branch configuration, and the
submodules happen to not have a master branch:
git clone git://gitorious.org/qt/qt5.git qt5
cd qt5
git submodule init qtbase
git submodule update
In current master, the last command fails with the following output:
... and with a bug-free system, what does it do instead? Just clone
'qtbase' and make a detached-head checkout at the commit recorded in
the superproject's tree, or something else?
After reverting 23d25e48f5ead73 on current master it clones 'qtbase'
nicely with a detached HEAD.
Fixing this for initial update clone is pretty easy, we just need to
unset start_point before calling module_clone if
submodule.name.branch is not set.
There is this bit for update in git-submodule.txt:
For updates that clone missing submodules, checkout-mode updates
will create submodules with detached HEADs; all other modes will
create submodules with a local branch named after
submodule.path.branch.
[side note] Isn't that a typo of submodule.name.branch?
Yep, thats is a typo. Trevor will you fix that as well? Or how should be
do that? Since its just such a small change.
So the proposed change is to make the part before semicolon true?
If we are not newly cloning (because we already have it), if the
submodule.name.branch is not set *OR* refers to a branch that does
not even exist, shouldn't we either (1) abort as an error, or (2) do
the same and detach?
I would expect (1) abort as an error since the user is not getting what
he would expect.
However, that's just going to
push remote branch ambiguity problems back to the --remote update
functionality. What should happen when submodule.name.branch is not
set and you run a --remote update, which has used:
git rev-parse ${remote_name}/${branch}
since the submodule.name.branch setting was introduced in 06b1abb
(submodule update: add --remote for submodule's upstream changes,
2012-12-19)?
Isn't --remote about following one specific branch the user who
issues that command has in mind? If you as the end user did not
give any indication which branch you meant, e.g. by leaving the
submodule.name.branch empty, shouldn't that be diagnosed as an
error?
Well to simplify things there was this fallback to origin/master
(similar to the master branch we create on init) since that is a branch
which many projects have. E.g. for the users that share one central
server and just directly commit, push and pull to/from master. They
would have an easy way to start working in a submodule, by simply saying
--remote and then committing to master. At least that is what I
imagine.
gitmodules(5) is pretty clear that 'submodule.name.branch' defaults
to master (and not upstream's HEAD), do we want to adjust this at the
same time?
That may be likely. If the value set to a configuration variable
causes an established behaviour of a program change a lot, silently
defaulting that variable to something many people are expected to
have (e.g. 'master') would likely to cause a usability regression.
IMO this branch configuration should completely ignored in the default,
non --remote, usage. Since we simply checkout a specific SHA1 in this
case, that should be possible.
Cheers Heiko
--
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
submodule.path.branch vs. submodule.name.branch (was: Possible regression in master? (submodules without a master branch).
I'm breaking this off into a sub-thread, so it doesn't distract from the main issue. On Thu, Mar 27, 2014 at 12:39:03PM -0700, Junio C Hamano wrote: There is this bit for update in git-submodule.txt: For updates that clone missing submodules, checkout-mode updates will create submodules with detached HEADs; all other modes will create submodules with a local branch named after submodule.path.branch. [side note] Isn't that a typo of submodule.name.branch? Good catch. 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. The first submodule.path.branch reference landed a short time later in b9289227 (submodule add: If --branch is given, record it in .gitmodules, 2012-12-19), and I was probably just not aware of 73b0898d. The second submodule.path.branch reference landed in 23d25e48 (submodule: explicit local branch creation in module_clone, 2014-01-26), and is just a copy paste error. Both should be updated to submodule.name.branch. 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
[PATCH] Documentation/submodule: Fix submodule.name - .path typos
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. The first submodule.path.branch reference landed a short time later in b9289227 (submodule add: If --branch is given, record it in .gitmodules, 2012-12-19), and I was probably just not aware of 73b0898d. The second submodule.path.branch reference landed in 23d25e48 (submodule: explicit local branch creation in module_clone, 2014-01-26), and is just a copy paste error. This commit updates both references to the current submodule.name.branch. Reported-by: Junio C Hamano gits...@pobox.com Signed-off-by: W. Trevor King wk...@tremily.us --- 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. Documentation/git-submodule.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index 46c1eeb..77588b0 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -162,7 +162,7 @@ update:: + For updates that clone missing submodules, checkout-mode updates will create submodules with detached HEADs; all other modes will create -submodules with a local branch named after `submodule.path.branch`. +submodules with a local branch named after `submodule.name.branch`. + For updates that do not clone missing submodules, the submodule's HEAD is only touched when the remote reference does not match the @@ -247,7 +247,7 @@ OPTIONS -b:: --branch:: Branch of repository to add as submodule. - The name of the branch is recorded as `submodule.path.branch` in + The name of the branch is recorded as `submodule.name.branch` in `.gitmodules` for `update --remote`. -f:: -- 1.9.1.352.gd393d14.dirty -- 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: submodule.path.branch vs. submodule.name.branch
W. Trevor King wk...@tremily.us writes: [side note] Isn't that a typo of submodule.name.branch? Good catch. 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. Thanks for digging. Strictly speaking, I think this was not even a transition (rather, there was no way to give a submodule a name that is different from its path). In any version of git whose git-submodule.sh has module_name helper function, the path and the name were conceptually two different things, and we should have been using the name, not path, throughout. ... Both should be updated to submodule.name.branch. I agree. 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 v8 03/12] Move lower case functions into wrapper.c
Junio C Hamano gits...@pobox.com writes: Christian Couder chrisc...@tuxfamily.org writes: Yeah, but it seems a bit wasteful to allocate memory for a new string, then downcase it, then compare it with strcmp() and then free it, instead of just using strcasecmp() on the original string. 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? -- 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 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. 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). -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
Jeff King p...@peff.net writes: All bool config values allow tRuE. I was expecting somebody will bring it up, but think about it. Bool is a very special case. Even among CS folks, depending on your background, true may be True may be TRUE may be 1. Conflating it with some random enum does not make a good argument. 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. I would say that the latter is the right thing to do. 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,... Actually, I think it ends up being hostile to the users to accept random cases without a good reason. If you see two trailer elements whose where are specified as after and AFTER in somebody's configuration file, wouldn't that give a wrong impression that a new line with the latter somehow has a stronger desire to come later than the former? If you consistently take only the fixed strings, you do not have to worry about many people writing the same things in different ways, confusing each other. I would however fully agree with you that using strcasecmp() would be the cleanest when reading and maintaining the code **IF** we want to accept values in random case, but I do not agree that accepting random cases is a good thing, so... -- 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 Wed, Mar 26, 2014 at 10:46:16PM +, Charles Bailey wrote: On Wed, Mar 26, 2014 at 05:57:41PM -0400, Jeff King wrote: Hmm, so the year you got is actually: 1623969404. That still seems off to me by a factor 20. I don't know if this is really worth digging into that much further, but I wonder what you would get for timestamps of: 9 999 etc. AIX goes negative at about the same time Linux and Solaris segfault: 999 Sun Apr 26 10:46:39 1970 -0700 Sat Mar 3 02:46:39 1973 -0700 9 Sat Sep 8 18:46:39 2001 -0700 99 Sat Nov 20 10:46:39 2286 -0700 999 Wed Nov 16 02:46:39 5138 -0700 Thu Sep 26 18:46:39 33658 -0700 9 Sun May 20 10:46:39 318857 -0700 99 Sat Nov 7 02:46:39 3170843 -0700 999 Sat Jul 4 18:46:39 31690708 -0700 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. 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. 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. -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: Possible regression in master? (submodules without a master branch)
Am 27.03.2014 19:30, schrieb Junio C Hamano: Jens Lehmann jens.lehm...@web.de writes: Am 27.03.2014 16:52, schrieb W. Trevor King: On Thu, Mar 27, 2014 at 03:21:49PM +0100, Johan Herland wrote: I just found a failure to checkout a project with submodules where there is no explicit submodule branch configuration, and the submodules happen to not have a master branch: The docs say [1]: A remote branch name for tracking updates in the upstream submodule. If the option is not specified, it defaults to 'master'. But the branch setting isn't configured for Qt, the .gitmodules file contains only this: [submodule qtbase] path = qtbase url = ../qtbase.git ... which is what we do now. Working around that to default to the upstream submodule's HEAD is possible (you can just use --branch HEAD), but I think it's easier to just explicitly specify your preferred branch. That is *not* easier, as Johan did not have to do that before. I think your patch 23d25e48f5ead73c9ce233986f90791abec9f1e8 does not do what the commit message promised: With this change, folks cloning submodules for the first time via: $ git submodule update ... will get a local branch instead of a detached HEAD, unless they are using the default checkout-mode updates. And Qt uses the default checkout-mode updates and doesn't have branch configured either. So we are facing a serious regression here. There are two potential issues (and a half) then: - When cloning with the default checkout-mode updates, the new feature to avoid detaching the HEAD should not kick in at all. Yep. - For a repository that does not have that branch thing configured, the doc says that it will default to 'master'. I do not think this was brought up during the review, but is it a sensible default if the project does not even have that branch? What are viable alternatives? - use 'master' and fail just the way Johan saw? - use any random branch that happens to be at the same commit as what is being checked out? - use the branch clone for the submodule repository saw the upstream was pointing at with its HEAD? - something else? Good question. Me thinks that when a superproject doesn't have 'branch' configured and does set 'update' to something other than 'checkout' for a submodule it should better make sure 'master' is a valid branch in there. Everything else sounds like a misconfiguration on the superproject's part that warrants an error. But I may be wrong here as I only use 'checkout' together with a detached HEADs myself. Comments welcome. - Johan's set-up was apparently not covered in the addition to t/ in 23d25e48 (submodule: explicit local branch creation in module_clone, 2014-01-26)---otherwise we would have caught this regression. Are there other conditions that are not covered? I suspect so. This is one of the reasons I started the submodule testing framework I posted an RFC for a few days ago, as an attempt to start a systematic approach to submodule testing. This is not the first time a breakage was not caught by the tests, so we need to do better here. (Note to self: test for the detached HEAD for the checkout case in the framework too) -- 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 Thu, Mar 27, 2014 at 03:47:01PM -0700, Junio C Hamano wrote: Actually, I think it ends up being hostile to the users to accept random cases without a good reason. If you see two trailer elements whose where are specified as after and AFTER in somebody's configuration file, wouldn't that give a wrong impression that a new line with the latter somehow has a stronger desire to come later than the former? If you consistently take only the fixed strings, you do not have to worry about many people writing the same things in different ways, confusing each other. I do not agree with this line of reasoning at all. After all, do we have confusion about the case differences between: [COLOR] diff = true [color] UI = false 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. -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: Possible regression in master? (submodules without a master branch)
Am 27.03.2014 21:27, schrieb Heiko Voigt:
On Thu, Mar 27, 2014 at 12:39:03PM -0700, Junio C Hamano wrote:
W. Trevor King wk...@tremily.us writes:
On Thu, Mar 27, 2014 at 06:31:27PM +0100, Jens Lehmann wrote:
Am 27.03.2014 18:16, schrieb Junio C Hamano:
Johan Herland jo...@herland.net writes:
I just found a failure to checkout a project with submodules where
there is no explicit submodule branch configuration, and the
submodules happen to not have a master branch:
git clone git://gitorious.org/qt/qt5.git qt5
cd qt5
git submodule init qtbase
git submodule update
In current master, the last command fails with the following output:
... and with a bug-free system, what does it do instead? Just clone
'qtbase' and make a detached-head checkout at the commit recorded in
the superproject's tree, or something else?
After reverting 23d25e48f5ead73 on current master it clones 'qtbase'
nicely with a detached HEAD.
Fixing this for initial update clone is pretty easy, we just need to
unset start_point before calling module_clone if
submodule.name.branch is not set.
There is this bit for update in git-submodule.txt:
For updates that clone missing submodules, checkout-mode updates
will create submodules with detached HEADs; all other modes will
create submodules with a local branch named after
submodule.path.branch.
[side note] Isn't that a typo of submodule.name.branch?
Yep, thats is a typo. Trevor will you fix that as well? Or how should be
do that? Since its just such a small change.
So the proposed change is to make the part before semicolon true?
Definitely. But not only for the initial clone, that should hold
true for all subsequent updates too.
If we are not newly cloning (because we already have it), if the
submodule.name.branch is not set *OR* refers to a branch that does
not even exist, shouldn't we either (1) abort as an error, or (2) do
the same and detach?
I would expect (1) abort as an error since the user is not getting what
he would expect.
I believe that depends on the 'update' setting. If it is either not
set or set to 'checkout', it should simply detach when 'branch' is
not set. So it is (2) do the same and detach in that case. Otherwise
I agree with Heiko.
However, that's just going to
push remote branch ambiguity problems back to the --remote update
functionality. What should happen when submodule.name.branch is not
set and you run a --remote update, which has used:
git rev-parse ${remote_name}/${branch}
since the submodule.name.branch setting was introduced in 06b1abb
(submodule update: add --remote for submodule's upstream changes,
2012-12-19)?
Isn't --remote about following one specific branch the user who
issues that command has in mind? If you as the end user did not
give any indication which branch you meant, e.g. by leaving the
submodule.name.branch empty, shouldn't that be diagnosed as an
error?
Well to simplify things there was this fallback to origin/master
(similar to the master branch we create on init) since that is a branch
which many projects have. E.g. for the users that share one central
server and just directly commit, push and pull to/from master. They
would have an easy way to start working in a submodule, by simply saying
--remote and then committing to master. At least that is what I
imagine.
I'd really like to see more feedback on this from people who actually
use the 'merge' and 'rebase' update modes with or without 'branch' set.
gitmodules(5) is pretty clear that 'submodule.name.branch' defaults
to master (and not upstream's HEAD), do we want to adjust this at the
same time?
That may be likely. If the value set to a configuration variable
causes an established behaviour of a program change a lot, silently
defaulting that variable to something many people are expected to
have (e.g. 'master') would likely to cause a usability regression.
IMO this branch configuration should completely ignored in the default,
non --remote, usage. Since we simply checkout a specific SHA1 in this
case, that should be possible.
Yes.
--
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] git-prompt.sh: make '+' work for unborn branches
On Thu, Mar 06, 2014 at 10:16:47PM +0100, Maurice Bos wrote: I have no clue why git diff --cached isn't used instead of git diff-index. I was wondering about it, but I decided I don't know enough about git and there are probably valid reasons for doing it this way. Though, replacing it with with git diff --cached seems to have the exact same behaviour, as far as I tested. That would make the patch a little prettier, as it doesn't contain the empty tree id any more: I think it probably goes in the wrong direction, though. The prompt code should probably be building on plumbing, not porcelain. So your original patch as-is is probably the most sensible thing (we may want to convert the first git-diff call to use plumbing, too, but that would be a separate patch). It looks like Junio did not pick up your patch. You may want to repost it. -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: SSL_CTX leak?
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. 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. -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] Documentation/submodule: Fix submodule.name - .path typos
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. The first submodule.path.branch reference landed a short time later in b9289227 (submodule add: If --branch is given, record it in .gitmodules, 2012-12-19), and I was probably just not aware of 73b0898d. The second submodule.path.branch reference landed in 23d25e48 (submodule: explicit local branch creation in module_clone, 2014-01-26), and is just a copy paste error. This commit updates both references to the current submodule.name.branch. Reported-by: Junio C Hamano gits...@pobox.com Signed-off-by: W. Trevor King wk...@tremily.us --- 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. Documentation/git-submodule.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index 46c1eeb..77588b0 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -162,7 +162,7 @@ update:: + For updates that clone missing submodules, checkout-mode updates will create submodules with detached HEADs; all other modes will create -submodules with a local branch named after `submodule.path.branch`. +submodules with a local branch named after `submodule.name.branch`. + For updates that do not clone missing submodules, the submodule's HEAD is only touched when the remote reference does not match the @@ -247,7 +247,7 @@ OPTIONS -b:: --branch:: Branch of repository to add as submodule. - The name of the branch is recorded as `submodule.path.branch` in + The name of the branch is recorded as `submodule.name.branch` in `.gitmodules` for `update --remote`. -f:: -- 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: Re: Possible regression in master? (submodules without a master branch)
(Thanks to all of you for picking this up and more or less resolving
it while I was away from email for a few hours...)
On Thu, Mar 27, 2014 at 9:27 PM, Heiko Voigt hvo...@hvoigt.net wrote:
On Thu, Mar 27, 2014 at 12:39:03PM -0700, Junio C Hamano wrote:
W. Trevor King wk...@tremily.us writes:
On Thu, Mar 27, 2014 at 06:31:27PM +0100, Jens Lehmann wrote:
Am 27.03.2014 18:16, schrieb Junio C Hamano:
Johan Herland jo...@herland.net writes:
I just found a failure to checkout a project with submodules where
there is no explicit submodule branch configuration, and the
submodules happen to not have a master branch:
git clone git://gitorious.org/qt/qt5.git qt5
cd qt5
git submodule init qtbase
git submodule update
In current master, the last command fails with the following output:
... and with a bug-free system, what does it do instead? Just clone
'qtbase' and make a detached-head checkout at the commit recorded in
the superproject's tree, or something else?
After reverting 23d25e48f5ead73 on current master it clones 'qtbase'
nicely with a detached HEAD.
...which is exactly the behaviour I (and the Qt project - I assume) expected.
Fixing this for initial update clone is pretty easy, we just need to
unset start_point before calling module_clone if
submodule.name.branch is not set.
There is this bit for update in git-submodule.txt:
For updates that clone missing submodules, checkout-mode updates
will create submodules with detached HEADs; all other modes will
create submodules with a local branch named after
submodule.path.branch.
[side note] Isn't that a typo of submodule.name.branch?
Yep, thats is a typo. Trevor will you fix that as well? Or how should be
do that? Since its just such a small change.
So the proposed change is to make the part before semicolon true?
If we are not newly cloning (because we already have it), if the
submodule.name.branch is not set *OR* refers to a branch that does
not even exist, shouldn't we either (1) abort as an error, or (2) do
the same and detach?
I would expect (1) abort as an error since the user is not getting what
he would expect.
FWIW, here is the behaviour I would expect from git submodule update:
- In checkout-mode, if submodule.name.branch is not set, we should
_always_ detach. Whether or not the submodule is already cloned does
not matter.
- In rebase/merge-mode, if submodule.name.branch is not set, we
should _always_ abort with an error.
- If submodule.name.branch is set - but the branch it refers to
does not exist - we should _always_ abort with an error. The current
checkout/rebase/merge-mode does not matter.
In other words, submodule.name.branch is _necessary_ in rebase/merge
mode, but _optional_ in checkout-mode (its absence indicating that we
should detach).
However, that's just going to
push remote branch ambiguity problems back to the --remote update
functionality. What should happen when submodule.name.branch is not
set and you run a --remote update, which has used:
git rev-parse ${remote_name}/${branch}
since the submodule.name.branch setting was introduced in 06b1abb
(submodule update: add --remote for submodule's upstream changes,
2012-12-19)?
Isn't --remote about following one specific branch the user who
issues that command has in mind? If you as the end user did not
give any indication which branch you meant, e.g. by leaving the
submodule.name.branch empty, shouldn't that be diagnosed as an
error?
Well to simplify things there was this fallback to origin/master
(similar to the master branch we create on init) since that is a branch
which many projects have.
I think the analogy to the master branch we create on init is false.
A better analogy is running git pull or git pull -rebase in a
branch where branch.name.merge has not yet been set. And this
currently fails with Please specify which branch you want to merge
with. So I would be inclined to agree with Junio here: We should
error out.
E.g. for the users that share one central
server and just directly commit, push and pull to/from master. They
would have an easy way to start working in a submodule, by simply saying
--remote and then committing to master. At least that is what I
imagine.
If there are compelling arguments for providing a default fallback
(and I'm not sure the above argument is enough), I say we should
rather follow clone's lead, and use the submodule's upstream's HEAD,
instead of blindly assuming origin/master to be present. I expect in
most cases where origin/master happens to be the Right Answer, using
the submodule's upstream's HEAD will yield the same result.
gitmodules(5) is pretty clear that 'submodule.name.branch' defaults
to master (and not upstream's HEAD), do we want to adjust this at the
same time?
That may be likely. If the value set to a configuration variable
causes an established behaviour of a program
Re: Possible regression in master? (submodules without a master branch)
On Thu, Mar 27, 2014 at 11:55 PM, Jens Lehmann jens.lehm...@web.de wrote: Am 27.03.2014 19:30, schrieb Junio C Hamano: - For a repository that does not have that branch thing configured, the doc says that it will default to 'master'. I do not think this was brought up during the review, but is it a sensible default if the project does not even have that branch? What are viable alternatives? - use 'master' and fail just the way Johan saw? - use any random branch that happens to be at the same commit as what is being checked out? - use the branch clone for the submodule repository saw the upstream was pointing at with its HEAD? - something else? Good question. Me thinks that when a superproject doesn't have 'branch' configured and does set 'update' to something other than 'checkout' for a submodule it should better make sure 'master' is a valid branch in there. Everything else sounds like a misconfiguration on the superproject's part that warrants an error. But I may be wrong here as I only use 'checkout' together with a detached HEADs myself. Comments welcome. I believe unset 'branch' and 'update' != 'checkout' is somewhat analogous to unset branch.name.merge while pulling. I.e. you have told me to merge/rebase, but you have not told me against which branch, therefore error out. ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- 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
git commit vs. ignore-submodules
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. 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. 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. Thanks in advance for any reply, Ronald Weiss -- 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] patch-id-test: test new --stable and --unstable flags
On Thu, Mar 27, 2014 at 5:25 AM, Michael S. Tsirkin m...@redhat.com wrote:
Verify that patch ID is now stable against hunk reordering.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
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..75f77ef 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
Broken -chain.
If you use -EOF, you can indent the content rather than having to hang
it on the left margin. Better, use -\EOF to indicate that you're not
interested in interpolation within the block.
+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
--
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 10/10] Documentation: Document --function-name usage
On Thu, Mar 27, 2014 at 2:50 PM, David A. Dalrymple (and Bhushan G. Lodha) dad-...@mit.edu wrote: From: Bhushan Lodha David A. Dalrymple dad-...@mit.edu Signed-off-by: David Dalrymple (on zayin) davi...@alum.mit.edu --- Documentation/diff-options.txt | 9 + Documentation/gitdiffcore.txt | 17 ++--- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index 9b37b2a..a778dff 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -427,6 +427,15 @@ information. --pickaxe-regex:: Treat the string given to `-S` as an extended POSIX regular expression to match. + +--function-nameregex:: Missing '=' before regex. Ditto for the several additional instances in this patch. + Look for differences whose patch text contains hunk headers that match + regex. This can be useful to locate changes to a particular function + or other semantic element in a program, since hunk headers are intended + to indicate the function context (in the sense of + `--function-context`) in which the particular change occurs. The + function context is determined by the diff driver corresponding to the + file in question; see linkgit:gitattributes[7] for details. endif::git-format-patch[] -Oorderfile:: diff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt index c8b3e51..b8477ce 100644 --- a/Documentation/gitdiffcore.txt +++ b/Documentation/gitdiffcore.txt @@ -222,10 +222,11 @@ version prefixed with '+'. diffcore-pickaxe: For Detecting Addition/Deletion of Specified String - -This transformation limits the set of filepairs to those that change +This transformation limits the set of filepairs to those that involve specified strings between the preimage and the postimage in a certain -way. -Sblock of text and -Gregular expression options are used to -specify different ways these strings are sought. +way. -Sblock of text, -Gregular expression, and +--function-nameregular expression options are used to specify +different ways these strings are sought. -Sblock of text detects filepairs whose preimage and postimage have different number of occurrences of the specified block of text. @@ -251,6 +252,16 @@ criterion in a changeset, the entire changeset is kept. This behavior is designed to make reviewing changes in the context of the whole changeset easier. +--function-nameregular expression detects filepairs whose textual +diff contains a hunk header that matches the given regular expression. +The hunk header is generated via the diff driver specified in +`.gitattributes`, and is intended to reflect the function context +(in the sense of `--function-context`) in which the change occurs, +with programming-language-dependent heuristics. As of now, the +programming language is not auto-detected in any way. Also note that +hunks whose headers do not match the regular expression are not +currently filtered out; this is only a filepair filter. + diffcore-order: For Sorting the Output Based on Filenames - -- 1.7.12.4 (Apple Git-37) -- 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
Found a bug in git 1.9.0 but can't reproduce it without copyrighted source code.
Hi, I found git sometimes can't detect working trees changes. But I can only reproduce this problem on several specific files, unfortunately these files are copyrighted source files so I can't send them to you. Is there anything I can do to narrow the problem and finally reproduce the bug without these commercial files? I posted a question on stackoverflow which shows the process. http://stackoverflow.com/questions/22684163/cant-reproduce-a-bug-in-git-without-a-specific-file Actually what I'm doing is: git init copy the first version of file into the working tree. git add . git commit -m 'init' copy and replace the file into working tree. git status and nothing is reported by git. these two files have the same timestamp, the same size, bug slightly different contents. These files were generated by `git difftool -d` I just manually copied them out from the temp directory just for future review. Git I'm using is msysgit 1.9.0 on windows 7 Best regards, Sheng Yun -- 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 06/17] ls-files: add --color to highlight file names
On Wed, Mar 26, 2014 at 7:15 PM, Duy Nguyen pclo...@gmail.com wrote: On Thu, Mar 27, 2014 at 2:13 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: Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/git-ls-files.txt | 9 + builtin/ls-files.c | 38 +++--- 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt index c0856a6..5c1b7f3 100644 --- a/Documentation/git-ls-files.txt +++ b/Documentation/git-ls-files.txt @@ -147,6 +147,15 @@ a space) at the start of each line: possible for manual inspection; the exact format may change at any time. +--color[=when]:: + Color file names. The value must be always (default), never, + or auto. Here, the default is always... These (.txt changes in other patches as well) are mostly copy and paste from existing .txt files. You may want to grep through and fix other places as well, in a separate series. +--no-color:: + Turn off coloring, even when the configuration file gives the + default to color output, same as `--color=never`. This is the + default. But, here the default is never. What I mean is color is turned off by default for ls-files (in contrast, ls has color on by default). The default 'always' means that if you write --color without the when part, then it's --color=always. How do I phrase to make it clear? Perhaps: Color file names. The value must be always, never, or auto. `--color` by itself is the same as `--color=always`. -- 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: Found a bug in git 1.9.0 but can't reproduce it without copyrighted source code.
On 28/03/2014 07:45, yun sheng wrote: Hi, I found git sometimes can't detect working trees changes. But I can only reproduce this problem on several specific files, unfortunately these files are copyrighted source files so I can't send them to you. Is there anything I can do to narrow the problem and finally reproduce the bug without these commercial files? I posted a question on stackoverflow which shows the process. http://stackoverflow.com/questions/22684163/cant-reproduce-a-bug-in-git-without-a-specific-file Actually what I'm doing is: git init copy the first version of file into the working tree. git add . git commit -m 'init' copy and replace the file into working tree. git status and nothing is reported by git. these two files have the same timestamp, the same size, bug slightly different contents. These files were generated by `git difftool -d` I just manually copied them out from the temp directory just for future review. Don't worry about copyright, please run sha1sum in order to make sure the content is changed! Git I'm using is msysgit 1.9.0 on windows 7 Is it Ok on Linux OS, on other git version? Best regards, Sheng Yun -- -- Trần Ngọc Quân. -- 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: Found a bug in git 1.9.0 but can't reproduce it without copyrighted source code.
Hi, yun sheng wrote: these two files have the same timestamp, the same size, bug slightly different contents. How did they get the same timestamp? [...] Git I'm using is msysgit 1.9.0 on windows 7 Unixy operating systems have other fields like inode number and ctime that make it possible to notice that a file might have been changed without actually rereading it. Unfortunately Git for Windows is limited to what's in the WIN32_FILE_ATTRIBUTE_DATA which means the size, mtime, and mode are basically all it has to go by. Do you know of some other Windows API call that could help? Hope that helps, Jonathan -- 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
Fwd: Found a bug in git 1.9.0 but can't reproduce it without copyrighted source code.
-- Forwarded message -- From: yun sheng uew...@gmail.com Date: Fri, Mar 28, 2014 at 9:28 AM Subject: Re: Found a bug in git 1.9.0 but can't reproduce it without copyrighted source code. To: Trần Ngọc Quân vnwild...@gmail.com The result of sha1sum is different. Following is my console log C:\shengy\tmp\shengyun\git-testgit init Initialized empty Git repository in C:/shengy/tmp/shengyun/git-test/.git/ C:\shengy\tmp\shengyun\git-testcopy C:\shengy\Dropbox\tmp\git-issue\old\epmdstyp.h 1 file(s) copied. C:\shengy\tmp\shengyun\git-testgit add . C:\shengy\tmp\shengyun\git-testgit commit -m 'init' [master (root-commit) 579385e] 'init' 1 file changed, 97 insertions(+) create mode 100644 epmdstyp.h C:\shengy\tmp\shengyun\git-testsha1sum epmdstyp.h c2310fe32891dc7269298814475d0ad083c5483c *epmdstyp.h C:\shengy\tmp\shengyun\git-testcopy C:\shengy\Dropbox\tmp\git-issue\new\epmdstyp.h Overwrite C:\shengy\tmp\shengyun\git-test\epmdstyp.h? (Yes/No/All): Y 1 file(s) copied. C:\shengy\tmp\shengyun\git-testsha1sum epmdstyp.h 7a98d5161b5e5ae201997b40fa5d5cebe1d14d1c *epmdstyp.h C:\shengy\tmp\shengyun\git-testgit status On branch master nothing to commit, working directory clean C:\shengy\tmp\shengyun\git-test On Fri, Mar 28, 2014 at 9:04 AM, Trần Ngọc Quân vnwild...@gmail.com wrote: On 28/03/2014 07:45, yun sheng wrote: Hi, I found git sometimes can't detect working trees changes. But I can only reproduce this problem on several specific files, unfortunately these files are copyrighted source files so I can't send them to you. Is there anything I can do to narrow the problem and finally reproduce the bug without these commercial files? I posted a question on stackoverflow which shows the process. http://stackoverflow.com/questions/22684163/cant-reproduce-a-bug-in-git-without-a-specific-file Actually what I'm doing is: git init copy the first version of file into the working tree. git add . git commit -m 'init' copy and replace the file into working tree. git status and nothing is reported by git. these two files have the same timestamp, the same size, bug slightly different contents. These files were generated by `git difftool -d` I just manually copied them out from the temp directory just for future review. Don't worry about copyright, please run sha1sum in order to make sure the content is changed! Git I'm using is msysgit 1.9.0 on windows 7 Is it Ok on Linux OS, on other git version? Best regards, Sheng Yun -- -- Trần Ngọc Quân. -- 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: Found a bug in git 1.9.0 but can't reproduce it without copyrighted source code.
The files get the same timestamp by using `git difftool -d` to view diffs, the diff tool I use id beyond compare 3, this command would generate temp files to feed the compare program, so these files get the same time stamp, I copied them out from the temp folder. I have no idea of the second quesiton, I am really not familiar with windows API. Do you mean this file may have been changed without rereading and git can't detect it? Best regards, Sheng Yun On Fri, Mar 28, 2014 at 9:40 AM, Jonathan Nieder jrnie...@gmail.com wrote: Hi, yun sheng wrote: these two files have the same timestamp, the same size, bug slightly different contents. How did they get the same timestamp? [...] Git I'm using is msysgit 1.9.0 on windows 7 Unixy operating systems have other fields like inode number and ctime that make it possible to notice that a file might have been changed without actually rereading it. Unfortunately Git for Windows is limited to what's in the WIN32_FILE_ATTRIBUTE_DATA which means the size, mtime, and mode are basically all it has to go by. Do you know of some other Windows API call that could help? Hope that helps, Jonathan -- 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
Git doesn't notice file has changed (Re: Found a bug in git 1.9.0 but can't reproduce it without copyrighted source code.)
(cc-ing msysgit list, where there are more Windows-knowledgeable people) yun sheng wrote: On Fri, Mar 28, 2014 at 9:40 AM, Jonathan Nieder jrnie...@gmail.com wrote: yun sheng wrote: these two files have the same timestamp, the same size, bug slightly different contents. How did they get the same timestamp? [...] Git I'm using is msysgit 1.9.0 on windows 7 Unixy operating systems have other fields like inode number and ctime that make it possible to notice that a file might have been changed without actually rereading it. Unfortunately Git for Windows is limited to what's in the WIN32_FILE_ATTRIBUTE_DATA which means the size, mtime, and mode are basically all it has to go by. Do you know of some other Windows API call that could help? The files get the same timestamp by using `git difftool -d` to view diffs, the diff tool I use id beyond compare 3, this command would generate temp files to feed the compare program, so these files get the same time stamp, I copied them out from the temp folder. I have no idea of the second quesiton, I am really not familiar with windows API. Do you mean this file may have been changed without rereading and git can't detect it? Sorry for the lack of clarity. I meant that normally git detects when a file might have been changed without actually reading the file. To do this, it uses heuristics like If all file attributes are unchanged, the file is unchanged which tend to work well on Unix. My question was whether there's some similar trick that could work better on Windows than the current code. For example, if some interested person ports something like Facebook's watchman[1] to Windows[2], then Git could take advantage of that work using something like Duy's file-watcher series[3], which would be one way to fix this problem. Thanks, Jonathan [1] https://github.com/facebook/watchman [2] using FindFirstChangeNotification and ReadDirectoryChangesW, perhaps [3] http://thread.gmane.org/gmane.comp.version-control.git/240339/focus=241395 -- 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 doesn't notice file has changed (Re: Found a bug in git 1.9.0 but can't reproduce it without copyrighted source code.)
The problem is I can't reproduce this bug if create some other files which have the same size and timestamp. It only happens on several files in my project. And it's even more frustrating that I can't send these files to the mailing list since it is a proprietary source file. On Fri, Mar 28, 2014 at 9:59 AM, Jonathan Nieder jrnie...@gmail.com wrote: (cc-ing msysgit list, where there are more Windows-knowledgeable people) yun sheng wrote: On Fri, Mar 28, 2014 at 9:40 AM, Jonathan Nieder jrnie...@gmail.com wrote: yun sheng wrote: these two files have the same timestamp, the same size, bug slightly different contents. How did they get the same timestamp? [...] Git I'm using is msysgit 1.9.0 on windows 7 Unixy operating systems have other fields like inode number and ctime that make it possible to notice that a file might have been changed without actually rereading it. Unfortunately Git for Windows is limited to what's in the WIN32_FILE_ATTRIBUTE_DATA which means the size, mtime, and mode are basically all it has to go by. Do you know of some other Windows API call that could help? The files get the same timestamp by using `git difftool -d` to view diffs, the diff tool I use id beyond compare 3, this command would generate temp files to feed the compare program, so these files get the same time stamp, I copied them out from the temp folder. I have no idea of the second quesiton, I am really not familiar with windows API. Do you mean this file may have been changed without rereading and git can't detect it? Sorry for the lack of clarity. I meant that normally git detects when a file might have been changed without actually reading the file. To do this, it uses heuristics like If all file attributes are unchanged, the file is unchanged which tend to work well on Unix. My question was whether there's some similar trick that could work better on Windows than the current code. For example, if some interested person ports something like Facebook's watchman[1] to Windows[2], then Git could take advantage of that work using something like Duy's file-watcher series[3], which would be one way to fix this problem. Thanks, Jonathan [1] https://github.com/facebook/watchman [2] using FindFirstChangeNotification and ReadDirectoryChangesW, perhaps [3] http://thread.gmane.org/gmane.comp.version-control.git/240339/focus=241395 -- 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 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 -- 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: Possible regression in master? (submodules without a master branch)
On Thu, Mar 27, 2014 at 11:55:21PM +0100, Jens Lehmann wrote: Me thinks that when a superproject doesn't have 'branch' configured and does set 'update' to something other than 'checkout' for a submodule it should better make sure 'master' is a valid branch in there. Everything else sounds like a misconfiguration on the superproject's part that warrants an error. submodule.name.branch should only matter for --remote updates (and the initial clone, which is a special case of remote update). So having an alternative update mode and no submodule.name.branch *is* a valid configuration. It says: * I want to integrate local submodule commits with superproject gitlink changes using the submodule.name.update strategy. * I never use --remote updates, so I haven't bothered to setup submodule.name.branch. I can imagine folks using a workflow like that. And I think erroring out if they *do* try a --remote update shouldn't be too surprising for them. 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: Re: Possible regression in master? (submodules without a master branch)
On Fri, Mar 28, 2014 at 12:21:23AM +0100, Johan Herland wrote: On Thu, Mar 27, 2014 at 9:27 PM, Heiko Voigt wrote: On Thu, Mar 27, 2014 at 12:39:03PM -0700, Junio C Hamano wrote: There is this bit for update in git-submodule.txt: For updates that clone missing submodules, checkout-mode updates will create submodules with detached HEADs; all other modes will create submodules with a local branch named after submodule.path.branch. … So the proposed change is to make the part before semicolon true? If we are not newly cloning (because we already have it), if the submodule.name.branch is not set *OR* refers to a branch that does not even exist, shouldn't we either (1) abort as an error, or (2) do the same and detach? I would expect (1) abort as an error since the user is not getting what he would expect. Branch-attachment is mostly a function of submodule.name.update, not a function of submodule.name.branch. We could certainly interpret a missing submodule.name.branch as: * Use the subproject's HEAD for the initial clone (clear start_point in cmd_update if submodule.$name.branch is not set). * Don't change the branch name on subsequent local updates (what we already do). * Do $something if the user tries a --remote update. I just don't know what that $something should be. FWIW, here is the behaviour I would expect from git submodule update: - In checkout-mode, if submodule.name.branch is not set, we should _always_ detach. Whether or not the submodule is already cloned does not matter. Agreed, checkout-mode should *always* detach the submodule's HEAD. - In rebase/merge-mode, if submodule.name.branch is not set, we should _always_ abort with an error. Why? Can't we mimic clone and use the remote's HEAD (for --remote updates)? That seems more intuitive to me. For local updates, we're just integrating the gitlinked commit with the submodule's HEAD, and you don't need submodule.name.branch for that at all. - If submodule.name.branch is set - but the branch it refers to does not exist - we should _always_ abort with an error. The current checkout/rebase/merge-mode does not matter. Sounds good to me, and should match the current functionality. In other words, submodule.name.branch is _necessary_ in rebase/merge mode, but _optional_ in checkout-mode (its absence indicating that we should detach). The main trigger for “we should detach” is the update mode (checkout-mode detaches, all others integrate with the submodule's HEAD (without changing submodule branches). You only need submodule.name.branch for determining which *remote* commit you're trying to integrate (or clone from). HEAD, master, and “die screaming” all sound like reasonable defaults in that case. Deciding between them is a policy/UI decision, not a technical decision. gitmodules(5) is pretty clear that 'submodule.name.branch' defaults to master (and not upstream's HEAD), do we want to adjust this at the same time? That may be likely. If the value set to a configuration variable causes an established behaviour of a program change a lot, silently defaulting that variable to something many people are expected to have (e.g. 'master') would likely to cause a usability regression. IMO this branch configuration should completely ignored in the default, non --remote, usage. Since we simply checkout a specific SHA1 in this case, that should be possible. Yes. Checkout-mode with no submodule.name.branch configured should always detach. Except for the initial clone (where it's easy to fix), submodule.name.branch *is* ignored in non --remote updates. 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 3/3] test-lib: '--run' to run only specific tests
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.
+ 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.
+ 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...
+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.
+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.)
+If --run starts with '+' or unprefixed pattern the initial set of
+tests to run is empty. If the first pattern starts with '-' or
+'!' all the tests are added to the initial set. After initial
+set is determined every pattern, test number or range is added or
+excluded from the set one by one, from left to right.
+
+For example, common case is to run several setup tests and then a
+specific test that relies on that setup:
Perhaps be a bit more specific:
...run several setup tests (1, 2, 3) and then a
specific test (21) that relies...
+$ sh ./t9200-git-cvsexport-commit.sh --run='1 2 3 21'
+
+or:
+
+$ sh ./t9200-git-cvsexport-commit.sh --run='4 21'
It might be clearer to say =3 rather than 4.
+To run only tests up to a specific test one could do this:
s/specific test/specific test,/
Also perhaps:
...up to a specific test (21), one...
+$ sh ./t9200-git-cvsexport-commit.sh --run='!=21'
+
+As noted above test set is build going though patterns left to
s/above/above,/
s/test set/the test set/
s/build/built/
As noted above, the test set is built...
+right, so this:
+
+$ sh ./t9200-git-cvsexport-commit.sh --run='5 !3'
+
+will run tests 1, 2, and 4.
+
+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.
+--run is mostly useful when you want to focus on a specific test
+and know what you are doing. Or when you want to run up to a
+certain test.
Naming Tests
diff --git a/t/test-lib.sh b/t/test-lib.sh
index e035f36..63e481a 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -191,6 +191,14 @@ do
[RFC] submodule: change submodule.name.branch default from master to HEAD
gitmodule(5) mentioned 'master' as the default remote branch, but
folks using checkout-style updates are unlikely to care which upstream
branch their commit comes from (they only care that the clone fetches
that commit). If they haven't set submodule.name.branch, it makes
more sense to mirror 'git clone' and use the subproject's HEAD than to
default to 'master' (which may not even exist).
After the initial clone, subsequent updates may be local or remote.
Local updates (integrating gitlink changes) have no need to fetch a
specific remote branch, and get along just fine without
submodule.name.branch. Remote updates do need a remote branch, but
HEAD works as well here as it did for the initial clone.
Reported-by: Johan Herland jo...@herland.net
Signed-off-by: W. Trevor King wk...@tremily.us
---
This still needs tests, but it gets through the following fine:
rm -rf superproject subproject
mkdir subproject
(cd subproject
git init
echo 'Subproject' README
git add README
git commit -m 'Subproject commit'
git branch -m master next
)
mkdir superproject
(cd superproject
git init
git submodule add ../subproject submod
git commit -am 'Add submod'
)
(cd subproject
echo 'work work work' README
git commit -am 'Subproject commit 2'
)
(cd superproject
git submodule update --remote
git commit -am 'Add submod'
)
The main drawback to this approach is that we're changing a default,
but I agree with John's:
On Fri, Mar 28, 2014 at 12:21:23AM +0100, Johan Herland wrote:
I expect in most cases where origin/master happens to be the Right
Answer, using the submodule's upstream's HEAD will yield the same
result.
so the default-change may not be particularly intrusive.
Cheers,
Trevor
Documentation/git-submodule.txt | 2 +-
Documentation/gitmodules.txt| 5 +++--
git-submodule.sh| 11 ---
3 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 46c1eeb..c485a17 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -284,7 +284,7 @@ OPTIONS
the superproject's recorded SHA-1 to update the submodule, use the
status of the submodule's remote-tracking branch. The remote used
is branch's remote (`branch.name.remote`), defaulting to `origin`.
- The remote branch used defaults to `master`, but the branch name may
+ The remote branch used defaults to `HEAD`, but the branch name may
be overridden by setting the `submodule.name.branch` option in
either `.gitmodules` or `.git/config` (with `.git/config` taking
precedence).
diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
index f539e3f..1aecce9 100644
--- a/Documentation/gitmodules.txt
+++ b/Documentation/gitmodules.txt
@@ -53,8 +53,9 @@ submodule.name.update::
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
+ HEAD. See the `--remote` documentation in linkgit:git-submodule[1]
+ for details.
+
This branch name is also used for the local branch created by
non-checkout cloning updates. See the `update` documentation in
diff --git a/git-submodule.sh b/git-submodule.sh
index 6135cfa..5f08e6c 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -819,8 +819,8 @@ cmd_update()
name=$(module_name $sm_path) || exit
url=$(git config submodule.$name.url)
config_branch=$(get_submodule_config $name branch)
- branch=${config_branch:-master}
- local_branch=$branch
+ branch=${config_branch:-HEAD}
+ local_branch=$config_branch
if ! test -z $update
then
update_module=$update
@@ -860,7 +860,12 @@ Maybe you want to use 'update --init'?)
if ! test -d $sm_path/.git -o -f $sm_path/.git
then
- start_point=origin/${branch}
+ if test -n $config_branch
+ then
+ start_point=origin/$branch
+ else
+ start_point=
+ fi
module_clone $sm_path $name $url $reference
$depth $start_point $local_branch || exit
cloned_modules=$cloned_modules;$name
subsha1=
--
1.9.1.352.gd393d14.dirty
--
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 Thu, Mar 27, 2014 at 11:36 PM, W. Trevor King wk...@tremily.us wrote:
gitmodule(5) mentioned 'master' as the default remote branch, but
folks using checkout-style updates are unlikely to care which upstream
branch their commit comes from (they only care that the clone fetches
that commit). If they haven't set submodule.name.branch, it makes
more sense to mirror 'git clone' and use the subproject's HEAD than to
default to 'master' (which may not even exist).
After the initial clone, subsequent updates may be local or remote.
Local updates (integrating gitlink changes) have no need to fetch a
specific remote branch, and get along just fine without
submodule.name.branch. Remote updates do need a remote branch, but
HEAD works as well here as it did for the initial clone.
Reported-by: Johan Herland jo...@herland.net
Signed-off-by: W. Trevor King wk...@tremily.us
---
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 46c1eeb..c485a17 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -284,7 +284,7 @@ OPTIONS
the superproject's recorded SHA-1 to update the submodule, use the
status of the submodule's remote-tracking branch. The remote used
is branch's remote (`branch.name.remote`), defaulting to `origin`.
- The remote branch used defaults to `master`, but the branch name may
+ The remote branch used defaults to `HEAD`, but the branch name may
be overridden by setting the `submodule.name.branch` option in
either `.gitmodules` or `.git/config` (with `.git/config` taking
precedence).
diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
index f539e3f..1aecce9 100644
--- a/Documentation/gitmodules.txt
+++ b/Documentation/gitmodules.txt
@@ -53,8 +53,9 @@ submodule.name.update::
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.
+
This branch name is also used for the local branch created by
non-checkout cloning updates. See the `update` documentation in
diff --git a/git-submodule.sh b/git-submodule.sh
index 6135cfa..5f08e6c 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -819,8 +819,8 @@ cmd_update()
name=$(module_name $sm_path) || exit
url=$(git config submodule.$name.url)
config_branch=$(get_submodule_config $name branch)
- branch=${config_branch:-master}
- local_branch=$branch
+ branch=${config_branch:-HEAD}
+ local_branch=$config_branch
if ! test -z $update
then
update_module=$update
@@ -860,7 +860,12 @@ Maybe you want to use 'update --init'?)
if ! test -d $sm_path/.git -o -f $sm_path/.git
then
- start_point=origin/${branch}
+ if test -n $config_branch
+ then
+ start_point=origin/$branch
+ else
+ start_point=
+ fi
module_clone $sm_path $name $url $reference
$depth $start_point $local_branch || exit
cloned_modules=$cloned_modules;$name
subsha1=
--
1.9.1.352.gd393d14.dirty
--
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 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” ;). 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 v2 13/17] ls: add -1 short for --no-column in the spirit of GNU ls
On Wed, Mar 26, 2014 at 9:48 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
Subject: ls: add -1 short for --no-column in the spirit of GNU ls
The -1 option is POSIX [1]; not a GNU extension.
[1]: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/ls.html
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Documentation/git-ls.txt | 3 +++
builtin/ls-files.c | 2 ++
2 files changed, 5 insertions(+)
diff --git a/Documentation/git-ls.txt b/Documentation/git-ls.txt
index 10df6b0..0480c42 100644
--- a/Documentation/git-ls.txt
+++ b/Documentation/git-ls.txt
@@ -51,6 +51,9 @@ OPTIONS
--recursive::
Equivalent of --max-depth=-1 (infinite recursion).
+-1::
+ Equivalent of --no-column.
+
--color[=when]::
Color file names. The value must be always (default), never,
or auto.
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 772a6ce..014de05 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -729,6 +729,8 @@ int cmd_ls(int argc, const char **argv, const char
*cmd_prefix)
N_(shortcut for --max-depth=-1), -1),
OPT__COLOR(use_color, N_(show color)),
OPT_COLUMN(0, column, colopts, N_(show files in
columns)),
+ OPT_SET_INT('1', NULL, colopts,
+ N_(shortcut for --no-column), COL_PARSEOPT),
{ OPTION_INTEGER, 0, max-depth, max_depth, N_(depth),
N_(descend at most depth levels), PARSE_OPT_NONEG,
NULL, 1 },
--
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
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? 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 v2 16/17] ls: do not show duplicate cached entries
On Wed, Mar 26, 2014 at 9:48 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
With the current show_files() ls -tcm will show
foo.c
M foo.c
The first item is redundant. If foo.c is modified, we know it's in
the cache. Introduce show_files_compact to do that because ls-files is
plumbing and scripts may already depend on current display behavior.
Another difference in show_files_compact() is it does not show
skip-worktree (aka outside sparse checkout) entries anymore, which
makes sense in porcelain context.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
builtin/ls-files.c | 52 +++-
1 file changed, 51 insertions(+), 1 deletion(-)
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 709d8b1..cd8e35c 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -337,6 +337,53 @@ static void show_files(struct dir_struct *dir)
}
}
+static void show_files_compact(struct dir_struct *dir)
+{
+ int i;
+
+ /* For cached/deleted files we don't need to even do the readdir */
+ if (show_others || show_killed) {
+ if (!show_others)
+ dir-flags |= DIR_COLLECT_KILLED_ONLY;
+ fill_directory(dir, pathspec);
+ if (show_others)
+ show_other_files(dir);
+ if (show_killed)
+ show_killed_files(dir);
+ }
+ if (!(show_cached || show_stage || show_deleted || show_modified))
+ return;
+ for (i = 0; i active_nr; i++) {
+ const struct cache_entry *ce = active_cache[i];
+ struct stat st;
+ int err, shown = 0;
+ if ((dir-flags DIR_SHOW_IGNORED)
+ !ce_excluded(dir, ce))
+ continue;
+ if (show_unmerged !ce_stage(ce))
+ continue;
+ if (ce-ce_flags CE_UPDATE)
+ continue;
+ if (ce_skip_worktree(ce))
+ continue;
+ 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?
+ show_ce_entry(tag_modified, ce);
+ shown = 1;
+ }
+ if (ce_stage(ce)) {
+ show_ce_entry(tag_unmerged, ce);
+ shown = 1;
+ }
+ if (!shown show_cached)
+ show_ce_entry(tag_cached, ce);
+ }
+}
+
/*
* Prune the index to only contain stuff starting with prefix
*/
@@ -606,7 +653,10 @@ static int ls_files(const char **argv, const char
*prefix)
refresh_index(the_index, REFRESH_QUIET, pathspec, NULL,
NULL);
setup_pager();
}
- show_files(dir);
+ if (porcelain)
+ show_files_compact(dir);
+ else
+ show_files(dir);
if (show_resolve_undo)
show_ru_info();
--
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
--
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
