Git development
 help / color / mirror / Atom feed
* [PATCH] Mak gitk bind keyboard actions to the command key on Mac OS
From: Shawn O. Pearce @ 2007-07-19  4:37 UTC (permalink / raw)
  To: Junio C Hamano, Paul Mackerras; +Cc: git

git-gui already uses the command key for accelerators, but gitk has
never done so.  I'm actually finding it very hard to move back and
forth between the two applications as git-gui is following the Mac
OS X conventions and gitk is not.

This trick is the same one that git-gui uses to determine which
key to bind actions to.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 gitk |   75 +++++++++++++++++++++++++++++++++++++----------------------------
 1 files changed, 43 insertions(+), 32 deletions(-)

diff --git a/gitk b/gitk
index 39e452a..6f58803 100755
--- a/gitk
+++ b/gitk
@@ -796,6 +796,12 @@ proc makewindow {} {
         wm geometry . "$geometry(main)"
     }
 
+    if {[tk windowingsystem] eq {aqua}} {
+        set M1B M1
+    } else {
+        set M1B Control
+    }
+
     bind .pwbottom <Configure> {resizecdetpanes %W %w}
     pack .ctop -fill both -expand 1
     bindall <1> {selcanvline %W %x %y}
@@ -814,12 +820,12 @@ proc makewindow {} {
     bindkey <Key-Left> "goback"
     bind . <Key-Prior> "selnextpage -1"
     bind . <Key-Next> "selnextpage 1"
-    bind . <Control-Home> "allcanvs yview moveto 0.0"
-    bind . <Control-End> "allcanvs yview moveto 1.0"
-    bind . <Control-Key-Up> "allcanvs yview scroll -1 units"
-    bind . <Control-Key-Down> "allcanvs yview scroll 1 units"
-    bind . <Control-Key-Prior> "allcanvs yview scroll -1 pages"
-    bind . <Control-Key-Next> "allcanvs yview scroll 1 pages"
+    bind . <$M1B-Home> "allcanvs yview moveto 0.0"
+    bind . <$M1B-End> "allcanvs yview moveto 1.0"
+    bind . <$M1B-Key-Up> "allcanvs yview scroll -1 units"
+    bind . <$M1B-Key-Down> "allcanvs yview scroll 1 units"
+    bind . <$M1B-Key-Prior> "allcanvs yview scroll -1 pages"
+    bind . <$M1B-Key-Next> "allcanvs yview scroll 1 pages"
     bindkey <Key-Delete> "$ctext yview scroll -1 pages"
     bindkey <Key-BackSpace> "$ctext yview scroll -1 pages"
     bindkey <Key-space> "$ctext yview scroll 1 pages"
@@ -839,15 +845,15 @@ proc makewindow {} {
     bindkey ? findprev
     bindkey f nextfile
     bindkey <F5> updatecommits
-    bind . <Control-q> doquit
-    bind . <Control-f> dofind
-    bind . <Control-g> {findnext 0}
-    bind . <Control-r> dosearchback
-    bind . <Control-s> dosearch
-    bind . <Control-equal> {incrfont 1}
-    bind . <Control-KP_Add> {incrfont 1}
-    bind . <Control-minus> {incrfont -1}
-    bind . <Control-KP_Subtract> {incrfont -1}
+    bind . <$M1B-q> doquit
+    bind . <$M1B-f> dofind
+    bind . <$M1B-g> {findnext 0}
+    bind . <$M1B-r> dosearchback
+    bind . <$M1B-s> dosearch
+    bind . <$M1B-equal> {incrfont 1}
+    bind . <$M1B-KP_Add> {incrfont 1}
+    bind . <$M1B-minus> {incrfont -1}
+    bind . <$M1B-KP_Subtract> {incrfont -1}
     wm protocol . WM_DELETE_WINDOW doquit
     bind . <Button-1> "click %W"
     bind $fstring <Key-Return> dofind
@@ -1088,12 +1094,17 @@ proc keys {} {
 	raise $w
 	return
     }
+    if {[tk windowingsystem] eq {aqua}} {
+	set M1T Cmd
+    } else {
+	set M1T Ctrl
+    }
     toplevel $w
     wm title $w "Gitk key bindings"
-    message $w.m -text {
+    message $w.m -text "
 Gitk key bindings:
 
-<Ctrl-Q>		Quit
+<$M1T-Q>		Quit
 <Home>		Move to first commit
 <End>		Move to last commit
 <Up>, p, i	Move up one commit
@@ -1102,12 +1113,12 @@ Gitk key bindings:
 <Right>, x, l	Go forward in history list
 <PageUp>	Move up one page in commit list
 <PageDown>	Move down one page in commit list
-<Ctrl-Home>	Scroll to top of commit list
-<Ctrl-End>	Scroll to bottom of commit list
-<Ctrl-Up>	Scroll commit list up one line
-<Ctrl-Down>	Scroll commit list down one line
-<Ctrl-PageUp>	Scroll commit list up one page
-<Ctrl-PageDown>	Scroll commit list down one page
+<$M1T-Home>	Scroll to top of commit list
+<$M1T-End>	Scroll to bottom of commit list
+<$M1T-Up>	Scroll commit list up one line
+<$M1T-Down>	Scroll commit list down one line
+<$M1T-PageUp>	Scroll commit list up one page
+<$M1T-PageDown>	Scroll commit list down one page
 <Shift-Up>	Move to previous highlighted line
 <Shift-Down>	Move to next highlighted line
 <Delete>, b	Scroll diff view up one page
@@ -1115,20 +1126,20 @@ Gitk key bindings:
 <Space>		Scroll diff view down one page
 u		Scroll diff view up 18 lines
 d		Scroll diff view down 18 lines
-<Ctrl-F>		Find
-<Ctrl-G>		Move to next find hit
+<$M1T-F>		Find
+<$M1T-G>		Move to next find hit
 <Return>	Move to next find hit
 /		Move to next find hit, or redo find
 ?		Move to previous find hit
 f		Scroll diff view to next file
-<Ctrl-S>		Search for next hit in diff view
-<Ctrl-R>		Search for previous hit in diff view
-<Ctrl-KP+>	Increase font size
-<Ctrl-plus>	Increase font size
-<Ctrl-KP->	Decrease font size
-<Ctrl-minus>	Decrease font size
+<$M1T-S>		Search for next hit in diff view
+<$M1T-R>		Search for previous hit in diff view
+<$M1T-KP+>	Increase font size
+<$M1T-plus>	Increase font size
+<$M1T-KP->	Decrease font size
+<$M1T-minus>	Decrease font size
 <F5>		Update
-} \
+" \
 	    -justify left -bg white -border 2 -relief groove
     pack $w.m -side top -fill both -padx 2 -pady 2
     $w.m configure -font $uifont
-- 
1.5.3.rc2.822.g75e72

^ permalink raw reply related

* Re: [PATCH] Proposal for git-svn
From: Eric Wong @ 2007-07-19  4:22 UTC (permalink / raw)
  To: Benoit SIGOURE; +Cc: git
In-Reply-To: <FAFA899D-EC45-4313-98ED-2D0A3FF37669@lrde.epita.fr>

Benoit SIGOURE <tsuna@lrde.epita.fr> wrote:
> Hello,
> 
> I'm importing many SVN repositories in Git and I ran across a problem:
> ufloat.h has mode 120000but is not a link
> 
> I've read the code and checked-out the revision where the problem  
> occured and it turns out that some stupid user commited a broken  
> symlink and I think that's where the problem came from.  I'm  
> proposing the following trivial change to let git-svn clone continue  
> its work:
> 
> diff --git a/git-svn.perl b/git-svn.perl
> index 01c3904..a82baf4 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -2555,8 +2555,8 @@ sub close_file {
>                 sysseek($fh, 0, 0) or croak $!;
>                 if ($fb->{mode_b} == 120000) {
>                         sysread($fh, my $buf, 5) == 5 or croak $!;
> -                       $buf eq 'link ' or die "$path has mode 120000",
> -                                              "but is not a link\n";
> +                       $buf eq 'link ' or warn "$path has mode 120000",
> +                                              " but is not a link\n";
>                 }
>                 defined(my $pid = open my $out,'-|') or die "Can't  
> fork: $!\n";
>                 if (!$pid) {
> 
> (I also added a whitespace because "120000but" does not look good :D)
> I checked out the problematic revision in git and I see the broken  
> symlink just like in SVN so I assume this change is correct.

Very strange.  Since $buf didn't have the string "link " in it, did it
have a path name in it?  If so, the sysread() would've advanced the $fh
offset by 5 bytes; causing an even more broken symlink to be added by git.

Would the following be more correct?

--- a/git-svn.perl
+++ b/git-svn.perl
@@ -2552,9 +2552,15 @@ sub close_file {
 		}
 		sysseek($fh, 0, 0) or croak $!;
 		if ($fb->{mode_b} == 120000) {
-			sysread($fh, my $buf, 5) == 5 or croak $!;
-			$buf eq 'link ' or die "$path has mode 120000",
-			                       "but is not a link\n";
+			eval {
+				sysread($fh, my $buf, 5) == 5 or croak $!;
+				$buf eq 'link ' or die "$path has mode 120000",
+						       " but is not a link";
+			};
+			if ($@) {
+				warn "$@\n";
+				sysseek($fh, 0, 0) or croak $!;
+			}
 		}
 		defined(my $pid = open my $out,'-|') or die "Can't fork: $!\n";
 		if (!$pid) {

-- 
Eric Wong

^ permalink raw reply

* Re: Proposal about --help options and man calls
From: Brian Gernhardt @ 2007-07-19  3:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: David Kastrup, git
In-Reply-To: <7v644h715y.fsf@assigned-by-dhcp.cox.net>


On Jul 18, 2007, at 7:16 PM, Junio C Hamano wrote:

> What's the reason to set GIT_PAGER and PAGER differently to
> begin with?  Can people give examples of the reason why?

I currently have GIT_PAGER (actually, core.pager) set to tig.  Tig  
has lots of useful features for visualizing history and jumping  
between patches and logs.

But for everything that isn't git, less is far better.

~~ Brian

^ permalink raw reply

* Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats
From: Luben Tuikov @ 2007-07-19  3:30 UTC (permalink / raw)
  To: Junio C Hamano, Matt McCutchen, Jakub Narebski; +Cc: git, Petr Baudis
In-Reply-To: <7vvech42nb.fsf@assigned-by-dhcp.cox.net>

--- Junio C Hamano <gitster@pobox.com> wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > On Tue, 17 July 2007, Matt McCutchen napisał:
> > ...
> >> Alert for gitweb site administrators: This patch changes the format of
> >> $feature{'snapshot'}{'default'} in gitweb_config.perl from a list of
> >> three pieces of information about a single format to a list of one or
> >> more formats you wish to offer from the set ('tgz', 'tbz2', 'zip').
> >> Update your gitweb_config.perl appropriately.  The preferred names for
> >> gitweb.snapshot in repository configuration have also changed from
> >> 'gzip' and 'bzip2' to 'tgz' and 'tbz2', but the old names are still
> >> recognized for compatibility.
> >
> > This alert/warning should probably be put in RelNotes for when it would
> > be in git.git
> 
> Does anybody else worry about the backward imcompatibility, I
> wonder...  List?

I wouldn't mind an improvement in the snapshot area of gitweb.
I wasn't really happy with the snapshot feature as it was originally
implemented, as it would generate a tar file with ".tar.bz2"
name extension, but the file was NOT bz2, and I had to always
manually rename, bz2, and rename back.

> I really hate to having to say something like that in the
> RelNotes.  I do not think this is a good enough reason to break
> existing configurations; I would not want to be defending that
> change.
> 
> >> I thought of another incompatibility: previously bookmarked snapshot
> >> URLs will no longer work because they lack the new "sf" parameter.  I
> >> don't care about this; do any of you?
> >
> > I think either having good error message, or using first format avaiable
> > would be good enough.
> 
> I doubt bookmarked snapshot URL would make sense to begin with,
> so this would be Ok.
> 
> I am wondering if something like this patch (totally untested,
> mind you) to convert the old style %feature in configuration at
> the site at runtime would be sufficient.

"totally untested" is a problem.  Anything going into gitweb for
public consumption (master branch, next ok), should be completely
and exhaustively tested.

   Luben

> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index f17c983..cdec4d0 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -236,9 +236,39 @@ our %feature = (
>  		'default' => [0]},
>  );
>  
> +# Functions to convert values from older gitweb configuration
> +# into the current data format
> +sub gitweb_bc_feature_snapshot {
> +	my $def = $feature{'snapshot'}{'default'};
> +	# Older definition was to have either undef (to disable), or
> +	# a three-element array whose first element was content encoding
> +	# without leading "application/".
> +	return if (ref $def ne 'ARRAY');
> +	if (!defined $def->[0] && @$def == 1) {
> +		# Disabled -- the new way to spell it is to have an empty
> +		# arrayref.
> +		$feature{'snapshot'}{'default'} = [];
> +		return;
> +	}
> +	return if (@$def != 3);
> +	for ($def->[0]) {
> +		if (/x-gzip/) {
> +			$feature{'snapshot'}{'default'} = ['tgz'];
> +		}
> +		if (/x-bz2/) {
> +			$feature{'snapshot'}{'default'} = ['tbz2'];
> +		}
> +		if (/x-zip/) {
> +			$feature{'snapshot'}{'default'} = ['zip'];
> +		}
> +	}
> +}
> +
>  sub gitweb_check_feature {
>  	my ($name) = @_;
>  	return unless exists $feature{$name};
> +	eval "gitweb_bc_feature_$name()";
> +
>  	my ($sub, $override, @defaults) = (
>  		$feature{$name}{'sub'},
>  		$feature{$name}{'override'},
> 
> 

^ permalink raw reply

* Re: [REVISED PATCH 2/6] Introduce commit notes
From: Linus Torvalds @ 2007-07-19  3:28 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, Alberto Bertogli, git, Johan Herland
In-Reply-To: <Pine.LNX.4.64.0707190258550.14781@racer.site>



On Thu, 19 Jul 2007, Johannes Schindelin wrote:
>
> There is one severe shortcoming, though.  Since tree objects can
> contain file names of a variable length, it is not possible to
> do a binary search for the correct base name in the tree object's
> contents.

Well, I've been thinking about this, and that's not really entirely 
correct.

It *is* possible to do a binary search, it's just a bit complicated, 
because you have to take the "halfway" thing, and find the beginning of 
an entry.

But the good news is that the tree entries have a very fixed format, and 
one that is actually amenable to finding where they start. It gets a bit 
complicated, but:

 - SHA1's contain random bytes, so we cannot really depend on their 
   content. Fair enough. But on the other hand, they are fixed length, 
   which means..

 - Each SHA1 is always preceded by a zero byte (it is what separates the 
   filename from the SHA1), and while filenames too can have arbitrary 
   content (and arbitrary length), we know that the *filename* doesn't 
   have a zero byte in it.

 - so finding the beginning of a tree entry should be as easy as finding 
   two zero bytes that are have at least 20 bytes in between them, and 
   then you *know* that the second zero byte is the one that starts a new 
   SHA1 (it cannot be _inside_ a SHA1: if it was, there would be less 
   than twenty bytes to the previous '\0', and it cannot be inside the
   filename either).

 - And you know that 20 bytes after that '\0' is the next tree entry!

Now, what does this mean? It means that if we actually know the filename 
we're looking for, and we're looking at a large range, we really *could* 
start out with binary searching. We would do something like this:

	unsigned char *start;
	unsigned long size;

	while (size > 200) {
		/*
		 * Look halfway, and then back up a bit, because we 
		 * expect it to take us about 20 characters to find
		 * the zero we look for, and an additional 20
		 * characters is the subsequent SHA1.
		 */
		unsigned long guess = size / 2 - 40;

		/*
		 * This is the offset past which a zero means that 
		 * we're good. If we don't find a zero in the first
		 * twenty bytes, that means that the first zero we
		 * find must be the beginning of a SHA1!
		 */
		unsigned long goal_zero = guess + 20;

		for (;;) {
			unsigned char c;

			/*
			 * We need at least 22 characters more: the
			 * '\0' and the SHA1, and then the next entry.
			 * We know the ASCII mode is 4 characters, so
			 * we migth as well make the rule "within 26 of
			 * end end".
			 */
			if (guess >= size-26)
				goto fall_back_to_linear_search;
			c = start[guess++];
			if (c)
				continue;
			/* Found it? */
			if (guess > goal_zero)
				break;
			/*
			 * We found a zero that wasn't 20 bytes away, 
			 * that means we have to reset out goal..
			 */
			last_zero = guess + 20;
		}
		/*
		 * "guess" now points to one past the '\0': the SHA1 of
		 * the previous entry. Add 20, and it points at the start
		 * of a valid tree entry.
		 */
		guess = guess + 20;

		/* Length of the entry: ascii string + '\0' + SHA1 */
		thisentrylen = strlen(start + guess) + 1 + 20;

		.. compare the entry we found with
		.. the entry we are looking for!
		if (found < lookedfor) {
			size = guess;
			continue;
		} else if (found == lookedfor) {
			Yay! FOUND!
		} else {
			guess += thisentry;
			size -= guess;
			start += guess;
			continue;
		}
	}
  fall_back_to_linear_search:

	.. linear search in [ start, size ] ..


Anyway, as you can tell, the above is totally untested, but I really think 
it should work. Whether it really helps, I dunno. But if somebody is 
interested in trying, it might be cool to see.

And yes, the "search for zero bytes" is not *guaranteed* to find any 
beginning at all, if you have lots of short names, *and* lots of zero 
bytes in the SHA1's. But while short names may be common, zero bytes in 
SHA1's are not so much (since you should expect to see a very even 
distribution of bytes, and as such most SHA1's by far should have no zero 
bytes at all!)

So if you're really really *really* unlucky, you might end up having to 
fall back on the linear search. But it still works!

Can anybody see anything wrong in my thinking above?

(And the real question is whether it really helps. I suspect it does 
actually help for big directories, and that it is worth doing, but maybe 
the magic number in "while (size > 200)" could be tweaked.

The logic of that was that binary searching doesn't work very well for 
just a few entries (and "size < 200" implies ~5-6 directory entries), but 
also that linear search is actually perfectly good when it's just a couple 
of cache-lines, and binary searching - especially with the complication of 
having to find the beginning - isn't worth it unless it really means that 
we can avoid a cache miss.

Of course, it may well be that the *real* cost of the directories is just 
the uncompression thing, and that the search is not the problem. Who 
knows? 

			Linus

^ permalink raw reply

* Re: [REVISED PATCH 3/6] Add git-notes
From: Johannes Schindelin @ 2007-07-19  2:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Alberto Bertogli, git, Johan Herland
In-Reply-To: <Pine.LNX.4.64.0707190331050.14781@racer.site>

Hi,

On Thu, 19 Jul 2007, Johannes Schindelin wrote:

> +MESSAGE="$GIT_DIR"/new-notes
> +trap '
> +	test -f "$MESSAGE" && rm "$MESSAGE"
> +' 0

Oh, well.  Probably this should use mktemp and should handle 
GIT_INDEX_FILE, too.

Will do that tomorrow.

Ciao,
Dscho

^ permalink raw reply

* [REVISED PATCH 3/6] Add git-notes
From: Johannes Schindelin @ 2007-07-19  2:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Alberto Bertogli, git, Johan Herland
In-Reply-To: <7v8x9h6igv.fsf@assigned-by-dhcp.cox.net>


This script allows you to edit and show commit notes easily.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 .gitignore   |    1 +
 Makefile     |    2 +-
 git-notes.sh |   62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 64 insertions(+), 1 deletions(-)
 create mode 100755 git-notes.sh

diff --git a/.gitignore b/.gitignore
index 20ee642..125613f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -83,6 +83,7 @@ git-mktag
 git-mktree
 git-name-rev
 git-mv
+git-notes
 git-pack-redundant
 git-pack-objects
 git-pack-refs
diff --git a/Makefile b/Makefile
index 119d949..10a9342 100644
--- a/Makefile
+++ b/Makefile
@@ -213,7 +213,7 @@ SCRIPT_SH = \
 	git-merge-resolve.sh git-merge-ours.sh \
 	git-lost-found.sh git-quiltimport.sh git-submodule.sh \
 	git-filter-branch.sh \
-	git-stash.sh
+	git-stash.sh git-notes.sh
 
 SCRIPT_PERL = \
 	git-add--interactive.perl \
diff --git a/git-notes.sh b/git-notes.sh
new file mode 100755
index 0000000..031e911
--- /dev/null
+++ b/git-notes.sh
@@ -0,0 +1,62 @@
+#!/bin/sh
+
+USAGE="(edit | show) [commit]"
+. git-sh-setup
+
+test -n "$3" && usage
+
+test -z "$GIT_NOTES_REF" && GIT_NOTES_REF="$(git config core.notesref)"
+test -z "$GIT_NOTES_REF" && GIT_NOTES_REF="refs/notes/commits"
+
+COMMIT=$(git rev-parse --verify --default HEAD "$2") || die "Invalid ref: $2"
+NAME=$(echo $COMMIT | sed "s/^../&\//")
+
+MESSAGE="$GIT_DIR"/new-notes
+trap '
+	test -f "$MESSAGE" && rm "$MESSAGE"
+' 0
+
+case "$1" in
+edit)
+	GIT_NOTES_REF= git log -1 $COMMIT | sed "s/^/#/" > "$MESSAGE"
+
+	GIT_INDEX_FILE="$MESSAGE".idx
+	export GIT_INDEX_FILE
+
+	CURRENT_HEAD=$(git show-ref "$GIT_NOTES_REF" | cut -f 1 -d ' ')
+	if [ -z "$CURRENT_HEAD" ]; then
+		PARENT=
+	else
+		PARENT="-p $CURRENT_HEAD"
+		git read-tree "$GIT_NOTES_REF" || die "Could not read index"
+		git cat-file blob :$NAME >> "$MESSAGE" 2> /dev/null
+	fi
+
+	${VISUAL:-${EDITOR:-vi}} "$MESSAGE"
+
+	grep -v ^# < "$MESSAGE" | git stripspace > "$MESSAGE".processed
+	mv "$MESSAGE".processed "$MESSAGE"
+	if [ -s "$MESSAGE" ]; then
+		BLOB=$(git hash-object -w "$MESSAGE") ||
+			die "Could not write into object database"
+		git update-index --add --cacheinfo 0644 $BLOB $NAME ||
+			die "Could not write index"
+	else
+		test -z "$CURRENT_HEAD" &&
+			die "Will not initialise with empty tree"
+		git update-index --force-remove $NAME ||
+			die "Could not update index"
+	fi
+
+	TREE=$(git write-tree) || die "Could not write tree"
+	NEW_HEAD=$(echo Annotate $COMMIT | git commit-tree $TREE $PARENT) ||
+		die "Could not annotate"
+	git update-ref -m "Annotate $COMMIT" \
+		"$GIT_NOTES_REF" $NEW_HEAD $CURRENT_HEAD
+;;
+show)
+	git show "$GIT_NOTES_REF":$NAME
+;;
+*)
+	usage
+esac
-- 
1.5.3.rc1.16.g9d6f-dirty

^ permalink raw reply related

* [REVISED PATCH 4/6] Add a test script for "git notes"
From: Johannes Schindelin @ 2007-07-19  2:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Alberto Bertogli, git, Johan Herland
In-Reply-To: <7v3azp6igt.fsf@assigned-by-dhcp.cox.net>


Incidentally, a test for "git notes" implies a test for the
whole commit notes machinery.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t3301-notes.sh |   65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 65 insertions(+), 0 deletions(-)
 create mode 100755 t/t3301-notes.sh

diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
new file mode 100755
index 0000000..ba42c45
--- /dev/null
+++ b/t/t3301-notes.sh
@@ -0,0 +1,65 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Johannes E. Schindelin
+#
+
+test_description='Test commit notes'
+
+. ./test-lib.sh
+
+cat > fake_editor.sh << \EOF
+echo "$MSG" > "$1"
+echo "$MSG" >& 2
+EOF
+chmod a+x fake_editor.sh
+VISUAL=./fake_editor.sh
+export VISUAL
+
+test_expect_success 'cannot annotate non-existing HEAD' '
+	! MSG=3 git notes edit
+'
+
+test_expect_success setup '
+	: > a1 &&
+	git add a1 &&
+	test_tick &&
+	git commit -m 1st &&
+	: > a2 &&
+	git add a2 &&
+	test_tick &&
+	git commit -m 2nd
+'
+
+test_expect_success 'need valid notes ref' '
+	! MSG=1 GIT_NOTES_REF='/' git notes edit &&
+	! MSG=2 GIT_NOTES_REF='/' git notes show
+'
+
+test_expect_success 'create notes' '
+	git config core.notesRef refs/notes/commits &&
+	MSG=b1 git notes edit &&
+	test ! -f .git/new-notes &&
+	test 1 = $(git ls-tree refs/notes/commits | wc -l) &&
+	test b1 = $(git notes show) &&
+	git show HEAD^ &&
+	! git notes show HEAD^
+'
+
+cat > expect << EOF
+commit 268048bfb8a1fb38e703baceb8ab235421bf80c5
+Author: A U Thor <author@example.com>
+Date:   Thu Apr 7 15:14:13 2005 -0700
+
+    2nd
+
+Notes:
+    b1
+EOF
+
+test_expect_success 'show notes' '
+	! (git cat-file commit HEAD | grep b1) &&
+	git log -1 > output &&
+	git diff expect output
+'
+
+test_done
-- 
1.5.3.rc1.16.g9d6f-dirty

^ permalink raw reply related

* [REVISED PATCH 2/6] Introduce commit notes
From: Johannes Schindelin @ 2007-07-19  2:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Alberto Bertogli, git, Johan Herland
In-Reply-To: <7vejj96igx.fsf@assigned-by-dhcp.cox.net>


Commit notes are blobs which are shown together with the commit
message.  These blobs are taken from the notes ref, which you can
configure by the config variable core.notesRef, which in turn can
be overridden by the environment variable GIT_NOTES_REF.

The notes ref is a branch which contains trees much like the
loose object trees in .git/objects/.  In other words, to get
at the commit notes for a given SHA-1, take the first two
hex characters as directory name, and the remaining 38 hex
characters as base name, and look that up in the notes ref.

The rationale for putting this information into a ref is this: we
want to be able to fetch and possibly union-merge the notes,
maybe even look at the date when a note was introduced, and we
want to store them efficiently together with the other objects.

There is one severe shortcoming, though.  Since tree objects can
contain file names of a variable length, it is not possible to
do a binary search for the correct base name in the tree object's
contents.  Therefore this approach does not scale well, because
the average lookup time will be proportional to the number of
commit objects, and therefore the slowdown will be quadratic in
that number.

However, a remedy is near: in a later commit, a .git/notes-index
will be introduced, a cached mapping from commits to commit notes,
to be written when the tree name of the notes ref changes.  In
case that notes-index cannot be written, the current (possibly
slow) code will come into effect again.

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

	On Sun, 15 Jul 2007, Junio C Hamano wrote:

	> This design forces "one blob and only one blob decorates a
	> commit".  It certainly makes the implementation and semantics
	> simpler -- if I have this note and you have that note on the
	> same commit, comparing notes eventually should result in a merge
	> of our notes.  But is it sufficient in real life usage scenarios
	> (what's the use case)?  One example that was raised on the list
	> is to collect "Acked-by", "Tested-by", etc., and in that case
	> perhaps one set "refs/notes/acks" may hold the former while
	> "refs/notes/tests" the latter.  If we wanted to show both at the
	> same time, is it the only option to put them in the same "note"
	> blob and not use "refs/notes/{acks,tests}"?

	Would that not make things even slower?  I am hesitant.

	All other concerns should be addressed, here and in the two 
	upcoming revised patches.

 Documentation/config.txt |   15 +++++++++
 Makefile                 |    3 +-
 cache.h                  |    3 ++
 commit.c                 |    5 +++
 config.c                 |    5 +++
 environment.c            |    1 +
 notes.c                  |   77 ++++++++++++++++++++++++++++++++++++++++++++++
 notes.h                  |    8 +++++
 8 files changed, 116 insertions(+), 1 deletions(-)
 create mode 100644 notes.c
 create mode 100644 notes.h

diff --git a/Documentation/config.txt b/Documentation/config.txt
index d0e9a17..5fe833d 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -285,6 +285,21 @@ core.pager::
 	The command that git will use to paginate output.  Can be overridden
 	with the `GIT_PAGER` environment variable.
 
+core.notesRef::
+	When showing commit messages, also show notes which are stored in
+	the given ref.  This ref is expected to contain paths of the form
+	??/*, where the directory name consists of the first two
+	characters of the commit name, and the base name consists of
+	the remaining 38 characters.
++
+If such a path exists in the given ref, the referenced blob is read, and
+appended to the commit message, separated by a "Notes:" line.  If the
+given ref itself does not exist, it is not an error, but means that no
+notes should be print.
++
+This setting defaults to "refs/notes/commits", and can be overridden by
+the `GIT_NOTES_REF` environment variable.
+
 alias.*::
 	Command aliases for the gitlink:git[1] command wrapper - e.g.
 	after defining "alias.last = cat-file commit HEAD", the invocation
diff --git a/Makefile b/Makefile
index d7541b4..119d949 100644
--- a/Makefile
+++ b/Makefile
@@ -322,7 +322,8 @@ LIB_OBJS = \
 	write_or_die.o trace.o list-objects.o grep.o match-trees.o \
 	alloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \
 	color.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \
-	convert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o
+	convert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o \
+	notes.o
 
 BUILTIN_OBJS = \
 	builtin-add.o \
diff --git a/cache.h b/cache.h
index 328c1ad..df45336 100644
--- a/cache.h
+++ b/cache.h
@@ -204,6 +204,8 @@ enum object_type {
 #define GITATTRIBUTES_FILE ".gitattributes"
 #define INFOATTRIBUTES_FILE "info/attributes"
 #define ATTRIBUTE_MACRO_PREFIX "[attr]"
+#define GIT_NOTES_REF_ENVIRONMENT "GIT_NOTES_REF"
+#define GIT_NOTES_DEFAULT_REF "refs/notes/commits"
 
 extern int is_bare_repository_cfg;
 extern int is_bare_repository(void);
@@ -309,6 +311,7 @@ extern size_t packed_git_window_size;
 extern size_t packed_git_limit;
 extern size_t delta_base_cache_limit;
 extern int auto_crlf;
+extern char *notes_ref_name;
 
 #define GIT_REPO_VERSION 0
 extern int repository_format_version;
diff --git a/commit.c b/commit.c
index 0c350bc..8911a18 100644
--- a/commit.c
+++ b/commit.c
@@ -6,6 +6,7 @@
 #include "interpolate.h"
 #include "diff.h"
 #include "revision.h"
+#include "notes.h"
 
 int save_commit_buffer = 1;
 
@@ -1254,6 +1255,10 @@ unsigned long pretty_print_commit(enum cmit_fmt fmt,
 	 */
 	if (fmt == CMIT_FMT_EMAIL && offset <= beginning_of_body)
 		buf[offset++] = '\n';
+
+	if (fmt != CMIT_FMT_ONELINE)
+		get_commit_notes(commit, buf_p, &offset, space_p, encoding);
+
 	buf[offset] = '\0';
 	free(reencoded);
 	return offset;
diff --git a/config.c b/config.c
index f89a611..05d2ad6 100644
--- a/config.c
+++ b/config.c
@@ -395,6 +395,11 @@ int git_default_config(const char *var, const char *value)
 		return 0;
 	}
 
+	if (!strcmp(var, "core.notesref")) {
+		notes_ref_name = xstrdup(value);
+		return 0;
+	}
+
 	if (!strcmp(var, "user.name")) {
 		strlcpy(git_default_name, value, sizeof(git_default_name));
 		return 0;
diff --git a/environment.c b/environment.c
index f83fb9e..2e677d3 100644
--- a/environment.c
+++ b/environment.c
@@ -34,6 +34,7 @@ char *pager_program;
 int pager_in_use;
 int pager_use_color = 1;
 int auto_crlf = 0;	/* 1: both ways, -1: only when adding git objects */
+char *notes_ref_name;
 
 static const char *git_dir;
 static char *git_object_dir, *git_index_file, *git_refs_dir, *git_graft_file;
diff --git a/notes.c b/notes.c
new file mode 100644
index 0000000..6207f95
--- /dev/null
+++ b/notes.c
@@ -0,0 +1,77 @@
+#include "cache.h"
+#include "commit.h"
+#include "notes.h"
+#include "refs.h"
+#include "utf8.h"
+
+static int initialized;
+
+void get_commit_notes(const struct commit *commit,
+		char **buf_p, unsigned long *offset_p, unsigned long *space_p,
+		const char *output_encoding)
+{
+        static const char *utf8 = "utf-8";
+	char name[80];
+	const char *hex;
+	unsigned char sha1[20];
+	char *msg;
+	unsigned long msgoffset, msglen;
+	enum object_type type;
+
+	if (!initialized) {
+		const char *env = getenv(GIT_NOTES_REF_ENVIRONMENT);
+		if (env)
+			notes_ref_name = getenv(GIT_NOTES_REF_ENVIRONMENT);
+		else if (!notes_ref_name)
+			notes_ref_name = GIT_NOTES_DEFAULT_REF;
+		if (notes_ref_name && read_ref(notes_ref_name, sha1))
+			notes_ref_name = NULL;
+		initialized = 1;
+	}
+	if (!notes_ref_name)
+		return;
+
+	hex = sha1_to_hex(commit->object.sha1);
+	if (snprintf(name, sizeof(name), "%s:%.*s/%.*s",
+			notes_ref_name, 2, hex, 38, hex + 2)
+			>= sizeof(name) - 1) {
+		error("Notes ref name too long: %.*s", 60, notes_ref_name);
+		return;
+	}
+	if (get_sha1(name, sha1))
+		return;
+
+	if (!(msg = read_sha1_file(sha1, &type, &msglen)) || !msglen ||
+			type != OBJ_BLOB)
+		return;
+        if (output_encoding && *output_encoding &&
+			strcmp(utf8, output_encoding)) {
+                char *reencoded = reencode_string(msg, output_encoding, utf8);
+		if (reencoded) {
+			free(msg);
+			msg = reencoded;
+			msglen = strlen(msg);
+		}
+	}
+	/* we will end the annotation by a newline anyway. */
+	if (msg[msglen - 1] == '\n')
+		msglen--;
+
+	ALLOC_GROW(*buf_p, *offset_p + 8 + msglen, *space_p);
+	*offset_p += sprintf(*buf_p + *offset_p, "\nNotes:\n");
+
+	for (msgoffset = 0; msgoffset < msglen;) {
+		int linelen = get_line_length(msg + msgoffset, msglen);
+
+		ALLOC_GROW(*buf_p, *offset_p + linelen + 5, *space_p);
+		*offset_p += sprintf(*buf_p + *offset_p,
+				"    %.*s", linelen, msg + msgoffset);
+		msgoffset += linelen;
+	}
+	ALLOC_GROW(*buf_p, *offset_p + 1, *space_p);
+	(*buf_p)[*offset_p] = '\n';
+	(*offset_p)++;
+	free(msg);
+}
+
+
diff --git a/notes.h b/notes.h
new file mode 100644
index 0000000..fe8f209
--- /dev/null
+++ b/notes.h
@@ -0,0 +1,8 @@
+#ifndef NOTES_H
+#define NOTES_H
+
+void get_commit_notes(const struct commit *commit,
+	char **buf_p, unsigned long *offset_p, unsigned long *space_p,
+	const char *output_encoding);
+
+#endif
-- 
1.5.3.rc1.16.g9d6f-dirty

^ permalink raw reply related

* [REVISED PATCH 1/2] filter-branch: provide the convenience functions also for commit filters
From: Johannes Schindelin @ 2007-07-19  1:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vfy3l5jo4.fsf@assigned-by-dhcp.cox.net>


By sourcing git-filter-branch and stopping after the function definitions,
the commit filter can now access the convenience functions like "map".

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

	On Wed, 18 Jul 2007, Junio C Hamano wrote:

	> Exporting this_script variable, and changing the above to
	> 
	> 	filter_commit='SOURCE_FUNCTIONS=1 . "$this_script";'" $OPTARG"
	> 
	> to arrange the shell that is invoked with 'sh -c' to expand its 
	> value would make it smaller problem, I suspect.

	Hereby done.

	<shameless plug>rebase -i rocks</shameless>

 git-filter-branch.sh |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 0d000ed..3113937 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -8,9 +8,6 @@
 # a new branch. You can specify a number of filters to modify the commits,
 # files and trees.
 
-USAGE="git-filter-branch [-d TEMPDIR] [FILTERS] DESTBRANCH [REV-RANGE]"
-. git-sh-setup
-
 warn () {
         echo "$*" >&2
 }
@@ -69,6 +66,14 @@ set_ident () {
 	echo "[ -n \"\$GIT_${uid}_NAME\" ] || export GIT_${uid}_NAME=\"\${GIT_${uid}_EMAIL%%@*}\""
 }
 
+# This script can be sourced by the commit filter to get the functions
+test "a$SOURCE_FUNCTIONS" = a1 && return
+this_script="$(cd "$(dirname "$0")"; pwd)"/$(basename "$0")
+export this_script
+
+USAGE="git-filter-branch [-d TEMPDIR] [FILTERS] DESTBRANCH [REV-RANGE]"
+. git-sh-setup
+
 tempdir=.git-rewrite
 filter_env=
 filter_tree=
@@ -118,7 +123,7 @@ do
 		filter_msg="$OPTARG"
 		;;
 	--commit-filter)
-		filter_commit="$OPTARG"
+		filter_commit='SOURCE_FUNCTIONS=1 . "$this_script";'" $OPTARG"
 		;;
 	--tag-name-filter)
 		filter_tag_name="$OPTARG"
-- 
1.5.3.rc1.16.g9d6f-dirty

^ permalink raw reply related

* [PATCH] Force listingblocks to be monospaced in manpages
From: Julian Phillips @ 2007-07-18 21:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Fredrik Tolf, git
In-Reply-To: <Pine.LNX.4.64.0707190157430.1964@beast.quantumfyre.co.uk>

For the html output we can use a stylesheet to make sure that the
listingblocks are presented in a monospaced font.  For the manpages do
it manually by inserting a ".ft C" before and ".ft" after the block in
question.  This makes the ascii-art diagrams readable in PS output.

In order for these roff commands to get through to the manpage they
have to be element encoded to prevent quoting.  In particular with
docbook xsl 1.72.0 and newer we have to use U+2302 instead of . to
prevent the roff command being escaped.  We also add a small perl
script for docbook < 1.72.0.

Signed-off-by: Julian Phillips <julian@quantumfyre.co.uk>
---

On Thu, 19 Jul 2007, Julian Phillips wrote:

> On Wed, 18 Jul 2007, Junio C Hamano wrote:
>
>>  I tried with your patch, both with asciidoc7 and asciidoc8.  Did
>>  you really mean "&#x2302;" above?  Replacing them with "."  gave
>>  me a series of these changes (diff between output before and
>>  after your patch with the "s/\&#x2302;/./g" fixup):
>
> I did mean it.  I originally just had .ft, but I was getting \&.ft in the
> manpage, which then just came out as .ft in the console.
>
> I got the &#x2302; from
> http://docbook.sourceforge.net/release/xsl/current/manpages/utility.xsl, so I
> assumed it was the "correct" thing to use ...
>
> This was with asciidoc 7 and docbook xsl stylesheet 1.72.0.
>
>
>>  whatever that 2302 is...
>
> &#x2302; (or U+2302) seems to be a character from the unicode "Misc.
> Technical" section ... looks a bit like a house.
>
> See sixth bullet from
> http://docbook.sourceforge.net/release/xsl/current/RELEASE-NOTES.html#V1.72.0$
>
> looks like it may need to depend on which docbook xsl version you are using
> ...
>
>

I couldn't find any way to detect the docbook version - perhaps someone more
with more knowledge of asciidoc might know?

Otherwise, something like this perhaps?

 Documentation/Makefile          |    3 +++
 Documentation/asciidoc.conf     |    6 ++++++
 Documentation/replace_U+2303.pl |    6 ++++++
 3 files changed, 15 insertions(+), 0 deletions(-)
 create mode 100755 Documentation/replace_U+2303.pl

diff --git a/Documentation/Makefile b/Documentation/Makefile
index b062757..e381b2e 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -121,6 +121,9 @@ clean:
 
 %.1 %.5 %.7 : %.xml
 	xmlto -m callouts.xsl man $<
+	mv $@ $@.tmp
+	./replace_U+2303.pl < $@.tmp > $@
+	$(RM) $@.tmp
 
 %.xml : %.txt
 	$(RM) $@+ $@
diff --git a/Documentation/asciidoc.conf b/Documentation/asciidoc.conf
index 6b6220d..d54fe29 100644
--- a/Documentation/asciidoc.conf
+++ b/Documentation/asciidoc.conf
@@ -27,7 +27,13 @@ ifdef::backend-docbook[]
 [listingblock]
 <example><title>{title}</title>
 <literallayout>
+ifdef::doctype-manpage[]
+&#10;&#x2302;ft C&#10;
+endif::doctype-manpage[]
 |
+ifdef::doctype-manpage[]
+&#10;&#x2302;ft&#10;
+endif::doctype-manpage[]
 </literallayout>
 {title#}</example>
 endif::backend-docbook[]
diff --git a/Documentation/replace_U+2303.pl b/Documentation/replace_U+2303.pl
new file mode 100755
index 0000000..b086949
--- /dev/null
+++ b/Documentation/replace_U+2303.pl
@@ -0,0 +1,6 @@
+#!/usr/bin/perl -w
+
+while ($line = <>) {
+	$line =~ s/^\x{2302}/./;
+	print $line;
+}
-- 
1.5.2.2

^ permalink raw reply related

* Re: [PATCH 0/6] Introduce commit notes
From: Johannes Schindelin @ 2007-07-19  1:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Andy Parkins, git, Alberto Bertogli, Johan Herland
In-Reply-To: <7vzm1w2pwk.fsf@assigned-by-dhcp.cox.net>

Hi,

On Mon, 16 Jul 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> Hmph, you are right.  In this sequence:
> >> 
> >> 	hex = sha1_to_hex(commit->object.sha1);
> >> 	snprintf(name, sizeof(name), "%s:%.*s/%.*s",
> >> 			notes_ref_name, 2, hex, 38, hex + 2);
> >> 	if (get_sha1(name, sha1))
> >> 		return;
> >> 
> >> Instead, we could read the tree object by hand in the commit that is 
> >> referenced by notes_ref_name, which has uniform two letter names for 
> >> subtrees which can be binary searched, open the tree for that entry, 
> >> again by hand, and do another binary search because that tree has 
> >> uniform 38-letter names.  That certainly could be done.
> >> 
> >> Sounds like a "fun" project for some definition of the word.
> >
> > I disagree.  One disadvantage to using tree objects is that it is much 
> > easier to have pilot errors.  You could even make a new working tree 
> > checking out refs/notes/commits and change/add/remove files.
> 
> I suspect you read me wrong.  I was saying that it is possible to use a 
> specialized tree object parser in place of get_sha1() only in the above 
> code to read the tree objects that represents a 'note'.  You obviously 
> would want to do a sanity check such as:
> 
>  - The size of the tree object your customized tree parser is
>    fed is multiple of expected entry size (mode word + 20 SHA1 +
>    2 + NUL for fan-out, replace 2 with 38 for lower level);
> 
>  - mode word for the entry is sane (an entry in the fan-out tree
>    would point at a tree object, an entry in lower level would
>    point at a blob);
> 
>  - The name part (2 or 38) are lowercase hexadecimal strings;

In which case it is not _that_ attractive any more, since you

- have to have a fallback anyway, and

- have a relatively complex thing.

Instead, I want to go with the hash map approach, if only to have a O(1) 
behaviour instead of O(log N).

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Force listingblocks to be monospaced in manpages
From: Julian Phillips @ 2007-07-19  1:21 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Fredrik Tolf, git
In-Reply-To: <7vr6n55krx.fsf@assigned-by-dhcp.cox.net>

On Wed, 18 Jul 2007, Junio C Hamano wrote:

> I tried with your patch, both with asciidoc7 and asciidoc8.  Did
> you really mean "&#x2302;" above?  Replacing them with "."  gave
> me a series of these changes (diff between output before and
> after your patch with the "s/\&#x2302;/./g" fixup):

I did mean it.  I originally just had .ft, but I was getting \&.ft in the 
manpage, which then just came out as .ft in the console.

I got the &#x2302; from 
http://docbook.sourceforge.net/release/xsl/current/manpages/utility.xsl, 
so I assumed it was the "correct" thing to use ...

This was with asciidoc 7 and docbook xsl stylesheet 1.72.0.


> whatever that 2302 is...

&#x2302; (or U+2302) seems to be a character from the unicode "Misc. 
Technical" section ... looks a bit like a house.

See sixth bullet from 
http://docbook.sourceforge.net/release/xsl/current/RELEASE-NOTES.html#V1.72.0_Manpages

looks like it may need to depend on which docbook xsl version you are 
using ...

-- 
Julian

  ---
The only way to amuse some people is to slip and fall on an icy pavement.

^ permalink raw reply

* Re: [PATCH 1/2] filter-branch: provide the convenience functions also for commit filters
From: Johannes Schindelin @ 2007-07-19  1:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vfy3l5jo4.fsf@assigned-by-dhcp.cox.net>

Hi,

On Wed, 18 Jul 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > +this_script="$(cd "$(dirname "$0")"; pwd)"/$(basename "$0")
> > ...
> > +		filter_commit="SOURCE_FUNCTIONS=1 . \"$this_script\"; $OPTARG"
> 
> Hmmmmmm.
> 
> Care to enlighten why this is not just:
> 
> 	filter_commit="SOURCE_FUNCTIONS=1 . \"$0\"; $OPTARG"
> 
> Is it because you cd(1) around in the script, and it can be
> relative to where you started?

Yes.

> In either case, are you quoting potential funnies (such as '"'
> or '\\') in "$0" sufficiently?  Exporting this_script variable,
> and changing the above to
> 
> 	filter_commit='SOURCE_FUNCTIONS=1 . "$this_script";'" $OPTARG"
> 
> to arrange the shell that is invoked with 'sh -c' to expand its
> value would make it smaller problem, I suspect.

Will do.  You know, I do have my problems with correct quoting, and the 
way I did it in this patch was always good enough for me.  Which does not 
mean much...

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats
From: Junio C Hamano @ 2007-07-19  1:12 UTC (permalink / raw)
  To: Matt McCutchen, Jakub Narebski; +Cc: git, Petr Baudis, Luben Tuikov
In-Reply-To: <200707190140.05235.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

> On Tue, 17 July 2007, Matt McCutchen napisał:
> ...
>> Alert for gitweb site administrators: This patch changes the format of
>> $feature{'snapshot'}{'default'} in gitweb_config.perl from a list of
>> three pieces of information about a single format to a list of one or
>> more formats you wish to offer from the set ('tgz', 'tbz2', 'zip').
>> Update your gitweb_config.perl appropriately.  The preferred names for
>> gitweb.snapshot in repository configuration have also changed from
>> 'gzip' and 'bzip2' to 'tgz' and 'tbz2', but the old names are still
>> recognized for compatibility.
>
> This alert/warning should probably be put in RelNotes for when it would
> be in git.git

Does anybody else worry about the backward imcompatibility, I
wonder...  List?

I really hate to having to say something like that in the
RelNotes.  I do not think this is a good enough reason to break
existing configurations; I would not want to be defending that
change.

>> I thought of another incompatibility: previously bookmarked snapshot
>> URLs will no longer work because they lack the new "sf" parameter.  I
>> don't care about this; do any of you?
>
> I think either having good error message, or using first format avaiable
> would be good enough.

I doubt bookmarked snapshot URL would make sense to begin with,
so this would be Ok.

I am wondering if something like this patch (totally untested,
mind you) to convert the old style %feature in configuration at
the site at runtime would be sufficient.

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index f17c983..cdec4d0 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -236,9 +236,39 @@ our %feature = (
 		'default' => [0]},
 );
 
+# Functions to convert values from older gitweb configuration
+# into the current data format
+sub gitweb_bc_feature_snapshot {
+	my $def = $feature{'snapshot'}{'default'};
+	# Older definition was to have either undef (to disable), or
+	# a three-element array whose first element was content encoding
+	# without leading "application/".
+	return if (ref $def ne 'ARRAY');
+	if (!defined $def->[0] && @$def == 1) {
+		# Disabled -- the new way to spell it is to have an empty
+		# arrayref.
+		$feature{'snapshot'}{'default'} = [];
+		return;
+	}
+	return if (@$def != 3);
+	for ($def->[0]) {
+		if (/x-gzip/) {
+			$feature{'snapshot'}{'default'} = ['tgz'];
+		}
+		if (/x-bz2/) {
+			$feature{'snapshot'}{'default'} = ['tbz2'];
+		}
+		if (/x-zip/) {
+			$feature{'snapshot'}{'default'} = ['zip'];
+		}
+	}
+}
+
 sub gitweb_check_feature {
 	my ($name) = @_;
 	return unless exists $feature{$name};
+	eval "gitweb_bc_feature_$name()";
+
 	my ($sub, $override, @defaults) = (
 		$feature{$name}{'sub'},
 		$feature{$name}{'override'},

^ permalink raw reply related

* Re: git-p4 pull request
From: Junio C Hamano @ 2007-07-19  0:25 UTC (permalink / raw)
  To: Simon Hausmann; +Cc: git
In-Reply-To: <200707181742.12046.simon@lst.de>

Thanks, pulled (but not pushed out yet).

^ permalink raw reply

* Re: [RFC PATCH] Re: Empty directories...
From: Linus Torvalds @ 2007-07-19  0:22 UTC (permalink / raw)
  To: David Kastrup
  Cc: Junio C Hamano, Matthieu Moy, Johannes Schindelin,
	Git Mailing List
In-Reply-To: <85abttwa7m.fsf@lola.goethe.zz>



On Thu, 19 Jul 2007, David Kastrup wrote:
> 
> Well, kudos.  Together with the analysis from Junio, this seems like a
> good start.  Would you have any recommendations about what stuff one
> should really read in order to get up to scratch about git internals?

Well, you do need to understand the index. That's where all the new 
subtlety happens.

The data structures themselves are trivial, and we've supported empty 
trees (at the top level) from the beginning, so that part is not anything 
new.

However, now having a new entry type in the index (S_IFDIR) means that 
anything that interacts with the index needs to think twice. But a lot of 
that is just testing what happens, and so the first thing to do is to have 
a test-suite.

There's also the question about how to show an empty tree in a diff. We've 
never had that: the only time we had empty trees was when we compared a 
totally empty "root" tree against another tree, and then it was obvious. 
But what if the empty tree is a subdirectory of another tree - how do you 
express that in a diff? Do you care? Right now, since we always recurse 
into the tree (and then not find anything), empty trees will simply not 
show up _at_all_ in any diffs.

And what about usability issues elsewhere? With my patch, doing something 
like a

	git add directory/

still won't do anything, because the behaviour of "git add" has always 
been to recurse into directories. So to add a new empty directory, you'd 
have to do

	git update-index --add directory

and that's not exactly user-friendly.

So do you add a "-n" flag to "git add" to tell it to not recurse? Or do 
you always recurse, but then if you notice that the end result is empty, 
you add it as a directory?

All the logic for that whole directory lookup is in git/dir.c, and that 
code takes various flags because different programs want different things 
(show "ignored" files, or ignore them? Show empty directories or ignore 
them? etc).

So primarily, I think the job is:

 - thinking about the index, and the interactions when adding a directory 
   or adding files under a directory that already exists.

   I *think* we get all the corner cases right, because they should be 
   exactly the same as with subprojects, but hey, maybe there's some piece 
   that tests S_ISGITLINK() and now needs a S_ISDIR() test too..

 - adding test cases

 - thinking about the user interfaces for this, and adding code to handle 
   directories where needed (eg the above "git add" issue).

 - thinking about merges (which is largely about the index too, but is a 
   whole 'nother set of issues, with multiple stages in the same index at 
   the same time)

It might all be trivial. The directory traversal already knows that empty 
directories are special, so getting the right behaviour to "git add" may 
be really really easy. Or maybe it's not. I think a lot of it is just 
finding what needs to be done, seeign if we already do it, and if not, 
seeign how to do it. Boring test-cases, in other words.

		Linus

^ permalink raw reply

* Re: Rsync fetch?
From: Johannes Schindelin @ 2007-07-19  0:21 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0707181226020.14596@iabervon.org>

Hi,

On Wed, 18 Jul 2007, Daniel Barkalow wrote:

> The only thing that's totally missing at this point from my 
> builtin-fetch is rsync. Do we still care? Any takers for actually 
> implementing it?

I'll have a go at it, but I will probably have time for that only on the 
weekend.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 1/2] filter-branch: provide the convenience functions also for commit filters
From: Junio C Hamano @ 2007-07-19  0:19 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <Pine.LNX.4.64.0707181650080.14781@racer.site>

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

> +this_script="$(cd "$(dirname "$0")"; pwd)"/$(basename "$0")
> ...
> +		filter_commit="SOURCE_FUNCTIONS=1 . \"$this_script\"; $OPTARG"

Hmmmmmm.

Care to enlighten why this is not just:

	filter_commit="SOURCE_FUNCTIONS=1 . \"$0\"; $OPTARG"

Is it because you cd(1) around in the script, and it can be
relative to where you started?

In either case, are you quoting potential funnies (such as '"'
or '\\') in "$0" sufficiently?  Exporting this_script variable,
and changing the above to

	filter_commit='SOURCE_FUNCTIONS=1 . "$this_script";'" $OPTARG"

to arrange the shell that is invoked with 'sh -c' to expand its
value would make it smaller problem, I suspect.

^ permalink raw reply

* Re: [PATCH] Force listingblocks to be monospaced in manpages
From: Junio C Hamano @ 2007-07-18 23:56 UTC (permalink / raw)
  To: Julian Phillips; +Cc: Fredrik Tolf, git
In-Reply-To: <20070718213725.31383.50523.julian@quantumfyre.co.uk>

Julian Phillips <julian@quantumfyre.co.uk> writes:

> For the html output we can use a stylesheet to make sure that the
> listingblocks are presented in a monospaced font.  For the manpages do
> it manually by inserting a ".ft C" before and ".ft" after the block in
> question.
>
> In order for these roff commands to get through to the manpage they
> have to be element encoded to prevent quoting.
>
> Signed-off-by: Julian Phillips <julian@quantumfyre.co.uk>
> ...
> How about this?
>
> Seems to work for me - but I'm not an asciidoc/docbook/roff expert ...
>
>  Documentation/asciidoc.conf |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/asciidoc.conf b/Documentation/asciidoc.conf
> index 6b6220d..d54fe29 100644
> --- a/Documentation/asciidoc.conf
> +++ b/Documentation/asciidoc.conf
> @@ -27,7 +27,13 @@ ifdef::backend-docbook[]
>  [listingblock]
>  <example><title>{title}</title>
>  <literallayout>
> +ifdef::doctype-manpage[]
> +&#10;&#x2302;ft C&#10;
> +endif::doctype-manpage[]
>  |
> +ifdef::doctype-manpage[]
> +&#10;&#x2302;ft&#10;
> +endif::doctype-manpage[]
>  </literallayout>
>  {title#}</example>
>  endif::backend-docbook[]
> -- 

I tried with your patch, both with asciidoc7 and asciidoc8.  Did
you really mean "&#x2302;" above?  Replacing them with "."  gave
me a series of these changes (diff between output before and
after your patch with the "s/\&#x2302;/./g" fixup):

        @@ -83,10 +83,13 @@
         .sp
         .RS 3n
         .nf
        +.ft C
             *** Commands ***
               1: status       2: update       3: revert       4: add untracked
               5: patch        6: diff         7: quit         8: help
             What now> 1
        +.ft
        +
         .fi
         .RE
         You also could say "s" or "sta" or "status" above as long as the choice is unique.

which seems reasonable, but I did not render them through roff.
WIth "&#x2302;" I was getting:

        @@ -83,10 +83,14 @@
         .sp
         .RS 3n
         .nf
        +
        +	ft C
             *** Commands ***
               1: status       2: update       3: revert       4: add untracked
               5: patch        6: diff         7: quit         8: help
             What now> 1
        +	ft
        +
         .fi
         .RE
         You also could say "s" or "sta" or "status" above as long as the choice is unique.

whatever that 2302 is...

^ permalink raw reply

* Re: [RFC PATCH] Re: Empty directories...
From: Linus Torvalds @ 2007-07-18 23:40 UTC (permalink / raw)
  To: Junio C Hamano, David Kastrup
  Cc: Matthieu Moy, Johannes Schindelin, Git Mailing List
In-Reply-To: <alpine.LFD.0.999.0707181557270.27353@woody.linux-foundation.org>



On Wed, 18 Jul 2007, Linus Torvalds wrote:
>
> +		if (!S_ISDIR(st->st_mode))
> +			return -1;
> +		ce->ce_mode = htonl(S_IFDIR);
> +		pretend_sha1_file(NULL, 0, OBJ_TREE, ce->sha1);

Oh, one word of warning: that whole "pretend_sha1_file()" thing won't 
create the object itself, and when I did the limited testing that I did, I 
actually made sure had a magic zero-sized tree object in my object 
directory.

If you don't, some things will complain, because they end up getting a 
SHA1 that they cannot look up, becasue *they* didn't create that pretend 
entry.

I didn't know which way I wanted to go with that thing. I was kind of 
thinking that maybe we would just have the zero-sized OBJ_BLOB and 
OBJ_TREE objects as special magical things, and have all git programs just 
do that "pretend" at the beginning.

But that kind of thing is probably just a totally unnecessary special 
case, and instead, that "pretend_sha1_file()" should have just been a

	write_sha1_file(NULL, 0, "tree", ce->sha1);

instead.

Anyway, if there are issues with not finding an object called 
4b825dc642cb6eb9a060e54bf8d69288fbee4904, then that's the empty tree 
object, and that pretend thing was the cause.

(The git repo itself has the empty tree as an object in it, because one of 
the commits has that - probably as a result of a bug, but there you have 
it)

		Linus

^ permalink raw reply

* Re: [RFC PATCH] Re: Empty directories...
From: David Kastrup @ 2007-07-18 23:42 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Junio C Hamano, Matthieu Moy, Johannes Schindelin,
	Git Mailing List
In-Reply-To: <alpine.LFD.0.999.0707181557270.27353@woody.linux-foundation.org>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> Gaah.
>
> I'm a damn softie (and soft in the head too, for writing the code).
>
> Ok, here's a trivial patch to start the ball rolling. I'm really not 
> interested in taking this patch any further personally, but I'm hoping 
> that maybe it can make somebody else who is actually _interested_ in 
> trackign empty directories (hint hint) decide that it's a good enough 
> start that they can fill in the details.

Well, kudos.  Together with the analysis from Junio, this seems like a
good start.  Would you have any recommendations about what stuff one
should really read in order to get up to scratch about git internals?

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply

* Re: Empty directories...
From: Junio C Hamano @ 2007-07-18 23:34 UTC (permalink / raw)
  To: David Kastrup; +Cc: git
In-Reply-To: <867ioyqhgc.fsf@lola.quinscape.zz>

David Kastrup <dak@gnu.org> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Having made it sound so easy, here are the issues I would expect
>> to be nontrivial (but probably not rocket surgery either).
>> ...
> This would seem to imply that the index does not need to be
> upwards-compatible: simplifying the code means that old indexes won't
> be treated all too well.

I did not imply any such thing, by the way.  These are off the
top of my head technical issues and there probably are more, but
I limited the list to technical side of the things.

You of course have social side to take care of.  If you are
breaking everybody else's index, you would need to tell
everybody: "I am sorry but if you upgrade your git to this
version that does what I want, you have to nuke your index and
start over, so commit all changes first, and then update the
git.  Sorry for causing you a minor inconvenience".  Everybody
at this point involves (obviously) the kernel folks, wine,
x.org, among many others.

I suspect your saying that to them is probably not good enough
for them to forgive the minor inconveniences, which means you
need to convince _me_ to join you in defending, in the release
notes, that this is a feature worth having even though there is
a minor inconvenience to redo everybody's index files.  Which I
suspect is quite unlikely to happen at this moment, though...

A much less troublesome approach might be to do things
differently from what I outlined, to keep the index compatible
as long as it does not contain an empty directory, which is what
we did for subprojects support.

^ permalink raw reply

* Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats
From: Jakub Narebski @ 2007-07-18 23:40 UTC (permalink / raw)
  To: Matt McCutchen; +Cc: Junio C Hamano, git, Petr Baudis, Luben Tuikov
In-Reply-To: <1184699486.9831.7.camel@mattlaptop2>

On Tue, 17 July 2007, Matt McCutchen napisał:
> - Centralize knowledge about snapshot formats (mime types, extensions,
>   commands) in %known_snapshot_formats and improve how some of that
>   information is specified.  In particular, zip files are no longer a
>   special case.
> 
> - Add support for offering multiple snapshot formats to the user so
>   that he/she can download a snapshot in the format he/she prefers.
>   The site-wide or project configuration now gives a list of formats
>   to offer, and if more than one format is offered, the "_snapshot_"
>   link becomes something like "snapshot (_tar.bz2_ _zip_)".
> 
> - If only one format is offered, a tooltip on the "_snapshot_" link
>   tells the user what it is.

Nice idea.

> - Fix out-of-date "tarball" -> "archive" in comment.
> 
> Alert for gitweb site administrators: This patch changes the format of
> $feature{'snapshot'}{'default'} in gitweb_config.perl from a list of
> three pieces of information about a single format to a list of one or
> more formats you wish to offer from the set ('tgz', 'tbz2', 'zip').
> Update your gitweb_config.perl appropriately.  The preferred names for
> gitweb.snapshot in repository configuration have also changed from
> 'gzip' and 'bzip2' to 'tgz' and 'tbz2', but the old names are still
> recognized for compatibility.

This alert/warning should probably be put in RelNotes for when it would
be in git.git

> Signed-off-by: Matt McCutchen <hashproduct@gmail.com>
> ---
> 
> Changes since the previous revision of the patch:
> 
> - Added display names.
> - Changed compressor command line to list form.
> - Added compatibility format aliases for repository configuration.
> - Tweaked filtering of unknown formats to apply only to repository
>   configuration.
> - Reformatted format_snapshot_links and added/modified comments to make it much
>   easier to understand.
> - When a single snapshot format is offered, added a tooltip showing the format
>   to the "snapshot" link.  This helps Junio's hypothetical end user without
>   using additional screen real estate.
> 
> I thought of another incompatibility: previously bookmarked snapshot
> URLs will no longer work because they lack the new "sf" parameter.  I
> don't care about this; do any of you?

I think either having good error message, or using first format avaiable
would be good enough.

> +# information about snapshot formats that gitweb is capable of serving
> +# name => [display name, mime type, filename suffix, --format for git-archive,
> +#          [compressor command and arguments] | undef]
> +our %known_snapshot_formats = (
> +	'tgz'  => ['tar.gz' , 'application/x-gzip' , '.tar.gz' , 'tar', ['gzip' ]],
> +	'tbz2' => ['tar.bz2', 'application/x-bzip2', '.tar.bz2', 'tar', ['bzip2']],
> +	'zip'  => ['zip',     'application/x-zip'  , '.zip'    , 'zip', undef    ],
> +);

First I'm not sure if we want to do the way it had to be done when
those info was in the subfield of %feature hash, or to imitate %feature
hash using instead:

+our %known_snapshot_formats = (
+	'tgz'  => {
+		'display'  => 'tar.gz',
+		'mimetype' => 'application/x-gzip',
+		'suffix'   => '.tar.gz',
+		'format'   => 'tar',
+		'compressor' => ['gzip' ]},
... 

which means that when using %known_snapshot_formats we don't have
to remember for example which of the elements in array is mimetype,
which display name, and which format to be passed to git-archive.


Second, I have thought that we might want to simply use the rest of
array for the compressor and it's arguments instead of adding it as
anonymous array reference (inner array). But this way we could in
princile add more pipelines... although I think it would be not useful.
I'd rather have first option implemented, even if it does not allow for
multiple pipelines.

Third, I think we don't need to say "undef" explicitely, I think.
"defined ('a')[1]" returns the same as "defined ('a', undef)[1]".

> +# Aliases so we understand old gitweb.snapshot values in repository
> +# configuration.
> +our %known_snapshot_format_aliases = (
> +	'gzip'  => 'tgz' ,
> +	'bzip2' => 'tbz2',
> +);

Good idea, better than tring to fit it in %known_snapshot_formats.

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: Proposal about --help options and man calls
From: David Kastrup @ 2007-07-18 23:28 UTC (permalink / raw)
  To: git
In-Reply-To: <7v644h715y.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <gitster@pobox.com> writes:

> David Kastrup <dak@gnu.org> writes:
>
>> Frequently, git somecommand --help will call the man command to
>> display help pages.  I think that when it does so, it should pass the
>> value of the GIT_PAGER variable copied into the PAGER variable: the
>> paging happens on behalf of git here.
>
> Hmph.  Is that to help people who uses GIT_PAGER but no PAGER,
> or have both but set it differently (setting both and in the
> same way does not make much sense).  But what it means is that
> "git command --help" and "man git-command" would be paged
> differently.  I highly doubt it is really desirable.
>
> What's the reason to set GIT_PAGER and PAGER differently to begin
> with?  Can people give examples of the reason why?

If I call command --help, I don't want a pager barfing at me.  Never.
I have scrollback for that.  It is my choice when I page and when I
page not.  There are manual pages who go through 50 pages or so,
however.  There are commands that fundamentally are connected with a
pager.  man is, for example.  But most cases where git calls a pager
(and that includes his way of calling man without getting asked for it
explicitly) utterly surprise me.  So I set GIT_PAGER to cat and hoped
that it would get git to behave.  Sometimes it does, sometimes it
doesn't.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ 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