* [PATCH] git-svn: add --authors-file test
From: Eric Wong @ 2009-01-11 23:44 UTC (permalink / raw)
To: git
I'm not sure how often this functionality is used, but in case
it's not, having an extra test here will help catch breakage
sooner.
Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
This was posted over a year ago and forgotten about. Updated and
cleaned up a bit to work with the new test suite.
t/t9130-git-svn-authors-file.sh | 94 +++++++++++++++++++++++++++++++++++++++
1 files changed, 94 insertions(+), 0 deletions(-)
create mode 100755 t/t9130-git-svn-authors-file.sh
diff --git a/t/t9130-git-svn-authors-file.sh b/t/t9130-git-svn-authors-file.sh
new file mode 100755
index 0000000..b8fb277
--- /dev/null
+++ b/t/t9130-git-svn-authors-file.sh
@@ -0,0 +1,94 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Eric Wong
+#
+
+test_description='git svn authors file tests'
+
+. ./lib-git-svn.sh
+
+cat > svn-authors <<EOF
+aa = AAAAAAA AAAAAAA <aa@example.com>
+bb = BBBBBBB BBBBBBB <bb@example.com>
+EOF
+
+test_expect_success 'setup svnrepo' '
+ for i in aa bb cc dd
+ do
+ svn mkdir -m $i --username $i "$svnrepo"/$i
+ done
+ '
+
+test_expect_success 'start import with incomplete authors file' '
+ ! git svn clone --authors-file=svn-authors "$svnrepo" x
+ '
+
+test_expect_success 'imported 2 revisions successfully' '
+ (
+ cd x
+ test "`git rev-list refs/remotes/git-svn | wc -l`" -eq 2 &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn | \
+ grep "^author BBBBBBB BBBBBBB <bb@example\.com> " &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~1 | \
+ grep "^author AAAAAAA AAAAAAA <aa@example\.com> "
+ )
+ '
+
+cat >> svn-authors <<EOF
+cc = CCCCCCC CCCCCCC <cc@example.com>
+dd = DDDDDDD DDDDDDD <dd@example.com>
+EOF
+
+test_expect_success 'continues to import once authors have been added' '
+ (
+ cd x
+ git svn fetch --authors-file=../svn-authors &&
+ test "`git rev-list refs/remotes/git-svn | wc -l`" -eq 4 &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn | \
+ grep "^author DDDDDDD DDDDDDD <dd@example\.com> " &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~1 | \
+ grep "^author CCCCCCC CCCCCCC <cc@example\.com> "
+ )
+ '
+
+test_expect_success 'authors-file against globs' '
+ svn mkdir -m globs --username aa \
+ "$svnrepo"/aa/trunk "$svnrepo"/aa/branches "$svnrepo"/aa/tags &&
+ git svn clone --authors-file=svn-authors -s "$svnrepo"/aa aa-work &&
+ for i in bb ee cc
+ do
+ branch="aa/branches/$i"
+ svn mkdir -m "$branch" --username $i "$svnrepo/$branch"
+ done
+ '
+
+test_expect_success 'fetch fails on ee' '
+ ( cd aa-work && ! git svn fetch --authors-file=../svn-authors )
+ '
+
+tmp_config_get () {
+ GIT_CONFIG=.git/svn/.metadata git config --get "$1"
+}
+
+test_expect_success 'failure happened without negative side effects' '
+ (
+ cd aa-work &&
+ test 6 -eq "`tmp_config_get svn-remote.svn.branches-maxRev`" &&
+ test 6 -eq "`tmp_config_get svn-remote.svn.tags-maxRev`"
+ )
+ '
+
+cat >> svn-authors <<EOF
+ee = EEEEEEE EEEEEEE <ee@example.com>
+EOF
+
+test_expect_success 'fetch continues after authors-file is fixed' '
+ (
+ cd aa-work &&
+ git svn fetch --authors-file=../svn-authors &&
+ test 8 -eq "`tmp_config_get svn-remote.svn.branches-maxRev`" &&
+ test 8 -eq "`tmp_config_get svn-remote.svn.tags-maxRev`"
+ )
+ '
+
+test_done
--
Eric Wong
^ permalink raw reply related
* Re: [PATCH 3/4] color-words: refactor to allow for 0-character word boundaries
From: Johannes Schindelin @ 2009-01-11 23:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Thomas Rast
In-Reply-To: <7viqolo2z9.fsf@gitster.siamese.dyndns.org>
Hi,
On Sun, 11 Jan 2009, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > This code was ugly, for a number of reasons:
> > ...
> > Fix all of these issues by processing the text such that
>
> Looks much cleaner than the original. I didn't compare it with
> Thomas's, but it seems he found some breakages, so I'd expect a second
> round sometime in the future.
Certainly,
Dscho
^ permalink raw reply
* Re: [PATCH v3 3/4] word diff: make regex configurable via attributes
From: Junio C Hamano @ 2009-01-11 23:20 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, Johannes Schindelin, Teemu Likonen
In-Reply-To: <72242bd75fa8d55c2afc723f8539ef56f2569d3e.1231669012.git.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
> Make the --color-words splitting regular expression configurable via
> the diff driver's 'wordregex' attribute. The user can then set the
> driver on a file in .gitattributes. If a regex is given on the
> command line, it overrides the driver's setting.
> ...
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index 6152d5b..d22c06b 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -96,7 +96,9 @@ endif::git-format-patch[]
> By default, a new word only starts at whitespace, so that a
> 'word' is defined as a maximal sequence of non-whitespace
> characters. The optional argument <regex> can be used to
> - configure this.
> + configure this. It can also be set via a diff driver, see
> + linkgit:gitattributes[1]; if a <regex> is given explicitly, it
> + overrides any diff driver setting.
> +
> The <regex> must be an (extended) regular expression. When set, every
> non-overlapping match of the <regex> is considered a word. (Regular
One bikeshedding I think is better to get over with is that this probably
should be called xwordregex for consistency with xfuncname where 'x'
variant means POSIX ERE and the one without means POSIX BRE, even if we
are not going to support diff.wordregex that uses BRE.
I am assuming [3/4] can be trivially adjusted if we were to adopt the
clean-up approach Dscho is taking?
^ permalink raw reply
* Re: [PATCH 3/4] color-words: refactor to allow for 0-character word boundaries
From: Junio C Hamano @ 2009-01-11 23:08 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Thomas Rast
In-Reply-To: <alpine.DEB.1.00.0901112059340.3586@pacific.mpi-cbg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> This code was ugly, for a number of reasons:
> ...
> Fix all of these issues by processing the text such that
Looks much cleaner than the original. I didn't compare it with Thomas's,
but it seems he found some breakages, so I'd expect a second round
sometime in the future.
^ permalink raw reply
* Re: stopping patches from just floating by
From: Junio C Hamano @ 2009-01-11 23:06 UTC (permalink / raw)
To: jidanni; +Cc: barkalow, git
In-Reply-To: <874p05k1ie.fsf@jidanni.org>
jidanni@jidanni.org writes:
> I see. Say, for my forthcoming 40 minor grammar fixes that affect 20
> files in Documentation/*, I just couldn't bear spamming this list with
> more that one or two [PATCH] mails. OK one long mail it will be then,
> with several commits encompassing many diffs. Hope that will be OK.
It probably won't be Ok, as earlier documentation patches from you that
were smaller had good bits that were applied and not so good bits that
were dropped, and I'd refuse to pick and choose only good bits from one
single large patch myself. The tradeoff between "minor grammar fixes" and
my time spent to do so is not good enough.
Please send in small isolated pieces so that people do not have to comment
"in this patch, this and that rewordings are very good clarification, but
this other hunk adds noise without adding much signal".
And by "isolated", I do not mean "one mail per patched file".
For example, if you fix many misused 3rd person singular verbs that ends
with 's' that are used for subject that is not 3rd person singular, that
is one topic and one patch can fix an error of that kind in all of 20
files.
On the other hand, if you conflate such a fix with a new explanation of an
option (which may or may not be adding more noise than the signal) to the
same file in a single e-mailed patch, you will force people to say "this
part is good but that part is not". That's not the isolation I am
referring to.
^ permalink raw reply
* Re: current git kernel has strange problems during bisect
From: Pierre Habouzit @ 2009-01-11 23:02 UTC (permalink / raw)
To: Alexey Zaytsev
Cc: Sam Ravnborg, Linus Torvalds, Christian Borntraeger,
Johannes Schindelin, git, Linux Kernel Mailing List
In-Reply-To: <f19298770901111147t625a2161t779bfcfc0317225c@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1949 bytes --]
On Sun, Jan 11, 2009 at 07:47:18PM +0000, Alexey Zaytsev wrote:
> On Sun, Jan 11, 2009 at 22:42, Sam Ravnborg <sam@ravnborg.org> wrote:
> >>
> >> For bisect, it's indeed somewhat annoying, and we could have perhaps done
> >> some things a bit differently, but it's about the closest you can get to
> >> "real history" without making the first btrfs merge-point a _total_
> >> disaster.
> >>
> >> For bisect purposes, if you know you're not chasing down a btrfs issue,
> >> you can do
> >>
> >> git bisect good 34353029534a08e41cfb8be647d734b9ce9ebff8
> >>
> >> where that commit 34353029 is the last one which has _just_ the btrfs
> >> files. The next commit is when it does "Merge Btrfs into fs/btrfs", and
> >> that one has the whole kernel tree again.
> >
> > The cost of moving this piece of history from one git tree to another
> > git tree is that we make it harder to debug the kernel for the advanced user
> > that knows how to do bisect.
>
> And wasn't is trivial to avoid? Just exporting the commits as
> patches and importing them into the kernel tree would preserve
> the history, and not break bisection.
And would have brought a whole history of totally irrelevant stuff that
never exited for real, with probably a lot of non-compiling sub-steps
which would be even worse.
No, the two possible choices were to squash the whole stuff at once, or
do what has been done IMNSHO. People have to grok how to take shortcuts
with git-bisect. I know that git-bisect puts people on the brainless
course of actions where they git-bisect; configure; compile; boot; test;
mark as good/bad and retry. And that's what I sometimes don't like with
it. Because people trust git-bisect too much and forget how to think
right.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH 0/4] refactor the --color-words to make it more hackable
From: Johannes Schindelin @ 2009-01-11 23:02 UTC (permalink / raw)
To: Thomas Rast; +Cc: git
In-Reply-To: <200901112253.27165.trast@student.ethz.ch>
Hi,
On Sun, 11 Jan 2009, Thomas Rast wrote:
> Johannes Schindelin wrote:
> >
> > But at least _I_ think it is easy to follow, and it actually makes the code
> > more readable/hackable. Correct me if I'm wrong.
>
> It indeed seems a sane approach.
Thanks.
> However, the final result segfaults and/or prints garbage (on
> apparently every commit except very small changes) when using the regex
> '\S+', which IMHO should give exactly the same result as not using a
> regex at all.
No, it should not. The correct regex is '^\S+'.
As it happens, your regex matches _anything_ + non-whitespace.
Unfortunately, this includes a newline which utterly confuses the diff,
and therefore the code that tries to get the true offsets.
Consequently, it crashes.
> Plain --color-words is not affected.
Of course, I did not change anything outside the code path of
--color-words.
Ciao,
Dscho
-- snipsnap --
[PATCH] color-words: \n must not be a part of the word.
Allowing \n as part of a word is a pilot error, but that is not a
reason for the code to crash.
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
diff.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/diff.c b/diff.c
index d6bba72..676eb79 100644
--- a/diff.c
+++ b/diff.c
@@ -381,8 +381,10 @@ static int find_word_boundary(mmfile_t *buffer, int i, regex_t *word_regex)
if (word_regex) {
regmatch_t match[1];
- if (!regexec(word_regex, buffer->ptr + i, 1, match, 0))
- i += match[0].rm_eo;
+ if (!regexec(word_regex, buffer->ptr + i, 1, match, 0)) {
+ char *p = memchr(buffer->ptr + i, '\n', match[0].rm_eo);
+ i = p ? p - buffer->ptr : match[0].rm_eo + i;
+ }
}
else
while (i < buffer->size && !isspace(buffer->ptr[i]))
^ permalink raw reply related
* Re: Removing a commit from a local branch
From: Björn Steinbrink @ 2009-01-11 22:52 UTC (permalink / raw)
To: Chris Packham; +Cc: git
In-Reply-To: <a038bef50901111442y16695664y4fed7cdd9d8af27@mail.gmail.com>
On 2009.01.12 11:42:24 +1300, Chris Packham wrote:
> Hi List,
>
> I'm part of a development team using git. We use a maintainer model
> where developers send patches/pull requests to a maintainer who
> applies the patches to a local branch, decides if they're good or not
> and pushes the good patches to the public repository.
>
> What I want to do is script the removal of a bad patch so that the
> maintainer identifies a patch in his local branch, sends an email to
> the author telling them why their patch is being rejected then removes
> the commit for that patch. Using git log a script can extract the
> author email address, hash and headline of each commit. Based on that
> information scripting the email is easy enough. Now I come to using
> git rebase to remove the bad commit based on its hash which leads me
> to my question - How do I refer to a commit based on the hash of its
> parent?
>
> Consider the following example. The maintainer has the following branch locally
>
> todeliver: A-B-C-D
>
> He is happy with commits A, C and D but wants to reject B. Ideally I
> want to be able to say
> git rebase --onto <parent of B> <child of B> todelvier
You don't want <child of B> there, just B.
git rebase --onto <onto> <upstream> <branch>
Rebases the commits from the range <upstream>..<branch>, and that
_excludes_ the commit (referenced by) <upstream>.
So:
git rebase --onto B^ B todeliver
Works on: B..todeliver == todeliver --not B
And that range contains commits C and D.
Björn
^ permalink raw reply
* Re: Removing a commit from a local branch
From: Jakub Narebski @ 2009-01-11 22:48 UTC (permalink / raw)
To: Chris Packham; +Cc: git, Stephan Beyer
In-Reply-To: <a038bef50901111442y16695664y4fed7cdd9d8af27@mail.gmail.com>
"Chris Packham" <judge.packham@gmail.com> writes:
> Consider the following example. The maintainer has the following
> branch locally
>
> todeliver: A-B-C-D
>
> He is happy with commits A, C and D but wants to reject B. Ideally I
> want to be able to say
> git rebase --onto <parent of B> <child of B> todelvier
>
> and get
> todeliver: A-C'-D'
>
> I know that <parent of B> can be referred to as B~1 but what about
> <child of B>? I've read through the man page for git-rev-parse and
> nothing stands out as child of commit X.
>
> Is there a better what do achieve what I'm after?
Yes, I think it would be easier to either use "git rebase --interactive"
(with some script taking place of EDITOR, or something), or prod Stephan
Beyer (CC-ed) to finish git-sequencer...
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH 1/4] Add color_fwrite(), a function coloring each line individually
From: Junio C Hamano @ 2009-01-11 22:43 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Thomas Rast
In-Reply-To: <alpine.DEB.1.00.0901112058570.3586@pacific.mpi-cbg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> +/*
> + * This function splits the buffer by newlines and colors the lines individually.
> + */
> +void color_fwrite(FILE *f, const char *color, size_t count, const char *buf)
Is it just me that this is grossly misnamed? It is not about fwrite of
count bytes starting at buf in the specified color. At list it should be
called color_fwrite_lines() or something like that.
> diff --git a/color.h b/color.h
> index 6cf5c88..9fb58f5 100644
> --- a/color.h
> +++ b/color.h
> @@ -19,5 +19,6 @@ int git_config_colorbool(const char *var, const char *value, int stdout_is_tty);
> void color_parse(const char *var, const char *value, char *dst);
> int color_fprintf(FILE *fp, const char *color, const char *fmt, ...);
> int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...);
> +void color_fwrite(FILE *f, const char *color, size_t count, const char *buf);
Also if other functions in the family all return int to indicate errors
and name the FILE * argument fp, I find it a very bad taste not to follow
their patterns without having a good reason (which I do not see).
^ permalink raw reply
* Removing a commit from a local branch
From: Chris Packham @ 2009-01-11 22:42 UTC (permalink / raw)
To: git
In-Reply-To: <a038bef50901111441w21959397tc41922656a25027c@mail.gmail.com>
Hi List,
I'm part of a development team using git. We use a maintainer model
where developers send patches/pull requests to a maintainer who
applies the patches to a local branch, decides if they're good or not
and pushes the good patches to the public repository.
What I want to do is script the removal of a bad patch so that the
maintainer identifies a patch in his local branch, sends an email to
the author telling them why their patch is being rejected then removes
the commit for that patch. Using git log a script can extract the
author email address, hash and headline of each commit. Based on that
information scripting the email is easy enough. Now I come to using
git rebase to remove the bad commit based on its hash which leads me
to my question - How do I refer to a commit based on the hash of its
parent?
Consider the following example. The maintainer has the following branch locally
todeliver: A-B-C-D
He is happy with commits A, C and D but wants to reject B. Ideally I
want to be able to say
git rebase --onto <parent of B> <child of B> todelvier
and get
todeliver: A-C'-D'
I know that <parent of B> can be referred to as B~1 but what about
<child of B>? I've read through the man page for git-rev-parse and
nothing stands out as child of commit X.
Is there a better what do achieve what I'm after?
Thanks,
Chris
^ permalink raw reply
* Re: current git kernel has strange problems during bisect
From: Daniel Barkalow @ 2009-01-11 22:34 UTC (permalink / raw)
To: Alexey Zaytsev
Cc: Linus Torvalds, Sam Ravnborg, Christian Borntraeger,
Johannes Schindelin, git, Linux Kernel Mailing List
In-Reply-To: <f19298770901111417t6762e1e3x79b2f488ee6f1243@mail.gmail.com>
On Mon, 12 Jan 2009, Alexey Zaytsev wrote:
> On Sun, Jan 11, 2009 at 23:04, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> >
> > On Sun, 11 Jan 2009, Sam Ravnborg wrote:
> >>
> >> The cost of moving this piece of history from one git tree to another
> >> git tree is that we make it harder to debug the kernel for the advanced user
> >> that knows how to do bisect.
> >>
> >> It is not like this history would be lost - one just had to look
> >> somewhere else to find it.
> >>
> >> That may be a bad pain/benefit ratio - time will tell.
> >
> > Umm. No.
> >
> > Time is exactly what makes it useful. It will make all the downsides
> > shrink, and the advantages stay.
> >
> >> There should be a way to avoid such pain when bisecting without
> >> having to mark a semi-random (for the average person) commit as good.
> >
> > Well, you don't actually have to mark that semi-random one as good either.
> > What you can do is to just mark anything that _only_ contains fs/btrfs as
> > good. IOW, you don't have to know the magic number - you just have to be
> > told that "oh, if you only have btrfs files, and you're not actively
> > bisecting a btrfs bug, just do 'git bisect good' and continue".
> >
> > Yeah, you'll hit it a few times, but you don't even have to compile things
> > or boot anything, so it's not actually going to be all that much slower
> > than just knowing about the magic point either.
>
> But would not such bug avoid being bisected if you blindly
> mark btrfs commits as good?
>
> v2.6.29 <-- bad
> ...
> ...
> ...
> btrfs stuff <-- mark as good
> ...
> the-real-bug
> ...
> v2.6.28 <-- good
>
> So you hit the btrfs commit, mark it as good, leaving the real bug below,
> and the bisection continues, with both sides being actually bad.
>
> Am I missing something?
Yes, there are no kernel bugs below the btrfs stuff, because there's no
kernel at all below the btrfs stuff. The history is actually like:
A -- B -- C -- D -- G
/
F -- E
F and E are the btrfs stuff, while A-D and G are commit containing the
kernel source (D and G also containing btrfs). Marking E as good cuts off
F, but doesn't cut off anything at all on the top line. Of course, if
you're actually debugging a problem with btrfs that you somehow know to
have worked while btrfs was a separate module at so point, you would want
to get into this history (and would build it as a separate module in order
to do so).
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: current git kernel has strange problems during bisect
From: Sam Ravnborg @ 2009-01-11 22:32 UTC (permalink / raw)
To: Alexey Zaytsev
Cc: Linus Torvalds, Christian Borntraeger, Johannes Schindelin, git,
Linux Kernel Mailing List
In-Reply-To: <f19298770901111417t6762e1e3x79b2f488ee6f1243@mail.gmail.com>
On Mon, Jan 12, 2009 at 01:17:31AM +0300, Alexey Zaytsev wrote:
> On Sun, Jan 11, 2009 at 23:04, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> >
> > On Sun, 11 Jan 2009, Sam Ravnborg wrote:
> >>
> >> The cost of moving this piece of history from one git tree to another
> >> git tree is that we make it harder to debug the kernel for the advanced user
> >> that knows how to do bisect.
> >>
> >> It is not like this history would be lost - one just had to look
> >> somewhere else to find it.
> >>
> >> That may be a bad pain/benefit ratio - time will tell.
> >
> > Umm. No.
> >
> > Time is exactly what makes it useful. It will make all the downsides
> > shrink, and the advantages stay.
> >
> >> There should be a way to avoid such pain when bisecting without
> >> having to mark a semi-random (for the average person) commit as good.
> >
> > Well, you don't actually have to mark that semi-random one as good either.
> > What you can do is to just mark anything that _only_ contains fs/btrfs as
> > good. IOW, you don't have to know the magic number - you just have to be
> > told that "oh, if you only have btrfs files, and you're not actively
> > bisecting a btrfs bug, just do 'git bisect good' and continue".
> >
> > Yeah, you'll hit it a few times, but you don't even have to compile things
> > or boot anything, so it's not actually going to be all that much slower
> > than just knowing about the magic point either.
>
> But would not such bug avoid being bisected if you blindly
> mark btrfs commits as good?
>
> v2.6.29 <-- bad
> ...
> ...
> ...
> btrfs stuff <-- mark as good
> ...
> the-real-bug
> ...
> v2.6.28 <-- good
>
> So you hit the btrfs commit, mark it as good, leaving the real bug below,
> and the bisection continues, with both sides being actually bad.
>
> Am I missing something?
Yep - you miss that people get confused when suddenly they have no kernel source.
Sam
^ permalink raw reply
* Re: current git kernel has strange problems during bisect
From: Daniel Barkalow @ 2009-01-11 22:27 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Linus Torvalds, Sam Ravnborg, Johannes Schindelin, git,
Linux Kernel Mailing List
In-Reply-To: <200901112239.20306.borntraeger@de.ibm.com>
On Sun, 11 Jan 2009, Christian Borntraeger wrote:
> Am Sonntag 11 Januar 2009 schrieb Linus Torvalds:
> > Well, you don't actually have to mark that semi-random one as good either.
> > What you can do is to just mark anything that _only_ contains fs/btrfs as
> > good. IOW, you don't have to know the magic number - you just have to be
> > told that "oh, if you only have btrfs files, and you're not actively
> > bisecting a btrfs bug, just do 'git bisect good' and continue".
>
> That should work.
>
> <rant>
> Still, I am a bit frustrated. During this weekend I reported 2 regressions
> (wlan and ata) and I still try to find out why suspend/resume stopped
> working. In the meantime I have identified 2 patches (one was already known,
> I reported the 2nd to the usb maintainers) after 2.6.28 that caused suspend
> to ram regressions. In rc1 S2R was broken again. So I tried bisecting the
> third patch - which finally brought me to the btrfs bisect problem.
>
> For me, this was the most annoying merge window ever.
>
> In my opinion we should really avoid subtree merges in the future as a curtesy
> to people who do the uncool work of testing, problem tracking and bisecting.
> </rant>
I think hitting a version without the actual kernel source in it should
actually make bisecting easier, not harder; you can say without even
building the kernel that that version doesn't have the problem you're
trying to find, because it doesn't have anything in it.
The alternative to having that part of the tree empty would be to stick in
some kernel version there (probably 2.6.28), and then you'd build and test
2.6.28 again, completely wasting a bunch of time.
Probably the bisect documentation or messages need to make it clear what
you should do when you land on this sort of commit.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: [PATCH] Cleanup of unused symcache variable inside diff-lib.c
From: Junio C Hamano @ 2009-01-11 22:25 UTC (permalink / raw)
To: Kjetil Barvik; +Cc: Johannes Schindelin, git
In-Reply-To: <86ab9x8t4a.fsf@broadpark.no>
Kjetil Barvik <barvik@broadpark.no> writes:
> And the 'a' and the 'd' in the DCO I do agree with in this particular
> situation, so I added a '--signoff' to the patches.
I think Dscho's suggestion was to sign-off when you commit, not when you
format-patch. It won't make any difference either way to me nor other
people who reads the list, because nobody can tell which way you used by
looking at your e-mail, but it is a good habit to get into if you work on
git or the kernel.
^ permalink raw reply
* Re: [PATCH] Cleanup of unused symcache variable inside diff-lib.c
From: Junio C Hamano @ 2009-01-11 22:23 UTC (permalink / raw)
To: Kjetil Barvik; +Cc: Johannes Schindelin, git
In-Reply-To: <86eiz98v0s.fsf@broadpark.no>
Kjetil Barvik <barvik@broadpark.no> writes:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> <snipp>
>> My question was more: why do you do additional work and put a git diff
>> --raw between the commit message and the diffstat when that information is
>> in the patch already?
>
> Ok, I see. I (re)used the 'git format-patch' command from previous
> run, and this time it was (without the line-breaks):
>
> git format-patch --stat --patch-with-raw -1 --signoff -M -C
> --summary --full-index --subject-prefix="PATCH"
> --output-directory ../diff_lib_c_symcache_cleanup_v1/
Please drop --patch-with-raw and --full-index. They are distracting.
I do not think using --subject-prefix=PATCH to repeat what is default adds
any value either.
^ permalink raw reply
* Re: [PATCH v3 2/4] word diff: customizable word splits
From: Junio C Hamano @ 2009-01-11 22:20 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, Johannes Schindelin, Teemu Likonen
In-Reply-To: <529cd830908f018f796dbc46d3b055c1f8ba9c1b.1231669012.git.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
> Allows for user-configurable word splits when using --color-words.
> This can make the diff more readable if the regex is configured
> according to the language of the file.
>
> Each non-overlapping match of the regex is a word; everything in
> between is whitespace.
What happens if the input "language" does not have any inter-word spacing
but its words can still be expressed by regexp patterns?
ImagineALanguageThatAllowsYouToWriteSomethingLikeThis. Does the mechanism
help users who want to do word-diff files written in such a language by
outputting:
ImagineALanguage<red>That</red><green>Which</green>AllowsYou...
when '[A-Z][a-z]*' is given by the word pattern?
> We disallow matching the empty string (because
> it results in an endless loop) or a newline (breaks color escapes and
> interacts badly with the input coming from the usual line diff). To
> help the user, we set REG_NEWLINE so that [^...] and . do not match
> newlines.
AndImagineALanguageWhoseWordStruc
tureDoesNotCareAboutLineBreak
Can you help users with such payload?
Side note. Yes, I am coming from Japanese background.
Side note 2. No, I am not saying your code must support both of
the above to be acceptable. I am just gauging the design
assumptions and limitations.
> Insertion of spaces is somewhat subtle. We echo a "context" space
> twice (once on each side of the diff) if it follows directly after a
> word. While this loses a tiny bit of accuracy, it runs together long
> sequences of changed word into one removed and one added block, making
> the diff much more readable.
I guess this part can be later enhanced to be more precise, so that it
keeps the original context space more faithfully (i.e. does not lose two
consecutive spaces in the original occidental script, and does not insert
any extra space to the oriental script), if we were to support the second
example I gave above in the future as a follow-up patch.
> +--color-words[=<regex>]::
> + Show colored word diff, i.e., color words which have changed.
> + By default, a new word only starts at whitespace, so that a
> + 'word' is defined as a maximal sequence of non-whitespace
> + characters. The optional argument <regex> can be used to
> + configure this.
> ++
> +The <regex> must be an (extended) regular expression. When set, every
> +non-overlapping match of the <regex> is considered a word. (Regular
> +expression semantics ensure that quantifiers grab a maximal sequence
> +of characters.) Anything between these matches is considered
> +whitespace and ignored for the purposes of finding differences. You
> +may want to append `|\S` to your regular expression to make sure that
> +it matches all non-whitespace characters.
Whose regexp library do we assume here? Traditionally we limited
ourselves to POSIX BRE, and I do not think anybody minds using POSIX ERE
here, but we need to be clear. In either case \S is a pcre outside
POSIX.
The rest I only skimmed but did not spot anything glaringly wrong; thanks.
^ permalink raw reply
* Re: [PATCH v3 1/4] word diff: comments, preparations for regex customization
From: Junio C Hamano @ 2009-01-11 22:19 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, Johannes Schindelin, Teemu Likonen
In-Reply-To: <4aea85caafd38a058145c5769fe8a30ffdbd4d13.1231669012.git.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
> +/* unused right now */
> +enum diff_word_boundaries {
> + DIFF_WORD_UNDEF,
> + DIFF_WORD_BODY,
> + DIFF_WORD_END,
> + DIFF_WORD_SPACE,
> + DIFF_WORD_SKIP
> +};
Don't do this. Please introduce them when you start using them.
> struct diff_words_buffer {
> mmfile_t text;
> long alloc;
> long current; /* output pointer */
> int suppressed_newline;
> + enum diff_word_boundaries *boundaries;
> };
Likewise. Especially because this raises eyebrows "Huh, a pointer to an
enum, or perhaps he allocates an array of enums?" without allowing the
reader to figure it out much later when the field is actually used.
> static void diff_words_append(char *line, unsigned long len,
> @@ -339,16 +349,23 @@ static void diff_words_append(char *line, unsigned long len,
> struct diff_words_data {
> struct diff_words_buffer minus, plus;
> FILE *file;
> + regex_t *word_regex; /* currently unused */
> };
I see having this here and keeping it NULL in this patch makes the later
patch to diff_words_show() more readable, so this probably should stay
here.
> -static void print_word(FILE *file, struct diff_words_buffer *buffer, int len, int color,
> +/*
> + * Print 'len' characters from the "real" diff data in 'buffer'. Also
> + * returns how many characters were printed (currently always 'len').
> + * With 'suppress_newline', we remember a final newline instead of
> + * printing it.
> + */
"... Even in such a case, 'len' that is returned counts the suppressed
newline", or something like that? If you can concisely describe why the
caller wants the returned count not match the number of actually printed
chars (i.e. it includes the suppressed newline), it would help the reader
understand the logic. I am _guessing_ it is because this is called to
print matching words from the preimage buffer, and the return value is
used to skip over the same part in the postimage buffer, and by definition
they have to be of the same length (otherwise they won't be matching).
> +static int print_word(FILE *file, struct diff_words_buffer *buffer, int len, int color,
> int suppress_newline)
> {
> const char *ptr;
> int eol = 0;
>
> if (len == 0)
> - return;
> + return len;
>
> ptr = buffer->text.ptr + buffer->current;
> buffer->current += len;
> @@ -368,18 +385,30 @@ static void print_word(FILE *file, struct diff_words_buffer *buffer, int len, in
> else
> putc('\n', file);
> }
> +
> + /* we need to return how many chars to skip on the other side,
> + * so account for the (held off) \n */
Multi-line comment style? I won't repeat this but you have many...
> + return len+eol;
> }
>
> +/*
> + * Callback for word diff output
> + */
Without saying "to do what", the comment adds more noise than signal.
"Called to parse diff between pre- and post- image files converted into
one-word-per-line format and concatenate them to into lines by dropping
some of the end-of-lines but keeping some others", or something like that?
Thanks.
^ permalink raw reply
* Re: [PATCH] Allow cloning to an existing empty directory
From: Junio C Hamano @ 2009-01-11 22:19 UTC (permalink / raw)
To: Alexander Potashev; +Cc: Git Mailing List
In-Reply-To: <1231676352-16082-1-git-send-email-aspotashev@gmail.com>
Thanks; will queue both with minor style fix-ups and a trivial test.
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2009, #02; Sun, 11)
From: Junio C Hamano @ 2009-01-11 22:19 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <m3ljthzzdq.fsf@localhost.localdomain>
Jakub Narebski <jnareb@gmail.com> writes:
>> ----------------------------------------------------------------
>> [Actively cooking]
>>
>> * sc/gitweb-category (Fri Dec 12 00:45:12 2008 +0100) 3 commits
>> - gitweb: Optional grouping of projects by category
>> - gitweb: Split git_project_list_body in two functions
>> - gitweb: Modularized git_get_project_description to be more generic
>
> This I think needs some further cooking. I guess with addition of one
> more patch to series categories could be sorted together with projects
> they contain, and not always have to be in fixed ordering.
These should be moved to the Stalled category; nobody seems to be
discussing improvements and sending updates to the series as far as I
recall.
>> * gb/gitweb-patch (Thu Dec 18 08:13:19 2008 +0100) 4 commits
>> - gitweb: link to patch(es) view in commit(diff) and (short)log view
>> - gitweb: add patches view
>> - gitweb: change call pattern for git_commitdiff
>> - gitweb: add patch view
>
> If I remember correctly the only point of discussion is calling
> convention for git_commitdiff, and whether 'patches' view should
> (re)use git_commitdiff or use its own subroutine.
Thanks; I take it that it is basically usable, useful and can be
incrementally improved in 'next'?
^ permalink raw reply
* Re: current git kernel has strange problems during bisect
From: Alexey Zaytsev @ 2009-01-11 22:17 UTC (permalink / raw)
To: Linus Torvalds
Cc: Sam Ravnborg, Christian Borntraeger, Johannes Schindelin, git,
Linux Kernel Mailing List
In-Reply-To: <alpine.LFD.2.00.0901111200330.6528@localhost.localdomain>
On Sun, Jan 11, 2009 at 23:04, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
>
> On Sun, 11 Jan 2009, Sam Ravnborg wrote:
>>
>> The cost of moving this piece of history from one git tree to another
>> git tree is that we make it harder to debug the kernel for the advanced user
>> that knows how to do bisect.
>>
>> It is not like this history would be lost - one just had to look
>> somewhere else to find it.
>>
>> That may be a bad pain/benefit ratio - time will tell.
>
> Umm. No.
>
> Time is exactly what makes it useful. It will make all the downsides
> shrink, and the advantages stay.
>
>> There should be a way to avoid such pain when bisecting without
>> having to mark a semi-random (for the average person) commit as good.
>
> Well, you don't actually have to mark that semi-random one as good either.
> What you can do is to just mark anything that _only_ contains fs/btrfs as
> good. IOW, you don't have to know the magic number - you just have to be
> told that "oh, if you only have btrfs files, and you're not actively
> bisecting a btrfs bug, just do 'git bisect good' and continue".
>
> Yeah, you'll hit it a few times, but you don't even have to compile things
> or boot anything, so it's not actually going to be all that much slower
> than just knowing about the magic point either.
But would not such bug avoid being bisected if you blindly
mark btrfs commits as good?
v2.6.29 <-- bad
...
...
...
btrfs stuff <-- mark as good
...
the-real-bug
...
v2.6.28 <-- good
So you hit the btrfs commit, mark it as good, leaving the real bug below,
and the bisection continues, with both sides being actually bad.
Am I missing something?
^ permalink raw reply
* Re: git-svn: File was not found in commit
From: Björn Steinbrink @ 2009-01-11 21:55 UTC (permalink / raw)
To: Morgan Christiansson; +Cc: git
In-Reply-To: <49678705.4040506@mog.se>
On 2009.01.09 18:19:01 +0100, Morgan Christiansson wrote:
> Hi, i'm trying to "git svn fetch" my repository from a local file:///
> repo and i'm running into this problem:
>
> $ git svn init -t tags -b branches -T trunk file:///path/to/svn/repo
> $ git svn fetch
> branches/rails/rails/vendor/plugins/acts_as_xapian/.git/refs/heads/master
> was not found in commit a643e882c557593f36bb9fd0966490010b9dba61 (r10576)
>
>
> I found another report that seems to describe the same error:
> http://marc.info/?l=git&m=121537767308135&w=2
> Investigating the the history it's committed in r10577 and it's looking
> for it in r10576, so it seems to be off by one revision number. Exactly
> like the other report.
> I've tried the latest git version of git-svn.perl and the problem is not
> fixed there.
>
>
> $ svn log file:///path/to/repo -r10576:10577 -v
> ------------------------------------------------------------------------
> r10576 | morgan | 2008-11-28 14:35:53 +0000 (Fri, 28 Nov 2008) | 3 lines
> Changed paths:
> A /branches/rails/rails/app/controllers/browse_sheetmusic_controller.rb
> M /branches/rails/rails/app/controllers/scores_controller.rb
> M /branches/rails/rails/app/models/composer.rb
> M /branches/rails/rails/app/models/score.rb
> M /branches/rails/rails/config/routes.rb
>
> Commit message.
>
> ------------------------------------------------------------------------
> r10577 | morgan | 2008-11-28 18:31:00 +0000 (Fri, 28 Nov 2008) | 3 lines
> Changed paths:
> A /branches/rails/rails/vendor/plugins/acts_as_xapian/.git/FETCH_HEAD
> M /branches/rails/rails/vendor/plugins/acts_as_xapian/.git/config
> M /branches/rails/rails/vendor/plugins/acts_as_xapian/.git/index
> M /branches/rails/rails/vendor/plugins/acts_as_xapian/.git/logs/HEAD
> M
> /branches/rails/rails/vendor/plugins/acts_as_xapian/.git/logs/refs/heads/master
>
>
> # <-- THIS FILE
Unless I totally misread that line, SVN reports that the file was
_modified_, not added. For the file to be modified, it seems reasonable
to expect that it existed in the previous commit.
Is there anything "special" about how that file came into existence in
that branch? Without further knowledge, that looks like a svn bug, but
hey, it's svn, it might do funny stuff on purpose ;-)
Maybe you could provide a small test case/repo?
Björn
^ permalink raw reply
* Re: current git kernel has strange problems during bisect
From: Sam Ravnborg @ 2009-01-11 21:54 UTC (permalink / raw)
To: Linus Torvalds
Cc: Christian Borntraeger, Johannes Schindelin, git,
Linux Kernel Mailing List
In-Reply-To: <alpine.LFD.2.00.0901111200330.6528@localhost.localdomain>
On Sun, Jan 11, 2009 at 12:04:12PM -0800, Linus Torvalds wrote:
>
>
> On Sun, 11 Jan 2009, Sam Ravnborg wrote:
> >
> > The cost of moving this piece of history from one git tree to another
> > git tree is that we make it harder to debug the kernel for the advanced user
> > that knows how to do bisect.
> >
> > It is not like this history would be lost - one just had to look
> > somewhere else to find it.
> >
> > That may be a bad pain/benefit ratio - time will tell.
>
> Umm. No.
>
> Time is exactly what makes it useful. It will make all the downsides
> shrink, and the advantages stay.
>
> > There should be a way to avoid such pain when bisecting without
> > having to mark a semi-random (for the average person) commit as good.
>
> Well, you don't actually have to mark that semi-random one as good either.
> What you can do is to just mark anything that _only_ contains fs/btrfs as
> good. IOW, you don't have to know the magic number - you just have to be
> told that "oh, if you only have btrfs files, and you're not actively
> bisecting a btrfs bug, just do 'git bisect good' and continue".
And we lost 24 hours due to timezone differences etc. and maybe
a few testers.
Thats my point.
There are other obvious ways to do this where we keep history in kernel
but do not impact bisect.
And we have one frustrated tester already - so this is not a made up example.
Sam
^ permalink raw reply
* Re: [PATCH 0/4] refactor the --color-words to make it more hackable
From: Thomas Rast @ 2009-01-11 21:53 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0901112057300.3586@pacific.mpi-cbg.de>
[-- Attachment #1: Type: text/plain, Size: 695 bytes --]
Johannes Schindelin wrote:
>
> But at least _I_ think it is easy to follow, and it actually makes the code
> more readable/hackable. Correct me if I'm wrong.
It indeed seems a sane approach. However, the final result segfaults
and/or prints garbage (on apparently every commit except very small
changes) when using the regex '\S+', which IMHO should give exactly
the same result as not using a regex at all. In git.git:
$ ./git-show --color-words='\S+' 7eb5bbdb645
Segmentation fault
$ ./git-show --color-words='\S+' d3240d935c4
[...garbled output...]
Segmentation fault
Plain --color-words is not affected.
--
Thomas Rast
trast@{inf,student}.ethz.ch
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: current git kernel has strange problems during bisect
From: Christian Borntraeger @ 2009-01-11 21:39 UTC (permalink / raw)
To: Linus Torvalds
Cc: Sam Ravnborg, Johannes Schindelin, git, Linux Kernel Mailing List
In-Reply-To: <alpine.LFD.2.00.0901111200330.6528@localhost.localdomain>
Am Sonntag 11 Januar 2009 schrieb Linus Torvalds:
> Well, you don't actually have to mark that semi-random one as good either.
> What you can do is to just mark anything that _only_ contains fs/btrfs as
> good. IOW, you don't have to know the magic number - you just have to be
> told that "oh, if you only have btrfs files, and you're not actively
> bisecting a btrfs bug, just do 'git bisect good' and continue".
That should work.
<rant>
Still, I am a bit frustrated. During this weekend I reported 2 regressions
(wlan and ata) and I still try to find out why suspend/resume stopped
working. In the meantime I have identified 2 patches (one was already known,
I reported the 2nd to the usb maintainers) after 2.6.28 that caused suspend
to ram regressions. In rc1 S2R was broken again. So I tried bisecting the
third patch - which finally brought me to the btrfs bisect problem.
For me, this was the most annoying merge window ever.
In my opinion we should really avoid subtree merges in the future as a curtesy
to people who do the uncool work of testing, problem tracking and bisecting.
</rant>
Christian
^ 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