Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Implement limited context matching in git-apply.
From: Eric W. Biederman @ 2006-04-10 18:35 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0604100821340.9504@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> On Mon, 10 Apr 2006, Eric W. Biederman wrote:
>> 
>> If I just loop through all of Andrews patches in order
>> and run git-apply --index -C1 I process the entire patchset
>> in 1m53s or about 6 patches per second.  So running
>> git-mailinfo, git-write-tree, git-commit-tree, and
>> git-update-ref everytime has a measurable impact,
>> and shows things can be speeded up even more.
>
> git-write-tree is actually a fairly expensive operation on the kernel. It 
> needs to write the 1000+ tree objects - and while _most_ of them already 
> exist (and thus don't actually need to be written out), we need to 
> generate the tree object and its SHA1 in order to notice that that is the 
> case.
>
> I'm almost certain that 90%+ of the overhead you see is the tree writing, 
> not the rest of the scripting.

Well it is easy enough to time.  Looking at the timings
going from just git-apply to git-apply && git-write-tree
does seem to about the double the amount of time taken,
or take me to about 4 minutes.  With everything else
in there things happen in the 6-7 minute range with
in the hot cache scenario.  So write-tree is closer
to 50% of the overhead.

Is it possible to cache the sha1 of unmodified directories?

If we did that we could probe to see if the hash already
existed before we attempted to look for the subdirectories.

The pain would is remembering which directory sha1 are
current.  If nothing else we can modify: 
remove_cache_entry, and add_file_to_cache to clear
the parent directories cached sha1 when we update an
index entry.  But I keep thinking there should
be something more elegant.  Like using ce_flags,
or comparing mtime values.

...

Ok taking a quick look at write-tree to see where
the bottle neck is:  

I made two modified versions of write-tree. 
- git-write-tree-nowritetree which calls return just before calling
    write_tree.
- git-write-tree-nosha1write which does everything except call
    sha1_file_write.

With just git-apply and git-write-tree-nosha1write it takes
me about 3m:20s to process 2.6.17-rc1-mm2.

With just git-apply and git-write-tree-nowritetree it takes:
real    2m59.985s
user    1m38.353s
sys     0m31.445s

With just git-apply and /bin/true it takes:
real    2m1.581s
user    1m3.169s
sys     0m29.903s


Looking at the individual numbers:
$ time git-write-tree-nowritetree --missing-ok

real    0m0.158s
user    0m0.052s
sys     0m0.008s
$ time git-write-tree-nowritetree --missing-ok

real    0m0.155s
user    0m0.057s
sys     0m0.003s
$ time git-write-tree-nowritetree --missing-ok 
real    0m0.065s
user    0m0.057s
sys     0m0.002s
$ time git-write-tree-nowritetree --missing-ok

real    0m0.159s
user    0m0.055s
sys     0m0.005s
$ time git-write-tree-nowritetree --missing-ok

real    0m0.151s
user    0m0.054s
sys     0m0.007s
$ time git-write-tree-nowritetree --missing-ok

real    0m0.154s
user    0m0.056s
sys     0m0.005s

$ time git-write-tree-nosha1write --missing-ok
0000000000000000000000000000000000000000

real    0m0.199s
user    0m0.091s
sys     0m0.008s
$ time git-write-tree-nosha1write --missing-ok
0000000000000000000000000000000000000000

real    0m0.195s
user    0m0.094s
sys     0m0.007s
$ time git-write-tree-nosha1write --missing-ok
0000000000000000000000000000000000000000

real    0m0.198s
user    0m0.092s
sys     0m0.009s

$ time git-write-tree --missing-ok
0ecfe3dbc2e65aa9638c62abf0cf05057c77f884

real    0m0.217s
user    0m0.113s
sys     0m0.012s

$ time git-write-tree
0ecfe3dbc2e65aa9638c62abf0cf05057c77f884

real    0m0.276s
user    0m0.169s
sys     0m0.008s

So at a quick inspection it looks to me like:
About .059s to perform to check for missing files.
About .019s to write the new tree.
About .155s in start up overhead, read_cache, and sanity checks.

So at a first glance it looks like librification to
allow the redundant work to be skipped, is where
the big speed win on my machine would be.

> Your patch looks ok from a quick read-through:

Thanks.

My import of 2.6.17-rc1-mm2 gives exactly the same
result as simply applying Andrews patch.  Which while
not definitive hits a lot of interesting cases.

> Acked-by: Linus Torvalds <torvalds@osdl.org>
>
> 		Linus

^ permalink raw reply

* Re: [PATCH] Implement limited context matching in git-apply.
From: Junio C Hamano @ 2006-04-10 19:29 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: git
In-Reply-To: <m1irphhj1p.fsf_-_@ebiederm.dsl.xmission.com>

ebiederm@xmission.com (Eric W. Biederman) writes:

> If I just loop through all of Andrews patches in order
> and run git-apply --index -C1 I process the entire patchset
> in 1m53s or about 6 patches per second.  So running
> git-mailinfo, git-write-tree, git-commit-tree, and
> git-update-ref everytime has a measurable impact,
> and shows things can be speeded up even more.

Although I haven't "read" it, but just only "looked at" it, the
patch looks OK.  I haven't managed to start beating on it yet
for time constraints.

If you are dealing with the kernel tree, I suspect most time is
spent on write-tree.  Statistically, a typical kernel patch (I
haven't counted the ones in -mm series, but only the ones
actually reacheable from Linus tip) touches only 3 files on
average, so most of the 1,100 tree objects in a typical kernel
tree are computed but found unchanged when write-tree happens.

I suspect we could make a backward incompatible change to the
index file format to record the top-level tree object names
somewhere where normal cache-entry walker would not see.  Then
when anybody makes a modification to invalidate that tree
object, mark that tree (or split that tree to read lower level
trees lazily) to force us recompute the tree object.

Theoretically you could do that recursively to record all 1,100
tree objects but that would make the cache slightly larger (say,
by 100kB).

^ permalink raw reply

* SVN_URL undocumented
From: Pavel Roskin @ 2006-04-10 21:22 UTC (permalink / raw)
  To: Eric Wong; +Cc: git

Hello, Eric!

I'm trying to use git-svn, but the documentation seems to be quite
unclear.  I'm using current git-svn from the git repository.

I'm running "git-svn init" in a Subversion working directory:

$ git-svn init
SVN repository location required
 at /home/proski/bin/git-svn line 223
        main::init() called at /home/proski/bin/git-svn line 104

The help shown by git-svn without arguments doesn't say anything about
arguments for "init", unlike other subcommands.  I'm assuming that
"git-svn init" is not taking any arguments.

The manual page doesn't say anything about arguments, but it mentions
that "the SVN_URL must be specified at this point".  This statement is
unclear to me.

There are several references to SVN_URL in the manual, but there is no
description what it is.  Is it supposed to be an environment variable,
or an undocumented argument, or is it taken from "svn info" output?

The environment variable has no effect:

$ SVN_URL=http://svn.madwifi.org/trunk git-svn init
SVN repository location required
 at /home/proski/bin/git-svn line 223
        main::init() called at /home/proski/bin/git-svn line 104

Looking at the sources, it looks like that SVN_URL is an undocumented
argument for "git-cvs init".  "git-svn fetch" appears to be taking that
value from the git repository, but it also takes care to protect the
original value (I have no idea where it can come from).

One of the "basic examples" further in the manual confirms that SVN_URL
is set on the command line:

git-svn init http://svn.foo.org/project/trunk

I believe that the documentation should be updated to describe where
SVN_URL comes from, and what arguments are accepted by "git-svn init".
Basic examples shouldn't recommend using undocumented arguments.

It seems to me that the current documentation describes the internals of
git-svn instead of its user interface.  Rather than tell the user that
"FOO must be specified at that point", the documentation should say what
(an how) should (or can) be specified when the command is invoked on the
command line.

Also, it would be great to refer to the "additional fetch arguments" in
the "fetch" description.  That section is easy to miss by somebody who
is specifically looking for "fetch arguments".

-- 
Regards,
Pavel Roskin

^ permalink raw reply

* Re: git-svnimport on OSX?
From: Rutger Nijlunsing @ 2006-04-10 23:00 UTC (permalink / raw)
  To: Martin Langhoff; +Cc: Randal L. Schwartz, git
In-Reply-To: <46a038f90604031911y415dd795nc1c8814f80a02ad7@mail.gmail.com>

On Tue, Apr 04, 2006 at 02:11:02PM +1200, Martin Langhoff wrote:
> On 03 Apr 2006 18:04:07 -0700, Randal L. Schwartz <merlyn@stonehenge.com> wrote:
> >
> > Working for anyone?  Not working for me, and just wondered if it was me or a
> > known thing.  Maybe I'm just holding my mouth wrong, and yes, I have SVN::Core
> > installed.  If anyone wants details, I can provide.
> 
> I think I tried and gave up on it a month or two ago, but can't
> remember the details. Fink's SVN::Core is too old, and having all the
> SVN toolchain is a pain. What is the problem?
> 
> BTW, getting git-svnimport to work normally takes me quite a few tries
> with different options, so OSX may be perfectly innocent this time...

Could you try http://www.wingding.demon.nl/git-svnconvert.rb on
MacOSX? This one doesn't need SVN::Core, and I'm curious on what to
change for MacOSX.

By default it imports all branches it can find (see in file itself):

...
$branch_dirs = %q{
  /branches/*
  /trunk
}
...

If your layout is different (for example, like
http://svn.perl.org/perl6 where every subdir is a collection of
branches itself), use something like:

...
$branch_dirs = %q{
  */branches/*
  */trunk
}
...



-- 
Rutger Nijlunsing ---------------------------------- eludias ed dse.nl
never attribute to a conspiracy which can be explained by incompetence
----------------------------------------------------------------------

^ permalink raw reply

* Re: [PATCH] git log [diff-tree options]...
From: Junio C Hamano @ 2006-04-10 23:42 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604090950590.9504@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> On Sun, 9 Apr 2006, Junio C Hamano wrote:
>>
>> And this makes "git log" to take common diff-tree options, so
>> that it can be used as "git whatchanged".
>
> I wonder... This all looks fine, but there are actually two different 
> "diffs" that can be shown for "git log --diff <pathlimiter>":
>
>  - the whole diff for a commit
>
>  - the path-limited diff
>
> and I think we'd likely want to have some way to select the output. 
> Probably with the path-limited diff being the default (that's what 
> "git-whatchanged" does), but perhaps with "--full-diff" showing the whole 
> commit diff (which is what "gitk" does).

Yes, but it turns out it is a bit tricky, given that the way
tree-diff.c is written it pretty much assumes the same pathspec
is used for diff operation during the lifetime of the process.

I think we should move (nr_paths, pathlens and paths) to struct
diff_options.  Two patches to follow shortly.

^ permalink raw reply

* [PATCH] tree-diff: do not assume we use only one pathspec
From: Junio C Hamano @ 2006-04-10 23:43 UTC (permalink / raw)
  To: git; +Cc: Linus Torvalds
In-Reply-To: <7vodz980az.fsf@assigned-by-dhcp.cox.net>

The way tree-diff was set up assumed we would use only one set
of pathspec during the entire life of the program.  Move the
pathspec related static variables out to diff_options structure
so that we can filter commits with one set of paths while show
the actual diffs using different set of paths.

I suspect this breaks blame.c, and makes "git log paths..." to
default to the --full-diff, the latter of which is dealt with
the next commit.

Signed-off-by: Junio C Hamano <junkio@cox.net>

---

 diff-tree.c |    2 +-
 diff.h      |    6 +++++-
 revision.c  |    2 +-
 tree-diff.c |   46 +++++++++++++++++++++++++---------------------
 4 files changed, 32 insertions(+), 24 deletions(-)

be7b412689f723a1008b40712d9f0b96773ad81a
diff --git a/diff-tree.c b/diff-tree.c
index 2a088d1..2b79dd0 100644
--- a/diff-tree.c
+++ b/diff-tree.c
@@ -120,7 +120,7 @@ int main(int argc, const char **argv)
 	if (opt->diffopt.output_format == DIFF_FORMAT_PATCH)
 		opt->diffopt.recursive = 1;
 
-	diff_tree_setup_paths(get_pathspec(prefix, argv));
+	diff_tree_setup_paths(get_pathspec(prefix, argv), opt);
 	diff_setup_done(&opt->diffopt);
 
 	switch (nr_sha1) {
diff --git a/diff.h b/diff.h
index a02ef28..cc7cc62 100644
--- a/diff.h
+++ b/diff.h
@@ -38,11 +38,15 @@ struct diff_options {
 	int setup;
 	int abbrev;
 
+	int nr_paths;
+	const char **paths;
+	int *pathlens;
 	change_fn_t change;
 	add_remove_fn_t add_remove;
 };
 
-extern void diff_tree_setup_paths(const char **paths);
+extern void diff_tree_setup_paths(const char **paths, struct diff_options *);
+extern void diff_tree_release_paths(struct diff_options *);
 extern int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
 		     const char *base, struct diff_options *opt);
 extern int diff_tree_sha1(const unsigned char *old, const unsigned char *new,
diff --git a/revision.c b/revision.c
index fe26562..634f9a5 100644
--- a/revision.c
+++ b/revision.c
@@ -707,7 +707,7 @@ int setup_revisions(int argc, const char
 		revs->limited = 1;
 
 	if (revs->prune_data) {
-		diff_tree_setup_paths(revs->prune_data);
+		diff_tree_setup_paths(revs->prune_data, &diff_opt);
 		revs->prune_fn = try_to_simplify_commit;
 	}
 
diff --git a/tree-diff.c b/tree-diff.c
index 701fbba..1cdf8aa 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -5,11 +5,6 @@ #include "cache.h"
 #include "diff.h"
 #include "tree.h"
 
-// What paths are we interested in?
-static int nr_paths = 0;
-static const char **paths = NULL;
-static int *pathlens = NULL;
-
 static char *malloc_base(const char *base, const char *path, int pathlen)
 {
 	int baselen = strlen(base);
@@ -72,14 +67,14 @@ static int compare_tree_entry(struct tre
 	return 0;
 }
 
-static int interesting(struct tree_desc *desc, const char *base)
+static int interesting(struct tree_desc *desc, const char *base, struct diff_options *opt)
 {
 	const char *path;
 	unsigned mode;
 	int i;
 	int baselen, pathlen;
 
-	if (!nr_paths)
+	if (!opt->nr_paths)
 		return 1;
 
 	(void)tree_entry_extract(desc, &path, &mode);
@@ -87,9 +82,9 @@ static int interesting(struct tree_desc 
 	pathlen = strlen(path);
 	baselen = strlen(base);
 
-	for (i=0; i < nr_paths; i++) {
-		const char *match = paths[i];
-		int matchlen = pathlens[i];
+	for (i=0; i < opt->nr_paths; i++) {
+		const char *match = opt->paths[i];
+		int matchlen = opt->pathlens[i];
 
 		if (baselen >= matchlen) {
 			/* If it doesn't match, move along... */
@@ -129,7 +124,7 @@ static int interesting(struct tree_desc 
 static void show_tree(struct diff_options *opt, const char *prefix, struct tree_desc *desc, const char *base)
 {
 	while (desc->size) {
-		if (interesting(desc, base))
+		if (interesting(desc, base, opt))
 			show_entry(opt, prefix, desc, base);
 		update_tree_entry(desc);
 	}
@@ -167,11 +162,11 @@ static int show_entry(struct diff_option
 int diff_tree(struct tree_desc *t1, struct tree_desc *t2, const char *base, struct diff_options *opt)
 {
 	while (t1->size | t2->size) {
-		if (nr_paths && t1->size && !interesting(t1, base)) {
+		if (opt->nr_paths && t1->size && !interesting(t1, base, opt)) {
 			update_tree_entry(t1);
 			continue;
 		}
-		if (nr_paths && t2->size && !interesting(t2, base)) {
+		if (opt->nr_paths && t2->size && !interesting(t2, base, opt)) {
 			update_tree_entry(t2);
 			continue;
 		}
@@ -229,19 +224,28 @@ static int count_paths(const char **path
 	return i;
 }
 
-void diff_tree_setup_paths(const char **p)
+void diff_tree_release_paths(struct diff_options *opt)
 {
+	free(opt->pathlens);
+}
+
+void diff_tree_setup_paths(const char **p, struct diff_options *opt)
+{
+	opt->nr_paths = 0;
+	opt->pathlens = NULL;
+	opt->paths = NULL;
+
 	if (p) {
 		int i;
 
-		paths = p;
-		nr_paths = count_paths(paths);
-		if (nr_paths == 0) {
-			pathlens = NULL;
+		opt->paths = p;
+		opt->nr_paths = count_paths(p);
+		if (opt->nr_paths == 0) {
+			opt->pathlens = NULL;
 			return;
 		}
-		pathlens = xmalloc(nr_paths * sizeof(int));
-		for (i=0; i<nr_paths; i++)
-			pathlens[i] = strlen(paths[i]);
+		opt->pathlens = xmalloc(opt->nr_paths * sizeof(int));
+		for (i=0; i < opt->nr_paths; i++)
+			opt->pathlens[i] = strlen(p[i]);
 	}
 }
-- 
1.3.0.rc3.g910a

^ permalink raw reply related

* [PATCH] git log --full-diff
From: Junio C Hamano @ 2006-04-10 23:44 UTC (permalink / raw)
  To: git; +Cc: Linus Torvalds
In-Reply-To: <7vodz980az.fsf@assigned-by-dhcp.cox.net>

Without this flag, "git log -p paths..." shows commits that
touch the specified paths, and diffs about the same specified
paths.  With this, the full diff is shown for commits that touch
the specified paths.

Signed-off-by: Junio C Hamano <junkio@cox.net>

---

 git.c |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)

fd3ed3d9667e034b90aad02b4f9e5efcc61f1ce3
diff --git a/git.c b/git.c
index 8776088..ad896da 100644
--- a/git.c
+++ b/git.c
@@ -288,7 +288,9 @@ static int cmd_log(int argc, const char 
 	int abbrev_commit = 0;
 	const char *commit_prefix = "commit ";
 	struct log_tree_opt opt;
+	int shown = 0;
 	int do_diff = 0;
+	int full_diff = 0;
 
 	init_log_tree_opt(&opt);
 	argc = setup_revisions(argc, argv, &rev, "HEAD");
@@ -315,6 +317,10 @@ static int cmd_log(int argc, const char 
 			else if (40 < abbrev)
 				abbrev = 40;
 		}
+		else if (!strcmp(arg, "--full-diff")) {
+			do_diff = 1;
+			full_diff = 1;
+		}
 		else {
 			int cnt = log_tree_opt_parse(&opt, argv+1, argc-1);
 			if (0 < cnt) {
@@ -328,6 +334,7 @@ static int cmd_log(int argc, const char 
 
 		argc--; argv++;
 	}
+
 	if (do_diff) {
 		opt.diffopt.abbrev = abbrev;
 		opt.verbose_header = 0;
@@ -339,12 +346,16 @@ static int cmd_log(int argc, const char 
 			opt.diffopt.output_format = DIFF_FORMAT_PATCH;
 		if (opt.diffopt.output_format == DIFF_FORMAT_PATCH)
 			opt.diffopt.recursive = 1;
+		if (!full_diff && rev.prune_data)
+			diff_tree_setup_paths(rev.prune_data, &opt.diffopt);
 		diff_setup_done(&opt.diffopt);
 	}
 
 	prepare_revision_walk(&rev);
 	setup_pager();
 	while ((commit = get_revision(&rev)) != NULL) {
+		if (commit_format != CMIT_FMT_ONELINE && shown)
+			putchar('\n');
 		fputs(commit_prefix, stdout);
 		if (abbrev_commit && abbrev)
 			fputs(find_unique_abbrev(commit->object.sha1, abbrev),
@@ -377,9 +388,9 @@ static int cmd_log(int argc, const char 
 		pretty_print_commit(commit_format, commit, ~0, buf,
 				    LOGSIZE, abbrev);
 		printf("%s\n", buf);
-
 		if (do_diff)
 			log_tree_commit(&opt, commit);
+		shown = 1;
 	}
 	free(buf);
 	return 0;
-- 
1.3.0.rc3.g910a

^ permalink raw reply related

* Re: [PATCH] git log [diff-tree options]...
From: Linus Torvalds @ 2006-04-10 23:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vodz980az.fsf@assigned-by-dhcp.cox.net>



On Mon, 10 Apr 2006, Junio C Hamano wrote:
> >
> > and I think we'd likely want to have some way to select the output. 
> > Probably with the path-limited diff being the default (that's what 
> > "git-whatchanged" does), but perhaps with "--full-diff" showing the whole 
> > commit diff (which is what "gitk" does).
> 
> Yes, but it turns out it is a bit tricky, given that the way
> tree-diff.c is written it pretty much assumes the same pathspec
> is used for diff operation during the lifetime of the process.

Hmm.. The hacky approach would seem to be to just call 
"diff_tree_setup_paths()" just before doing the diff (and re-set it 
afterwards). That should work, I don't think we keep any real state 
around.

> I think we should move (nr_paths, pathlens and paths) to struct
> diff_options.  Two patches to follow shortly.

That certainly seems to be the cleanest option. Not just because it should 
solve this problem, but because it's also what looks most natural anyway 
(and matches the "struct revs" approach).

		Linus

^ permalink raw reply

* [PATCH] Support printing diffs in the raw and patch format at once
From: Petr Baudis @ 2006-04-10 23:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

This is required when using cg-Xfollowrenames as a backend for
a tool which wants to show the end-user patches; parsing patches
properly is a nightmare, so we need to receive the diff in the
raw diff format from git-diff-tree, while also receiving the diff
in the patch format for further processing by the frontend.

This patch adds a -P option which is similar to the -p option
and achieves this. First the raw diff format is printed, then
a line terminator and then the patch.

It is fairly simple and should be straightforward, so it would be
great if this could yet make it to 1.3.0, or cg-log patch output
would have to stay with broken rename detection until the next Git
major release. :-(

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 Documentation/diff-options.txt |    3 +++
 combine-diff.c                 |   32 ++++++++++++++++++++++++++------
 diff-files.c                   |    2 +-
 diff-index.c                   |    2 +-
 diff-stages.c                  |    2 +-
 diff-tree.c                    |    4 ++--
 diff.c                         |   29 +++++++++++++++++++++--------
 diff.h                         |    3 ++-
 8 files changed, 57 insertions(+), 20 deletions(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index ec6811c..67c8867 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -4,6 +4,9 @@
 -u::
 	Synonym for "-p".
 
+-P::
+	Generate patch but keep also the default raw diff output.
+
 -z::
 	\0 line termination on output
 
diff --git a/combine-diff.c b/combine-diff.c
index eb0d757..65e110e 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -816,10 +816,33 @@ int show_combined_diff(struct combine_di
 
 	default:
 	case DIFF_FORMAT_PATCH:
+	case DIFF_FORMAT_RAW_PATCH:
 		return show_patch_diff(p, num_parent, dense, header, opt);
 	}
 }
 
+const char *show_combined_diffs(struct combine_diff_path *paths,
+                                int num_parent, int dense,
+				const char *header, struct diff_options *opt)
+{
+	struct combine_diff_path *p;
+
+	if (opt->output_format == DIFF_FORMAT_RAW_PATCH) {
+		opt->output_format = DIFF_FORMAT_RAW;
+		header = show_combined_diffs(paths, num_parent, dense,
+		                             header, opt);
+		putchar(opt->line_termination);
+		opt->output_format = DIFF_FORMAT_RAW_PATCH;
+	}
+
+	for (p = paths; p; p = p->next) {
+		if (show_combined_diff(p, num_parent, dense,
+				       header, opt))
+			header = NULL;
+	}
+	return header;
+}
+
 const char *diff_tree_combined_merge(const unsigned char *sha1,
 			     const char *header, int dense,
 			     struct diff_options *opt)
@@ -849,7 +872,7 @@ const char *diff_tree_combined_merge(con
 			       &diffopts);
 		diffcore_std(&diffopts);
 		paths = intersect_paths(paths, i, num_parent);
-		diff_flush(&diffopts);
+		diff_flush(&diffopts, 1);
 	}
 
 	/* find out surviving paths */
@@ -858,11 +881,8 @@ const char *diff_tree_combined_merge(con
 			num_paths++;
 	}
 	if (num_paths) {
-		for (p = paths; p; p = p->next) {
-			if (show_combined_diff(p, num_parent, dense,
-					       header, opt))
-				header = NULL;
-		}
+		header = show_combined_diffs(paths, num_parent, dense,
+		                             header, opt);
 	}
 
 	/* Clean things up */
diff --git a/diff-files.c b/diff-files.c
index 3e7f5f1..4e2e5cb 100644
--- a/diff-files.c
+++ b/diff-files.c
@@ -208,6 +208,6 @@ int main(int argc, const char **argv)
 			      ce->name);
 	}
 	diffcore_std(&diff_options);
-	diff_flush(&diff_options);
+	diff_flush(&diff_options, 1);
 	return 0;
 }
diff --git a/diff-index.c b/diff-index.c
index e376d65..dade539 100644
--- a/diff-index.c
+++ b/diff-index.c
@@ -248,6 +248,6 @@ int main(int argc, const char **argv)
 	ret = diff_cache(active_cache, active_nr, pathspec);
 
 	diffcore_std(&diff_options);
-	diff_flush(&diff_options);
+	diff_flush(&diff_options, 1);
 	return ret;
 }
diff --git a/diff-stages.c b/diff-stages.c
index dcd20e7..fa146a9 100644
--- a/diff-stages.c
+++ b/diff-stages.c
@@ -99,6 +99,6 @@ int main(int ac, const char **av)
 
 	diff_stages(stage1, stage2, pathspec);
 	diffcore_std(&diff_options);
-	diff_flush(&diff_options);
+	diff_flush(&diff_options, 1);
 	return 0;
 }
diff --git a/diff-tree.c b/diff-tree.c
index d1265d7..5327df8 100644
--- a/diff-tree.c
+++ b/diff-tree.c
@@ -23,7 +23,7 @@ static int call_diff_flush(void)
 	if (diff_queue_is_empty()) {
 		int saved_fmt = diff_options.output_format;
 		diff_options.output_format = DIFF_FORMAT_NO_OUTPUT;
-		diff_flush(&diff_options);
+		diff_flush(&diff_options, 1);
 		diff_options.output_format = saved_fmt;
 		return 0;
 	}
@@ -32,7 +32,7 @@ static int call_diff_flush(void)
 			printf("%s%c", header, diff_options.line_termination);
 		header = NULL;
 	}
-	diff_flush(&diff_options);
+	diff_flush(&diff_options, 1);
 	return 1;
 }
 
diff --git a/diff.c b/diff.c
index 2fa285a..f57d82e 100644
--- a/diff.c
+++ b/diff.c
@@ -862,6 +862,8 @@ int diff_opt_parse(struct diff_options *
 	const char *arg = av[0];
 	if (!strcmp(arg, "-p") || !strcmp(arg, "-u"))
 		options->output_format = DIFF_FORMAT_PATCH;
+	else if (!strcmp(arg, "-P"))
+		options->output_format = DIFF_FORMAT_RAW_PATCH;
 	else if (!strcmp(arg, "-z"))
 		options->line_termination = 0;
 	else if (!strncmp(arg, "-l", 2))
@@ -1270,7 +1272,7 @@ static void diff_resolve_rename_copy(voi
 	diff_debug_queue("resolve-rename-copy done", q);
 }
 
-void diff_flush(struct diff_options *options)
+void diff_flush(struct diff_options *options, int shall_free)
 {
 	struct diff_queue_struct *q = &diff_queued_diff;
 	int i;
@@ -1278,6 +1280,13 @@ void diff_flush(struct diff_options *opt
 	int diff_output_format = options->output_format;
 	int line_termination = options->line_termination;
 
+	if (diff_output_format == DIFF_FORMAT_RAW_PATCH) {
+		options->output_format = DIFF_FORMAT_RAW;
+		diff_flush(options, 0);
+		putchar(options->line_termination);
+		options->output_format = DIFF_FORMAT_RAW_PATCH;
+	}
+
 	if (!line_termination)
 		inter_name_termination = 0;
 
@@ -1292,15 +1301,16 @@ void diff_flush(struct diff_options *opt
 			break;
 		default:
 			switch (diff_output_format) {
-			case DIFF_FORMAT_PATCH:
-				diff_flush_patch(p, options);
-				break;
 			case DIFF_FORMAT_RAW:
 			case DIFF_FORMAT_NAME_STATUS:
 				diff_flush_raw(p, line_termination,
 					       inter_name_termination,
 					       options);
 				break;
+			case DIFF_FORMAT_PATCH:
+			case DIFF_FORMAT_RAW_PATCH:
+				diff_flush_patch(p, options);
+				break;
 			case DIFF_FORMAT_NAME:
 				diff_flush_name(p,
 						inter_name_termination,
@@ -1310,11 +1320,14 @@ void diff_flush(struct diff_options *opt
 				break;
 			}
 		}
-		diff_free_filepair(p);
+		if (shall_free)
+			diff_free_filepair(p);
 	}
-	free(q->queue);
-	q->queue = NULL;
-	q->nr = q->alloc = 0;
+	if (shall_free) {
+		free(q->queue);
+		q->queue = NULL;
+		q->nr = q->alloc = 0;
+	}
 }
 
 static void diffcore_apply_filter(const char *filter)
diff --git a/diff.h b/diff.h
index a02ef28..a2dacac 100644
--- a/diff.h
+++ b/diff.h
@@ -135,8 +135,9 @@ #define DIFF_FORMAT_PATCH	2
 #define DIFF_FORMAT_NO_OUTPUT	3
 #define DIFF_FORMAT_NAME	4
 #define DIFF_FORMAT_NAME_STATUS	5
+#define DIFF_FORMAT_RAW_PATCH	6
 
-extern void diff_flush(struct diff_options*);
+extern void diff_flush(struct diff_options *, int shall_free);
 
 /* diff-raw status letters */
 #define DIFF_STATUS_ADDED		'A'

^ permalink raw reply related

* [PATCH] diff-* --with-raw
From: Junio C Hamano @ 2006-04-11  0:00 UTC (permalink / raw)
  To: git; +Cc: pasky

Pasky wanted to have an option to get both diff-raw output and
diff-patch output.  This implements "git-diff-* --with-raw"
(which obviously implies -p as well) to do so.

Because all the necessary information is already on extended
header lines such as "index xxxxxx..yyyyyy" and "rename from"
lines, this is not strictly necessary, but if it helps
Porcelains...

--
diff --git a/diff.c b/diff.c
index 2fa285a..1a9fdab 100644
--- a/diff.c
+++ b/diff.c
@@ -862,6 +862,10 @@ int diff_opt_parse(struct diff_options *
 	const char *arg = av[0];
 	if (!strcmp(arg, "-p") || !strcmp(arg, "-u"))
 		options->output_format = DIFF_FORMAT_PATCH;
+	else if (!strcmp(arg, "--with-raw")) {
+		options->output_format = DIFF_FORMAT_PATCH;
+		options->with_raw = 1;
+	}
 	else if (!strcmp(arg, "-z"))
 		options->line_termination = 0;
 	else if (!strncmp(arg, "-l", 2))
@@ -1293,6 +1297,10 @@ void diff_flush(struct diff_options *opt
 		default:
 			switch (diff_output_format) {
 			case DIFF_FORMAT_PATCH:
+				if (options->with_raw)
+					diff_flush_raw(p, line_termination,
+						       inter_name_termination,
+						       options);
 				diff_flush_patch(p, options);
 				break;
 			case DIFF_FORMAT_RAW:
diff --git a/diff.h b/diff.h
index a02ef28..07b153b 100644
--- a/diff.h
+++ b/diff.h
@@ -24,6 +24,7 @@ struct diff_options {
 	const char *orderfile;
 	const char *pickaxe;
 	unsigned recursive:1,
+		 with_raw:1,
 		 tree_in_recursive:1,
 		 full_index:1;
 	int break_opt;

^ permalink raw reply related

* Re: [PATCH] git log [diff-tree options]...
From: Randal L. Schwartz @ 2006-04-11  0:04 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Linus Torvalds, Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.63.0604092312340.29136@wbgn013.biozentrum.uni-wuerzburg.de>

>>>>> "Johannes" == Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Johannes> Hi,
Johannes> On Sun, 9 Apr 2006, Linus Torvalds wrote:

>> - keep it - for historical reasons - as a internal shorthand, and just 
>> turn it into "git log --diff -cc"

Johannes> It is "git log --cc", right? And BTW, I was burnt by the difference of 
Johannes> "git-log" and "git log" this time. "git-log" does not understand "--cc". 
Johannes> Could we kill "git-log", please?

Wait.  Why is there a git log and a git-log?  Shouldn't those
be *absolutely* identical?  Or have we now finally diverged, violating
rules that were established earlier?

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!

^ permalink raw reply

* Re: [PATCH] diff-* --with-raw
From: Junio C Hamano @ 2006-04-11  0:06 UTC (permalink / raw)
  To: git
In-Reply-To: <7v7j5x7zh3.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> writes:

> Pasky wanted to have an option to get both diff-raw output and
> diff-patch output.  This implements "git-diff-* --with-raw"
> (which obviously implies -p as well) to do so.
>
> Because all the necessary information is already on extended
> header lines such as "index xxxxxx..yyyyyy" and "rename from"
> lines, this is not strictly necessary, but if it helps
> Porcelains...

And this alternative gives raw upfront followed by patch.  It
was unclear which one Pasky wanted, so...

--
diff --git a/diff.c b/diff.c
index 2fa285a..12924f2 100644
--- a/diff.c
+++ b/diff.c
@@ -862,6 +862,10 @@ int diff_opt_parse(struct diff_options *
 	const char *arg = av[0];
 	if (!strcmp(arg, "-p") || !strcmp(arg, "-u"))
 		options->output_format = DIFF_FORMAT_PATCH;
+	else if (!strcmp(arg, "--with-raw")) {
+		options->output_format = DIFF_FORMAT_PATCH;
+		options->with_raw = 1;
+	}
 	else if (!strcmp(arg, "-z"))
 		options->line_termination = 0;
 	else if (!strncmp(arg, "-l", 2))
@@ -1270,46 +1274,58 @@ static void diff_resolve_rename_copy(voi
 	diff_debug_queue("resolve-rename-copy done", q);
 }
 
-void diff_flush(struct diff_options *options)
+static void flush_one_pair(struct diff_filepair *p,
+			   int diff_output_format,
+			   struct diff_options *options)
 {
-	struct diff_queue_struct *q = &diff_queued_diff;
-	int i;
 	int inter_name_termination = '\t';
-	int diff_output_format = options->output_format;
 	int line_termination = options->line_termination;
-
 	if (!line_termination)
 		inter_name_termination = 0;
 
-	for (i = 0; i < q->nr; i++) {
-		struct diff_filepair *p = q->queue[i];
-
-		switch (p->status) {
-		case DIFF_STATUS_UNKNOWN:
+	switch (p->status) {
+	case DIFF_STATUS_UNKNOWN:
+		break;
+	case 0:
+		die("internal error in diff-resolve-rename-copy");
+		break;
+	default:
+		switch (diff_output_format) {
+		case DIFF_FORMAT_PATCH:
+			diff_flush_patch(p, options);
 			break;
-		case 0:
-			die("internal error in diff-resolve-rename-copy");
+		case DIFF_FORMAT_RAW:
+		case DIFF_FORMAT_NAME_STATUS:
+			diff_flush_raw(p, line_termination,
+				       inter_name_termination,
+				       options);
 			break;
-		default:
-			switch (diff_output_format) {
-			case DIFF_FORMAT_PATCH:
-				diff_flush_patch(p, options);
-				break;
-			case DIFF_FORMAT_RAW:
-			case DIFF_FORMAT_NAME_STATUS:
-				diff_flush_raw(p, line_termination,
-					       inter_name_termination,
-					       options);
-				break;
-			case DIFF_FORMAT_NAME:
-				diff_flush_name(p,
-						inter_name_termination,
-						line_termination);
-				break;
-			case DIFF_FORMAT_NO_OUTPUT:
-				break;
-			}
+		case DIFF_FORMAT_NAME:
+			diff_flush_name(p,
+					inter_name_termination,
+					line_termination);
+			break;
+		case DIFF_FORMAT_NO_OUTPUT:
+			break;
 		}
+	}
+}
+
+void diff_flush(struct diff_options *options)
+{
+	struct diff_queue_struct *q = &diff_queued_diff;
+	int i;
+	int diff_output_format = options->output_format;
+
+	if (options->with_raw) {
+		for (i = 0; i < q->nr; i++) {
+			struct diff_filepair *p = q->queue[i];
+			flush_one_pair(p, DIFF_FORMAT_RAW, options);
+		}
+	}
+	for (i = 0; i < q->nr; i++) {
+		struct diff_filepair *p = q->queue[i];
+		flush_one_pair(p, diff_output_format, options);
 		diff_free_filepair(p);
 	}
 	free(q->queue);
diff --git a/diff.h b/diff.h
index a02ef28..07b153b 100644
--- a/diff.h
+++ b/diff.h
@@ -24,6 +24,7 @@ struct diff_options {
 	const char *orderfile;
 	const char *pickaxe;
 	unsigned recursive:1,
+		 with_raw:1,
 		 tree_in_recursive:1,
 		 full_index:1;
 	int break_opt;

^ permalink raw reply related

* Re: [PATCH] git log [diff-tree options]...
From: Junio C Hamano @ 2006-04-11  0:12 UTC (permalink / raw)
  To: Randal L. Schwartz; +Cc: git
In-Reply-To: <86k69xasg0.fsf@blue.stonehenge.com>

merlyn@stonehenge.com (Randal L. Schwartz) writes:

> Wait.  Why is there a git log and a git-log?  Shouldn't those
> be *absolutely* identical?  Or have we now finally diverged, violating
> rules that were established earlier?

What rule ;-)?

I kept "git-log.sh" for two reasons.  (1) to see how long it
takes people to notice, and (2) to have a handy way to verify
potential regressions.

There is no reason for them to be *absolutely* identical -- if
the git.c embedded one turns out to be usable, useful and even
superiour, git-log.sh based one should be retired, and will be
made again as synonyms, perhaps like this:

--
diff --git a/Makefile b/Makefile
index a979205..5239526 100644
--- a/Makefile
+++ b/Makefile
@@ -116,7 +116,7 @@ SCRIPT_SH = \
 	git-add.sh git-bisect.sh git-branch.sh git-checkout.sh \
 	git-cherry.sh git-clean.sh git-clone.sh git-commit.sh \
 	git-count-objects.sh git-diff.sh git-fetch.sh \
-	git-format-patch.sh git-log.sh git-ls-remote.sh \
+	git-format-patch.sh git-ls-remote.sh \
 	git-merge-one-file.sh git-parse-remote.sh \
 	git-prune.sh git-pull.sh git-push.sh git-rebase.sh \
 	git-repack.sh git-request-pull.sh git-reset.sh \
@@ -167,6 +167,8 @@ PROGRAMS = \
 	git-name-rev$X git-pack-redundant$X git-repo-config$X git-var$X \
 	git-describe$X git-merge-tree$X git-blame$X git-imap-send$X
 
+BUILT_INS = git-log$X
+
 # what 'all' will build and 'install' will install, in gitexecdir
 ALL_PROGRAMS = $(PROGRAMS) $(SIMPLE_PROGRAMS) $(SCRIPTS)
 
@@ -448,7 +450,7 @@ LIB_OBJS += $(COMPAT_OBJS)
 export prefix TAR INSTALL DESTDIR SHELL_PATH template_dir
 ### Build rules
 
-all: $(ALL_PROGRAMS) git$X gitk
+all: $(ALL_PROGRAMS) git$X $(BUILT_INS) gitk
 
 all:
 	$(MAKE) -C templates
@@ -460,6 +462,9 @@ git$X: git.c common-cmds.h $(GITLIBS)
 	$(CC) -DGIT_VERSION='"$(GIT_VERSION)"' \
 		$(ALL_CFLAGS) -o $@ $(filter %.c,$^) \
 		$(ALL_LDFLAGS) $(LIBS)
+
+$(BUILT_INS): git$X
+	rm -f $@ && ln git$X $@
 
 common-cmds.h: Documentation/git-*.txt
 	./generate-cmdlist.sh > $@
@@ -642,7 +647,7 @@ ### Cleaning rules
 
 clean:
 	rm -f *.o mozilla-sha1/*.o arm/*.o ppc/*.o compat/*.o xdiff/*.o \
-		$(LIB_FILE) $(XDIFF_LIB)
+		$(LIB_FILE) $(XDIFF_LIB) $(BUILT_INS)
 	rm -f $(ALL_PROGRAMS) git$X
 	rm -f *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h TAGS tags
 	rm -rf $(GIT_TARNAME)

^ permalink raw reply related

* Re: [PATCH] diff-* --with-raw
From: Petr Baudis @ 2006-04-11  0:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v3bgl7z80.fsf@assigned-by-dhcp.cox.net>

Dear diary, on Tue, Apr 11, 2006 at 02:06:07AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> Junio C Hamano <junkio@cox.net> writes:
> 
> > Pasky wanted to have an option to get both diff-raw output and
> > diff-patch output.  This implements "git-diff-* --with-raw"
> > (which obviously implies -p as well) to do so.
> >
> > Because all the necessary information is already on extended
> > header lines such as "index xxxxxx..yyyyyy" and "rename from"
> > lines, this is not strictly necessary, but if it helps
> > Porcelains...
> 
> And this alternative gives raw upfront followed by patch.  It
> was unclear which one Pasky wanted, so...

Technically, I don't really care, I can parse both. :-) But I prefer
this format since then people can use it even when visually inspecting
the diff output, as a kind of quick patch summary followed with the
patch itself. It might be nice to separate the patch by another newline,
like my patch does (but again, I can parse both).

Your patch is nicer, but the --with-raw option name is strange -
git-diff-* output is by default the raw format, and with the --with-raw
option you tell it to furthermore include the raw format... sounds
wrong, doesn't it? ;-) I'd call it --patch-with-raw or -P.

Also, it would be nice to handle the -c case as well. Not strictly
necessary for cg-log right now, but other cg-Xfollowrenames users might
want to have that for merges... (Potentially, this might break renames
detection but the case is really obscure.)

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Right now I am having amnesia and deja-vu at the same time.  I think
I have forgotten this before.

^ permalink raw reply

* Re: [PATCH] git log [diff-tree options]...
From: Petr Baudis @ 2006-04-11  0:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Randal L. Schwartz, git
In-Reply-To: <7vveth6kdg.fsf@assigned-by-dhcp.cox.net>

Dear diary, on Tue, Apr 11, 2006 at 02:12:11AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> There is no reason for them to be *absolutely* identical

One is that people expect them to be. It's been implied that "git xyzzy"
is functionally fully equivalent to "git-xyzzy" for too long to silently
break this now.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Right now I am having amnesia and deja-vu at the same time.  I think
I have forgotten this before.

^ permalink raw reply

* Re: [PATCH] diff-* --with-raw
From: Junio C Hamano @ 2006-04-11  0:35 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <20060411002311.GW27689@pasky.or.cz>

Petr Baudis <pasky@suse.cz> writes:

> git-diff-* output is by default the raw format, and with the --with-raw
> option you tell it to furthermore include the raw format... sounds
> wrong, doesn't it? ;-) I'd call it --patch-with-raw or -P.

Since --raw-with-raw would be oximoron, I would say --with-raw
would naturally mean --patch-with-raw, but that's fine.

> Also, it would be nice to handle the -c case as well. Not strictly
> necessary for cg-log right now, but other cg-Xfollowrenames users might
> want to have that for merges... (Potentially, this might break renames
> detection but the case is really obscure.)

This would cover that request, comes on top of the previous one.

--

diff --git a/combine-diff.c b/combine-diff.c
index eb0d757..748dc30 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -832,6 +832,7 @@ const char *diff_tree_combined_merge(con
 
 	diffopts = *opt;
 	diffopts.output_format = DIFF_FORMAT_NO_OUTPUT;
+	diffopts.with_raw = 0;
 	diffopts.recursive = 1;
 
 	/* count parents */
@@ -858,6 +859,15 @@ const char *diff_tree_combined_merge(con
 			num_paths++;
 	}
 	if (num_paths) {
+		if (opt->with_raw) {
+			opt->output_format = DIFF_FORMAT_RAW;
+			for (p = paths; p; p = p->next) {
+				if (show_combined_diff(p, num_parent, dense,
+						       header, opt))
+					header = NULL;
+			}
+			opt->output_format = DIFF_FORMAT_PATCH;
+		}
 		for (p = paths; p; p = p->next) {
 			if (show_combined_diff(p, num_parent, dense,
 					       header, opt))

^ permalink raw reply related

* Re: [PATCH] git log [diff-tree options]...
From: Randal L. Schwartz @ 2006-04-11  0:50 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Junio C Hamano, git
In-Reply-To: <20060411003126.GX27689@pasky.or.cz>

>>>>> "Petr" == Petr Baudis <pasky@suse.cz> writes:

Petr> Dear diary, on Tue, Apr 11, 2006 at 02:12:11AM CEST, I got a letter
Petr> where Junio C Hamano <junkio@cox.net> said that...
>> There is no reason for them to be *absolutely* identical

Petr> One is that people expect them to be. It's been implied that "git xyzzy"
Petr> is functionally fully equivalent to "git-xyzzy" for too long to silently
Petr> break this now.

Yes, this is what I was referencing.  I would *never* have considered
that git-log and "git log" be different things now that it's been firmly
engrained that they are the same.  Please don't violate the model.

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!

^ permalink raw reply

* What's in git.git
From: Junio C Hamano @ 2006-04-11  4:40 UTC (permalink / raw)
  To: git

No real 1.3.0 yet...

* The 'master' branch has these since the last announcement.
  All of them are fixes and clean-ups (except --parents stuff
  from Linus which seems safe enough).

Junio C Hamano:
      git-log: match rev-list --abbrev and --abbrev-commit
      diff: fix output of total-rewrite diff.
      diffcore-rename: fix merging back a broken pair.
      Retire diffcore-pathspec.
      Retire git-log.sh
      Retire git-log.sh (take#2)

Linus Torvalds:
      Make "--parents" logs also be incremental

Marco Roeland:
      xdiff/xdiffi.c: fix warnings about possibly uninitialized variables

Petr Baudis:
      Improve the git-diff-tree -c/-cc documentation


* The 'next' branch, in addition, has these.

  - diff-* --patch-with-raw
  - Implement limited context matching in git-apply (Eric W. Biederman)

  - log-tree: separate major part of diff-tree.
  - git log [diff-tree options]...

I should not be moving anything new from "next" to "master"
before 1.3.0, but I am inclined to include the --patch-with-raw
change, which Cogito wants.  Also apply -Cn by Eric seems safe
enough; as long as you do not ask for it, it keeps the existing
behaviour.  The initial part of "git log with diff-tree options"
seems safe enough so it is also in 'next'.

I have the rest of "git log with diff-tree options", along with
its fallouts, which turned out to be more than what I wanted to
see, in "pu".

^ permalink raw reply

* [PATCH] Rename --with-raw to --patch-with-raw and document it
From: Petr Baudis @ 2006-04-11 11:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr7456jb4.fsf@assigned-by-dhcp.cox.net>

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 Documentation/diff-options.txt |    3 +++
 diff.c                         |    2 +-
 diff.h                         |    2 ++
 3 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index ec6811c..338014c 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -4,6 +4,9 @@
 -u::
 	Synonym for "-p".
 
+--patch-with-raw::
+	Generate patch but keep also the default raw diff output.
+
 -z::
 	\0 line termination on output
 
diff --git a/diff.c b/diff.c
index 12924f2..00c79aa 100644
--- a/diff.c
+++ b/diff.c
@@ -862,7 +862,7 @@ int diff_opt_parse(struct diff_options *
 	const char *arg = av[0];
 	if (!strcmp(arg, "-p") || !strcmp(arg, "-u"))
 		options->output_format = DIFF_FORMAT_PATCH;
-	else if (!strcmp(arg, "--with-raw")) {
+	else if (!strcmp(arg, "--patch-with-raw")) {
 		options->output_format = DIFF_FORMAT_PATCH;
 		options->with_raw = 1;
 	}
diff --git a/diff.h b/diff.h
index 07b153b..c5372b9 100644
--- a/diff.h
+++ b/diff.h
@@ -113,6 +113,8 @@ #define COMMON_DIFF_OPTIONS_HELP \
 "  -z            output diff-raw with lines terminated with NUL.\n" \
 "  -p            output patch format.\n" \
 "  -u            synonym for -p.\n" \
+"  --patch-with-raw\n" \
+"                output both a patch and the diff-raw format.\n" \
 "  --name-only   show only names of changed files.\n" \
 "  --name-status show names and status of changed files.\n" \
 "  --full-index  show full object name on index lines.\n" \

^ permalink raw reply related

* [PATCH] Separate the raw diff and patch with a newline
From: Petr Baudis @ 2006-04-11 11:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr7456jb4.fsf@assigned-by-dhcp.cox.net>

More friendly for human reading I believe, and possibly friendlier to some
parsers (although only by an epsilon).

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 combine-diff.c |    1 +
 diff.c         |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/combine-diff.c b/combine-diff.c
index 748dc30..0e25788 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -867,6 +867,7 @@ const char *diff_tree_combined_merge(con
 					header = NULL;
 			}
 			opt->output_format = DIFF_FORMAT_PATCH;
+			putchar(opt->line_termination);
 		}
 		for (p = paths; p; p = p->next) {
 			if (show_combined_diff(p, num_parent, dense,
diff --git a/diff.c b/diff.c
index 00c79aa..86e4251 100644
--- a/diff.c
+++ b/diff.c
@@ -1322,6 +1322,7 @@ void diff_flush(struct diff_options *opt
 			struct diff_filepair *p = q->queue[i];
 			flush_one_pair(p, DIFF_FORMAT_RAW, options);
 		}
+		putchar(options->line_termination);
 	}
 	for (i = 0; i < q->nr; i++) {
 		struct diff_filepair *p = q->queue[i];

^ permalink raw reply related

* Re: What's in git.git
From: Linus Torvalds @ 2006-04-11 13:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v3bgk7mhy.fsf@assigned-by-dhcp.cox.net>



On Mon, 10 Apr 2006, Junio C Hamano wrote:
>
>       Retire git-log.sh
>       Retire git-log.sh (take#2)

I think you need a (take#3).

This creates "git-log" as a link to "git", but does so at _build_ time, 
not install time.

Which means that when it actually gets installed, it gets installed as a 
copy of that link, and you get two different executables instead of one 
single executable that is just linked to two names:

  [torvalds@g5 git]$ ls -li /home/torvalds/bin/git ~/bin/git-log
  67272830 -rwxr-xr-x 1 torvalds torvalds 333613 Apr 11 06:45 /home/torvalds/bin/git
  67272781 -rwxr-xr-x 1 torvalds torvalds 333613 Apr 11 06:45 /home/torvalds/bin/git-log

so it _works_, but it wastes 300kB+ for each "builtin". Which was kind of 
against the whole point.

I think the builtins should be a install-time only issue. 

		Linus

^ permalink raw reply

* Re: What's in git.git
From: Petr Baudis @ 2006-04-11 15:55 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0604110645060.3722@g5.osdl.org>

Dear diary, on Tue, Apr 11, 2006 at 03:50:14PM CEST, I got a letter
where Linus Torvalds <torvalds@osdl.org> said that...
> I think the builtins should be a install-time only issue. 

I don't care about git-log in particular since I don't use it, but I use
development Git versions without actually installing them and if it's
not a huge hurdle, it would be nice to keep this possible.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Right now I am having amnesia and deja-vu at the same time.  I think
I have forgotten this before.

^ permalink raw reply

* Build fixes for Solaris 9
From: Dennis Stosberg @ 2006-04-11 16:37 UTC (permalink / raw)
  To: git

Hello,

The attached patch allows git to be built on Solaris 9, which does
not have setenv() and unsetenv().

Also, Solaris needs strings.h for index().  A few days ago there was
a thread about this on the list and the result was that strchr() is
more portable than index() and is used everywhere in git anyway.  So
instead of including strings.h, this patch replaces all remaining
calls to index() by strchr().

Regards,
Dennis


diff --git a/Makefile b/Makefile
index c0409f3..ff645d8 100644
--- a/Makefile
+++ b/Makefile
@@ -248,6 +248,10 @@ ifeq ($(uname_S),SunOS)
 		NO_UNSETENV = YesPlease
 		NO_SETENV = YesPlease
 	endif
+	ifeq ($(uname_R),5.9)
+		NO_UNSETENV = YesPlease
+		NO_SETENV = YesPlease
+	endif
 	INSTALL = ginstall
 	TAR = gtar
 	ALL_CFLAGS += -D__EXTENSIONS__
diff --git a/http-fetch.c b/http-fetch.c
index 71a7daf..861644b 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -597,7 +597,7 @@ static void process_alternates_response(
 				newalt->packs = NULL;
 				path = strstr(target, "//");
 				if (path) {
-					path = index(path+2, '/');
+					path = strchr(path+2, '/');
 					if (path)
 						newalt->path_len = strlen(path);
 				}
@@ -678,7 +678,7 @@ static void
 xml_start_tag(void *userData, const char *name, const char **atts)
 {
 	struct xml_ctx *ctx = (struct xml_ctx *)userData;
-	const char *c = index(name, ':');
+	const char *c = strchr(name, ':');
 	int new_len;
 
 	if (c == NULL)
@@ -707,7 +707,7 @@ static void
 xml_end_tag(void *userData, const char *name)
 {
 	struct xml_ctx *ctx = (struct xml_ctx *)userData;
-	const char *c = index(name, ':');
+	const char *c = strchr(name, ':');
 	char *ep;
 
 	ctx->userFunc(ctx, 1);
@@ -1261,7 +1261,7 @@ int main(int argc, char **argv)
 	alt->next = NULL;
 	path = strstr(url, "//");
 	if (path) {
-		path = index(path+2, '/');
+		path = strchr(path+2, '/');
 		if (path)
 			alt->path_len = strlen(path);
 	}
diff --git a/http-push.c b/http-push.c
index 57cefde..994ee90 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1211,7 +1211,7 @@ static void
 xml_start_tag(void *userData, const char *name, const char **atts)
 {
 	struct xml_ctx *ctx = (struct xml_ctx *)userData;
-	const char *c = index(name, ':');
+	const char *c = strchr(name, ':');
 	int new_len;
 
 	if (c == NULL)
@@ -1240,7 +1240,7 @@ static void
 xml_end_tag(void *userData, const char *name)
 {
 	struct xml_ctx *ctx = (struct xml_ctx *)userData;
-	const char *c = index(name, ':');
+	const char *c = strchr(name, ':');
 	char *ep;
 
 	ctx->userFunc(ctx, 1);
@@ -2350,7 +2350,7 @@ int main(int argc, char **argv)
 			char *path = strstr(arg, "//");
 			remote->url = arg;
 			if (path) {
-				path = index(path+2, '/');
+				path = strchr(path+2, '/');
 				if (path)
 					remote->path_len = strlen(path);
 			}

^ permalink raw reply related

* t5501-old-fetch-and-upload.sh fails with NO_PYTHON=1
From: Dennis Stosberg @ 2006-04-11 17:05 UTC (permalink / raw)
  To: git

Hello,

t/t5501-old-fetch-and-upload.sh fails on Solaris 9 with NO_PYTHON=1.
The test doesn't work correctly on Linux with NO_PYTHON=1, too, but it
doesn't cause a failure there.

When NO_PYTHON=1 is set, t/Makefile passes "--no-python" to the test as
an argument.  That causes the $list variable to be set to "--no-python"
instead of "fetch upload".  Since that string does not identify a
program to be tested, $pgm remains unset.

On Linux the return code of "which $pgm" is 1 in that case, which
causes the test to do nothing and exit without failure.  In contrast,
the return code of "which" without any argument is 0 on Solaris, so
the test is being run and fails.

I have attached a simple fix, but is this test still useful at all?

Regards,
Dennis

diff --git a/t/t5501-old-fetch-and-upload.sh b/t/t5501-old-fetch-and-upload.sh
index 596c88b..df69d97 100755
--- a/t/t5501-old-fetch-and-upload.sh
+++ b/t/t5501-old-fetch-and-upload.sh
@@ -13,10 +13,11 @@ tmp=`pwd`/.tmp$$
 
 retval=0
 
-if [ -z "$1" ]; then
+tests=`echo "$@"| sed -e 's/--[a-zA-Z\-]*//g'`
+if [ -z "$tests" ]; then
 	list="fetch upload"
 else
-	list="$@"
+	list="$tests"
 fi
 
 for i in $list; do

^ permalink raw reply related

* Re: What's in git.git
From: Junio C Hamano @ 2006-04-11 17:58 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Petr Baudis, git
In-Reply-To: <20060411155518.GY27689@pasky.or.cz>

Petr Baudis <pasky@suse.cz> writes:

>> I think the builtins should be a install-time only issue. 
>
> I don't care about git-log in particular since I don't use it, but I use
> development Git versions without actually installing them and if it's
> not a huge hurdle, it would be nice to keep this possible.

Also not making the link would probably break test suite (I do
not know if we have git-log test right now, though).

But installing *copies* is obviously not right as you pointed out.

^ 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