Git development
 help / color / mirror / Atom feed
* log/diff option parsing unification
From: Junio C Hamano @ 2006-04-15 12:04 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git

Linus,

I kept your log/diff option parsing stuff with a few fixups for
about 3 hours in the "next" branch, but dropped and replaced
them with an alternative with lesser impact.  I freely admit
that what you write is nicer and cleaner than what I do 99.9% of
the time, but I think this time it is justified.  We have been
telling people that their scripts can expect certain output
format out of the lowest level Plumbing such as rev-list and
diff-tree.  I just do not want to break it, and doing things
with very unified way seemed to be too much hassle to keep
everything compatible.

On the other hand, I am much more relaxed if 'git-log' and
'git-whatchanged' output are not bit-for-bit identical to their
shell script versions.  You (not "Linus, you" but figuratively
you) would be crazy if you have been feeding their output to
your own scripts with such an expectation.  The rule-of-thumb I
apply here is that anything that has built-in pager is primarily
for human consumption, and the stability of its output format is
a fair game, especially when the modified behaviour results in
better output for humans.

I might regret this later when we try to do the internal "git
diff", but it is already tomorrow morning now, so I'll crash
first.

This option parsing unification will be post 1.3.0, but it is
important enough that I suspect it will be the first topic to be
merged and disrupt the "master" branch for a while after 1.3.0.

^ permalink raw reply

* Re: [PATCH] diff-options: add --patch-with-stat
From: Johannes Schindelin @ 2006-04-15 11:56 UTC (permalink / raw)
  To: Marco Costalba; +Cc: git
In-Reply-To: <e5bfff550604150450u37ce1660u2db4f6e97c586e13@mail.gmail.com>

Hi,

On Sat, 15 Apr 2006, Marco Costalba wrote:

> On 4/15/06, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
> >         BTW I really would like to have a diffstat for combined diffs.
> >         Any ideas?
> 
> Well..hem..why do not count the (shifted) + and - in the combined diffs 
> ouput?

This does not help. The combined diff is so useful, because it contains 
the information as to which parent has this difference, and which parent 
has not.

By just counting the plusses and minusses, this information is filtered 
out.

Ciao,
Dscho

^ permalink raw reply

* Re: [WISH] prepend diffstat in front of the patch
From: Johannes Schindelin @ 2006-04-15 11:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vejzzi4ft.fsf@assigned-by-dhcp.cox.net>

Hi,

On Sat, 15 Apr 2006, Junio C Hamano wrote:

> [... speaking about --patch-with-stat ...]
> 
> It is also what I want, but there is only 24 hours in a day and
> there is this thing called day-job.

I want to add that there is a real life also, and that in a few parts of 
this lovely world, there is Easter right now.

And most importantly: an itch like that is a great opportunity for 
non-veterans to get involved with git.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] diff-options: add --patch-with-stat
From: Marco Costalba @ 2006-04-15 11:50 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, junkio
In-Reply-To: <Pine.LNX.4.63.0604151340210.24303@wbgn013.biozentrum.uni-wuerzburg.de>

On 4/15/06, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
> With this option, git prepends a diffstat in front of the patch.
>

Thanks!  qgit will set this as default!

>
>         Buggeth, and you shall be given.
>
>         BTW I really would like to have a diffstat for combined diffs.
>         Any ideas?
>

Well..hem..why do not  count  the (shifted) + and - in the combined diffs ouput?

I suspect this can be a total idiocy, but now I'm missing why.  :-)

Marco

^ permalink raw reply

* Re: [WISH] prepend diffstat in front of the patch
From: Johannes Schindelin @ 2006-04-15 11:48 UTC (permalink / raw)
  To: Marco Costalba; +Cc: git
In-Reply-To: <e5bfff550604150207h6fdb0042x3a9bbfa63269a8c8@mail.gmail.com>

Hi,

On Sat, 15 Apr 2006, Marco Costalba wrote:

> Perhaps I missed something, but I would like to see --stat and -p as 
> _cumulative_ options .
> 
> Would be great if git-diff-tree -r -c --stat -p 84981f9 prepends
> diffstat in front of the patch as Junio suggested some days ago.
> 
> Is it already planned?

We are a little impatient, aren't we?

Anyway, as you probably saw already, I sent out a patch which does that. I 
wanted to wait a little to introduce it, because I had the feeling that 
the option parsing would be volatile for a few days.

Ciao,
Dscho

^ permalink raw reply

* Re: Recent unresolved issues
From: Johannes Schindelin @ 2006-04-15 11:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk69ri5cp.fsf@assigned-by-dhcp.cox.net>

Hi,

On Sat, 15 Apr 2006, Junio C Hamano wrote:

> Honestly, the longer I look at it, the more I feel that this way
> might break more things than it fixes.  I haven't even looked at
> blame.c or http-push.c to see what's broken yet.

I do not have time to look at this closely, but it sounds to me like you 
need a two-stage approach:

setup_diff_options(&options);
[... set defaults ...]
handle_cmdline_arguments(&options);
[... possibly check if the user overrode some defaults ...]

I think that the unified option parsing is the right approach.

Ciao,
Dscho

^ permalink raw reply

* [PATCH] diff-options: add --patch-with-stat
From: Johannes Schindelin @ 2006-04-15 11:41 UTC (permalink / raw)
  To: git, junkio


With this option, git prepends a diffstat in front of the patch.

Since I really, really do not know what a diffstat of a combined diff
("merge diff") should look like, the diffstat is not generated for these.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

	Buggeth, and you shall be given.

	BTW I really would like to have a diffstat for combined diffs.
	Any ideas?

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

c06cf94fe5a2f0b004e7b46c0322554e7ec4ff99
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 447e522..c183dc9 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -10,6 +10,9 @@
 --stat::
 	Generate a diffstat instead of a patch.
 
+--patch-with-stat::
+	Generate patch and prepend its diffstat.
+
 -z::
 	\0 line termination on output
 
diff --git a/diff.c b/diff.c
index f1b672d..1b33465 100644
--- a/diff.c
+++ b/diff.c
@@ -1049,6 +1049,10 @@ int diff_opt_parse(struct diff_options *
 	}
 	else if (!strcmp(arg, "--stat"))
 		options->output_format = DIFF_FORMAT_DIFFSTAT;
+	else if (!strcmp(arg, "--patch-with-stat")) {
+		options->output_format = DIFF_FORMAT_PATCH;
+		options->with_stat = 1;
+	}
 	else if (!strcmp(arg, "-z"))
 		options->line_termination = 0;
 	else if (!strncmp(arg, "-l", 2))
@@ -1518,7 +1522,7 @@ void diff_flush(struct diff_options *opt
 	int diff_output_format = options->output_format;
 	struct diffstat_t *diffstat = NULL;
 
-	if (diff_output_format == DIFF_FORMAT_DIFFSTAT) {
+	if (diff_output_format == DIFF_FORMAT_DIFFSTAT || options->with_stat) {
 		diffstat = xcalloc(sizeof (struct diffstat_t), 1);
 		diffstat->xm.consume = diffstat_consume;
 	}
@@ -1530,6 +1534,17 @@ void diff_flush(struct diff_options *opt
 		}
 		putchar(options->line_termination);
 	}
+	if (options->with_stat) {
+		for (i = 0; i < q->nr; i++) {
+			struct diff_filepair *p = q->queue[i];
+			flush_one_pair(p, DIFF_FORMAT_DIFFSTAT, options,
+					diffstat);
+		}
+		show_stats(diffstat);
+		free(diffstat);
+		diffstat = NULL;
+		putchar(options->line_termination);
+	}
 	for (i = 0; i < q->nr; i++) {
 		struct diff_filepair *p = q->queue[i];
 		flush_one_pair(p, diff_output_format, options, diffstat);
diff --git a/diff.h b/diff.h
index 2f8aff2..f783bae 100644
--- a/diff.h
+++ b/diff.h
@@ -25,6 +25,7 @@ struct diff_options {
 	const char *pickaxe;
 	unsigned recursive:1,
 		 with_raw:1,
+		 with_stat:1,
 		 tree_in_recursive:1,
 		 full_index:1;
 	int break_opt;
@@ -120,6 +121,8 @@ #define COMMON_DIFF_OPTIONS_HELP \
 "  --patch-with-raw\n" \
 "                output both a patch and the diff-raw format.\n" \
 "  --stat        show diffstat instead of patch.\n" \
+"  --patch-with-stat\n" \
+"                output a patch and prepend its diffstat.\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" \
-- 
1.3.0.rc4.ga1167e-dirty

^ permalink raw reply related

* Re: git-svn and Author files question
From: Rutger Nijlunsing @ 2006-04-15  9:25 UTC (permalink / raw)
  To: Seth Falcon; +Cc: git
In-Reply-To: <m21wvzx5e6.fsf@ziti.fhcrc.org>

On Fri, Apr 14, 2006 at 01:34:57PM -0700, Seth Falcon wrote:
> Hi all,
> 
> I've been using git to manually track changes to a project that uses
> svn as its primary SCM.
> 
> git-svn looks like it can help me streamline my workflow, but I'm
> getting stuck with the following:
> 
>     mkdir foo
>     cd foo
>     git-svn init $URL  <--- the svn URL
>     git-svn fetch
>     Author: dfcimm3 not defined in  file
> 
> :-(
> 
> Can someone point me to the file and the place that describes what I
> should put in it?  There are many committers to the svn project.  I'm
> hoping that I will not have to enumerate all of their names in some
> file.

I'm not familiar with git-svn, but the $GIT_DIR/svn-authors file used
by git-svnimport.perl and
http://www.wingding.demon.nl/git-svnconvert.rb contains lines like:

svn-author = Full Name <email@domain>

And yes, you've got to enumerate the names you want
transformed. Another option is to give no authors, but then the SVN
author names will be used (with email 'unknown' or something).


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

^ permalink raw reply

* Re: [WISH] prepend diffstat in front of the patch
From: Marco Costalba @ 2006-04-15  9:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vejzzi4ft.fsf@assigned-by-dhcp.cox.net>

On 4/15/06, Junio C Hamano <junkio@cox.net> wrote:
> "Marco Costalba" <mcostalba@gmail.com> writes:
>
> > Perhaps I missed something, but I would like to see --stat and -p as
> > _cumulative_ options .
>
> Yes, you missed my write-up on "Recent unresolved issues",
> especially this entry:
>
>     * Message-ID: <7vek02ynif.fsf@assigned-by-dhcp.cox.net>
>       diff --with-raw, --with-stat? (me)
>
> and the thread that introduced the --stat option.
>
> It is also what I want, but there is only 24 hours in a day and
> there is this thing called day-job.
>

Sorry, I  didn't mean to force anything, just asking.

Day-job thing is not unknown to me too.

Marco

^ permalink raw reply

* Re: [WISH] prepend diffstat in front of the patch
From: Junio C Hamano @ 2006-04-15  9:16 UTC (permalink / raw)
  To: Marco Costalba; +Cc: git
In-Reply-To: <e5bfff550604150207h6fdb0042x3a9bbfa63269a8c8@mail.gmail.com>

"Marco Costalba" <mcostalba@gmail.com> writes:

> Perhaps I missed something, but I would like to see --stat and -p as 
> _cumulative_ options .

Yes, you missed my write-up on "Recent unresolved issues",
especially this entry:

    * Message-ID: <7vek02ynif.fsf@assigned-by-dhcp.cox.net>
      diff --with-raw, --with-stat? (me)

and the thread that introduced the --stat option.

It is also what I want, but there is only 24 hours in a day and
there is this thing called day-job.

^ permalink raw reply

* [WISH] prepend diffstat in front of the patch
From: Marco Costalba @ 2006-04-15  9:07 UTC (permalink / raw)
  To: Johannes.Schindelin; +Cc: junkio, git

>From today git tree

$ git-diff-tree -r -c -p --stat 84981f9
84981f9ad963f050abf4fe33ac07d36b4ea90c6d
---
 diff.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)


$ git-diff-tree -r -c --stat -p 84981f9
84981f9ad963f050abf4fe33ac07d36b4ea90c6d
diff --git a/diff.c b/diff.c
index c120239..f1b672d 100644
--- a/diff.c
+++ b/diff.c
@@ -438,8 +438,8 @@ static void builtin_diffstat(const char
                xdemitcb_t ecb;

                xpp.flags = XDF_NEED_MINIMAL;
-               xecfg.ctxlen = 3;
-               xecfg.flags = XDL_EMIT_FUNCNAMES;
+               xecfg.ctxlen = 0;
+               xecfg.flags = 0;
                ecb.outf = xdiff_outf;
                ecb.priv = diffstat;
                xdl_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);


Perhaps I missed something, but I would like to see --stat and -p as 
_cumulative_ options .

Would be great if git-diff-tree -r -c --stat -p 84981f9 prepends
diffstat in front of the patch as Junio suggested some days ago.

Is it already planned?

Thanks
Marco

^ permalink raw reply related

* Re: Recent unresolved issues
From: Junio C Hamano @ 2006-04-15  8:57 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <7vu08vjra5.fsf@assigned-by-dhcp.cox.net>

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

> Although I've already decided to merge it up, there are small
> fallout from this.  I've fixed the ones I noticed, but there
> probably remain some backward compatibility issues in commands
> that I do not usually use.  We'll see.

I am very to sorry to say this, but...


				Pain


"git log" wants default abbrev (to show Merge: lines and
"whatchanged -r" output compactly) while "git diff-tree -r" by
default wants to show full SHA1 unless asked, which means
"memset(revs, 0, sizeof(*revs))" in revision.c::init_revisions()
needs to be defeated by the caller.

"git rev-list" wants to know if any --pretty was specified to
set verbose_header, but there is no way to tell if the user did
not say anything or said --pretty because revs->commit_format
will be CMIT_FMT_DEFAULT either way.  This is the worst breakage
I found so far -- "git rev-list --pretty" no longer works,
although "git rev-list --header" works so you probably did not
notice the breakage with gitk.

Honestly, the longer I look at it, the more I feel that this way
might break more things than it fixes.  I haven't even looked at
blame.c or http-push.c to see what's broken yet.

^ permalink raw reply

* Re: Test fails on ubuntu breezy
From: Aneesh Kumar @ 2006-04-15  7:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: linux, Peter Eriksen, git, Carl Worth
In-Reply-To: <cc723f590604142347p7646aa40r52506a0d85b7d817@mail.gmail.com>

On 4/15/06, Aneesh Kumar <aneesh.kumar@gmail.com> wrote:
> I am still having failure with the top of the tree. I guess it is
> because git rm exit with status 0.
>
> kvaneesh@home:/tmp/test$ git rm -f a
> rm: cannot remove `a': Permission denied
> kvaneesh@home:/tmp/test$ echo $?
> 0
>


I think it is xargs
kvaneesh@home:/tmp/test$ echo a | xargs rm
rm: cannot remove `a': Permission denied
kvaneesh@home:/tmp/test$ echo $?
0
kvaneesh@home:/tmp/test$

on ubuntu Dapper it returns 123 status.   I will file a bug report
against xargs.

-aneesh

^ permalink raw reply

* Re: Test fails on ubuntu breezy
From: Junio C Hamano @ 2006-04-15  7:24 UTC (permalink / raw)
  To: Aneesh Kumar; +Cc: git
In-Reply-To: <cc723f590604142347p7646aa40r52506a0d85b7d817@mail.gmail.com>

"Aneesh Kumar" <aneesh.kumar@gmail.com> writes:

> I am still having failure with the top of the tree. I guess it is
> because git rm exit with status 0.

Please don't guess, but validate (I know you are capable of
doing so).

What I would do to see what is happening would be to do
something like the attached patch and re-run the test.

BTW, you did "sh -x t3600-rm.sh" which is very good.  I usually
do it when debugging hard-to-debug testsuite problem, especially
ones which I do not know intimately, written by somebody else,
like this:

	sh -x tXXXX-that-test.sh -i

The -i option makes it immediately stop at the first test failure.

-- >8 --
diff --git a/git-rm.sh b/git-rm.sh
index fda4541..622ffca 100755
--- a/git-rm.sh
+++ b/git-rm.sh
@@ -43,6 +43,8 @@ case "$#" in
 	;;
 esac
 
+set -x
+
 if test -f "$GIT_DIR/info/exclude"
 then
 	git-ls-files -z \

^ permalink raw reply related

* Re: Test fails on ubuntu breezy
From: Aneesh Kumar @ 2006-04-15  6:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: linux, Peter Eriksen, git, Carl Worth
In-Reply-To: <7vpsjl1ezb.fsf@assigned-by-dhcp.cox.net>

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

I am still having failure with the top of the tree. I guess it is
because git rm exit with status 0.

kvaneesh@home:/tmp/test$ git rm -f a
rm: cannot remove `a': Permission denied
kvaneesh@home:/tmp/test$ echo $?
0
kvaneesh@home:/tmp/test$


I am attaching the logs below

-aneesh

[-- Attachment #2: test2 --]
[-- Type: application/octet-stream, Size: 1435 bytes --]


+ touch -- foo bar baz 'space embedded' -q
+ git-add -- foo bar baz 'space embedded' -q
+ git-commit -m 'add normal files'
Committing initial tree e5c556e46aae6124ff4a2a466c95004e92d9a2e4
+ test_tabs=y
+ touch -- 'tab embedded' 'newline
embedded'
+ git-add -- 'tab       embedded' 'newline
embedded'
+ git-commit -m 'add files with tabs and newlines'
+ :
+ chmod a-w .
+ rm -f test-file
rm: cannot remove `test-file': Permission denied
+ test -f test-file
+ test_failed_remove=y
+ chmod 775 .
+ rm -f test-file
+ test y = y
+ chmod a-w .
+ test_expect_failure 'Test that "git-rm -f" fails if its rm fails' 'git-rm -f baz'
+ test 2 = 2
+ say 'expecting failure: git-rm -f baz'
+ echo '* expecting failure: git-rm -f baz'
+ test_run_ 'git-rm -f baz'
+ eval 'git-rm -f baz'
+ eval_ret=0
+ return 0
+ '[' 0 = 0 -a 0 '!=' 0 ']'
+ test_failure_ 'Test that "git-rm -f" fails if its rm fails' 'git-rm -f baz'
++ expr 0 + 1
+ test_count=1
++ expr 0 + 1
+ test_failure=1
+ say 'FAIL 1: Test that "git-rm -f" fails if its rm fails'
+ echo '* FAIL 1: Test that "git-rm -f" fails if its rm fails'
* FAIL 1: Test that "git-rm -f" fails if its rm fails
+ shift
+ echo 'git-rm -f baz'
+ sed -e 's/^/  /'
        git-rm -f baz
+ test '' = ''
+ chmod 775 .
+ test_done
+ trap - exit
+ case "$test_failure" in
+ say 'failed 1 among 1 test(s)'
+ echo '* failed 1 among 1 test(s)'
* failed 1 among 1 test(s)
+ exit 1
kvaneesh@home:~/git-work/git.build/t$

^ permalink raw reply

* Re: Recent unresolved issues
From: Junio C Hamano @ 2006-04-15  6:18 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604141751270.3701@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> On Fri, 14 Apr 2006, Linus Torvalds wrote:
>> 
>> I think you're right, and I've probably broken "--full-diff" (causing the 
>> revparse to also use the empty set of paths). Gaah.
>
> In fact, it's broken path-limited revisions entirely. Duh. We should have 
> a test for that, so I would have noticed.
>
> I think we need two diffopt structures there - one for the actual diff, 
> and one for the pruning.

Although I've already decided to merge it up, there are small
fallout from this.  I've fixed the ones I noticed, but there
probably remain some backward compatibility issues in commands
that I do not usually use.  We'll see.

Also I merged the commit prettyprinter change, but I was hoping
we could instead pass down the commit object and commit format
to places where log message needs to be output and write it out
to the standard output instead of formatting in core.

^ permalink raw reply

* Re: Recent unresolved issues
From: Junio C Hamano @ 2006-04-15  5:06 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604142104140.3701@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> Well, it's easy enough to do something like
>
> 	if (rev->diff)
> 		usage(no_diff_cmd_usage);
>
> for something like that.

You're right.  I've swallowed all four patches with a fixlet on
top; thanks.

> Although, the thing is, once we have a built-in "git diff", there's 
> actually little enough reason to ever use the old "git-diff-tree" vs 
> "git-diff-index" vs "git-diff-files" at all. 

True, unless you are writing a Porcelain, that is.

> It might actually be nice to prune some of the tons of git commands. At 
> some point, the fact that
>
> 	echo bin/git-* | wc -w
>
> returns 122 just makes you go "Hmm..".

Yes, but I thought the plan to deal with that was to set gitexecdir
somewhere other than $(prefix)/bin; removing git-diff-* siblings
would be unfriendly to Porcelains.

^ permalink raw reply

* Clean up trailing whitespace when pretty-printing commits
From: Linus Torvalds @ 2006-04-15  4:20 UTC (permalink / raw)
  To: Junio C Hamano, Git Mailing List


Partly because we've messed up and now have some commits with trailing 
whitespace, but partly because this also just simplifies the code, let's 
remove trailing whitespace from the end when pretty-printing commits.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---
diff --git a/commit.c b/commit.c
index c7bb8db..ca25574 100644
--- a/commit.c
+++ b/commit.c
@@ -557,16 +557,11 @@ unsigned long pretty_print_commit(enum c
 		if (fmt == CMIT_FMT_ONELINE)
 			break;
 	}
-	if (fmt == CMIT_FMT_ONELINE) {
-		/* We do not want the terminating newline */
-		if (buf[offset - 1] == '\n')
-			offset--;
-	}
-	else {
-		/* Make sure there is an EOLN */
-		if (buf[offset - 1] != '\n')
-			buf[offset++] = '\n';
-	}
+	while (offset && isspace(buf[offset-1]))
+		offset--;
+	/* Make sure there is an EOLN for the non-oneline case */
+	if (fmt != CMIT_FMT_ONELINE)
+		buf[offset++] = '\n';
 	buf[offset] = '\0';
 	return offset;
 }

^ permalink raw reply related

* Re: Recent unresolved issues
From: Linus Torvalds @ 2006-04-15  4:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vacanmxhe.fsf@assigned-by-dhcp.cox.net>



On Fri, 14 Apr 2006, Junio C Hamano wrote:
> 
> Another thing is that some revision.c users are not interested
> in taking diff options at all.

Well, it's easy enough to do something like

	if (rev->diff)
		usage(no_diff_cmd_usage);

for something like that.

> I was going to suggest a new structure that captures struct
> rev_info, struct log_tree_opt and miscellaneous bits cmd_log
> uses such as do_diff, full_diff, etc., and move the option
> parser out of cmd_log() to a separate function, and have that
> shared across cmd_log(), cmd_show(), cmd_whatchanged(), and
> cmd_diff() without affecting any of the existing revision.c
> users.  That way, "rev-list --cc HEAD" will remain nonsense.

Well, I actually was going to make git-rev-list just take the diff 
options, and it ends up doing the same thing as "git log" with them. 
There's no real downside.

> One nice property your approach has is that it makes
> "git diff-tree a..b" magically starts working, unlike what
> I suggested above.

Yeah. It just fell out automatically from using the rev-list parsing.

Although, the thing is, once we have a built-in "git diff", there's 
actually little enough reason to ever use the old "git-diff-tree" vs 
"git-diff-index" vs "git-diff-files" at all. 

It might actually be nice to prune some of the tons of git commands. At 
some point, the fact that

	echo bin/git-* | wc -w

returns 122 just makes you go "Hmm..".

		Linus

^ permalink raw reply

* [PATCH] Fix a warning.
From: Mike McCormack @ 2006-04-15  3:50 UTC (permalink / raw)
  To: git

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

Signed-off-by: Mike McCormack <mike@codeweavers.com>


---

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


[-- Attachment #2: 2fa977d19bee245aef86d9beb307d742111890d7.diff --]
[-- Type: text/x-patch, Size: 476 bytes --]

2fa977d19bee245aef86d9beb307d742111890d7
diff --git a/diff-tree.c b/diff-tree.c
index 2b79dd0..7015b06 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), opt);
+	diff_tree_setup_paths(get_pathspec(prefix, argv), &opt->diffopt);
 	diff_setup_done(&opt->diffopt);
 
 	switch (nr_sha1) {


^ permalink raw reply related

* Re: Recent unresolved issues
From: Junio C Hamano @ 2006-04-15  2:22 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604141808480.3701@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> Ok, fourth time lucky?
>
> 		Linus

With this, perhaps.

---
diff --git a/revision.c b/revision.c
index 2061ca8..1d26e0d 100644
--- a/revision.c
+++ b/revision.c
@@ -792,6 +792,7 @@ int setup_revisions(int argc, const char
 	}
 	if (revs->diffopt.output_format == DIFF_FORMAT_PATCH)
 		revs->diffopt.recursive = 1;
+	revs->diffopt.abbrev = revs->abbrev;
 	diff_setup_done(&revs->diffopt);
 
 	return left;

^ permalink raw reply related

* Re: Recent unresolved issues: shallow clone
From: Junio C Hamano @ 2006-04-15  2:11 UTC (permalink / raw)
  To: Carl Worth; +Cc: git
In-Reply-To: <7vr73zn0rb.fsf@assigned-by-dhcp.cox.net>

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

> Maybe after the update to G happens (which means you now have C,
> F, G but not A B D E commits), the client side could enumerate
> commits on "rev-list ^C G" and cauterize the ones with missing
> parents (in this case, F does not have one of its parents).
> While doing this would help keeping the resulting commit
> ancestry sane, it does not solve the problem of missing blobs
> and trees.  See below.

Actually, it is more involved than the above.

The sender would give you A B D E as well, so we would not be
able to cauterise at F; instead you would do so at A, making
your shallow clone a bit deeper.  When you look at the objects
the parent gave you by running "rev-list ^C G", you would notice
that you do not have any of real parents of A, and add a new
graft.  While you are at it, you would hopefully notice that the
real parent of commit C is something you now have -- so you
remove the graft entry for C.

However, depending on what you care about more, having the
sender not to send the side branch and cauterising the result at
F _might_ be a better thing to do.  This is probably quite
involved and I offhand would not know how to efficiently
compute.

^ permalink raw reply

* Re: Recent unresolved issues
From: Junio C Hamano @ 2006-04-15  1:35 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604141748070.3701@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> On Fri, 14 Apr 2006, Junio C Hamano wrote:
>> 
>> I was thinking long because I had an impression that anything
>> based on revision.c interface, if it wants to do a tree-diff on
>> the commit stream, would need two different diff options.  One
>> is used by revision.c internally so that it can use its own
>> add_remove/change for parent pruning, and another to control the
>> way diff is run by the user of revision.c.
>
> I think you're right, and I've probably broken "--full-diff" (causing the 
> revparse to also use the empty set of paths). Gaah.

Another thing is that some revision.c users are not interested
in taking diff options at all.

I was going to suggest a new structure that captures struct
rev_info, struct log_tree_opt and miscellaneous bits cmd_log
uses such as do_diff, full_diff, etc., and move the option
parser out of cmd_log() to a separate function, and have that
shared across cmd_log(), cmd_show(), cmd_whatchanged(), and
cmd_diff() without affecting any of the existing revision.c
users.  That way, "rev-list --cc HEAD" will remain nonsense.

One nice property your approach has is that it makes
"git diff-tree a..b" magically starts working, unlike what
I suggested above.

^ permalink raw reply

* Re: Recent unresolved issues
From: Linus Torvalds @ 2006-04-15  1:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604141751270.3701@g5.osdl.org>



Ok, fourth time lucky?

		Linus

----
diff --git a/revision.c b/revision.c
index 0f98960..2061ca8 100644
--- a/revision.c
+++ b/revision.c
@@ -246,7 +246,7 @@ int rev_compare_tree(struct rev_info *re
 		return REV_TREE_DIFFERENT;
 	tree_difference = REV_TREE_SAME;
 	if (diff_tree_sha1(t1->object.sha1, t2->object.sha1, "",
-			   &revs->diffopt) < 0)
+			   &revs->pruning) < 0)
 		return REV_TREE_DIFFERENT;
 	return tree_difference;
 }
@@ -269,7 +269,7 @@ int rev_same_tree_as_empty(struct rev_in
 	empty.size = 0;
 
 	tree_difference = 0;
-	retval = diff_tree(&empty, &real, "", &revs->diffopt);
+	retval = diff_tree(&empty, &real, "", &revs->pruning);
 	free(tree);
 
 	return retval >= 0 && !tree_difference;
@@ -476,9 +476,9 @@ static void handle_all(struct rev_info *
 void init_revisions(struct rev_info *revs)
 {
 	memset(revs, 0, sizeof(*revs));
-	revs->diffopt.recursive = 1;
-	revs->diffopt.add_remove = file_add_remove;
-	revs->diffopt.change = file_change;
+	revs->pruning.recursive = 1;
+	revs->pruning.add_remove = file_add_remove;
+	revs->pruning.change = file_change;
 	revs->lifo = 1;
 	revs->dense = 1;
 	revs->prefix = setup_git_directory();
@@ -780,8 +780,10 @@ int setup_revisions(int argc, const char
 		revs->limited = 1;
 
 	if (revs->prune_data) {
-		diff_tree_setup_paths(revs->prune_data, &revs->diffopt);
+		diff_tree_setup_paths(revs->prune_data, &revs->pruning);
 		revs->prune_fn = try_to_simplify_commit;
+		if (!revs->full_diff)
+			diff_tree_setup_paths(revs->prune_data, &revs->diffopt);
 	}
 	if (revs->combine_merges) {
 		revs->ignore_merges = 0;
@@ -790,8 +792,6 @@ int setup_revisions(int argc, const char
 	}
 	if (revs->diffopt.output_format == DIFF_FORMAT_PATCH)
 		revs->diffopt.recursive = 1;
-	if (!revs->full_diff && revs->prune_data)
-		diff_tree_setup_paths(revs->prune_data, &revs->diffopt);
 	diff_setup_done(&revs->diffopt);
 
 	return left;
diff --git a/revision.h b/revision.h
index 9a45986..6eaa904 100644
--- a/revision.h
+++ b/revision.h
@@ -61,8 +61,9 @@ struct rev_info {
 	unsigned long max_age;
 	unsigned long min_age;
 
-	/* paths limiting */
+	/* diff info for patches and for paths limiting */
 	struct diff_options diffopt;
+	struct diff_options pruning;
 
 	topo_sort_set_fn_t topo_setter;
 	topo_sort_get_fn_t topo_getter;

^ permalink raw reply related

* RFE:  Fix IMAP-send TCP connect
From: Jeff Garzik @ 2006-04-15  1:10 UTC (permalink / raw)
  To: Git Mailing List


Request:  Somebody please fix imap-send.c to use git_tcp_connect() so 
that it supports IPv6 and getaddrinfo().

Bonus points for also fixing:

On Linux, daemon should use the default Linux behavior, binding to both 
IPv4 and IPv6 sockets at the same time.  git is actually doing _more 
work_ to avoid this.

	Jeff

^ 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