Git development
 help / color / mirror / Atom feed
* Re: [BUG] git-diff-* --color oddness
From: Jeff King @ 2008-01-04  8:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, win
In-Reply-To: <7vsl1ekmg5.fsf@gitster.siamese.dyndns.org>

On Fri, Jan 04, 2008 at 12:46:18AM -0800, Junio C Hamano wrote:

> > It's for user diff drivers, and it looks like the funcname pattern. Not
> > sure why we want to demand-load that stuff at all...I wonder if it
> > should just go into git_default_config (or a git_basic_diff_config).
> 
> Yeah, moving some to the diff-basic-config sounds sane.

OK, below is a patch that fixes the color issues I was having.

>  * I'd prefer to keep low-level produce consistent diff that can
>    be reliably applied with git-apply without getting broken by
>    user configuration while Porcelain level diff can be tweaked
>    by the user to do whatever "human readable" crap they would
>    want to see on the screen (such as "side by side"), and my
>    gut feeling is that we should keep the user-level driver
>    definition in ui-config, never to affect plumbing.

OK. In that case, we need a way for the plumbing to tell the diff
machinery "don't ever try loading the ui config."

>  * using or not-using colors should stay in ui-config;

Agreed.

>  * funcname-pattern can go either way; that affects what appears
>    at the end of @@ context @@ lines, and would not have risk to
>    corrupt the patch for plumbing.

I don't personally use this, so I don't care too much. But I am inclined
to say low-level since it cannot break the patch (and it will be used
and presented by porcelains like "git add -i").

>  * color choice can also go either way, but probably is better
>    to be in the low-level.  Cnce color is used the output cannot
>    be fed sanely to patch or git-apply _anyway_ so we can be
>    sure that "git diff-plumbing --color" is run to produce human
>    readable output.

Definitely low-level IMHO, since we have porcelain which relies on the
low-level's output (and there is no way to set it manually from the
command line).

Patch to add diff_basic_config infrastructure and fix the color
selection issue is below. It just does the color, but we can move other
stuff over as discussion ensues.

-- >8 --
add a "basic" diff config callback

The diff porcelain uses git_diff_ui_config to set
porcelain-ish config options, like automatically turning on
color. The plumbing specifically avoids calling this
function, since it doesn't want things like automatic color
or rename detection.

However, some diff options should be set for both plumbing
and porcelain. For example, one can still turn on color in
git-diff-files using the --color command line option. This
means we want the color config from color.diff.* (so that
once color is on, we use the user's preferred scheme), but
_not_ the color.diff variable.

We split the diff config into "ui" and "basic", where
"basic" is suitable for use by plumbing (so _most_ things
affecting the output should still go into the "ui" part).

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin-diff-files.c |    2 +-
 builtin-diff-index.c |    2 +-
 builtin-diff-tree.c  |    2 +-
 diff.c               |    6 ++++++
 diff.h               |    1 +
 5 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/builtin-diff-files.c b/builtin-diff-files.c
index 9c04111..4abe3c2 100644
--- a/builtin-diff-files.c
+++ b/builtin-diff-files.c
@@ -21,7 +21,7 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)
 
 	prefix = setup_git_directory_gently(&nongit);
 	init_revisions(&rev, prefix);
-	git_config(git_default_config); /* no "diff" UI options */
+	git_config(git_diff_basic_config); /* no "diff" UI options */
 	rev.abbrev = 0;
 
 	if (!setup_diff_no_index(&rev, argc, argv, nongit, prefix))
diff --git a/builtin-diff-index.c b/builtin-diff-index.c
index 0f2390a..2b955de 100644
--- a/builtin-diff-index.c
+++ b/builtin-diff-index.c
@@ -17,7 +17,7 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)
 	int result;
 
 	init_revisions(&rev, prefix);
-	git_config(git_default_config); /* no "diff" UI options */
+	git_config(git_diff_basic_config); /* no "diff" UI options */
 	rev.abbrev = 0;
 
 	argc = setup_revisions(argc, argv, &rev, NULL);
diff --git a/builtin-diff-tree.c b/builtin-diff-tree.c
index ebc50ef..832797f 100644
--- a/builtin-diff-tree.c
+++ b/builtin-diff-tree.c
@@ -68,7 +68,7 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
 	int read_stdin = 0;
 
 	init_revisions(opt, prefix);
-	git_config(git_default_config); /* no "diff" UI options */
+	git_config(git_diff_basic_config); /* no "diff" UI options */
 	nr_sha1 = 0;
 	opt->abbrev = 0;
 	opt->diff = 1;
diff --git a/diff.c b/diff.c
index f6a8515..6bb0f62 100644
--- a/diff.c
+++ b/diff.c
@@ -183,6 +183,12 @@ int git_diff_ui_config(const char *var, const char *value)
 				return parse_funcname_pattern(var, ep, value);
 		}
 	}
+
+	return git_diff_basic_config(var, value);
+}
+
+int git_diff_basic_config(const char *var, const char *value)
+{
 	if (!prefixcmp(var, "diff.color.") || !prefixcmp(var, "color.diff.")) {
 		int slot = parse_diff_color_slot(var, 11);
 		color_parse(value, var, diff_colors[slot]);
diff --git a/diff.h b/diff.h
index beccf85..073d5cb 100644
--- a/diff.h
+++ b/diff.h
@@ -172,6 +172,7 @@ extern void diff_unmerge(struct diff_options *,
 #define DIFF_SETUP_USE_CACHE		2
 #define DIFF_SETUP_USE_SIZE_CACHE	4
 
+extern int git_diff_basic_config(const char *var, const char *value);
 extern int git_diff_ui_config(const char *var, const char *value);
 extern void diff_setup(struct diff_options *);
 extern int diff_opt_parse(struct diff_options *, const char **, int);
-- 
1.5.4.rc2.1123.gce734-dirty

^ permalink raw reply related

* git-pull several branches merge conflict resolved
From: Cyrill Gorcunov @ 2008-01-04  9:03 UTC (permalink / raw)
  To: GIT-LIST; +Cc: Junio C Hamano

this message is just an attempt to summarize a problem and its decision
graciously explained by Junio C Hamano in hope it would help someone else
who trip on the same problem

lets start ;)


--- the problem ---

i'm keeping Linus's kernel git tree on my local harddrive so i update it from time to
time using git-pull command with explicitly defined URL:

	git-pull git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git

for some reasons i wish to track changes from Ingo's repository so i made a special
branch for it

	git-checkout -b x86

this branch i'm trying to keep up to date with

	git-pull --force git://git.kernel.org/pub/scm/linux/kernel/git/x86/linux-2.6-x86.git mm

after some point (as only Linus has his tree updated) i could not update Ingo's branch
without merge conflicts

so Junio explained what is going on there


--- the decision ---

Here is what is happening.

 (0) Ingo has this:

                      A---B (== I)
                     /
    ----o---o---o---L

     where L is the tip of Linus at some point, I is his changes
     for x86.  You pull and get the same thing.  Your local x86
     tip points at commit B.

 (1) Then Linus advances and Ingo rebases.  Updated Linus's tip
     is at L' and Ingo has old patches rebased (A' and B') while
     he added more changes (C and D).  His tip is at I'.

                      A---B (==I) A'--B'--C---D (==I')
                     /           /
    ----o---o---o---L---o---o---L'

 (2) You pull.  What is involved is:

     * git-pull is just "git fetch" followed by "git merge", and
       in your repository "git fetch" can be configured to use a
       remote tracking branch to keep track of Ingo's progress
       (but I suspect you don't).  Your "git branch" output
       shows your local branches, and "git branch -r" would show
       these remote tracking branches.

     * The remote tracking is typically configured in
       .git/config and would look like this:

        [remote "mingo"]
        url = git://git.kernel.org/pub/...
        fetch = refs/heads/*:refs/remotes/mingo/*

        Although I _suspect_ you do not have it (your $ipull
        script pulls with explicit URL without using configured
        information).

     The above (for normal people who have the tracking set up)
     fetches the branch tip's from Ingo, and store them in
     corresponding places in .git/refs/remotes/mingo/;  his 'mm'
     branch will be stored in .git/refs/remotes/mingo/mm.

     But remote.mingo.fetch configuration above does not start
     with '+' (e.g. "+refs/heads/*:refs/remotes/mingo/*", which
     means "do allow non-fast-forward").  For people with such
     configuration, "git pull" from him will fail because
     remotes/mingo/mm points at commit B before you initiated
     the fetch and now it points at D which is _NOT_ a
     descendant of B.

     His recommendation about --force applies _ONLY_ to override
     this, and allow your remote tracking branch that used to
     point at B to be replaced to point at D.  I suspect it does
     not even apply to you as I do not think you are using
     remote tracking branch at all.

     In any case, once "git fetch" completes, "git merge"
     happens.  --force does not affect this step at all.

     What's merged?

     Your 'x86' branch is still at B and you try to merge D into
     it.

                            .-------------------*
                           /                   / 
                      A---B       A'--B'--C---D
                     /           /
    ----o---o---o---L---o---o---L'

     Because Ingo's tree was rebased, the resulting merge wants
     to have both versions of A and B (the original and the
     rebased).  As corresponding patches (say A and A') would
     want to touch same parts of the code, and Ingo may have
     improved the latter while all of this has been happening
     (i.e. A and A' may not be literal rebase but can do things
     differently), it will inevitably conflict with each other.

Even though the conflict resolution would be trivial (you would
basically want to pick what's from A' over A), this is not what
you would typically want to happen.  When dealing with a
rebasing upstream, you often do not want to merge but instead
rebase yourself.

So backing up a bit, here is how people would follow rebasing
upstream:

 (0) Ingo has this:

                      A---B (== I)
                     /
    ----o---o---o---L

     where L is the tip of Linus at some point, I is his changes
     for x86.  You pull and get the same thing.  Your local x86
     tip points at commit B.

 (1) You develop on top of Ingo (although you hinted in your
     description that you are strictly following, that is just a
     degenerated case of this where (X,Y,Z) is empty in this
     picture):

                            X---Y---Z
                           /
                      A---B (== I)
                     /
    ----o---o---o---L

 (2) Then Linus advances and Ingo rebases.  Updated Linus's tip
     is at L' and Ingo has old patches rebased (A' and B') while
     he added more changes (C and D).  His tip is at I'.

                      A---B (==I) A'--B'--C---D (==I')
                     /           /
    ----o---o---o---L---o---o---L'

 (3) You do not pull but instead fetch from Ingo to get what
     happened outside your tree.

                            X---Y---Z
                           /
                      A---B (==I) A'--B'--C---D (==I')
                     /           /
    ----o---o---o---L---o---o---L'

    Note that your 'x86' is at Z and Ingo's tip is now at D.

 (4) You rebase on top of Ingo's updated tip.

                                                X'--Y'--Z'
                                               /
                      A---B (==I) A'--B'--C---D (==I')
                     /           /
    ----o---o---o---L---o---o---L'


I was told that our user manual is very good these days covering
both workflows based on merges and workflows based on rebases.
You may want to check it and also git-rebase(1).

--- end ---

Thanks you very much, Junio!

		- Cyrill -

^ permalink raw reply

* Re: [ANNOUNCE] ugit: the pythonic git gui
From: Abdelrazak Younes @ 2008-01-04  9:05 UTC (permalink / raw)
  To: git
In-Reply-To: <e5bfff550801032315v1d2b98f7s4c22cd466e16f524@mail.gmail.com>

Marco Costalba wrote:
> On Jan 3, 2008 4:20 AM, David <davvid@gmail.com> wrote:
>> I don't have any fancy mac or windows installers at the moment (Linux
>> is my primary platform) though I am working on packaging soon.
>>
> 
> Regarding windows installer I was very happy with Inno Setuo that I
> didn't know but I found out be very simple to learn and powerful.

Just a question: why don't you package the two programs together? From a 
packager POV it would makes sense because the Qt libs will be shared. 
And from a user POV you definitely don't want to intall two very related 
  packages that ask the same question: where is git? :-)

Abdel.

^ permalink raw reply

* [PATCH] diff: load funcname patterns in "basic" config
From: Jeff King @ 2008-01-04  9:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

The funcname patterns influence the "comment" on @@ lines of
the diff. They are safe to use with plumbing since they
don't fundamentally change the meaning of the diff in any
way.

Since all diff users call either diff_ui_config or
diff_basic_config, we can get rid of the lazy reading of the
config.

Signed-off-by: Jeff King <peff@peff.net>
---
I am slightly worried that I'm missing something fundamental here. Why
were these ever lazily loaded in the first place? Ditto for the user
diff drivers, which should just get loaded when the command parses the
config for the first time.

 diff.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/diff.c b/diff.c
index 6bb0f62..1ea10c8 100644
--- a/diff.c
+++ b/diff.c
@@ -179,8 +179,6 @@ #include "run-command.h"
 		if (ep != var + 4) {
 			if (!strcmp(ep, ".command"))
 				return parse_lldiff_command(var, ep, value);
-			if (!strcmp(ep, ".funcname"))
-				return parse_funcname_pattern(var, ep, value);
 		}
 	}
 
@@ -195,6 +193,14 @@ #include "run-command.h"
 		return 0;
 	}
 
+	if (!prefixcmp(var, "diff.")) {
+		const char *ep = strrchr(var, '.');
+		if (ep != var + 4) {
+			if (!strcmp(ep, ".funcname"))
+				return parse_funcname_pattern(var, ep, value);
+		}
+	}
+
 	return git_default_config(var, value);
 }
 
@@ -1165,7 +1171,6 @@ #include "run-command.h"
 {
 	struct funcname_pattern *pp;
 
-	read_config_if_needed();
 	for (pp = funcname_pattern_list; pp; pp = pp->next)
 		if (!strcmp(ident, pp->name))
 			return pp->pattern;
-- 
1.5.4.rc2.1124.g7c85e-dirty

^ permalink raw reply related

* Re: [PATCH] diff: load funcname patterns in "basic" config
From: Jeff King @ 2008-01-04  9:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20080104091613.GA7535@coredump.intra.peff.net>

On Fri, Jan 04, 2008 at 04:16:14AM -0500, Jeff King wrote:

> --- a/diff.c
> +++ b/diff.c
> @@ -179,8 +179,6 @@ #include "run-command.h"

Heh, I had a goofy diff.default.funcname option set up to test this out.
A slightly more readable version of the patch is below.

-- >8 --
diff: load funcname patterns in "basic" config

The funcname patterns influence the "comment" on @@ lines of
the diff. They are safe to use with plumbing since they
don't fundamentally change the meaning of the diff in any
way.

Since all diff users call either diff_ui_config or
diff_basic_config, we can get rid of the lazy reading of the
config.

Signed-off-by: Jeff King <peff@peff.net>
---
 diff.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/diff.c b/diff.c
index 6bb0f62..1ea10c8 100644
--- a/diff.c
+++ b/diff.c
@@ -179,8 +179,6 @@ int git_diff_ui_config(const char *var, const char *value)
 		if (ep != var + 4) {
 			if (!strcmp(ep, ".command"))
 				return parse_lldiff_command(var, ep, value);
-			if (!strcmp(ep, ".funcname"))
-				return parse_funcname_pattern(var, ep, value);
 		}
 	}
 
@@ -195,6 +193,14 @@ int git_diff_basic_config(const char *var, const char *value)
 		return 0;
 	}
 
+	if (!prefixcmp(var, "diff.")) {
+		const char *ep = strrchr(var, '.');
+		if (ep != var + 4) {
+			if (!strcmp(ep, ".funcname"))
+				return parse_funcname_pattern(var, ep, value);
+		}
+	}
+
 	return git_default_config(var, value);
 }
 
@@ -1165,7 +1171,6 @@ static const char *funcname_pattern(const char *ident)
 {
 	struct funcname_pattern *pp;
 
-	read_config_if_needed();
 	for (pp = funcname_pattern_list; pp; pp = pp->next)
 		if (!strcmp(ident, pp->name))
 			return pp->pattern;
-- 
1.5.4.rc2.1124.g7c85e-dirty

^ permalink raw reply related

* Re: [BUG] git-diff-* --color oddness
From: Jeff King @ 2008-01-04  9:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, win
In-Reply-To: <20080104085934.GA3706@coredump.intra.peff.net>

On Fri, Jan 04, 2008 at 03:59:34AM -0500, Jeff King wrote:

> OK. In that case, we need a way for the plumbing to tell the diff
> machinery "don't ever try loading the ui config."

> >  * funcname-pattern can go either way; that affects what appears
> >    at the end of @@ context @@ lines, and would not have risk to
> >    corrupt the patch for plumbing.

I just sent a patch to put the funcname pattern in the "basic" config,
and to get rid of the lazy config loading. So that fixes one call by the
plumbing to read_config_if_needed.

And it looks like the second call is already OK. We don't try parsing
the config to get the external diff command unless ALLOW_EXTERNAL is
set, which the plumbing already disallows (though I am still confused
why it would need to be loaded lazily in the first place -- I wonder if
read_config_if_needed is needed at all).

-Peff

^ permalink raw reply

* Re: [BUG] git-diff-* --color oddness
From: Junio C Hamano @ 2008-01-04  9:37 UTC (permalink / raw)
  To: Jeff King; +Cc: git, win
In-Reply-To: <20080104092505.GA2320@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Fri, Jan 04, 2008 at 03:59:34AM -0500, Jeff King wrote:
>
>> OK. In that case, we need a way for the plumbing to tell the diff
>> machinery "don't ever try loading the ui config."
>
>> >  * funcname-pattern can go either way; that affects what appears
>> >    at the end of @@ context @@ lines, and would not have risk to
>> >    corrupt the patch for plumbing.
>
> I just sent a patch to put the funcname pattern in the "basic" config,
> and to get rid of the lazy config loading. So that fixes one call by the
> plumbing to read_config_if_needed.
>
> And it looks like the second call is already OK. We don't try parsing
> the config to get the external diff command unless ALLOW_EXTERNAL is
> set, which the plumbing already disallows (though I am still confused
> why it would need to be loaded lazily in the first place -- I wonder if
> read_config_if_needed is needed at all).

I think that was a premature optimization without benching.  It
is expected that most trees would not have attributes to define
custom low-level diff types, and without them we do not need to
parse the configuration to find out the external commands to be
used.

^ permalink raw reply

* Re: [BUG] git-diff-* --color oddness
From: Jeff King @ 2008-01-04  9:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, win
In-Reply-To: <7vodc2kk2d.fsf@gitster.siamese.dyndns.org>

On Fri, Jan 04, 2008 at 01:37:46AM -0800, Junio C Hamano wrote:

> > And it looks like the second call is already OK. We don't try parsing
> > the config to get the external diff command unless ALLOW_EXTERNAL is
> > set, which the plumbing already disallows (though I am still confused
> > why it would need to be loaded lazily in the first place -- I wonder if
> > read_config_if_needed is needed at all).
> 
> I think that was a premature optimization without benching.  It
> is expected that most trees would not have attributes to define
> custom low-level diff types, and without them we do not need to
> parse the configuration to find out the external commands to be
> used.

Ah. But we were parsing them anyway in git_diff_ui_config at the
beginning of the program (and we need to, since you will always have
diff.default.*). So I think this is safe to do, and can replace my other
"only read config once" patch from a few minutes ago:

-- >8 --
diff: remove lazy config loading

There is no point to this. Either:

  1. The program has already loaded git_diff_ui_config, in
     which case this is a noop.
  2. The program didn't, which means it is plumbing that
     does not _want_ git_diff_ui_config to be loaded.

Signed-off-by: Jeff King <peff@peff.net>
---
 diff.c |   12 ------------
 1 files changed, 0 insertions(+), 12 deletions(-)

diff --git a/diff.c b/diff.c
index ecf2fd6..2c78d74 100644
--- a/diff.c
+++ b/diff.c
@@ -59,17 +59,6 @@ static struct ll_diff_driver {
 	char *cmd;
 } *user_diff, **user_diff_tail;
 
-static void read_config_if_needed(void)
-{
-	static int done = 0;
-	if (done)
-		return;
-
-	user_diff_tail = &user_diff;
-	git_config(git_diff_ui_config);
-	done = 1;
-}
-
 /*
  * Currently there is only "diff.<drivername>.command" variable;
  * because there are "diff.color.<slot>" variables, we are parsing
@@ -1825,7 +1814,6 @@ static const char *external_diff_attr(const char *name)
 		    !ATTR_UNSET(value)) {
 			struct ll_diff_driver *drv;
 
-			read_config_if_needed();
 			for (drv = user_diff; drv; drv = drv->next)
 				if (!strcmp(drv->name, value))
 					return drv->cmd;
-- 
1.5.4.rc2.1125.ga305e-dirty

^ permalink raw reply related

* [PATCH] Document git-reset defaults to HEAD if no commit is given
From: Marco Costalba @ 2008-01-04  9:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List

Signed-off by: Marco Costalba <mcostalba@gmail.com>
---
 Documentation/git-reset.txt |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index 050e4ea..69722d1 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -9,7 +9,7 @@ SYNOPSIS
 --------
 [verse]
 'git-reset' [--mixed | --soft | --hard] [-q] [<commit>]
-'git-reset' [--mixed] [-q] <commit> [--] <paths>...
+'git-reset' [--mixed] [-q] [<commit>] [--] <paths>...

 DESCRIPTION
 -----------
@@ -49,7 +49,7 @@ OPTIONS
 	Be quiet, only report errors.

 <commit>::
-	Commit to make the current HEAD.
+	Commit to make the current HEAD. If not given defaults to HEAD.

 Examples
 --------
-- 
1.5.4.rc2.20.gab44

^ permalink raw reply related

* Re: Retroactively change email signature? [resend]
From: Sam Vilain @ 2008-01-03  5:52 UTC (permalink / raw)
  To: Junio Hamano; +Cc: git
In-Reply-To: <alpine.LFD.1.00.0801021316080.3010@woody.linux-foundation.org>

Linus Torvalds wrote:
> (I don't think git-filter-branch even exposes any easy way to remap the 
> "revert commit xyz" messages in the message format. It's not technically 
> hard to use a --msg-filter for it, but it seems a big bother).

Assuming, of course, that the commit you refer to in a message like that
is in your history.  If it's referring to another commit in a different
branch, which may be being re-written, you've got two problems:

1. rewriting the commit

2. making sure that git-filter-branch processes the two commits in the
right order.

The first is easily fixable; the second requires a hook in
git-filter-branch to customize the tournament.

Subject: [PATCH] git-filter-branch: allow a custom tournament for rewrite ordering

The topological sort by git rev-list may be inconvenient for history
modification where commit IDs are used in comments to refer to other
changes.  So, allow for a program that can take a list of commits, and
return that list in the order that the rewriter must follow.  Borrow
"Tournament" term from Graph Theory.

Signed-off-by: Sam Vilain <sam.vilain@catalyst.net.nz>
---
 Documentation/git-filter-branch.txt         |   18 ++++
 contrib/filter-branch/msg-links.perl        |   50 +++++++++
 contrib/filter-branch/tournament-links.perl |  148 +++++++++++++++++++++++++++
 git-filter-branch.sh                        |   11 ++
 t/t7003-filter-branch.sh                    |   21 ++++
 5 files changed, 248 insertions(+), 0 deletions(-)
 create mode 100755 contrib/filter-branch/msg-links.perl
 create mode 100755 contrib/filter-branch/tournament-links.perl

diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt
index 895d750..ed3a839 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -118,6 +118,24 @@ convenience functions, too.  For example, calling 'skip_commit "$@"'
 will leave out the current commit (but not its changes! If you want
 that, use gitlink:git-rebase[1] instead).
 
+--tournament <command>::
+	This command is responsible for re-ordering the commit history
+	after it is retrieved from the input list.  It is passed on
+	standard input the deduced list based upon a regular
+	topological search, and the command is expected to print to
+	standard output a list in the same form.
++
+Again, this can't be used to re-order patches like
+gitlink:git-rebase[1] does, as the tree IDs of the original positions
+will remain untouched, so the differences between revisions will be
+scrambled.
++
+However, it is required when you need to refer to a commit by ID in a
+commit message, where the commit you are referring to is on an
+unrelated branch.  See the contributed filters in the Git distribution
+for an example script that can let you preserve such referential
+integrity during a rewrite.
+
 --tag-name-filter <command>::
 	This is the filter for rewriting tag names. When passed,
 	it will be called for every tag ref that points to a rewritten
diff --git a/contrib/filter-branch/msg-links.perl b/contrib/filter-branch/msg-links.perl
new file mode 100755
index 0000000..1ff780c
--- /dev/null
+++ b/contrib/filter-branch/msg-links.perl
@@ -0,0 +1,50 @@
+#!/usr/bin/perl -nl
+#
+#   msg-links.perl
+#
+# This is the companion script to tournament-links.perl, it is
+# configured in the same way.  If you want this script, you will
+# probably also want the other one.
+#
+# What it does is rewrites all commit IDs of at least 12 digits long
+# to the post-rewritten version.  Note that this works only for
+# commits seen by the current git-filter-branch run!  If you need to
+# correct "dead" links from a previous run, then supply the rewrite
+# file.
+#
+# You don't need the tournament filter if all of the commits IDs you
+# are changing to are "ancient" history, or if you are rewriting
+# history that is a straight line.
+
+no strict;
+no warnings;
+
+use constant REV_MIN_LENGTH => $ENV{MSG_LINK_MIN} || 12;
+
+BEGIN {
+	$mlr = "$ENV{GITDIR}/msg-link-rewrite";
+	if ( -e $mlr ) {
+		open FH, "<", $mlr or die $!;
+		local($/);
+		%remap = m{([0-9a-f]{40})}g while <FH>;
+	}
+	( -d "../map" ) or die "can't see the git-filter-branch map dir";
+}
+
+my $old = $_;
+s{([0-9a-f]{${\(REV_MIN_LENGTH)},40})}{substr(($remap{$1}?do {
+	print STDERR " Remapped: $1 => $remap{$1} (rule)\n";
+	$remap{$1}; }
+		   : $1), 0, length($1))}eg;
+
+s{([0-9a-f]{${\(REV_MIN_LENGTH)},40})}{substr((-e "../map/$1" ? do {
+	$x = `cat ../map/$1`; chomp($x);
+		print STDERR " Remapped: $1 => $x (ripple)\n";
+	$x}
+: $1),0,length($1))}eg;
+
+if ( $old ne $_ ) {
+	print STDERR "*** filtered: pre: \n$old\n post:\n$_\n";
+}
+
+print;
diff --git a/contrib/filter-branch/tournament-links.perl b/contrib/filter-branch/tournament-links.perl
new file mode 100755
index 0000000..479a28f
--- /dev/null
+++ b/contrib/filter-branch/tournament-links.perl
@@ -0,0 +1,148 @@
+#!/usr/bin/perl
+#
+#  tournament-links.perl: a tournament that allows simple re-ordering.
+#
+#  This provides the requirement of getting the commits in the right
+#  order if there are any (full length) commitids in the commit
+#  messages.  It also provides for some remapping along the way (if,
+#  say, a referenced commit ID was wrong).  To do that, put it in a
+#  file in GIT_DIR called "msg-link-rewrite".
+#
+#  The format of msg-link-rewrite is very simple - any full-length
+#  SHA1s that are seen in the file are put into a single hash.  So,
+#  every other item is a key, starting with the first.  Everything
+#  else in the file is ignored.  This is combined with the remapping
+#  that the current git-filter-branch run has performed.
+#
+#  This script is only called once, at the beginning of the run, and
+#  can be relatively expensive on this basis.  This one is practically
+#  guaranteed to feed quicksort it's worst nightmare every time.
+#
+#  Example:
+#    $ cat > .git/msg-link-rewrite <<EOF
+#        Rewrite the commit ID 123456789012 to
+#              '92712632981923'
+#    EOF
+#    $ path=$ENV{HOME}/src/git/contrib/filter-branch
+#    $ git-filter-branch --tournament $path/tournament.perl \
+#         --msg-filter $path/msg-links.perl
+#
+
+no strict;
+no warnings;
+# use sort '_mergesort'; # Perl 5.10+ - use a mergesort instead
+
+use constant WATCH_BATTLE => $ENV{WATCH_BATTLE};
+use constant TOURNAMENT_RESULTS => $ENV{TOURNAMENT_RESULTS};
+use constant REV_MIN_LENGTH => $ENV{MSG_LINK_MIN} || 12;
+
+BEGIN {
+	$mlr = "$ENV{GITDIR}/msg-link-rewrite";
+	if ( -e $mlr ) {
+		open FH, "<", $mlr or die $!;
+		local($/);
+		%remap = m{([0-9a-f]{${\(REV_MIN_LENGTH)},40})}g while <FH>;
+	}
+	( -d "../map" ) or die "can't see the git-filter-branch map dir";
+}
+
+@commits = <>;
+%refers;
+
+# shorten a commit ID to something human-readable for display to
+# STDERR only
+sub n{
+	my $commitid = shift;
+	substr($commitid, 0, 12);
+}
+
+print STDERR "tournament: read ".@commits." commit IDs\n"
+	if TOURNAMENT_RESULTS;
+
+for my $line ( @commits ) {
+	chomp($line);
+	my ($commit, @parents) = split " ", $line;
+	my @refers_commitids;
+	open COMMIT, "-|", qw(git cat-file commit), $commit;
+	while ( <COMMIT> ) {
+		push @refers_commitids, $1 if m{^parent (.*)$};
+		last if m{^\s*$};
+	}
+
+	my $msg = join "", <COMMIT>;
+
+	my @refers_commitids = map {
+		my ($match) = $remap{$_} ? ($remap{$_})
+			: map { $remap{$_} } grep m{^$_}, keys %remap;
+		if ( $match ) {
+			print STDERR "msg-link in ".n($commit).": "
+				.n($_)." treated as ".n($match)."\n"
+					if WATCH_BATTLE;
+			$match;
+		}
+		elsif ( do { $match = `git rev-parse $_`; !$? } ) {
+			chomp($match);
+			print STDERR "msg-link seen in ".n($commit)
+				." to ".n($match)."\n"
+					if WATCH_BATTLE;
+			$match;
+		}
+		else {
+			print STDERR "dead msg-link seen in ".n($commit)
+				." to ".n($_)."\n";
+			$_;
+		}
+	} ($msg =~ m{([0-9a-f]{${\(REV_MIN_LENGTH)},40})}g);
+
+	warn("$commit is not an extant revision\n"), next if $?;
+
+	$refers{$commit} = { map { $_ => 1 } @refers_commitids, @parents };
+}
+print STDERR "tournament: read ".@commits." commits\n"
+	if TOURNAMENT_RESULTS;
+
+my $O;
+
+# returns 1 if source refers to target, or target is a parent of
+# anything referred to by source.
+my $refers = sub {
+	my $source = shift;
+	my $target = shift;
+	my @todo = keys %{ $refers{$source}||{} };
+	my %seen = map { $_ => 1 } @todo;
+	while ( my $commit = shift @todo ) {
+		$O++;
+		do {
+			print STDERR n($source)." refers to ".n($target)."\n"
+				if WATCH_BATTLE;
+			return 1;
+		} if $commit eq $target;
+		push @todo, grep { $O++; !$seen{$_}++ }
+			keys %{ $refers{$commit} || {} };
+	}
+	print STDERR n($source)." does not refer to ".n($target)."\n"
+		if WATCH_BATTLE;
+	return 0;
+};
+
+# It's tournament time!
+print "$_\n" for sort {
+	(my $_a = $a) =~ s{ .*}{};
+	(my $_b = $b) =~ s{ .*}{};
+	my $cmp_val = 0;
+	if ( $refers->($_a, $_b) ) {
+		++$cmp_val;
+	}
+	elsif ( $refers->($_b, $_a) ) {
+		--$cmp_val;
+	}
+	print STDERR n($a)." ".
+		(($cmp_val == 0 ? "EQUIVALENT TO" :
+		  $cmp_val == -1 ? "COMES BEFORE" :
+		  $cmp_val == 1 ? "COMES AFTER" : "TSNH")
+		 ." ".n($b)."\n")
+			if WATCH_BATTLE;
+	$cmp_val;
+} @commits;
+
+print STDERR "tournament-links.perl: O(".@commits.") = ".$O."\n";
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index ae29f47..6c099ea 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -110,6 +110,7 @@ filter_msg=cat
 filter_commit='git commit-tree "$@"'
 filter_tag_name=
 filter_subdir=
+tournament=
 orig_namespace=refs/original/
 force=
 while :
@@ -166,6 +167,9 @@ do
 	--subdirectory-filter)
 		filter_subdir="$OPTARG"
 		;;
+	--tournament)
+		tournament="$OPTARG"
+		;;
 	--original)
 		orig_namespace=$(expr "$OPTARG/" : '\(.*[^/]\)/*$')/
 		;;
@@ -254,6 +258,13 @@ commits=$(wc -l <../revs | tr -d " ")
 
 test $commits -eq 0 && die "Found nothing to rewrite"
 
+if [ -n "$tournament" ]
+then
+	cat ../revs | (eval "$tournament") > ../revs.sorted
+	mv ../revs ../revs.pre-sorted
+	mv ../revs.sorted ../revs
+fi
+
 # Rewrite the commits
 
 i=0
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 5f60b22..8d5679d 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -165,4 +165,25 @@ test_expect_success '"map" works in commit filter' '
 	git rev-parse --verify master
 '
 
+test_expect_success "Specifying a custom tournament" '
+        git checkout -b custom-tournament &&
+	echo "Hello">&2
+        echo j > j &&
+	git add j &&
+	echo "There">&2
+        git commit -m j j &&
+        echo z > z &&
+	git add z &&
+	echo "Dude">&2
+        git commit -m z &&
+        echo x > x &&
+	git add x &&
+	echo "Dude">&2
+        git commit -m x &&
+        git-filter-branch -f --parent-filter cat --tournament "cat > foo; fp=\$(head -1 foo | awk \"{print \\\$2}\"); ( head -2 foo | tac; tail -1 foo ) | awk \"{print \\\$1}\" | while read x y; do echo \$x \$fp; fp=\$x; done"  master..HEAD &&
+	order=$(git-log --pretty=format:"%s" master...custom-tournament | xargs) &&
+	echo "order is <$order>" &&
+	test "$order" = "x j z"
+'
+
 test_done
-- 
1.5.3.5

^ permalink raw reply related

* Re: [PATCH] git-clean: make "Would remove ..." path relative to cwd again
From: Miklos Vajna @ 2008-01-04 15:41 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmyrmmc21.fsf@gitster.siamese.dyndns.org>

[-- Attachment #1: Type: text/plain, Size: 248 bytes --]

On Thu, Jan 03, 2008 at 08:47:50PM -0800, Junio C Hamano <gitster@pobox.com> wrote:
> The rewrite changed the output to use the path relative to the
> top of the work tree without a good reason.  This fixes it.

works here fine.

thanks,
- VMiklos

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: [PATCH] git-filter-branch could be confused by similar names
From: Dmitry Potapov @ 2008-01-04 15:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Johannes Schindelin
In-Reply-To: <7v7iiqppkw.fsf@gitster.siamese.dyndns.org>

On Thu, Jan 03, 2008 at 01:27:27PM -0800, Junio C Hamano wrote:
> Dmitry Potapov <dpotapov@gmail.com> writes:
> >  		ref="$(git for-each-ref --format='%(refname)' |
> > -			grep /"$ref")"
> > +			grep '^refs/\([^/]\+/\)\?'"$ref"'$')"
> >  	esac
> 
> Do we assume everybody's grep groks ERE these days?  I had an
> impression that we try to stick to a subset of BRE (namely, no
> \{m,n\}, [::], [==], nor [..]).

I was not aware about this policy, and I am not aware about
existing any grep that does not grok the expressions I used
above. So, I thought they are commonly accepted, but I might
be wrong.

> 
> Also as a general rule when dealing with refname, we use
> fileglob not regex.

Actually, refname was not meant to be used as regex here, and
it was written in the hope that there will be no special regex
symbols in the refname, but, yes, this use looks like hack.

BTW, accordingly to man, git filter-branch has <rev-list options>,
and git rev-list described as <commit(s)>, so fileglob may not
be used here. I look also at git for-each-ref and git show-ref,
and though they could have <pattern> as arguments, they meant
completely different by that. git for-each-ref requires the
full specification starting with refs but allows fileglob, while
git-show-ref does not allow fileglob, but it goes deeper in
refs, so it will match with those refs that are inside origin,
which git ref-list does not do. Here are a few examples:

===
$ git rev-list -1 master
257f3020f69f3222cdefc1d84b148fb35b2c4f5b
$ git rev-list -1 heads/master
257f3020f69f3222cdefc1d84b148fb35b2c4f5b
$ git rev-list -1 refs/heads/master
257f3020f69f3222cdefc1d84b148fb35b2c4f5b
$ git rev-list -1 'refs/heads/maste?'
fatal: ambiguous argument 'refs/heads/maste?': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions
$ git rev-list -1 maint
fatal: ambiguous argument 'maint': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions
===
$ git for-each-ref refs/heads/master
257f3020f69f3222cdefc1d84b148fb35b2c4f5b commit	refs/heads/master
$ git for-each-ref refs/heads/maste?
257f3020f69f3222cdefc1d84b148fb35b2c4f5b commit	refs/heads/master
$ git for-each-ref heads/master
$ git for-each-ref master
$ git for-each-ref maint
===
$ git show-ref refs/heads/master
257f3020f69f3222cdefc1d84b148fb35b2c4f5b refs/heads/master
$ git show-ref refs/heads/maste?
$ git show-ref heads/master
257f3020f69f3222cdefc1d84b148fb35b2c4f5b refs/heads/master
$ git show-ref master
257f3020f69f3222cdefc1d84b148fb35b2c4f5b refs/heads/master
257f3020f69f3222cdefc1d84b148fb35b2c4f5b refs/remotes/origin/master
$ git show-ref maint
4f3d37035a7c735a3b69f962656819f4ff7e4927 refs/remotes/origin/maint
===

> 
> What's the goal here?  Is it to make sure given refname is
> unambiguous by being a unique suffix of tags or heads, as in
> 
> 	test $(git show-ref "$ref" | wc -l) = 1

Because, I am not the author of this script, I can't be sure,
but it seems to me, the goal is to select among all parameters
only those that represents tops of branches, for example,
being given: A..B ^C D, we should choose only B and D and
convert them into the full refname in the same way as rev-list
does that.

> 
> or is there anything more going on?
> 
> Ah, it also wants the full name of the ref.  How about...
> 
> 	ref=$(git show-ref "$ref" | sed -e 's/^.* //')

It works only if the name "unambiguous" for git show-ref, which
interprets refname differently than rev-list as I wrote above.
Nevertheless, I believe we can use 'git show-ref' if we try
something like this:

for prefix in refs refs/tags refs/heads refs/remote; do
  fullref=$(git show-ref "$prefix/$ref" | sed -e 's/^.* //')
  test -n "$fullref" && break
done
ref="$fullref"

If this idea does not raise any objection, I will test it a bit
and then send the patch.

Dmitry

^ permalink raw reply

* git ftp push ?
From: Felipe Balbi @ 2008-01-04 15:54 UTC (permalink / raw)
  To: git

Hello all,

how could I issue a git push via ftp ??
Is that possible ?

Thanks in advance

-- 
Best Regards,

Felipe Balbi
felipebalbi@users.sourceforge.net

^ permalink raw reply

* [PATCH] git stash: one bug and one feature request
From: Marco Costalba @ 2008-01-04 16:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List

Currently git-stash writes to stderr also if there is nothing to error
out, also it would be very nice ;-) if git 'stash clear command' would
support deleting of only one patch, so as example to write

stg stash clear stash@{0}

To remove only the last added.


------------------  cut --------------------------

From: Marco Costalba <mcostalba@gmail.com>
Date: Fri, 4 Jan 2008 17:08:01 +0100
Subject: [PATCH] git-stash: avoid writing to stderr when is not an error

Otherwise git-stash is unusable by scripts that check
stderr to detect fail/success of launched command.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>
---
 git-stash.sh |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index 06cb177..a05a47a 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -86,7 +86,7 @@ save_stash () {

 	if no_changes
 	then
-		echo >&2 'No local changes to save'
+		echo > 'No local changes to save'
 		exit 0
 	fi
 	test -f "$GIT_DIR/logs/$ref_stash" ||
@@ -99,7 +99,7 @@ save_stash () {

 	git update-ref -m "$stash_msg" $ref_stash $w_commit ||
 		die "Cannot save the current status"
-	printf >&2 'Saved working directory and index state "%s"\n' "$stash_msg"
+	printf > 'Saved working directory and index state "%s"\n' "$stash_msg"
 }

 have_stash () {
@@ -229,7 +229,7 @@ create)
 	if test $# -eq 0
 	then
 		save_stash &&
-		echo >&2 '(To restore them type "git stash apply")' &&
+		echo > '(To restore them type "git stash apply")' &&
 		git-reset --hard
 	else
 		usage
-- 
1.5.4.rc2.18.g530e6

^ permalink raw reply related

* Re: git ftp push ?
From: Miklos Vajna @ 2008-01-04 16:32 UTC (permalink / raw)
  To: Felipe Balbi; +Cc: git
In-Reply-To: <31e679430801040754m552b7650g50405b6e2e607da9@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 301 bytes --]

On Fri, Jan 04, 2008 at 10:54:16AM -0500, Felipe Balbi <felipebalbi@users.sourceforge.net> wrote:
> how could I issue a git push via ftp ??
> Is that possible ?

at the moment, not. (http push is supported and it's a dumb protocol, so
it would be possible to support ftp but nobody did so)

- VMiklos

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: [PATCH 3/2] attribute "coding": specify blob encoding
From: Tsugikazu Shibata @ 2008-01-04 16:16 UTC (permalink / raw)
  To: gitster; +Cc: nanako3, git, tshibata
In-Reply-To: <7vejcyo9ql.fsf@gitster.siamese.dyndns.org>

On Thu, 03 Jan 2008 13:54:58 -0800, gitster wrote:
> しらいしななこ  <nanako3@bluebottle.com> writes:
> 
> > Quoting Junio C Hamano <gitster@pobox.com>:
> > 
> >> This teaches "diff hunk header" function about custom character
> >> encoding per path via gitattributes(5) so that it can sensibly
> >> chomp a line without truncating a character in the middle.
> >>
> >> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> >> ---
> >>
> >>  * This is not intended for serious inclusion, but was done more
> >>    as a demonstration of the concept, hence [3/2].
> >
...
> >> +static struct {
> >> +	const char *coding;
> >> +	sane_truncate_fn fn;
> >> +} builtin_truncate_fn[] = {
> >> +	{ "euc-jp", truncate_euc_jp },
> >> +	{ "utf-8", NULL },
> >> +};
> >
> >Can you also do JIS and Shift JIS?  I ask because many of my
> >old notes are in Shift JIS and I think it is the same for many
> >other people. 
> 
> I guess somebody else could (hint, hint,...).  Shift_JIS should
> be more or less straightforward to add.
> 
> With the current code structure, however, ISO-2022 (you said
> "JIS" -- Japanese often use that word to mean 7-bit ISO-2022 and
> so did you in this context) is a bit cumbersome to handle, as
> you cannot just truncate but also have to add a few escape bytes
> to go back to ASCII at the end of line.

I guess that there are many other encodings and support everything are
not reasonable.
In my experience, It seems that chopping a multi-byte character
occurs the field after "@@". I believe this field is for the name of
the appropriate function. Also, I believe most of the function names
are in ASCII. 
So, question is why we should think of this field in case of non
programming language files ? 
In case of text file using any coding, Should we need to add something
after "@@"?
How about not to add anything after "@@" when the file name was .txt
or no extension (ie. HOWTO)?

^ permalink raw reply

* libgit: git_setup_directory() is not reentrant
From: Christian Thaeter @ 2008-01-04 17:07 UTC (permalink / raw)
  To: git

While working on a git-browser, I noticed that git_setup_directory() is
not reentrant. Once called it initializes (by calling other functions in
turn) several static/hidden variables which can't be altered afterwards.
This is ugly when one wants  to iterate over different repositories
(generating a list of available repos).

How could we fix this?
I would propose that subsequent 'git_setup_directory()' calls will
reinit the setup completely.

Another way would be to provide a 'git_setup_reset()' function which
lets one cleanup all setup things before calling 'git_setup_directory()'
again.

Comments?

	Christian

^ permalink raw reply

* Re: [PATCH] git stash: one bug and one feature request
From: Brandon Casey @ 2008-01-04 16:36 UTC (permalink / raw)
  To: Marco Costalba; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <e5bfff550801040814n82f34b2g17c485a207093440@mail.gmail.com>

On Fri, 4 Jan 2008, Marco Costalba wrote:

> Currently git-stash writes to stderr also if there is nothing to error
> out, also it would be very nice ;-) if git 'stash clear command' would
> support deleting of only one patch, so as example to write
>
> stg stash clear stash@{0}
>
> To remove only the last added.

Maybe it should be named 'drop'. 'drop' sounds better than
'clear' for this usage.

   git stash drop [<stash>]

Not sure how often such a command would be used though, so
it may not be worth it.

-brandon

^ permalink raw reply

* Re: [PATCH] git stash: one bug and one feature request
From: Pascal Obry @ 2008-01-04 17:30 UTC (permalink / raw)
  To: Brandon Casey; +Cc: Marco Costalba, Junio C Hamano, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0801041030420.31161@torch.nrlssc.navy.mil>

Brandon Casey a écrit :
> Not sure how often such a command would be used though, so
> it may not be worth it.

I've missed it many times. Especially in some scripts when I want to use
the stash-stack to store current working tree and clear it before
exiting. This is not possible today as all the stash-stack would be cleared.

I agree that drop seems better.

Pascal.

-- 

--|------------------------------------------------------
--| Pascal Obry                           Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--|              http://www.obry.net
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595

^ permalink raw reply

* [PATCH] Don't access line[-1] for a zero-length "line" from fgets.
From: Jim Meyering @ 2008-01-04 17:37 UTC (permalink / raw)
  To: git list

A NUL byte at beginning of file, or just after a newline
would provoke an invalid buf[-1] access in a few places.

* builtin-grep.c (cmd_grep): Don't access buf[-1].
* builtin-pack-objects.c (get_object_list): Likewise.
* builtin-rev-list.c (read_revisions_from_stdin): Likewise.
* bundle.c (read_bundle_header): Likewise.
* server-info.c (read_pack_info_file): Likewise.
* transport.c (insert_packed_refs): Likewise.

Signed-off-by: Jim Meyering <meyering@redhat.com>
---
 builtin-grep.c         |    2 +-
 builtin-pack-objects.c |    2 +-
 builtin-rev-list.c     |    2 +-
 bundle.c               |    2 +-
 server-info.c          |    2 +-
 transport.c            |    2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/builtin-grep.c b/builtin-grep.c
index f1ff8dc..0d6cc73 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -644,7 +644,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 				die("'%s': %s", argv[1], strerror(errno));
 			while (fgets(buf, sizeof(buf), patterns)) {
 				int len = strlen(buf);
-				if (buf[len-1] == '\n')
+				if (len && buf[len-1] == '\n')
 					buf[len-1] = 0;
 				/* ignore empty line like grep does */
 				if (!buf[0])
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index e0ce114..a39cb82 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -2013,7 +2013,7 @@ static void get_object_list(int ac, const char **av)

 	while (fgets(line, sizeof(line), stdin) != NULL) {
 		int len = strlen(line);
-		if (line[len - 1] == '\n')
+		if (len && line[len - 1] == '\n')
 			line[--len] = 0;
 		if (!len)
 			break;
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 1cb5f67..de80158 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -520,7 +520,7 @@ static void read_revisions_from_stdin(struct rev_info *revs)

 	while (fgets(line, sizeof(line), stdin) != NULL) {
 		int len = strlen(line);
-		if (line[len - 1] == '\n')
+		if (len && line[len - 1] == '\n')
 			line[--len] = 0;
 		if (!len)
 			break;
diff --git a/bundle.c b/bundle.c
index 9b9b916..be204d8 100644
--- a/bundle.c
+++ b/bundle.c
@@ -48,7 +48,7 @@ int read_bundle_header(const char *path, struct bundle_header *header)
 			: &header->references;
 		char delim;

-		if (buffer[len - 1] == '\n')
+		if (len && buffer[len - 1] == '\n')
 			buffer[len - 1] = '\0';
 		if (get_sha1_hex(buffer + offset, sha1)) {
 			warning("unrecognized header: %s", buffer);
diff --git a/server-info.c b/server-info.c
index a051e49..c1c073b 100644
--- a/server-info.c
+++ b/server-info.c
@@ -101,7 +101,7 @@ static int read_pack_info_file(const char *infofile)

 	while (fgets(line, sizeof(line), fp)) {
 		int len = strlen(line);
-		if (line[len-1] == '\n')
+		if (len && line[len-1] == '\n')
 			line[--len] = 0;

 		if (!len)
diff --git a/transport.c b/transport.c
index 4e151a9..babaa21 100644
--- a/transport.c
+++ b/transport.c
@@ -118,7 +118,7 @@ static void insert_packed_refs(const char *packed_refs, struct ref **list)
 		if (hexval(buffer[0]) > 0xf)
 			continue;
 		len = strlen(buffer);
-		if (buffer[len - 1] == '\n')
+		if (len && buffer[len - 1] == '\n')
 			buffer[--len] = '\0';
 		if (len < 41)
 			continue;
--
1.5.4.rc2.18.ga0289

^ permalink raw reply related

* Re: [PATCH] git stash: one bug and one feature request
From: Jakub Narebski @ 2008-01-04 17:51 UTC (permalink / raw)
  To: Pascal Obry
  Cc: Brandon Casey, Marco Costalba, Junio C Hamano, Git Mailing List
In-Reply-To: <477E6D26.9020809@obry.net>

Pascal Obry <pascal@obry.net> writes:
> Brandon Casey a écrit :
> >
> > Not sure how often such a command would be used though, so
> > it may not be worth it.
> 
> I've missed it many times. Especially in some scripts when I want to use
> the stash-stack to store current working tree and clear it before
> exiting. This is not possible today as all the stash-stack would be cleared.
> 
> I agree that drop seems better.

or "git stash delete"

This probably would require the command to delete single reflog,
which was posted some time ago and is in either pu or in offcuts,
or in next.

But I guess this is post 1.5.4
-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [PATCH] git stash: one bug and one feature request
From: Brandon Casey @ 2008-01-04 18:00 UTC (permalink / raw)
  To: Marco Costalba; +Cc: Pascal Obry, Junio C Hamano, Git Mailing List
In-Reply-To: <e5bfff550801040944p7f8e722asfa726b34a4a712fa@mail.gmail.com>

Marco Costalba wrote:
> Ok, drop is better then clear, but, if we need to add a new command I
> vote for 'delete' or 'rm' to be consistent with git naming.

If the stash list is thought of as a stack, then drop makes sense.

I imagine using it like

   git stash apply
   git stash drop
   git stash apply stash@{3}
   git stash drop stash@{3}

'git stash delete' and 'git stash rm' when used without arguments
both sound like 'git stash clear' to me.

-brandon

^ permalink raw reply

* Re: [PATCH] git stash: one bug and one feature request
From: Marco Costalba @ 2008-01-04 18:05 UTC (permalink / raw)
  To: Brandon Casey; +Cc: Pascal Obry, Junio C Hamano, Git Mailing List
In-Reply-To: <477E7439.9090209@nrlssc.navy.mil>

On Jan 4, 2008 7:00 PM, Brandon Casey <casey@nrlssc.navy.mil> wrote:
> Marco Costalba wrote:
> > Ok, drop is better then clear, but, if we need to add a new command I
> > vote for 'delete' or 'rm' to be consistent with git naming.
>
> If the stash list is thought of as a stack, then drop makes sense.
>

Yes, but is _not_ as a stack because you can say

git stash apply stash@{3}
git stash apply stash@{1}
git stash apply stash@{4}

i.e. you can access reflogs in any order, so thinking to a stack is
misleading IMHO.

Marco

^ permalink raw reply

* Octopus?  Really?  Interesting...
From: Jon Loeliger @ 2008-01-04 18:28 UTC (permalink / raw)
  To: Git List

Guys,

So, I brain-wedged a merge here just now
and was surprised to see that a simple
fetch-n-merge yielded an octopus merge.
I was expecting a fast-forward.  So I went
back and reviewed.  What actually happend
was this:

    $ git checkout master
    $ git fetch origin
    # check it out... looks great ... let's merge it
    $ git merge master origin/master
    merge performed by octopus.

Oh, yeah.  Duh.  I did name two branches.  Feh.

Couple questions:

    Is it ever NOT the case, that if you are on one
    branch ("master") and name it as a "to be merged"
    branch along with some others, that we can simplify
    the request by noting that it is the same as the
    current "to be merged into" target branch?

    Other than creating a log message with "merged
    by octopus", will this merge be content-identical
    to the obvious simplified merge?

Thanks,
jdl

^ permalink raw reply

* Re: [PATCH] git stash: one bug and one feature request
From: Brandon Casey @ 2008-01-04 18:34 UTC (permalink / raw)
  To: Marco Costalba; +Cc: Pascal Obry, Junio C Hamano, Git Mailing List
In-Reply-To: <e5bfff550801041005x3ab682dam8535c7bde75038dc@mail.gmail.com>

Marco Costalba wrote:
> On Jan 4, 2008 7:00 PM, Brandon Casey <casey@nrlssc.navy.mil> wrote:
>> Marco Costalba wrote:
>>> Ok, drop is better then clear, but, if we need to add a new command I
>>> vote for 'delete' or 'rm' to be consistent with git naming.
>> If the stash list is thought of as a stack, then drop makes sense.
>>
> 
> Yes, but is _not_ as a stack because you can say
> 
> git stash apply stash@{3}
> git stash apply stash@{1}
> git stash apply stash@{4}
> 
> i.e. you can access reflogs in any order, so thinking to a stack is
> misleading IMHO.

I think it is like a stack because new things are always added to the top
and shift everything else down.
i.e. we can't say 'git stash replace stash@{3}' and we probably wouldn't
want to.

When we call git stash, the previous item on 'top' is pushed down
so that it is the second item stash@{1}. The new item just stashed
(pushed), is now on top at stash@{0}.

Doesn't seem like too far of a stretch.

-brandon

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox