* Re: [PATCH] Add --contains flag to git tag
From: Jake Goulding @ 2009-01-21 3:54 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git
In-Reply-To: <20090121032058.GG21473@genesis.frugalware.org>
That is actually in a subsequent commit (in my unmarked patch sequence).
I actually have 1 commit for the code change, one for documentation, and one for the test case. Should I squash all these together?
Also, my test case is in a separate file (t7704-tag-contains.sh) as that is how I read t/README. Was this incorrect?
Thanks!
-Jake
----- Original Message -----
From: "Miklos Vajna" <vmiklos@frugalware.org>
To: "Jake Goulding" <goulding@vivisimo.com>
Cc: git@vger.kernel.org
Sent: Tuesday, January 20, 2009 10:20:58 PM GMT -05:00 US/Canada Eastern
Subject: Re: [PATCH] Add --contains flag to git tag
On Tue, Jan 20, 2009 at 08:37:09PM -0500, Jake Goulding <goulding@vivisimo.com> wrote:
> Please let me know what else I have inevitably messed up, and I shall
> endeavor to fix and resubmit.
Please document your improvements in Documentation/git-tag.txt and don't
forget to add a testcase to t7004-tag.sh.
^ permalink raw reply
* [PATCH] color-words: Support diff.color-words config option
From: Boyd Stephen Smith Jr. @ 2009-01-21 3:46 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Santi Béjar, Thomas Rast, git, Junio C Hamano, Teemu Likonen
In-Reply-To: <alpine.DEB.1.00.0901201058520.3586@pacific.mpi-cbg.de>
When diff is invoked with --color-words (w/o =regex), use the regular
expression the user has configured as diff.wordregex.
diff drivers configured via attributes take precedence over the
diff.wordregex-words setting. If the user wants to change them, they have
their own configuration variables.
Signed-off-by: Boyd Stephen Smith Jr <bss@iguanasuicide.net>
---
This version is squashed into one patch and includes documentation and
rewritten tests. It was generated against js/diff-color-words~2,
80c49c3d (color-words: make regex configurable via attributes), replacing
my previous 2 patches. It uses "diff.wordregex" for reasons mention by
Dscho and because that was already what the diff drivers were using.
I'm not entirely satisfied with it. There should probably be some way
to force the default behavior (which is a bit faster) even if a global
config or diff driver exists. Also, I think camelCase is better than
runtogether so I'd prefer to change "wordregex" -> "wordRegex" across
the entire patch set.
Documentation/config.txt | 6 +++++
Documentation/diff-options.txt | 7 +++--
diff.c | 5 ++++
t/t4034-diff-words.sh | 45 ++++++++++++++++++++++++++++++++++++++-
4 files changed, 58 insertions(+), 5 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 7408bb2..0ca983a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -639,6 +639,12 @@ diff.suppress-blank-empty::
A boolean to inhibit the standard behavior of printing a space
before each empty output line. Defaults to false.
+diff.wordregex::
+ A POSIX Extended Regular Expression used to determine what is a "word"
+ when performing word-by-word difference calculations. Character
+ sequences that match the regular expression are "words", all other
+ characters are *ignorable* whitespace.
+
fetch.unpackLimit::
If the number of objects fetched over the git native
transfer is below this
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 1edb82e..164e2c5 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -103,9 +103,10 @@ expression to make sure that it matches all non-whitespace characters.
A match that contains a newline is silently truncated(!) at the
newline.
+
-The regex can also be set via a diff driver, see
-linkgit:gitattributes[1]; giving it explicitly overrides any diff
-driver setting.
+The regex can also be set via a diff driver or configuration option, see
+linkgit:gitattributes[1] or linkgit:git-config[1]. Giving it explicitly
+overrides any diff driver or configuration setting. Diff drivers
+override configuration settings.
--no-renames::
Turn off rename detection, even when the configuration
diff --git a/diff.c b/diff.c
index 9fcde96..ed8b83c 100644
--- a/diff.c
+++ b/diff.c
@@ -23,6 +23,7 @@ static int diff_detect_rename_default;
static int diff_rename_limit_default = 200;
static int diff_suppress_blank_empty;
int diff_use_color_default = -1;
+static const char *diff_word_regex_cfg;
static const char *external_diff_cmd_cfg;
int diff_auto_refresh_index = 1;
static int diff_mnemonic_prefix;
@@ -92,6 +93,8 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
}
if (!strcmp(var, "diff.external"))
return git_config_string(&external_diff_cmd_cfg, var, value);
+ if (!strcmp(var, "diff.wordregex"))
+ return git_config_string(&diff_word_regex_cfg, var, value);
return git_diff_basic_config(var, value, cb);
}
@@ -1550,6 +1553,8 @@ static void builtin_diff(const char *name_a,
o->word_regex = userdiff_word_regex(one);
if (!o->word_regex)
o->word_regex = userdiff_word_regex(two);
+ if (!o->word_regex)
+ o->word_regex = diff_word_regex_cfg;
if (o->word_regex) {
ecbdata.diff_words->word_regex = (regex_t *)
xmalloc(sizeof(regex_t));
diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index 744221b..6bcc153 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -77,6 +77,7 @@ a = b + c<RESET>
<GREEN>aeff = aeff * ( aaa<RESET> )
EOF
+cp expect expect.letter-runs-are-words
test_expect_success 'word diff with a regular expression' '
@@ -92,7 +93,7 @@ post diff=testdriver
EOF
'
-test_expect_success 'option overrides default' '
+test_expect_success 'option overrides .gitattributes' '
word_diff --color-words="[a-z]+"
@@ -112,13 +113,53 @@ a = b + c<RESET>
<GREEN>aeff = aeff * ( aaa )<RESET>
EOF
+cp expect expect.non-whitespace-is-word
-test_expect_success 'use default supplied by driver' '
+test_expect_success 'use regex supplied by driver' '
word_diff --color-words
'
+test_expect_success 'set diff.wordregex option' '
+ git config diff.wordregex "[[:alnum:]]+"
+'
+
+cp expect.letter-runs-are-words expect
+
+test_expect_success 'command-line overrides config' '
+ word_diff --color-words="[a-z]+"
+'
+
+cp expect.non-whitespace-is-word expect
+
+test_expect_success '.gitattributes override config' '
+ word_diff --color-words
+'
+
+test_expect_success 'remove diff driver regex' '
+ git config --unset diff.testdriver.wordregex
+'
+
+cat > expect <<\EOF
+<WHITE>diff --git a/pre b/post<RESET>
+<WHITE>index 330b04f..5ed8eff 100644<RESET>
+<WHITE>--- a/pre<RESET>
+<WHITE>+++ b/post<RESET>
+<BROWN>@@ -1,3 +1,7 @@<RESET>
+h(4),<GREEN>hh[44<RESET>]
+<RESET>
+a = b + c<RESET>
+
+<GREEN>aa = a<RESET>
+
+<GREEN>aeff = aeff * ( aaa<RESET> )
+EOF
+
+test_expect_success 'use configured regex' '
+ word_diff --color-words
+'
+
echo 'aaa (aaa)' > pre
echo 'aaa (aaa) aaa' > post
--
1.5.6.5
--
Boyd Stephen Smith Jr. ,= ,-_-. =.
bss@iguanasuicide.net ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy `-'(. .)`-'
http://iguanasuicide.net/ \_/
^ permalink raw reply related
* Re: [PATCH (topgit)] tg-patch: add support for generating patches against worktree and index
From: martin f krafft @ 2009-01-21 3:20 UTC (permalink / raw)
To: Kirill Smelkov, Petr Baudis, Git Mailing List
In-Reply-To: <20090120175635.GA5721@roro3.zxlink>
[-- Attachment #1: Type: text/plain, Size: 749 bytes --]
also sprach Kirill Smelkov <kirr@landau.phys.spbu.ru> [2009.01.21.0456 +1100]:
> No problem and take your time.
>
> It was just me having no idea about what's going on.
I am a little confused about the patches, and git-am does not seem
to be able to apply two patches from the same email. I use Maildir,
so I am passing the single message with multiple patches to git-am,
and it applies all changes into one single commit.
Anyway, do you have a repo which I can cherry-pick from?
--
martin | http://madduck.net/ | http://two.sentenc.es/
"out of the crooked timber of humanity,
no straight thing was ever made."
-- imanuel kant
spamtraps: madduck.bogus@madduck.net
[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] Add --contains flag to git tag
From: Miklos Vajna @ 2009-01-21 3:20 UTC (permalink / raw)
To: Jake Goulding; +Cc: git
In-Reply-To: <4448209.246381232501828959.JavaMail.root@scalix.vivisimo.com>
[-- Attachment #1: Type: text/plain, Size: 314 bytes --]
On Tue, Jan 20, 2009 at 08:37:09PM -0500, Jake Goulding <goulding@vivisimo.com> wrote:
> Please let me know what else I have inevitably messed up, and I shall
> endeavor to fix and resubmit.
Please document your improvements in Documentation/git-tag.txt and don't
forget to add a testcase to t7004-tag.sh.
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] [TOPGIT] make creating a commit from a topgit branch a function
From: martin f krafft @ 2009-01-21 3:19 UTC (permalink / raw)
To: Uwe Kleine-König, git, Petr Baudis
In-Reply-To: <1231968443-13960-2-git-send-email-u.kleine-koenig@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 564 bytes --]
also sprach Uwe Kleine-König <u.kleine-koenig@pengutronix.de> [2009.01.15.0827 +1100]:
> This helps avoiding code duplication for the next commit.
What's the "next commit"?
--
.''`. martin f. krafft <madduck@d.o> Related projects:
: :' : proud Debian developer http://debiansystem.info
`. `'` http://people.debian.org/~madduck http://vcs-pkg.org
`- Debian - when you have better things to do than fixing systems
i'd rather be riding a high speed tractor
with a beer on my lap,
and a six pack of girls next to me.
[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: John (zzz) Doe <john.doe@xz> (Comment)
From: Junio C Hamano @ 2009-01-21 3:12 UTC (permalink / raw)
To: Kirill Smelkov; +Cc: Johannes Schindelin, git
In-Reply-To: <20090120191446.GB5721@roro3.zxlink>
Kirill Smelkov <kirr@landau.phys.spbu.ru> writes:
> On Sun, Jan 18, 2009 at 10:50:12AM -0800, Junio C Hamano wrote:
>> So we can separate "John (zzz) Doe <john.doe@xz> (Comment)" into:
>>
>> AUTHOR_EMAIL=john.doe@xz
>> AUTHOR_NAME="John (zzz) Doe (Comment)"
>>
>> and leave it like so, I think.
>
> Ok, here you are:
>
> Subject: [PATCH 1/3] mailinfo: cleanup extra spaces for complex 'From'
>
> As described in RFC822 (3.4.3 COMMENTS, and A.1.4.), comments, as e.g.
>
> John (zzz) Doe <john.doe@xz> (Comment)
>
> should "NOT [be] included in the destination mailbox"
>
> On the other hand, quoting Junio:
>
>> The above quote from the RFC is irrelevant. Note that it is only about
>> how you extract the e-mail address, discarding everything else.
>>
>> What mailinfo wants to do is to separate the human-readable name and the
>> e-mail address, and we want to use _both_ results from it.
>>
>> We separate a few example From: lines like this:
>>
>> Kirill Smelkov <kirr@smelkov.xz>
>> ==> AUTHOR_EMAIL="kirr@smelkov.xz" AUTHOR_NAME="Kirill Smelkov"
>>
>> kirr@smelkov.xz (Kirill Smelkov)
>> ==> AUTHOR_EMAIL="kirr@smelkov.xz" AUTHOR_NAME="Kirill Smelkov"
>>
>> Traditionally, the way people spelled their name on From: line has been
>> either one of the above form. Typically comment form (i.e. the second
>> one) adds the name at the end, while "Name <addr>" form has the name at
>> the front. But I do not think RFC requires that, primarily because it is
>> all about discarding non-address part to find the e-mail address aka
>> "destination mailbox". It does not specify how humans should interpret
>> the human readable name and the comment.
>>
>> Now, why is the name not AUTHOR_NAME="(Kirill Smelkov)" in the latter
>> form?
>>
>> It is just common sense transformation. Otherwise it looks simply ugly,
>> and it is obvious that the parentheses is not part of the name of the
>> person who used "kirr@smelkov.xz (Kirill Smelkov)" on his From: line.
>>
>> So we can separate "John (zzz) Doe <john.doe@xz> (Comment)" into:
>>
>> AUTHOR_EMAIL=john.doe@xz
>> AUTHOR_NAME="John (zzz) Doe (Comment)"
>>
>> and leave it like so, I think.
>
> So let's just correctly remove extra spaces which could be left inside
> name.
>
> We need this functionality to pass all RFC2047 based tests in the next commit.
>
> Signed-off-by: Kirill Smelkov <kirr@landau.phys.spbu.ru>
> ---
> ...
> Is it ok?
I think the patch text looks good, but what you have as the proposed
commit log message does not look anything like log message we usually use.
- If you agree with my comment that "should NOT be included" from the RFC
you quoted is irrelevant, then I do not think you would even want to
have anything before "On the other hand,...".
- If you disagree, then why are you bending the patch text to match what
I say? ;-)
Also I am not sure "passing RFC2047 based tests" is a valid purpose nor
justification for adding this patch (because you could argue that the
tests and the results the tests expect are faulty).
The way we (and your patches) do thinks is to have the desired outcome
designed first, and then have tests to make sure that the implementation
matches the desired outcome.
Given that, what I would probably suggest would be...
-- 8< -- cut from here -- 8< --
mailinfo: handle comment fields in From: line sanely
Most commonly, people have their human readable name and their e-mail
address on their From: line in one of two formats:
Kirill Smelkov <kirr@smelkov.xz>
kirr@smelkov.xz (Kirill Smelkov)
In addition, you can have "Comments" in parentheses at random places, like
this:
John (zzz) Doe <john.doe@xz> (Comment)
RFC822 defines rules to extract the "destination mailbox" out of such
input (and we correctly extract "kirr@smelkov.xz" and "john.doe@xz" from
these examples). It however does not specify how to pick up the human
readable name from the remainder, and the existing code randomly dropped
pieces of information in <<<this and that way --- explain the breakage you
wanted to fix with your patch, perhaps "and left a newline in the result"
may be one of the breakages>>>.
This patch changes the rule so that <<<explain what it does here. I think
what the code does is (1) remove the e-mail (and angle brackets around
it), (2) sanitize LF into a single SP to keep the result a single line,
and (3) as a special case, if the result is enclosed by () pair, remove
them---this rule is to format the second common case listed above
sanely>>>.
A subsequent patch using From: lines taken from the example section of
RFC2047 will test this feature.
-- >8 --
^ permalink raw reply
* Re: unmerged flag at git diff-index
From: Johannes Schindelin @ 2009-01-21 2:48 UTC (permalink / raw)
To: Frank Li; +Cc: git
In-Reply-To: <1976ea660901201813r7b03525ax39fd0e1ecfb1700e@mail.gmail.com>
Hi,
On Wed, 21 Jan 2009, Frank Li wrote:
> There are conflict when I call git merge
> git ls-files -u
> 100644 54d05e1c40c686027b611b702e014284e9ab9a31 1 a.c
> 100644 a8a1b77c0d92f33491a0a69a8645f02ececb1eb6 2 a.c
> 100644 642ba972b2234f63048abcca544b3926f94e04ee 3 a.c
>
> but when use git diff-index HEAD
> :100644 100644 a8a1b77c0d92f33491a0a69a8645f02ececb1eb6 0000000000000000000000
> 0000000000000000 M a.c
>
> a.c flag is M: Modification of the contents or mode of a file.
>
> I think it should be U: file is unmerged(you must complete the merge
> before it can be commited)
You're talking plumbing here. And diff-index is about the differences
between the working directory and the index _stage 0_.
I don't think that's a bug (what would you display? stage 1? stage 2?
stage 3?), but even if it weren't, it is plumbing, and plumbing's UI is
guaranteed to be stable.
So I guess that you really have to use "git ls-files --stage" at some
stage :-)
Ciao,
Dscho
^ permalink raw reply
* Re: unmerged flag at git diff-index
From: Junio C Hamano @ 2009-01-21 2:40 UTC (permalink / raw)
To: Frank Li; +Cc: git
In-Reply-To: <1976ea660901201813r7b03525ax39fd0e1ecfb1700e@mail.gmail.com>
Frank Li <lznuaa@gmail.com> writes:
> There are conflict when I call git merge
> git ls-files -u
> 100644 54d05e1c40c686027b611b702e014284e9ab9a31 1 a.c
> 100644 a8a1b77c0d92f33491a0a69a8645f02ececb1eb6 2 a.c
> 100644 642ba972b2234f63048abcca544b3926f94e04ee 3 a.c
>
> but when use git diff-index HEAD
> :100644 100644 a8a1b77c0d92f33491a0a69a8645f02ececb1eb6 0000000000000000000000
> 0000000000000000 M a.c
>
> a.c flag is M: Modification of the contents or mode of a file.
That is correct.
By asking "git diff-index HEAD", you are asking to compare the work tree
contents through the index against HEAD commit. The work tree path by
definition cannot be "unmerged" as there is no stages in the work tree.
"git diff-index --cached HEAD" would compare the index against the HEAD
commit, and would say it is unmerged.
^ permalink raw reply
* unmerged flag at git diff-index
From: Frank Li @ 2009-01-21 2:13 UTC (permalink / raw)
To: git
There are conflict when I call git merge
git ls-files -u
100644 54d05e1c40c686027b611b702e014284e9ab9a31 1 a.c
100644 a8a1b77c0d92f33491a0a69a8645f02ececb1eb6 2 a.c
100644 642ba972b2234f63048abcca544b3926f94e04ee 3 a.c
but when use git diff-index HEAD
:100644 100644 a8a1b77c0d92f33491a0a69a8645f02ececb1eb6 0000000000000000000000
0000000000000000 M a.c
a.c flag is M: Modification of the contents or mode of a file.
I think it should be U: file is unmerged(you must complete the merge
before it can be commited)
^ permalink raw reply
* Re: [PATCH] Add --contains flag to git tag
From: Jake Goulding @ 2009-01-21 1:37 UTC (permalink / raw)
To: git
In-Reply-To: <1232501631-21531-3-git-send-email-goulding@vivisimo.com>
Ack - I sent that a bit before I meant to (and before I added patch sequence numbers).
This is a patch that adds a --contains flag to git tag, providing similar functionality as git branch --contains.
Please let me know what else I have inevitably messed up, and I shall endeavor to fix and resubmit.
-Jake
----- Original Message -----
From: "Jake Goulding" <goulding@vivisimo.com>
To: git@vger.kernel.org
Cc: "Jake Goulding" <goulding@vivisimo.com>
Sent: Tuesday, January 20, 2009 8:33:49 PM GMT -05:00 US/Canada Eastern
Subject: [PATCH] Add --contains flag to git tag
This functions similar to how --contains works for git branch - it
will show all tags that contain the specified commit. Indeed, it uses
the same lookup mechanisms as git branch.
Signed-off-by: Jake Goulding <goulding@vivisimo.com>
---
builtin-tag.c | 28 ++++++++++++++++++++++++++--
1 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/builtin-tag.c b/builtin-tag.c
index a398499..453ed26 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -26,6 +26,7 @@ static char signingkey[1000];
struct tag_filter {
const char *pattern;
int lines;
+ struct commit_list *with_commit;
};
#define PGP_SIGNATURE "-----BEGIN PGP SIGNATURE-----"
@@ -35,6 +36,15 @@ static int show_reference(const char *refname, const unsigned char *sha1,
{
struct tag_filter *filter = cb_data;
+ if (filter->with_commit) {
+ struct commit *commit;
+
+ commit = lookup_commit_reference_gently(sha1, 1);
+ if (!commit)
+ return error("branch '%s' does not point at a commit", refname);
+ if (!has_commit(commit, filter->with_commit))
+ return 0;
+ }
if (!fnmatch(filter->pattern, refname, 0)) {
int i;
unsigned long size;
@@ -79,7 +89,8 @@ static int show_reference(const char *refname, const unsigned char *sha1,
return 0;
}
-static int list_tags(const char *pattern, int lines)
+static int list_tags(const char *pattern, int lines,
+ struct commit_list *with_commit)
{
struct tag_filter filter;
@@ -88,6 +99,7 @@ static int list_tags(const char *pattern, int lines)
filter.pattern = pattern;
filter.lines = lines;
+ filter.with_commit = with_commit;
for_each_tag_ref(show_reference, (void *) &filter);
@@ -360,6 +372,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
list = 0, delete = 0, verify = 0;
const char *msgfile = NULL, *keyid = NULL;
struct msg_arg msg = { 0, STRBUF_INIT };
+ struct commit_list *with_commit = NULL;
struct option options[] = {
OPT_BOOLEAN('l', NULL, &list, "list tag names"),
{ OPTION_INTEGER, 'n', NULL, &lines, NULL,
@@ -378,6 +391,14 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
OPT_STRING('u', NULL, &keyid, "key-id",
"use another key to sign the tag"),
OPT_BOOLEAN('f', NULL, &force, "replace the tag if exists"),
+
+ OPT_GROUP("Tag listing options"),
+ {
+ OPTION_CALLBACK, 0, "contains", &with_commit, "commit",
+ "print only tags that contain the commit",
+ PARSE_OPT_LASTARG_DEFAULT,
+ parse_opt_with_commit, (intptr_t)"HEAD",
+ },
OPT_END()
};
@@ -402,9 +423,12 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
if (list + delete + verify > 1)
usage_with_options(git_tag_usage, options);
if (list)
- return list_tags(argv[0], lines == -1 ? 0 : lines);
+ return list_tags(argv[0], lines == -1 ? 0 : lines,
+ with_commit);
if (lines != -1)
die("-n option is only allowed with -l.");
+ if (with_commit)
+ die("--contains option is only allowed with -l.");
if (delete)
return for_each_tag_name(argv, delete_tag);
if (verify)
--
1.6.0.6
^ permalink raw reply related
* [PATCH 2/2 v2] valgrind: ignore ldso errors
From: Johannes Schindelin @ 2009-01-21 1:36 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0901210216440.19014@racer>
On some Linux systems, we get a host of Cond and Addr errors
from calls to dlopen that are caused by nss modules. We
should be able to safely ignore anything happening in
ld-*.so as "not our problem."
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Only change vs v1: adds Addr4 suppression, so that ld.so "errors"
are ignored on 32-bit, too.
t/valgrind/default.supp | 18 ++++++++++++++++++
1 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/t/valgrind/default.supp b/t/valgrind/default.supp
index 2482b3b..6061283 100644
--- a/t/valgrind/default.supp
+++ b/t/valgrind/default.supp
@@ -11,6 +11,24 @@
}
{
+ ignore-ldso-cond
+ Memcheck:Cond
+ obj:*ld-*.so
+}
+
+{
+ ignore-ldso-addr8
+ Memcheck:Addr8
+ obj:*ld-*.so
+}
+
+{
+ ignore-ldso-addr4
+ Memcheck:Addr4
+ obj:*ld-*.so
+}
+
+{
writing-data-from-zlib-triggers-errors
Memcheck:Param
write(buf)
--
1.6.1.243.g6c8bb35
^ permalink raw reply related
* Re: how to keeping track of cherry-pick?
From: Junio C Hamano @ 2009-01-21 1:29 UTC (permalink / raw)
To: Knut Olav Bøhmer; +Cc: git
In-Reply-To: <497663E4.4000302@telenor.com>
Knut Olav Bøhmer <knut-olav.bohmer@telenor.com> writes:
> svnmerge.py can give us a list of revisions available for merging. The
> result is similar to "git log --chery-pick master..dev" The difference
> is that svnmerge.py operates on revision-numbers, and --chery-pick looks
> at the diffs. The result of that is that when we get a conflict when a
> patch is cherry-picked, it will still show up as "available" when I run
> "git log --cherry-pick master..dev"
I think you are looking at it a wrong way.
Because subversion (at least the older one) does not keep track of merges,
you had to track cherry-picks. But cherry-pick is not how you usually do
things in git. You keep many topic branches with different doneness, and
you merge well-cooked ones to the more stable integration branch while
leaving others still cooking. So what you want to know is not cherry-pick
status, but merge status.
Because git tracks merges, output from "git log master..$topic" gives all
you need to know about $topic. The command lists changes that are still
not merged to master on the $topic branch. If it is empty, $topic is
already merged fully to master. Otherwise you still have things to
merge.
To get a list of topics that have been merged (or not merged) to a
particular integration branch, you should be able to use "git branch" with
its --merged and --no-merged options. This does not list what commits
each yet-to-be-merged topics have, though.
^ permalink raw reply
* Re: valgrind patches, was Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)
From: Johannes Schindelin @ 2009-01-21 1:26 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20090121003739.GA18373@coredump.intra.peff.net>
Hi,
On Tue, 20 Jan 2009, Jeff King wrote:
> On Wed, Jan 21, 2009 at 01:28:01AM +0100, Johannes Schindelin wrote:
>
> > Actually, I test first if it is there, and only if it is not, try to
> > create the symlink.
> >
> > Now, there is still a very minor chance for a race, namely if two
> > processes happen to test the existence of the missing symlink at exactly
> > the same time, and both do not find it, so both processes will try to
> > create it.
>
> Yep. Though I find "minor chance" when it comes to races to really mean
> "annoying to debug". But...
Well, in this case, you will find that the "bug" is _at most_ some
binaries not being found.
And really, the chance is so small as to be forgotten in the clutter:
after the valgrind setup, there are so many other things which are done
that by the time we actually use the Git binaries, everything should be
okay.
And keep in mind, this _only_ matters if you do make -j _and_ you haven't
run --valgrind _ever_. Once the symlinks are there, they are there.
(Actually, with my new patch, the may be replaced, but _only_ if
necessary, and the same thing would apply as I said earlier: the binary
would not be found, or a binary from the PATH would be run without
valgrind; but the next runs will not have the problem.)
> > However, the symlink creation is not checked for success, so the
> > processes will still both run just fine.
>
> Yes, so there is no race in what is there currently. It's just sad that
> we can't detect any actual errors.
Now we can. I actually check for the correct link target now (which means
I also check for a link), and if it is incorrect, the link is recreated
(and the deletion is checked for errors).
> > There is a very subtle problem, though. If you screw with your
> > configuration, replacing a link in t/valgrind/ by a script, my code
> > will not try to undo it. However, I think that's really asking for
> > trouble, and you can get out of the mess by "rm -r t/valgrind/git*".
>
> I think we can safely ignore such mucking about in the valgrind
> directory as craziness.
You'll find that v2 copes with that, too.
> > Another problem which is potentially much more troublesome is this:
> > when there was a script by a certain name, my code would symlink it to
> > $GIT_DIR/$BASENAME (actually a relative path, but you get the idea).
> > If that script is turned into a builtin -- this list has certainly
> > known a certain person to push for that kind of conversion :-) -- that
> > fact is not picked up.
>
> Yes. One way around this is to generate a "want" and a "have" list, and
> then just operate on the differences. Something like (totally untested):
>
> (cd $GIT_VALGRIND && ls) | sort >have
> (cd $TEST_DIRECTORY/.. && ls git git-*) | sort >want
> comm -23 have want | xargs -r rm -v
> comm -13 have want | while read f; do ln -s ../../$f $GIT_VALGRIND/$f; done
>
> and then you are also cleaning every time you create.
The script will now pick up on those changes, and recreate the symlink
correctly.
We don't need cleaning, as we only link to $TEST_DIRECTORY/.. (at least
via valgrind.sh), and if the binary does not exist there, well, it does
not exist there, and the script will error out, saying so.
Ciao,
Dscho
^ permalink raw reply
* [INTERDIFF of PATCH 1/2 v2] Add valgrind support in test scripts
From: Johannes Schindelin @ 2009-01-21 1:11 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0901210209580.19014@racer>
t/README | 6 +++++-
t/test-lib.sh | 32 +++++++++++++++++++++-----------
t/valgrind/.gitignore | 3 +--
t/valgrind/valgrind.sh | 4 ++--
4 files changed, 29 insertions(+), 16 deletions(-)
diff --git a/t/README b/t/README
index 8f12d48..0cee429 100644
--- a/t/README
+++ b/t/README
@@ -39,7 +39,8 @@ this:
* passed all 3 test(s)
You can pass --verbose (or -v), --debug (or -d), and --immediate
-(or -i) command line argument to the test.
+(or -i) command line argument to the test, or by setting GIT_TEST_OPTS
+appropriately before running "make".
--verbose::
This makes the test more verbose. Specifically, the
@@ -58,6 +59,9 @@ You can pass --verbose (or -v), --debug (or -d), and --immediate
This causes additional long-running tests to be run (where
available), for more exhaustive testing.
+--valgrind::
+ Execute all Git binaries with valgrind and stop on errors (the
+ exit code will be 126).
Skipping Tests
--------------
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 6bd893d..f031905 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -488,27 +488,37 @@ then
GIT_EXEC_PATH=$TEST_DIRECTORY/..
else
# override all git executables in PATH and TEST_DIRECTORY/..
- GIT_VALGRIND=$TEST_DIRECTORY/valgrind
+ GIT_VALGRIND=$TEST_DIRECTORY/valgrind/bin
mkdir -p "$GIT_VALGRIND"
OLDIFS=$IFS
IFS=:
- for path in $PATH:$TEST_DIRECTORY/..
+ for path in $PATH $TEST_DIRECTORY/..
do
- ls "$TEST_DIRECTORY"/../git "$path"/git-* 2> /dev/null |
+ ls "$path"/git "$path"/git-* 2> /dev/null |
while read file
do
# handle only executables
- test -x "$file" || continue
+ test -x "$file" && test ! -d "$file" || continue
base=$(basename "$file")
- test ! -h "$GIT_VALGRIND"/"$base" || continue
-
- if test "#!" = "$(head -c 2 < "$file")"
+ symlink_target=$TEST_DIRECTORY/../$base
+ # do not override scripts
+ if test -x "$symlink_target" &&
+ test "#!" != "$(head -c 2 < "$symlink_target")"
+ then
+ symlink_target=../valgrind.sh
+ fi
+ case "$base" in
+ *.sh|*.perl)
+ symlink_target=../unprocessed-script
+ esac
+ # create the link, or replace it if it is out of date
+ if test ! -h "$GIT_VALGRIND"/"$base" ||
+ test "$symlink_target" != \
+ "$(readlink "$GIT_VALGRIND"/"$base")"
then
- # do not override scripts
- ln -s ../../"$base" "$GIT_VALGRIND"/"$base"
- else
- ln -s valgrind.sh "$GIT_VALGRIND"/"$base"
+ rm -f "$GIT_VALGRIND"/"$base" || exit
+ ln -s "$symlink_target" "$GIT_VALGRIND"/"$base"
fi
done
done
diff --git a/t/valgrind/.gitignore b/t/valgrind/.gitignore
index d781a63..ae3c172 100644
--- a/t/valgrind/.gitignore
+++ b/t/valgrind/.gitignore
@@ -1,2 +1 @@
-/git
-/git-*
+/bin/
diff --git a/t/valgrind/valgrind.sh b/t/valgrind/valgrind.sh
index 24f3a4e..2c4b54b 100755
--- a/t/valgrind/valgrind.sh
+++ b/t/valgrind/valgrind.sh
@@ -4,9 +4,9 @@ base=$(basename "$0")
exec valgrind -q --error-exitcode=126 \
--leak-check=no \
- --suppressions="$GIT_VALGRIND/default.supp" \
+ --suppressions="$GIT_VALGRIND/../default.supp" \
--gen-suppressions=all \
--log-fd=4 \
--input-fd=4 \
$GIT_VALGRIND_OPTIONS \
- "$GIT_VALGRIND"/../../"$base" "$@"
+ "$GIT_VALGRIND"/../../../"$base" "$@"
^ permalink raw reply related
* [PATCH 1/2 v2] Add valgrind support in test scripts
From: Johannes Schindelin @ 2009-01-21 1:10 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20090121001219.GA18169@coredump.intra.peff.net>
This patch adds the ability to use valgrind's memcheck tool to
diagnose memory problems in git while running the test scripts. It
works by placing a "fake" git in the front of the test script's PATH;
this fake git runs the real git under valgrind. It also points the
exec-path such that any stand-alone dashed git programs are run using
the same script. In this way we avoid having to modify the actual git
code in any way.
To be certain that every call to any git executable is intercepted,
the PATH is searched for executables beginning with "git-"; Scripts
are excluded however.
Valgrind can be used by specifying "GIT_TEST_OPTS=--valgrind" in the
make invocation. Any invocation of git that finds any errors under
valgrind will exit with failure code 126. Any valgrind output will go
to the usual stderr channel for tests (i.e., /dev/null, unless -v has
been specified).
If you need to pass options to valgrind -- you might want to run
another tool than memcheck, for example -- you can set the environment
variable GIT_VALGRIND_OPTIONS.
A few default suppressions are included, since libz seems to
trigger quite a few false positives. We'll assume that libz
works and that we can ignore any errors which are reported
there.
Initial patch and all the hard work by Jeff King.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Changes vs v1:
- the symlinks will be created in t/valgrind/bin/ again, as it is
easier to remove the whole directory than weed out the unwanted
files from t/valgrind/.
- symbolic links are inspected for correct targets now, and if they
point somewhere else than expected, they are removed (this can
error out if the file could not be removed) and recreated.
- if the executable ends in ".sh" or ".perl", the target will be
set to a non-existing file (to catch invocations, erroring out).
- the Git binaries from the root are actually found now (IFS is
only interpreted after the file is parsed, it seems).
- added rudimentary documentation to t/README.
Interdiff to follow.
t/README | 6 ++++-
t/test-lib.sh | 49 +++++++++++++++++++++++++++++++++++++++++++++-
t/valgrind/.gitignore | 1 +
t/valgrind/default.supp | 21 ++++++++++++++++++++
t/valgrind/valgrind.sh | 12 +++++++++++
5 files changed, 86 insertions(+), 3 deletions(-)
create mode 100644 t/valgrind/.gitignore
create mode 100644 t/valgrind/default.supp
create mode 100755 t/valgrind/valgrind.sh
diff --git a/t/README b/t/README
index 8f12d48..0cee429 100644
--- a/t/README
+++ b/t/README
@@ -39,7 +39,8 @@ this:
* passed all 3 test(s)
You can pass --verbose (or -v), --debug (or -d), and --immediate
-(or -i) command line argument to the test.
+(or -i) command line argument to the test, or by setting GIT_TEST_OPTS
+appropriately before running "make".
--verbose::
This makes the test more verbose. Specifically, the
@@ -58,6 +59,9 @@ You can pass --verbose (or -v), --debug (or -d), and --immediate
This causes additional long-running tests to be run (where
available), for more exhaustive testing.
+--valgrind::
+ Execute all Git binaries with valgrind and stop on errors (the
+ exit code will be 126).
Skipping Tests
--------------
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 79f69de..f031905 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -94,6 +94,8 @@ do
--no-python)
# noop now...
shift ;;
+ --va|--val|--valg|--valgr|--valgri|--valgrin|--valgrind)
+ valgrind=t; shift ;;
*)
break ;;
esac
@@ -480,8 +482,51 @@ test_done () {
# Test the binaries we have just built. The tests are kept in
# t/ subdirectory and are run in 'trash directory' subdirectory.
TEST_DIRECTORY=$(pwd)
-PATH=$TEST_DIRECTORY/..:$PATH
-GIT_EXEC_PATH=$(pwd)/..
+if test -z "$valgrind"
+then
+ PATH=$TEST_DIRECTORY/..:$PATH
+ GIT_EXEC_PATH=$TEST_DIRECTORY/..
+else
+ # override all git executables in PATH and TEST_DIRECTORY/..
+ GIT_VALGRIND=$TEST_DIRECTORY/valgrind/bin
+ mkdir -p "$GIT_VALGRIND"
+ OLDIFS=$IFS
+ IFS=:
+ for path in $PATH $TEST_DIRECTORY/..
+ do
+ ls "$path"/git "$path"/git-* 2> /dev/null |
+ while read file
+ do
+ # handle only executables
+ test -x "$file" && test ! -d "$file" || continue
+
+ base=$(basename "$file")
+ symlink_target=$TEST_DIRECTORY/../$base
+ # do not override scripts
+ if test -x "$symlink_target" &&
+ test "#!" != "$(head -c 2 < "$symlink_target")"
+ then
+ symlink_target=../valgrind.sh
+ fi
+ case "$base" in
+ *.sh|*.perl)
+ symlink_target=../unprocessed-script
+ esac
+ # create the link, or replace it if it is out of date
+ if test ! -h "$GIT_VALGRIND"/"$base" ||
+ test "$symlink_target" != \
+ "$(readlink "$GIT_VALGRIND"/"$base")"
+ then
+ rm -f "$GIT_VALGRIND"/"$base" || exit
+ ln -s "$symlink_target" "$GIT_VALGRIND"/"$base"
+ fi
+ done
+ done
+ IFS=$OLDIFS
+ PATH=$GIT_VALGRIND:$PATH
+ GIT_EXEC_PATH=$GIT_VALGRIND
+ export GIT_VALGRIND
+fi
GIT_TEMPLATE_DIR=$(pwd)/../templates/blt
unset GIT_CONFIG
GIT_CONFIG_NOSYSTEM=1
diff --git a/t/valgrind/.gitignore b/t/valgrind/.gitignore
new file mode 100644
index 0000000..ae3c172
--- /dev/null
+++ b/t/valgrind/.gitignore
@@ -0,0 +1 @@
+/bin/
diff --git a/t/valgrind/default.supp b/t/valgrind/default.supp
new file mode 100644
index 0000000..2482b3b
--- /dev/null
+++ b/t/valgrind/default.supp
@@ -0,0 +1,21 @@
+{
+ ignore-zlib-errors-cond
+ Memcheck:Cond
+ obj:*libz.so*
+}
+
+{
+ ignore-zlib-errors-value4
+ Memcheck:Value4
+ obj:*libz.so*
+}
+
+{
+ writing-data-from-zlib-triggers-errors
+ Memcheck:Param
+ write(buf)
+ obj:/lib/ld-*.so
+ fun:write_in_full
+ fun:write_buffer
+ fun:write_loose_object
+}
diff --git a/t/valgrind/valgrind.sh b/t/valgrind/valgrind.sh
new file mode 100755
index 0000000..2c4b54b
--- /dev/null
+++ b/t/valgrind/valgrind.sh
@@ -0,0 +1,12 @@
+#!/bin/sh
+
+base=$(basename "$0")
+
+exec valgrind -q --error-exitcode=126 \
+ --leak-check=no \
+ --suppressions="$GIT_VALGRIND/../default.supp" \
+ --gen-suppressions=all \
+ --log-fd=4 \
+ --input-fd=4 \
+ $GIT_VALGRIND_OPTIONS \
+ "$GIT_VALGRIND"/../../../"$base" "$@"
--
1.6.1.243.g6c8bb35
^ permalink raw reply related
* Re: [PATCH 1/2] Add valgrind support in test scripts
From: Johannes Schindelin @ 2009-01-21 0:41 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20090121001219.GA18169@coredump.intra.peff.net>
Hi,
On Tue, 20 Jan 2009, Jeff King wrote:
> On Tue, Jan 20, 2009 at 04:04:28PM +0100, Johannes Schindelin wrote:
>
> > +else
> > + # override all git executables in PATH and TEST_DIRECTORY/..
> > + GIT_VALGRIND=$TEST_DIRECTORY/valgrind
> > + mkdir -p "$GIT_VALGRIND"
>
> Isn't this mkdir unnecessary, since it is actually part of the
> repository (i.e., there is a gitignore there already).
>
> However, I think it makes more sense to put the symlink cruft into
> "$GIT_VALGRIND/bin". That way you can clean up the cruft very easily. In
> which case you do need to "mkdir" that directory.
Hmm. I actually liked the hierarchy to be shallow, but I could be
convinced...
> > + OLDIFS=$IFS
> > + IFS=:
> > + for path in $PATH:$TEST_DIRECTORY/..
> > + do
> > + ls "$TEST_DIRECTORY"/../git "$path"/git-* 2> /dev/null |
>
> Why aren't these both "$path"/ ?
Yeah. Makes it more readable, doesn't it?
> But more importantly, do we really need to bother overriding the whole
> $PATH? In theory, we aren't calling anything git-* that isn't in
> "$TEST_DIRECTORY/..". And while it might be nice to catch it if we do,
> it seems like detecting that is totally orthogonal to running valgrind,
> and we get different behavior from valgrind versus not. And I think the
> two should be as similar as possible (with the obvious except of
> actually, you know, running valgrind).
Actually, the two _are_ orthogonal from the technical viewpoint.
But with the infrastructure we have in place, it was already very easy to
make sure that calls to a Git program we no longer ship are caught.
I vividly remember such a bug costing me 3 hours of my life, and a few
hairs.
So I think "as it's already _that_ easy, we should catch them bugs, too".
Needs some documentation though, I agree.
> > + base=$(basename "$file")
> > + test ! -h "$GIT_VALGRIND"/"$base" || continue
> > +
> > + if test "#!" = "$(head -c 2 < "$file")"
> > + then
> > + # do not override scripts
> > + ln -s ../../"$base" "$GIT_VALGRIND"/"$base"
> > + else
> > + ln -s valgrind.sh "$GIT_VALGRIND"/"$base"
> > + fi
>
> It would be nice to actually detect errors. But you have to
> differentiate between EEXIST and other errors, which is a pain. And you
> can't use "ln -sf" because it isn't atomic.
I really would not care all that much about that.
'GIT_TEST_OPTS==--valgrind make test' should be run by experts. And even
if it is a dummy driving the test, the next "make" call should take care
of that.
> Copying would solve that (provided you copied to a tempfile and did
> an atomic rename). Or writing this snippet as a C helper.
Nah, that is really too much work for such a rare thing. Think about it.
The symlinks are set up once. And even if you do that with -j50, there is
hardly a chance that two processes conflict with each other, and even if
they do, they do the same thing.
No, what I really want to fix is a script being replaced by a binary.
> > --- /dev/null
> > +++ b/t/valgrind/valgrind.sh
> > @@ -0,0 +1,12 @@
> > +#!/bin/sh
> > +
> > +base=$(basename "$0")
> > +
> > +exec valgrind -q --error-exitcode=126 \
> > + --leak-check=no \
> > + --suppressions="$GIT_VALGRIND/default.supp" \
> > + --gen-suppressions=all \
> > + --log-fd=4 \
> > + --input-fd=4 \
> > + $GIT_VALGRIND_OPTIONS \
> > + "$GIT_VALGRIND"/../../"$base" "$@"
>
> Hm. My version had to do some magic with the GIT_EXEC_PATH, but I think
> that is because I didn't set GIT_EXEC_PATH in the first place. If yours
> works (and I haven't really tested it -- I remember it being a real pain
> in the butt to make sure valgrind was getting called from every code
> path), then I like your approach much better.
I set GIT_EXEC_PATH... to $GIT_VALGRIND.
Ciao,
Dscho
^ permalink raw reply
* Re: valgrind patches, was Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)
From: Jeff King @ 2009-01-21 0:37 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0901210119510.19014@racer>
On Wed, Jan 21, 2009 at 01:28:01AM +0100, Johannes Schindelin wrote:
> Actually, I test first if it is there, and only if it is not, try to
> create the symlink.
>
> Now, there is still a very minor chance for a race, namely if two
> processes happen to test the existence of the missing symlink at exactly
> the same time, and both do not find it, so both processes will try to
> create it.
Yep. Though I find "minor chance" when it comes to races to really mean
"annoying to debug". But...
> However, the symlink creation is not checked for success, so the processes
> will still both run just fine.
Yes, so there is no race in what is there currently. It's just sad that
we can't detect any actual errors.
> There is a very subtle problem, though. If you screw with your
> configuration, replacing a link in t/valgrind/ by a script, my code will
> not try to undo it. However, I think that's really asking for trouble,
> and you can get out of the mess by "rm -r t/valgrind/git*".
I think we can safely ignore such mucking about in the valgrind
directory as craziness.
> Another problem which is potentially much more troublesome is this:
> when there was a script by a certain name, my code would symlink it
> to $GIT_DIR/$BASENAME (actually a relative path, but you get the
> idea). If that script is turned into a builtin -- this list has certainly
> known a certain person to push for that kind of conversion :-) -- that
> fact is not picked up.
Yes. One way around this is to generate a "want" and a "have" list, and
then just operate on the differences. Something like (totally untested):
(cd $GIT_VALGRIND && ls) | sort >have
(cd $TEST_DIRECTORY/.. && ls git git-*) | sort >want
comm -23 have want | xargs -r rm -v
comm -13 have want | while read f; do ln -s ../../$f $GIT_VALGRIND/$f; done
and then you are also cleaning every time you create.
-Peff
^ permalink raw reply
* Re: valgrind patches, was Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)
From: Johannes Schindelin @ 2009-01-21 0:28 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20090121001551.GB18169@coredump.intra.peff.net>
Hi,
On Tue, 20 Jan 2009, Jeff King wrote:
> On Wed, Jan 21, 2009 at 01:10:22AM +0100, Johannes Schindelin wrote:
>
> > > Hmm. I suppose that would work, since every test run is trying to create
> > > the same state.
> >
> > Yep, that's what I meant with "no race".
>
> Right, but it is still possible to screw it up, if your creation process
> does a delete-create. But it looks like you did it correctly in your
> patch (try to create, and if you fail because it's there, assume it's
> right).
Actually, I test first if it is there, and only if it is not, try to
create the symlink.
Now, there is still a very minor chance for a race, namely if two
processes happen to test the existence of the missing symlink at exactly
the same time, and both do not find it, so both processes will try to
create it.
However, the symlink creation is not checked for success, so the processes
will still both run just fine.
There is a very subtle problem, though. If you screw with your
configuration, replacing a link in t/valgrind/ by a script, my code will
not try to undo it. However, I think that's really asking for trouble,
and you can get out of the mess by "rm -r t/valgrind/git*".
Another problem which is potentially much more troublesome is this:
when there was a script by a certain name, my code would symlink it
to $GIT_DIR/$BASENAME (actually a relative path, but you get the
idea). If that script is turned into a builtin -- this list has certainly
known a certain person to push for that kind of conversion :-) -- that
fact is not picked up.
But I think I have an easy solution for that.
> > In any case, I already found a bug in the nth_last series, thanks to
> > your work, which I'll send in a minute.
>
> Yay! It's nice when infrastructure work like this actually pays off.
Yep! Thanks!
> Thanks for picking up this topic...I can drop the size of my
> ever-growing git todo list by one. :)
Actually, don't remind me... of my TODO list.
Ciao,
Dscho
^ permalink raw reply
* [VALGRIND PATCH for nth_last patch series] Fix invalid memory access
From: Johannes Schindelin @ 2009-01-21 0:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Thomas Rast, git, Johannes Sixt, Johan Herland
In-Reply-To: <alpine.DEB.1.00.0901191331590.3586@pacific.mpi-cbg.de>
Please squash in.
---
On Mon, 19 Jan 2009, Johannes Schindelin wrote:
> @@ -720,7 +718,7 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
> if (target[len] == '\n' && !strncmp(match, target, len))
> return 0;
This code is still not valid, as target[len] can be well after the
NUL marker.
Found by valgrind.
sha1_name.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index 4d10705..803f9d2 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -735,7 +735,7 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
if ((target = strstr(match, " to ")) != NULL) {
len = target - match;
target += 4;
- if (target[len] == '\n' && !strncmp(match, target, len))
+ if (len == strlen(target) && !strncmp(match, target, len))
return 0;
}
else
--
1.6.1.243.g6c8bb35
^ permalink raw reply related
* how to keeping track of cherry-pick?
From: Knut Olav Bøhmer @ 2009-01-20 23:53 UTC (permalink / raw)
To: git
Hi,
We are changing from subversion to git. We used to merge revisions from
development branch to stable with svnmerge.py.
svnmerge.py can give us a list of revisions available for merging. The
result is similar to "git log --chery-pick master..dev" The difference
is that svnmerge.py operates on revision-numbers, and --chery-pick looks
at the diffs. The result of that is that when we get a conflict when a
patch is cherry-picked, it will still show up as "available" when I run
"git log --cherry-pick master..dev"
svnmerge.py can also mark revisions as blocked, and will not show up in
the list of available revisions.
How can I keep track of cherry-picked patches, and mark patches as
"blocked", in the same (or similar) way as I did in subversion with
svnmerge.py?
--
Knut Olav Bøhmer
^ permalink raw reply
* Re: valgrind patches, was Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)
From: Jeff King @ 2009-01-21 0:15 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0901210105470.19014@racer>
On Wed, Jan 21, 2009 at 01:10:22AM +0100, Johannes Schindelin wrote:
> > Hmm. I suppose that would work, since every test run is trying to create
> > the same state.
>
> Yep, that's what I meant with "no race".
Right, but it is still possible to screw it up, if your creation process
does a delete-create. But it looks like you did it correctly in your
patch (try to create, and if you fail because it's there, assume it's
right).
> Heh. Okay. I was convinced that your valgrind patch predated my -j<n>
> patch...
I think I did an early version that did predate it, but then another
round afterwards.
> In any case, I already found a bug in the nth_last series, thanks to your
> work, which I'll send in a minute.
Yay! It's nice when infrastructure work like this actually pays off.
Thanks for picking up this topic...I can drop the size of my
ever-growing git todo list by one. :)
-Peff
^ permalink raw reply
* Re: [PATCH 1/2] Add valgrind support in test scripts
From: Jeff King @ 2009-01-21 0:12 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0901201602410.5159@intel-tinevez-2-302>
On Tue, Jan 20, 2009 at 04:04:28PM +0100, Johannes Schindelin wrote:
> +else
> + # override all git executables in PATH and TEST_DIRECTORY/..
> + GIT_VALGRIND=$TEST_DIRECTORY/valgrind
> + mkdir -p "$GIT_VALGRIND"
Isn't this mkdir unnecessary, since it is actually part of the
repository (i.e., there is a gitignore there already).
However, I think it makes more sense to put the symlink cruft into
"$GIT_VALGRIND/bin". That way you can clean up the cruft very easily. In
which case you do need to "mkdir" that directory.
> + OLDIFS=$IFS
> + IFS=:
> + for path in $PATH:$TEST_DIRECTORY/..
> + do
> + ls "$TEST_DIRECTORY"/../git "$path"/git-* 2> /dev/null |
Why aren't these both "$path"/ ?
But more importantly, do we really need to bother overriding the whole
$PATH? In theory, we aren't calling anything git-* that isn't in
"$TEST_DIRECTORY/..". And while it might be nice to catch it if we do,
it seems like detecting that is totally orthogonal to running valgrind,
and we get different behavior from valgrind versus not. And I think the
two should be as similar as possible (with the obvious except of
actually, you know, running valgrind).
> + base=$(basename "$file")
> + test ! -h "$GIT_VALGRIND"/"$base" || continue
> +
> + if test "#!" = "$(head -c 2 < "$file")"
> + then
> + # do not override scripts
> + ln -s ../../"$base" "$GIT_VALGRIND"/"$base"
> + else
> + ln -s valgrind.sh "$GIT_VALGRIND"/"$base"
> + fi
It would be nice to actually detect errors. But you have to
differentiate between EEXIST and other errors, which is a pain. And you
can't use "ln -sf" because it isn't atomic.
Copying would solve that (provided you copied to a tempfile and did
an atomic rename). Or writing this snippet as a C helper.
> --- /dev/null
> +++ b/t/valgrind/valgrind.sh
> @@ -0,0 +1,12 @@
> +#!/bin/sh
> +
> +base=$(basename "$0")
> +
> +exec valgrind -q --error-exitcode=126 \
> + --leak-check=no \
> + --suppressions="$GIT_VALGRIND/default.supp" \
> + --gen-suppressions=all \
> + --log-fd=4 \
> + --input-fd=4 \
> + $GIT_VALGRIND_OPTIONS \
> + "$GIT_VALGRIND"/../../"$base" "$@"
Hm. My version had to do some magic with the GIT_EXEC_PATH, but I think
that is because I didn't set GIT_EXEC_PATH in the first place. If yours
works (and I haven't really tested it -- I remember it being a real pain
in the butt to make sure valgrind was getting called from every code
path), then I like your approach much better.
-Peff
^ permalink raw reply
* Re: valgrind patches, was Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)
From: Johannes Schindelin @ 2009-01-21 0:10 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20090120232439.GA17746@coredump.intra.peff.net>
Hi,
On Tue, 20 Jan 2009, Jeff King wrote:
> On Tue, Jan 20, 2009 at 03:50:28PM +0100, Johannes Schindelin wrote:
>
> > > How will you deal with race conditions between two simultaneously
> > > running scripts? I.e., where are you going to put it?
> >
> > There are no race conditions, as for every git executable, a symbolic
> > link is created, pointing to the valgrind.sh script [*1*].
>
> Hmm. I suppose that would work, since every test run is trying to create
> the same state.
Yep, that's what I meant with "no race".
> > Besides, what with valgrind being a memory hog, you'd be nuts to call
> > valgrinded scripts simultaneously.
>
> I have to disagree there. I think there are two obvious usage patterns:
>
> - run script $X specifically under valgrind to track down a bug
>
> - run the whole test suite under valgrind occasionally to find
> latent bugs that wouldn't otherwise show up
>
> In the latter, you want a pretty beefy box. When I did the original
> patches, I ran through the whole test suite under valgrind. It took
> several hours on a 6GB quad-core box, using "-j4". I would hate for it
> to have taken an entire day. :)
Heh. Okay. I was convinced that your valgrind patch predated my -j<n>
patch...
In any case, I already found a bug in the nth_last series, thanks to your
work, which I'll send in a minute.
Ciao,
Dscho
^ permalink raw reply
* Re: Newbie Query
From: Johannes Schindelin @ 2009-01-21 0:05 UTC (permalink / raw)
To: Jeff King; +Cc: Nicolas Morey-Chaisemartin, Boyd Stephen Smith Jr., git
In-Reply-To: <20090120235125.GD17746@coredump.intra.peff.net>
Hi,
On Tue, 20 Jan 2009, Jeff King wrote:
> On Tue, Jan 20, 2009 at 10:46:47PM +0100, Nicolas Morey-Chaisemartin wrote:
>
> > Well I know there are solutions to convert it to a bare repo.
> > I was just wondering if there was a "clean" one which really converts
> > the repo to a bare one and not create a copy which is bare.
>
> It has been a long time since I have done this. It used to be that you
> could simply "mv foo/.git foo.git" and be done with it. These days I
> think you would also need "git config core.bare true". But I haven't
> actually tested it recently.
Yep, you need to set that option. At least if you initialized your
repository with anything newer than v1.5.0-rc1~3^2~2.
Ciao,
Dscho
^ permalink raw reply
* Re: An idea: maybe Git should use a lock/unlock file mode for problematic files? [Was: Re: after first git clone of linux kernel repository there are changed files in working dir]
From: Daniel Barkalow @ 2009-01-21 0:03 UTC (permalink / raw)
To: Alex Riesen; +Cc: John Chapman, Hannu Koivisto, rdkrsr, git
In-Reply-To: <81b0412b0901201525w22513418p57acc19457908a3@mail.gmail.com>
On Wed, 21 Jan 2009, Alex Riesen wrote:
> 2009/1/20 Daniel Barkalow <barkalow@iabervon.org>:
> > My impression was that this didn't happen in practice, because teams
> > would tend to not have two people create the same file at the same time,
> > but with different cases, and people interacting with the same file at
> > different times would use whatever case it was introduced with.
>
> It will and does happen in practice (annoingly too often even). Not with Git
> yet (with Perforce), where people do "branching" by simply copying things
> in another directory (perforce world does not know real branches),
> renaming files randomly, and putting the new directory back in the
> system (or maybe it is the strange tools here which do that - often
> it is the first character of a directory or file which gets down- or up-cased).
How does the resulting code work at all? With a case-sensitive filesystem,
most of the files you're using don't have the expected names any more, and
most systems will therefore not actually build or run.
I have to assume it's your strange tools, because we never have this
problem at my work, where we also use Perforce. Perhaps it's that we
always use "p4 integrate //some/project/version/...
//some/other/project/version/..." which inherently preserves the case of
all of the filenames within the project.
> As Perforce itself is case sensitive (like Git), using of such branches
> is a nightmare: the files get overwritten in checkout order which is
> not always sorted in predictable order. Combined with case-stupidity
> of the file system the working directories sometimes cause "interesting
> time" for unlucky users.
> Luckily (sadly) it is all-opening-in-a-wall shop, so the problem with "fanthom"
> files is rare (it is hard to notice) for most. Which actually makes it more
> frustrating when the real shit happens.
>
> And it will happen to Git as well, especially if development go crossplatform.
> It is not that hard to accidentally rename a file on case-sensitive file system,
> "git add *" it and commit without thinking (that's how most of software
> development happens, come to think of it).
People can accidentally rename files? And still have things work when they
do it on a case-sensitive filesystem?
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox