Git development
 help / color / mirror / Atom feed
* Re: [PATCH] gitweb: Add "next" link to commitdiff view
From: Jakub Narebski @ 2006-10-22 22:43 UTC (permalink / raw)
  To: git
In-Reply-To: <200610230037.57183.jnareb@gmail.com>

BTW there are some errors in displaying commitdiff of initial commit, 
take for example a=commitdiff;h=161332a521fe10c41979bcd493d95e2ac562b7f
for git.git repository in gitweb.

Needs fixing.
-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH 1/4] gitweb: Whitespace cleanup - tabs are for indent, spaces are for align (2)
From: Junio C Hamano @ 2006-10-22 22:55 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Jakub Narebski, git
In-Reply-To: <Pine.LNX.4.64.0610221359090.3962@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> On Sun, 22 Oct 2006, Jakub Narebski wrote:
>> 
>> To be true I do those "whitespace cleanup" patches when I notice
>> that something is mis-aligned for _my_ tab width (2 spaces).
>
> Oh, wow.
>
> You have clearly been damaged by some nasty GNU coding style or similar.
>
> Please immediately turn tabs into 8 character hard-tabs, and read the 
> kernel Documentation/CodingStyle document.

Offtopic comments.

I am from old school, so a TAB to me is always equivalent to a
run of spaces that pads up to the next column that is dividible
by 8, but there is one valid reason to occasionally take a look
at your code with unusual TAB settings to catch coding style
violations, if you _were_ following an indentation policy that
is different from what the kernel and git uses.

Aside from the textual comparison order habit (aka "never use >
or >= comparison operators") that everybody hates (and I've been
trying not to inroduce new ones, albeit slowly), I have picked
up two other "unusual" coding style habits, which I deliberately
have stayed away from applying to the code I write for git.

They both relate to the way the code is indented; unlike the
textual order comparison one, they do not affect readability to
me that much and it is easier for me to shake them when I want
to be.

They are:

 - There is no "aligning" between two lines, other than the case
   in which they start with _identical_ substring.  Not just
   that a TAB does not necessarily look the same as spaces to
   the next display column that is divisible by 8 by everybody;
   think about proportinal fonts.

   When you take this position, there is only one thing you can
   do when your logical line extends over more than one physical
   lines because it is too long.  Since there is no aligning,
   the only sensible thing to do is to indent it one level
   deeper than the first physical line of the logical line.
   Your indentation becomes like this:

   <BOL><TAB>if (very long expression that extends over<EOL>
   <BOL><TAB><TAB>more than one line) {<EOL>
   <BOL><TAB><TAB>do this and do that...<EOL>
   <BOL><TAB>}<EOL>

   When you are following this principle (and git sources do not
   and should not), viewing your code with an unusual tab width
   is a good way to check the coding style violation.

 - If you have to split a long expression over multiple lines,
   split the line _before_ a binary operator, not after, so when
   you turn your head sideways 90 degrees, you would see the
   parse tree of the expression:

        foo = a-term
                * (a-very-very-long-expression-term
                        + yet-another-term);

   If there _were_ alignment, this would perhaps look like this:

        foo =     a-term
                * (       a-very-very-long-expression-term
                        + yet-another-term);

^ permalink raw reply

* Re: [PATCH] threeway_merge: if file will not be touched, leave it alone
From: Junio C Hamano @ 2006-10-22 23:11 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0610222301080.14200@wbgn013.biozentrum.uni-wuerzburg.de>

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

> 	How about this? It passes the testsuite, and I tested it with the 
> 	test case you did, and with the same test case with recursive 
> 	merge.
>
>  unpack-trees.c |    5 ++---
>  1 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/unpack-trees.c b/unpack-trees.c
> index 3ac0289..b4994c4 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -658,10 +658,9 @@ int threeway_merge(struct cache_entry **
>  	 * up-to-date to avoid the files getting overwritten with
>  	 * conflict resolution files.
>  	 */
> -	if (index) {
> +	if (index)
>  		verify_uptodate(index, o);
> -	}
> -	else if (path)
> +	else if (no_anc_exists)
>  		verify_absent(path, "overwritten", o);
>  
>  	o->nontrivial_merge = 1;

This feels wrong at the philosophical level.  unpack-trees and
read-tree do not know, and more importantly, do not want to
decide, the outcome of the merge, so it should not be doing
verify_absent because it does not know if the path will be
overwritten by the merge.

Complaining when no_anc_exists means that threeway_merge() is
deciding that the merge result should have the path in this
case.  It might be true for the current merge-recursive and
merge-resolve, but I do not think we should force that decision
on future merge strategies, since that is the whole point of
declaring the merge to be nontrivial and _not_ deciding the
outcome ourselves here.

^ permalink raw reply

* Re: [PATCH qgit 1/2] Directly draw lanes in ListView::paintCell()
From: Marco Costalba @ 2006-10-22 22:35 UTC (permalink / raw)
  To: Josef Weidendorfer; +Cc: git
In-Reply-To: <200610222242.45313.Josef.Weidendorfer@gmx.de>

>
> Hi Marco,
>
> the change looks bigger than it really is because of the
> pure rearrangement of the drawing code.
> I hope I used your indentation style.
>
> Josef
>

Hi Josef,

Thanks for the patches. Applied locally both, unfortunately I cannot push due to
a problem with hard disk: all my data is lost, so I don't have anymore
the ssh keys necessary to login at kernel.org

I've already asked admin for help, but I think we have to wait few
days before to push patches again.

Anyhow patches are very good, really, a breath of fresh air!

Marco

^ permalink raw reply

* Re: prune/prune-packed
From: J. Bruce Fields @ 2006-10-22 23:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy7r954k7.fsf@assigned-by-dhcp.cox.net>

On Sat, Oct 21, 2006 at 09:59:20PM -0700, Junio C Hamano wrote:
> "J. Bruce Fields" <bfields@fieldses.org> writes:
> 
> > Both "man prune" and everyday.txt say that git-prune also runs
> > git-prune-packed.  But that doesn't seem to be true.  Is the bug in the
> > documentation?
> 
> I think it is a regression when prune was rewritten as a
> built-in.

So would it be as simple as this?

--b.

>From d8a01cf8e2d4ccc02dc52fe5dd22b8462997c1ca Mon Sep 17 00:00:00 2001
From: J. Bruce Fields <bfields@citi.umich.edu>
Date: Sun, 22 Oct 2006 19:01:23 -0400
Subject: [PATCH] Make prune also run prune-packed

Both the git-prune manpage and everday.txt say that git-prune should also prune
unpacked objects that are also found in packs, by running git prune-packed.

Junio thought this was "a regression when prune was rewritten as a built-in."

So modify prune to call prune-packed again.

Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
 builtin-prune-packed.c |   11 +++++------
 builtin-prune.c        |    2 ++
 builtin.h              |    1 +
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/builtin-prune-packed.c b/builtin-prune-packed.c
index 960db49..e12b6cf 100644
--- a/builtin-prune-packed.c
+++ b/builtin-prune-packed.c
@@ -4,9 +4,7 @@ #include "cache.h"
 static const char prune_packed_usage[] =
 "git-prune-packed [-n]";
 
-static int dryrun;
-
-static void prune_dir(int i, DIR *dir, char *pathname, int len)
+static void prune_dir(int i, DIR *dir, char *pathname, int len, int dryrun)
 {
 	struct dirent *de;
 	char hex[40];
@@ -31,7 +29,7 @@ static void prune_dir(int i, DIR *dir, c
 	rmdir(pathname);
 }
 
-static void prune_packed_objects(void)
+void prune_packed_objects(int dryrun)
 {
 	int i;
 	static char pathname[PATH_MAX];
@@ -50,7 +48,7 @@ static void prune_packed_objects(void)
 		d = opendir(pathname);
 		if (!d)
 			continue;
-		prune_dir(i, d, pathname, len + 3);
+		prune_dir(i, d, pathname, len + 3, dryrun);
 		closedir(d);
 	}
 }
@@ -58,6 +56,7 @@ static void prune_packed_objects(void)
 int cmd_prune_packed(int argc, const char **argv, const char *prefix)
 {
 	int i;
+	int dryrun;
 
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
@@ -73,6 +72,6 @@ int cmd_prune_packed(int argc, const cha
 		usage(prune_packed_usage);
 	}
 	sync();
-	prune_packed_objects();
+	prune_packed_objects(dryrun);
 	return 0;
 }
diff --git a/builtin-prune.c b/builtin-prune.c
index 6228c79..7290e6d 100644
--- a/builtin-prune.c
+++ b/builtin-prune.c
@@ -255,5 +255,7 @@ int cmd_prune(int argc, const char **arg
 
 	prune_object_dir(get_object_directory());
 
+	sync();
+	prune_packed_objects(show_only);
 	return 0;
 }
diff --git a/builtin.h b/builtin.h
index f9fa9ff..f71b962 100644
--- a/builtin.h
+++ b/builtin.h
@@ -11,6 +11,7 @@ extern int mailinfo(FILE *in, FILE *out,
 extern int split_mbox(const char **mbox, const char *dir, int allow_bare, int nr_prec, int skip);
 extern void stripspace(FILE *in, FILE *out);
 extern int write_tree(unsigned char *sha1, int missing_ok, const char *prefix);
+extern void prune_packed_objects(int);
 
 extern int cmd_add(int argc, const char **argv, const char *prefix);
 extern int cmd_apply(int argc, const char **argv, const char *prefix);
-- 
1.4.3.1.g87b78

^ permalink raw reply related

* Re: [PATCH] gitweb: Add "next" link to commitdiff view
From: Junio C Hamano @ 2006-10-22 23:16 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <200610230037.57183.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

> Add a kind of "next" view in the bottom part of navigation bar for
> "commitdiff" view.
>
> For commitdiff between two commits:
>   (from: _commit_)
> For commitdiff for one single parent commit:
>   (parent: _commit_)
> For commitdiff for one merge commit
>   (merge: _commit_ _commit_ ...)
> For commitdiff for root (parentless) commit
>   (initial)
> where _link_ denotes hyperlink. SHA1 is shortened to 7 characters on
> display, everything is perhaps unnecessary esc_html on display.
>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>

Would it even be necessary to use any SHA-1 name in these cases,
I wonder.  Would it make the page less useful if we replace all
of the above _commit_ with a fixed string, say, "parent"?

I always hated gitweb diffs that prefix each filepair with their
full 40-byte SHA-1 blob object names.  It just adds noise to the
output without adding any meaningful information.

^ permalink raw reply

* Re: Commit-ish shortcut for immediate parent range
From: Junio C Hamano @ 2006-10-22 23:36 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <ehgrj9$8u9$1@sea.gmane.org>

Jakub Narebski <jnareb@gmail.com> writes:

> Johannes Schindelin wrote:
>
>> On Sun, 22 Oct 2006, Jakub Narebski wrote:
>> 
>>> BTW. what does "recursive diff" mean (in git)?
>> 
>> AFAIU it means that the diff code recurses into subdirectories.
>
> When git-diff _doesn't_ recurse into subdirectories?

When it is not told to.

The Porcelainish commands (git-diff, git-show) default to
recursive, but low-level commands don't.

Compare these three:

	git diff --raw v1.4.3.1^^^..v1.4.3.1^^
	git diff-tree v1.4.3.1^^^..v1.4.3.1^^
	git diff-tree -r v1.4.3.1^^^..v1.4.3.1^^

^ permalink raw reply

* Re: [PATCH 1/4] gitweb: Whitespace cleanup - tabs are for indent, spaces are for align (2)
From: Luben Tuikov @ 2006-10-22 23:36 UTC (permalink / raw)
  To: Linus Torvalds, Jakub Narebski; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0610221359090.3962@g5.osdl.org>

--- Linus Torvalds <torvalds@osdl.org> wrote:
> On Sun, 22 Oct 2006, Jakub Narebski wrote:
> > 
> > To be true I do those "whitespace cleanup" patches when I notice
> > that something is mis-aligned for _my_ tab width (2 spaces).
> 
> Oh, wow.
> 
> You have clearly been damaged by some nasty GNU coding style or similar.
> 
> Please immediately turn tabs into 8 character hard-tabs, and read the 
> kernel Documentation/CodingStyle document.
> 
> After that, you may have to go to some detox unit to purge yourself of the 
> habit of 2-character indents, and I realize it will be hard and you'll 
> feel like shit for a while, but you'll be a better person for it.
> 
> It's like kicking a bad drug habit. It may be painful for a while, and you 
> might wistfully think back on the "happy days" when things went by in a 
> haze of unclear indentations, but really, it's worth it.

Funny, but true.

One way to do it is:

(defun my-perl-mode ()
  (setq perl-indent-level 8)
  (setq perl-continued-statement-offset 8)
)

(add-hook 'perl-mode-hook 'my-perl-mode)

     Luben

^ permalink raw reply

* Re: [PATCH] gitweb: Add "next" link to commitdiff view
From: Jakub Narebski @ 2006-10-22 23:41 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd58k0wmx.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> Add a kind of "next" view in the bottom part of navigation bar for
>> "commitdiff" view.
>>
>> For commitdiff between two commits:
>>   (from: _commit_)
Perhaps we should use "(from: _commit_ to: _commit_)" here...

>> For commitdiff for one single parent commit:
>>   (parent: _commit_)
>> For commitdiff for one merge commit
>>   (merge: _commit_ _commit_ ...)
>> For commitdiff for root (parentless) commit
>>   (initial)
>> where _link_ denotes hyperlink. SHA1 is shortened to 7 characters on
>> display, everything is perhaps unnecessary esc_html on display.
>>
>> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> 
> Would it even be necessary to use any SHA-1 name in these cases,
> I wonder.  Would it make the page less useful if we replace all
> of the above _commit_ with a fixed string, say, "parent"?

I decided on using _shortened_ SHA1 because I didn't like neither 
"(parent parent ...) " nor "(parent1 parent2 ...)" for merges. Perhaps 
I should have used 8-characters abbreviation, like in git_blame2.
And I was inspired by git-show output for merges:

 commit ff49fae6a547e5c70117970e01c53b64d983cd10
 Merge: 7ad4ee7... 75f9007... 14eab2b... 0b35995... eee4609...

> I always hated gitweb diffs that prefix each filepair with their
> full 40-byte SHA-1 blob object names.  It just adds noise to the
> output without adding any meaningful information.

I always thought about this only as a (somewhat sophisticated) separator 
marking where individual patch (patch for given files) begin. And
a place to click (non-hidden link) for blob before and after. Please 
remember that this gitweb diff header was from the times where we 
didn't have difftree in commitdiff view.

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH] threeway_merge: if file will not be touched, leave it alone
From: Johannes Schindelin @ 2006-10-23  0:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vlkn80wv2.fsf@assigned-by-dhcp.cox.net>

Hi,

On Sun, 22 Oct 2006, Junio C Hamano wrote:

> Complaining when no_anc_exists means that threeway_merge() is deciding 
> that the merge result should have the path in this case.

Two points:

- you are correct for at least the case of choosing the merge strategy 
"theirs". (Which does not exist yet.)

- in merge-recursive.c:process_entry() (which is called on _all_ unmerged 
entries after threeway merge), "Case A" reads "deleted in one branch". 
Reading the code again, I believe there is a bug, which should be fixed by

diff --git a/merge-recursive.c b/merge-recursive.c
index 2ba43ae..9f6538a 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1005,9 +1005,10 @@ static int process_entry(const char *pat
 		    (!a_sha && sha_eq(b_sha, o_sha))) {
 			/* Deleted in both or deleted in one and
 			 * unchanged in the other */
-			if (a_sha)
+			if (!a_sha) {
 				output("Removing %s", path);
-			remove_file(1, path);
+				remove_file(1, path);
+			}
 		} else {
 			/* Deleted in one and changed in the other */
 			clean_merge = 0;

Note that not only it groups the call to output() and remove_file(), which 
matches the expectation, but also changes the condition to "!a_sha", 
meaning that the file is deleted in branch "a", but existed in the merge 
base, where it is identical to what is in branch "b".

Of course, this assumes that even in the recursive case, branch "a" is to 
be preferred over branch "b". (If I still remember correctly, then branch 
"a" is either the current head, or the temporary recursive merge, so this 
would make sense to me.)

So, after applying this patchlet, merge-recursive (more precisely: the 
function process_entry()) should behave correctly with the change to 
unpack-trees.c you have in pu, i.e. the change that drops that 
verify_absent() call to the floor.

However, I could use some additional optical lobes here.

Ciao,
Dscho

P.S.: Maybe I was wrong on my earlier assessment, that merge-recursive 
does not optimize the "subtrees have identical SHA1s" case. This should be 
handled pretty well by the call to unpack_trees() with threeway merge.

^ permalink raw reply related

* Re: prune/prune-packed
From: J. Bruce Fields @ 2006-10-23  0:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v4ptylfvw.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> writes:
> I am considering the following to address irritation some people
> (including me, actually) are experiencing with this change when
> viewing a small (or no) diff.  Any objections?

So for me, if I run

	less -FRS file

where "file" is less than a page, I see nothing happen whatsoever.

At a guess, maybe it's clearing the screen, displaying the file, the
restoring, all before I see anything happen?

--b.

^ permalink raw reply

* Re: prune/prune-packed
From: A Large Angry SCM @ 2006-10-23  1:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: J. Bruce Fields, git
In-Reply-To: <20061023005336.GA12932@fieldses.org>

J. Bruce Fields wrote:
> Junio C Hamano <junkio@cox.net> writes:
>> I am considering the following to address irritation some people
>> (including me, actually) are experiencing with this change when
>> viewing a small (or no) diff.  Any objections?
> 
> So for me, if I run
> 
> 	less -FRS file
> 
> where "file" is less than a page, I see nothing happen whatsoever.
> 
> At a guess, maybe it's clearing the screen, displaying the file, the
> restoring, all before I see anything happen?

Junio,

How about reverting this change? From the reports here, is causing 
problems on a number of different distributions.

These settings are probably something that is better set by the user in 
an environment variable. Or, make the default something that does work 
everywhere and have a config item for those that wish to customize their UI.

^ permalink raw reply

* Re: [ANNOUNCE] GIT 1.4.3
From: J. Bruce Fields @ 2006-10-23  2:36 UTC (permalink / raw)
  To: A Large Angry SCM; +Cc: Junio C Hamano, git
In-Reply-To: <453C1A35.70504@gmail.com>

On Sun, Oct 22, 2006 at 06:26:13PM -0700, A Large Angry SCM wrote:
> J. Bruce Fields wrote:
> >So for me, if I run
> >
> >	less -FRS file
> >
> >where "file" is less than a page, I see nothing happen whatsoever.
> >
> >At a guess, maybe it's clearing the screen, displaying the file, the
> >restoring, all before I see anything happen?
...
> 
> How about reverting this change? From the reports here, is causing 
> problems on a number of different distributions.

I'm using gnome-terminal on Debian/Sid, by the way.

> These settings are probably something that is better set by the user in 
> an environment variable. Or, make the default something that does work 
> everywhere and have a config item for those that wish to customize their UI.

(Um, sorry for my mail screwups, by the way....)

--b.

^ permalink raw reply

* Re: prune/prune-packed
From: Junio C Hamano @ 2006-10-23  3:27 UTC (permalink / raw)
  To: gitzilla; +Cc: J. Bruce Fields, git
In-Reply-To: <453C1A35.70504@gmail.com>

A Large Angry SCM <gitzilla@gmail.com> writes:

> J. Bruce Fields wrote:
>> Junio C Hamano <junkio@cox.net> writes:
>>> I am considering the following to address irritation some people
>>> (including me, actually) are experiencing with this change when
>>> viewing a small (or no) diff.  Any objections?
>>
>> So for me, if I run
>>
>> 	less -FRS file
>>
>> where "file" is less than a page, I see nothing happen whatsoever.
>>
>> At a guess, maybe it's clearing the screen, displaying the file, the
>> restoring, all before I see anything happen?
>
> Junio,
>
> How about reverting this change? From the reports here, is causing
> problems on a number of different distributions.

Hmmm.  I thought I was using gnome-terminal as well, but I
always work in screen and did not see this problem.

Sorry, but you are right and Linus is more right.  How about
doing FRSX.

diff --git a/pager.c b/pager.c
index 8bd33a1..4587fbb 100644
--- a/pager.c
+++ b/pager.c
@@ -50,7 +50,7 @@ void setup_pager(void)
 	close(fd[0]);
 	close(fd[1]);
 
-	setenv("LESS", "FRS", 0);
+	setenv("LESS", "FRSX", 0);
 	run_pager(pager);
 	die("unable to execute pager '%s'", pager);
 	exit(255);

^ permalink raw reply related

* Re: Commit-ish shortcut for immediate parent range
From: Linus Torvalds @ 2006-10-23  3:33 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <ehgoii$1ap$1@sea.gmane.org>



On Sun, 22 Oct 2006, Jakub Narebski wrote:
> 
> BTW. what does "recursive diff" mean (in git)?

For projects with subdirectories (and git itself has almost none), the 
default "raw, nonrecursive" diff looks something like this:

	f8829caee311207afbc882794bdc5aa0db5caf33
	:040000 040000 db7ae247da2ede4d0f932b86771424534d2960b8 9033be5eb62db6fd778793f9f51e28734bb3d9f8 M      arch
	:040000 040000 c96e5293819986ae7c13a8ef779c5f2066b9575f 5950afceabd99053964778b49df19ba794a21b75 M      include

while the same commit with "raw, recursive" looks like

	f8829caee311207afbc882794bdc5aa0db5caf33
	:100644 100644 88b72c9a84957f2ac787ccf83fa46c4dbb0818d2 2de4d3c367a2c2da9adb6bcf29b5105c46c01f78 M      arch/mips/mm/init.c
	:100644 100644 4bdaa05f485b446e0d66587015cbd8378abf4a69 4a61e624b0ecfcd921a560d426e92e1df2df1de2 M      arch/mips/mm/pgtable-32.c
	:100644 100644 44b5e97fff65f75286fdd15f33c2bcf40841082a 8d600d307d5ddb3f617ffc34929ea98d4613b4a7 M      arch/mips/mm/pgtable-64.c
	:100644 100644 9ab59e2bb23368530fa67c95af0d6ab2c4f7fe8f e3c9925876a3ce4eb80ec67937362cd7d014ad2f M      include/asm-mips/cacheflush.h
	:100644 100644 6959bdb59310b096ec7797e0a31c78fde5aa9afc 02c8a13fc894838b27336ae42fbb542f87132e01 M      include/asm-mips/fixmap.h

ie the "recursive" diff will recurse into subdirectories.

A _patch_ is always recursive (you cannot make a patch-like diff for just 
the "tree entry", only for actual blobs), so you can only see this 
difference for raw git SHA1 diffs.

		Linus

^ permalink raw reply

* Re: [PATCH] threeway_merge: if file will not be touched, leave it alone
From: Junio C Hamano @ 2006-10-23  4:17 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0610230228340.14200@wbgn013.biozentrum.uni-wuerzburg.de>

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

> diff --git a/merge-recursive.c b/merge-recursive.c
> index 2ba43ae..9f6538a 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -1005,9 +1005,10 @@ static int process_entry(const char *pat
>  		    (!a_sha && sha_eq(b_sha, o_sha))) {
>  			/* Deleted in both or deleted in one and
>  			 * unchanged in the other */
> -			if (a_sha)
> +			if (!a_sha) {
>  				output("Removing %s", path);
> -			remove_file(1, path);
> +				remove_file(1, path);
> +			}
>  		} else {
>  			/* Deleted in one and changed in the other */
>  			clean_merge = 0;
>
> Note that not only it groups the call to output() and remove_file(), which 
> matches the expectation, but also changes the condition to "!a_sha", 
> meaning that the file is deleted in branch "a", but existed in the merge 
> base, where it is identical to what is in branch "b".

I think the conditional "output" is to mimic the first case in
git-merge-one-file; there we conditionally give that message
only when ours had that path.  If we lost the path while they
have it the same way as the common ancestor, then we do not have
the path to begin with when we start the merge.  It is not
correct to say "Removing" in such a case.

So the output() call being tied to if (a_sha) _is_ correct in
your code.

What we would want to prevent is to remove the path from the
working tree when we did not have the path at the beginning of
the merge and the merge result says we do not want that path.
In such a case, the file in the working tree is an untracked
file that is not touched by the merge.

E.g gitweb/gitweb.cgi is not tracked in the current "master",
but used to be around v1.4.0 time.  If you try to merge a
branch forked from v1.4.0 because you are interested in a work
on other part of the system (i.e. the branch did not touch
gitweb/ at all), we want to successfully merge that branch into
our "master" even after "make" created gitweb/gitweb.cgi.

Such a merge would start with your HEAD and index missing
gitweb/gitweb.cgi but the path still in your working tree.  The
common ancestor and their tree has the path tracked, so you
would end up with identical stage #1 and #3 with missing stage
#2.

The merge machinery should say the merge result does not have
the path, so it should remove it from the index.  However, it
should _not_ touch the untracked (from the beginning of the time
the merge started) working tree file.  So remove_file() call you
touch in your patch needs to be told not to update working
directory in such a case.

Under "aggressive" rule, threeway_merge() is requested to make
the merge policy decision, so it should also loosen this check
itself.  The change by commit 0b35995 needs to be updated with
this patch:

diff --git a/unpack-trees.c b/unpack-trees.c
index b1d78b8..7cfd628 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -642,7 +642,7 @@ int threeway_merge(struct cache_entry **
 		    (remote_deleted && head && head_match)) {
 			if (index)
 				return deleted_entry(index, index, o);
-			else if (path)
+			else if (path && !head_deleted)
 				verify_absent(path, "removed", o);
 			return 0;
 		}

^ permalink raw reply related

* [PATCH] rev-list --left-right
From: Junio C Hamano @ 2006-10-23  4:47 UTC (permalink / raw)
  To: git

The output from "symmetric diff", i.e. A...B, does not
distinguish between commits that are reachable from A and the
ones that are reachable from B.  In this picture, such a
symmetric diff includes commits marked with a and b.

         x---b---b  branch B
        / \ /
       /   .
      /   / \
     o---x---a---a  branch A

However, you cannot tell which ones are 'a' and which ones are
'b' from the output.  Sometimes this is frustrating.  This adds
an output option, --left-right, to rev-list.

        rev-list --left-right A...B

would show ones reachable from A prefixed with '<' and the ones
reachable from B prefixed with '>'.

When combined with --boundary, boundary commits (the ones marked
with 'x' in the above picture) are shown with prefix '-', so you
would see list that looks like this:

    git rev-list --left-right --boundary --pretty=oneline A...B

    >bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb 3rd on b
    >bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb 2nd on b
    <aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa 3rd on a
    <aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa 2nd on a
    -xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx 1st on b
    -xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx 1st on a

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

 * This lets you write something like git-cherry with a single
   invocation of rev-list instead of two.  We would need a diff
   format that gives patch-id in addition to this patch to make
   git-cherry more efficient though.

 builtin-rev-list.c |   11 +++++++++++
 commit.c           |   10 +++++-----
 revision.c         |    5 ++++-
 revision.h         |    1 +
 4 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index fb7fc92..4364035 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -45,6 +45,7 @@ static int bisect_list;
 static int show_timestamp;
 static int hdr_termination;
 static const char *header_prefix;
+static int show_left_right;
 
 static void show_commit(struct commit *commit)
 {
@@ -54,6 +55,12 @@ static void show_commit(struct commit *c
 		fputs(header_prefix, stdout);
 	if (commit->object.flags & BOUNDARY)
 		putchar('-');
+	else if (show_left_right) {
+		if (commit->object.flags & SYMMETRIC_LEFT)
+			putchar('<');
+		else
+			putchar('>');
+	}
 	if (revs.abbrev_commit && revs.abbrev)
 		fputs(find_unique_abbrev(commit->object.sha1, revs.abbrev),
 		      stdout);
@@ -240,6 +247,10 @@ int cmd_rev_list(int argc, const char **
 			bisect_list = 1;
 			continue;
 		}
+		if (!strcmp(arg, "--left-right")) {
+			show_left_right = 1;
+			continue;
+		}
 		if (!strcmp(arg, "--stdin")) {
 			if (read_from_stdin++)
 				die("--stdin given twice?");
diff --git a/commit.c b/commit.c
index a6d543e..289ef65 100644
--- a/commit.c
+++ b/commit.c
@@ -868,11 +868,11 @@ void sort_in_topological_order_fn(struct
 
 /* merge-rebase stuff */
 
-/* bits #0..7 in revision.h */
-#define PARENT1		(1u<< 8)
-#define PARENT2		(1u<< 9)
-#define STALE		(1u<<10)
-#define RESULT		(1u<<11)
+/* bits #0..15 in revision.h */
+#define PARENT1		(1u<<16)
+#define PARENT2		(1u<<17)
+#define STALE		(1u<<18)
+#define RESULT		(1u<<19)
 
 static struct commit *interesting(struct commit_list *list)
 {
diff --git a/revision.c b/revision.c
index f1e0caa..26ddf09 100644
--- a/revision.c
+++ b/revision.c
@@ -344,6 +344,7 @@ static void try_to_simplify_commit(struc
 static void add_parents_to_list(struct rev_info *revs, struct commit *commit, struct commit_list **list)
 {
 	struct commit_list *parent = commit->parents;
+	unsigned left_flag;
 
 	if (commit->object.flags & ADDED)
 		return;
@@ -388,6 +389,7 @@ static void add_parents_to_list(struct r
 	if (revs->no_walk)
 		return;
 
+	left_flag = (commit->object.flags & SYMMETRIC_LEFT);
 	parent = commit->parents;
 	while (parent) {
 		struct commit *p = parent->item;
@@ -395,6 +397,7 @@ static void add_parents_to_list(struct r
 		parent = parent->next;
 
 		parse_commit(p);
+		p->object.flags |= left_flag;
 		if (p->object.flags & SEEN)
 			continue;
 		p->object.flags |= SEEN;
@@ -643,7 +646,7 @@ int handle_revision_arg(const char *arg,
 				add_pending_commit_list(revs, exclude,
 							flags_exclude);
 				free_commit_list(exclude);
-				a->object.flags |= flags;
+				a->object.flags |= flags | SYMMETRIC_LEFT;
 			} else
 				a->object.flags |= flags_exclude;
 			b->object.flags |= flags;
diff --git a/revision.h b/revision.h
index 3adab95..f92a4d4 100644
--- a/revision.h
+++ b/revision.h
@@ -9,6 +9,7 @@ #define TMP_MARK	(1u<<4) /* for isolated
 #define BOUNDARY	(1u<<5)
 #define BOUNDARY_SHOW	(1u<<6)
 #define ADDED		(1u<<7)	/* Parents already parsed and added? */
+#define SYMMETRIC_LEFT	(1u<<8)
 
 struct rev_info;
 struct log_info;
-- 
1.4.3.1.g7ad4

^ permalink raw reply related

* [PATCH] Use column indexes in git-cvsserver where necessary.
From: Shawn Pearce @ 2006-10-23  5:09 UTC (permalink / raw)
  To: martyn, martin, Junio C Hamano; +Cc: git

Tonight I found a git-cvsserver instance spending a lot of time in
disk IO while trying to process operations against a Git repository
with >30,000 objects contained in it.

Blowing away my SQLLite database and rebuilding all tables with
indexes on the attributes that git-cvsserver frequently runs queries
against seems to have resolved the issue quite nicely.

Since the indexes shouldn't hurt performance on small repositories
and always helps on larger repositories we should just always create
them when creating the revision storage tables.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 git-cvsserver.perl |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 08ad831..8817f8b 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -2118,9 +2118,17 @@ sub new
                 mode       TEXT NOT NULL
             )
         ");
+        $self->{dbh}->do("
+            CREATE INDEX revision_ix1
+            ON revision (name,revision)
+        ");
+        $self->{dbh}->do("
+            CREATE INDEX revision_ix2
+            ON revision (name,commithash)
+        ");
     }
 
-    # Construct the revision table if required
+    # Construct the head table if required
     unless ( $self->{tables}{head} )
     {
         $self->{dbh}->do("
@@ -2134,6 +2142,10 @@ sub new
                 mode       TEXT NOT NULL
             )
         ");
+        $self->{dbh}->do("
+            CREATE INDEX head_ix1
+            ON head (name)
+        ");
     }
 
     # Construct the properties table if required
-- 
1.4.3.g4e51

^ permalink raw reply related

* Re: VCS comparison table
From: Matthew Hannigan @ 2006-10-23  5:19 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Tim Webster, bazaar-ng, git, Jakub Narebski
In-Reply-To: <Pine.LNX.4.64.0610220926170.3962@g5.osdl.org>

On Sun, Oct 22, 2006 at 10:12:00AM -0700, Linus Torvalds wrote:
> [ ... ]
> 
>    Again, the way to solve this would tend to be to have a few helper 
>    scripts that use regular file-contents that _describe_ these things to 
>    do "realdiff" and "install".
> 
> In other words, for at least three _totally_ different reasons, you really 
> don't want to do tracking/development directly in /etc, but you want to 
> have a buffer zone to do it. And once you have that, you might as well do 
> _that_ as the repository, and just add a few specialty commands (let's 
> call them "plugins" to make everybody happy) to do the special things.

Damn you stole my idea!  I had this scheme brewing in my head too,
with some slight variations:

> 	# copy the data, set up a PERMISSIONS file to track extra info
> 	sudo cp /etc/group /etc/passwd /etc/shadow .
> 	sudo chown user:user *
> 	cat <<EOF > PERMISSIONS
> 	group root:root 0644
> 	passwd root:root 0644
> 	shadow root:root 0400
> 	EOF

You may want one perms/metadata file per real file (file.meta?) with contents
like:
	owner root
	group root
	perms u=r,go=

for possibly easier to digest diff output. You could omit "don't care" variables.
You could still have one overarching file (DEFAULT.meta) for defaults.  Also, you
may want to track the implied umask instead of the real perms.

You could also track the pathname, (e.g. path /etc/group, path /etc/inet/hosts) so you
didn't have to match the structure of the working tree to the actual destination.

> And again, I'm not going to even claim that the above two "plugins" are 
> the right ones (maybe you want other operations too to interact with the 
> "real" installed files),  [ ... ]

Yes, there are other very useful transformations possible.  One example is to
split the /etc/group file into a series of files, each named after the group,
with contents the sorted list of members.  Again, this is useful for 'diff' and
any SCM. It's important that it's a lossless transformation in both
directions; you may want to scan the destination and make sure
your base revision matches it before 'git install'.

> Btw: none of this is really "git-specific". The above tells you how to do 
> local "git plugins", and it's obviously fairly trivial, but I suspect any 
> SCM can be used in this manner.

Indeed, the essential thing about this is you're representing any
system modification as a text diff, so it makes sense for any
SCM.  In fact the 'plugin' for any SCM would be 95% the same code.

This might also be useful for SCMs that don't handle symlinks
natively.

--
Matt Hannigan

^ permalink raw reply

* Re: [PATCH] Use column indexes in git-cvsserver where necessary.
From: Martin Langhoff (CatalystIT) @ 2006-10-23  6:02 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: martyn, Junio C Hamano, git
In-Reply-To: <20061023050934.GA25018@spearce.org>

Shawn Pearce wrote:
> Tonight I found a git-cvsserver instance spending a lot of time in
> disk IO while trying to process operations against a Git repository
> with >30,000 objects contained in it.
> 
> Blowing away my SQLLite database and rebuilding all tables with
> indexes on the attributes that git-cvsserver frequently runs queries
> against seems to have resolved the issue quite nicely.
> 
> Since the indexes shouldn't hurt performance on small repositories
> and always helps on larger repositories we should just always create
> them when creating the revision storage tables.
 >
 > Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

Ack.

I am thinking we need a lightweight schema versioning mechanism to 
decide whether the DB schema needs changes such as this. Too much work 
for this though. ;-)

We have a simple one in Moodle I could port when the time comes.

cheers,


martin
-- 
-----------------------------------------------------------------------
Martin @ Catalyst .Net .NZ  Ltd, PO Box 11-053, Manners St,  Wellington
WEB: http://catalyst.net.nz/           PHYS: Level 2, 150-154 Willis St
OFFICE: +64(4)916-7224                              MOB: +64(21)364-017
       Make things as simple as possible, but no simpler - Einstein
-----------------------------------------------------------------------

^ permalink raw reply

* Re: [PATCH] cvsserver: fix "cvs diff" in a subdirectory
From: Martin Langhoff (CatalystIT) @ 2006-10-23  6:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git, Martyn Smith
In-Reply-To: <7vslhvy8bo.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:

> Will park this and the "U for update, M for modified" patch in
> "pu", waiting for acks from actual git-cvsserver users.  It
> feels it is safe enough change, so with an Ack from Mart[yi]n
> it may be woth having it in 1.4.3

Late late Ack. Sorry about the delay. I see it's in master already ;-)

cheers,


martin
-- 
-----------------------------------------------------------------------
Martin @ Catalyst .Net .NZ  Ltd, PO Box 11-053, Manners St,  Wellington
WEB: http://catalyst.net.nz/           PHYS: Level 2, 150-154 Willis St
OFFICE: +64(4)916-7224                              MOB: +64(21)364-017
       Make things as simple as possible, but no simpler - Einstein
-----------------------------------------------------------------------

^ permalink raw reply

* [PATCH] git-send-email: do not pass custom Date: header
From: Eric Wong @ 2006-10-23  7:46 UTC (permalink / raw)
  To: git, Junio C Hamano; +Cc: Eric Wong

We already generate a Date: header based on when the patch was
emailed.  git-format-patch includes the Date: header of the
patch.  Having two Date: headers is just confusing, so we
just use the current Date:

Often the mailed patches in a patch series are created over a
series of several hours or days, so the Date: header from the
original commit is incorrect for email, and often far off enough
for spam filters to complain.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-send-email.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 1c6d2cc..c42dc3b 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -514,7 +514,7 @@ foreach my $t (@files) {
 						$2, $_) unless $quiet;
 					push @cc, $2;
 				}
-				elsif (/^[-A-Za-z]+:\s+\S/) {
+				elsif (!/^Date:\s/ && /^[-A-Za-z]+:\s+\S/) {
 					push @xh, $_;
 				}
 
-- 
1.4.3.1.g3e3bc

^ permalink raw reply related

* Re: [PATCH] git-send-email: do not pass custom Date: header
From: Jakub Narebski @ 2006-10-23  8:41 UTC (permalink / raw)
  To: git
In-Reply-To: <11615895973387-git-send-email-normalperson@yhbt.net>

Eric Wong wrote:

> We already generate a Date: header based on when the patch was
> emailed.  git-format-patch includes the Date: header of the
> patch.  Having two Date: headers is just confusing, so we
> just use the current Date:
> 
> Often the mailed patches in a patch series are created over a
> series of several hours or days, so the Date: header from the
> original commit is incorrect for email, and often far off enough
> for spam filters to complain.

But that makes us lose original commit date. And git format-patch
is if I remember correctly together with git-am used in git-rebase.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [PATCH] git-send-email: do not pass custom Date: header
From: Eric Wong @ 2006-10-23  9:38 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <ehhv6g$4js$1@sea.gmane.org>

Jakub Narebski <jnareb@gmail.com> wrote:
> Eric Wong wrote:
> 
> > We already generate a Date: header based on when the patch was
> > emailed. ?git-format-patch includes the Date: header of the
> > patch. ?Having two Date: headers is just confusing, so we
> > just use the current Date:
> > 
> > Often the mailed patches in a patch series are created over a
> > series of several hours or days, so the Date: header from the
> > original commit is incorrect for email, and often far off enough
> > for spam filters to complain.
> 
> But that makes us lose original commit date. And git format-patch
> is if I remember correctly together with git-am used in git-rebase.

This patch is for git-send-email, and only affects the way they
are sent over SMTP.  Output of git-format-patch is unchanged.

-- 
Eric Wong

^ permalink raw reply

* Re: [RFC] git-split: Split the history of a git repository by subdirectories and ranges
From: Josh Triplett @ 2006-10-23 10:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vlko5d3bx.fsf@assigned-by-dhcp.cox.net>

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

Junio C Hamano wrote:
> Josh Triplett <josh@freedesktop.org> writes:
>> Jamey Sharp and I wrote a script called git-split to accomplish this
>> repository split. git-split reconstructs the history of a sub-project
>> previously stored in a subdirectory of a larger repository. It
>> constructs new commit objects based on the existing tree objects for the
>> subtree in each commit, and discards commits which do not affect the
>> history of the sub-project, as well as merges made unnecessary due to
>> these discarded commits.
> 
> Very nicely done.

Thanks!

>> We would like to acknowledge the work of the gobby team in creating a
>> collaborative editor which greatly aided the development of git-split.
> 
>> from itertools import izip
>> from subprocess import Popen, PIPE
>> import os, sys
> 
> How recent a Python are we assuming here?  Is late 2.4 recent
> enough?

We ran it with 2.4, so yes.  git-split does require at least 2.4,
though, because it uses set(), str.rsplit(), and subprocess, none of
which existed in 2.3.

>> def walk(commits, new_commits, commit_hash, project):
>>     commit = commits[commit_hash]
>>     if not(commit.has_key("new_hash")):
>>         tree = get_subtree(commit["tree"], project)
>>         commit["new_tree"] = tree
>>         if not tree:
>>             raise Exception("Did not find project in tree for commit " + commit_hash)
>>         new_parents = list(set([walk(commits, new_commits, parent, project)
>>                                 for parent in commit["parents"]]))
>>
>>         new_hash = None
>>         if len(new_parents) == 1:
>>             new_hash = new_parents[0]
>>         elif len(new_parents) == 2: # Check for unnecessary merge
>>             if is_ancestor(new_commits, new_parents[0], new_parents[1]):
>>                 new_hash = new_parents[0]
>>             elif is_ancestor(new_commits, new_parents[1], new_parents[0]):
>>                 new_hash = new_parents[1]
>>         if new_hash and new_commits[new_hash]["new_tree"] != tree:
>>             new_hash = None
> 
> This is a real gem.  I really like reading well-written Python
> programs.

Thanks.  We had some fun writing this; git's elegant repository
structure made it a joy to work with.

> I wonder if using "git-log --full-history -- $project" to let
> the core side omit commits that do not change the $project (but
> still give you all merged branches) would have made your job any
> easier?

I don't think it would.  We still need to know what commit to use as the
parent of any given commit, so we don't want commits in the log output
with parents that don't exist in the log output.  And rewriting parents
in git-log based on which revisions change the specified subdirectory
seems like a bad idea.

> You are handling grafts by hand because --pretty=raw is special
> in that it displays the real parents (although traversal does
> use grafts).  Maybe it would have helped if we had a --pretty
> format that is similar to raw but rewrites the parents?

Yes, that would help.  We could then avoid dealing with grafts manually.


How would you feel about including git-split in the GIT tree?  We could
certainly write up the necessary documentation for it.

- Josh Triplett


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

^ 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