Git development
 help / color / mirror / Atom feed
* [PATCH 1/3] Documentation: lost-found is now deprecated.
From: Junio C Hamano @ 2007-11-09  1:21 UTC (permalink / raw)
  To: Jonas Fonseca; +Cc: Andreas Ericsson, Johannes Schindelin, git
In-Reply-To: <20071109002001.GB5082@diku.dk>

This makes it possible to mark commands that are deprecated in the
command list of the primary manual page git(7), and uses it to
mark "git lost-found" as deprecated.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * Here is what I have been preparing for queuing.

 Documentation/cmd-list.perl |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/Documentation/cmd-list.perl b/Documentation/cmd-list.perl
index 8d21d42..0066064 100755
--- a/Documentation/cmd-list.perl
+++ b/Documentation/cmd-list.perl
@@ -3,7 +3,8 @@
 use File::Compare qw(compare);
 
 sub format_one {
-	my ($out, $name) = @_;
+	my ($out, $nameattr) = @_;
+	my ($name, $attr) = @$nameattr;
 	my ($state, $description);
 	$state = 0;
 	open I, '<', "$name.txt" or die "No such file $name.txt";
@@ -26,8 +27,11 @@ sub format_one {
 		die "No description found in $name.txt";
 	}
 	if (my ($verify_name, $text) = ($description =~ /^($name) - (.*)/)) {
-		print $out "gitlink:$name\[1\]::\n";
-		print $out "\t$text.\n\n";
+		print $out "gitlink:$name\[1\]::\n\t";
+		if ($attr) {
+			print $out "($attr) ";
+		}
+		print $out "$text.\n\n";
 	}
 	else {
 		die "Description does not match $name: $description";
@@ -39,8 +43,8 @@ while (<DATA>) {
 	next if /^#/;
 
 	chomp;
-	my ($name, $cat) = /^(\S+)\s+(.*)$/;
-	push @{$cmds{$cat}}, $name;
+	my ($name, $cat, $attr) = /^(\S+)\s+(.*?)(?:\s+(.*))?$/;
+	push @{$cmds{$cat}}, [$name, $attr];
 }
 
 for my $cat (qw(ancillaryinterrogators
@@ -126,7 +130,7 @@ git-instaweb                            ancillaryinterrogators
 gitk                                    mainporcelain
 git-local-fetch                         synchingrepositories
 git-log                                 mainporcelain
-git-lost-found                          ancillarymanipulators
+git-lost-found                          ancillarymanipulators	deprecated
 git-ls-files                            plumbinginterrogators
 git-ls-remote                           plumbinginterrogators
 git-ls-tree                             plumbinginterrogators
-- 
1.5.3.5.1622.g41d10

^ permalink raw reply related

* Re: [PATCH 1/2] Add strchrnul()
From: Johannes Schindelin @ 2007-11-09  1:21 UTC (permalink / raw)
  To: René Scharfe; +Cc: Junio C Hamano, Pierre Habouzit, Git Mailing List
In-Reply-To: <4733AEA0.1060602@lsrfire.ath.cx>

Hi,

On Fri, 9 Nov 2007, Ren? Scharfe wrote:

> diff --git a/Makefile b/Makefile
> index 0d5590f..578c999 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -30,6 +30,8 @@ all::
>  #
>  # Define NO_MEMMEM if you don't have memmem.
>  #
> +# Define NO_STRCHRNUL if you don't have strchrnul.
> +#
>  # Define NO_STRLCPY if you don't have strlcpy.
>  #
>  # Define NO_STRTOUMAX if you don't have strtoumax in the C library.
> @@ -406,6 +408,7 @@ ifeq ($(uname_S),Darwin)
>  	OLD_ICONV = UnfortunatelyYes
>  	NO_STRLCPY = YesPlease
>  	NO_MEMMEM = YesPlease
> +	NO_STRCHRNUL = YesPlease
>  endif
>  ifeq ($(uname_S),SunOS)
>  	NEEDS_SOCKET = YesPlease
> @@ -413,6 +416,7 @@ ifeq ($(uname_S),SunOS)
>  	SHELL_PATH = /bin/bash
>  	NO_STRCASESTR = YesPlease
>  	NO_MEMMEM = YesPlease
> +	NO_STRCHRNUL = YesPlease
>  	NO_HSTRERROR = YesPlease
>  	ifeq ($(uname_R),5.8)
>  		NEEDS_LIBICONV = YesPlease
> @@ -438,6 +442,7 @@ ifeq ($(uname_O),Cygwin)
>  	NO_D_INO_IN_DIRENT = YesPlease
>  	NO_STRCASESTR = YesPlease
>  	NO_MEMMEM = YesPlease
> +	NO_STRCHRNUL = YesPlease
>  	NO_SYMLINK_HEAD = YesPlease
>  	NEEDS_LIBICONV = YesPlease
>  	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
> @@ -452,12 +457,14 @@ endif
>  ifeq ($(uname_S),FreeBSD)
>  	NEEDS_LIBICONV = YesPlease
>  	NO_MEMMEM = YesPlease
> +	NO_STRCHRNUL = YesPlease
>  	BASIC_CFLAGS += -I/usr/local/include
>  	BASIC_LDFLAGS += -L/usr/local/lib
>  endif
>  ifeq ($(uname_S),OpenBSD)
>  	NO_STRCASESTR = YesPlease
>  	NO_MEMMEM = YesPlease
> +	NO_STRCHRNUL = YesPlease
>  	NEEDS_LIBICONV = YesPlease
>  	BASIC_CFLAGS += -I/usr/local/include
>  	BASIC_LDFLAGS += -L/usr/local/lib
> @@ -473,6 +480,7 @@ endif
>  ifeq ($(uname_S),AIX)
>  	NO_STRCASESTR=YesPlease
>  	NO_MEMMEM = YesPlease
> +	NO_STRCHRNUL = YesPlease
>  	NO_STRLCPY = YesPlease
>  	NEEDS_LIBICONV=YesPlease
>  endif
> @@ -485,6 +493,7 @@ ifeq ($(uname_S),IRIX64)
>  	NO_SETENV=YesPlease
>  	NO_STRCASESTR=YesPlease
>  	NO_MEMMEM = YesPlease
> +	NO_STRCHRNUL = YesPlease
>  	NO_STRLCPY = YesPlease
>  	NO_SOCKADDR_STORAGE=YesPlease
>  	SHELL_PATH=/usr/gnu/bin/bash

Might be easier to define HAVE_STRCHRNUL ;-)

> diff --git a/compat/strchrnul.c b/compat/strchrnul.c
> new file mode 100644
> index 0000000..51839fe
> --- /dev/null
> +++ b/compat/strchrnul.c
> @@ -0,0 +1,8 @@
> +#include "../git-compat-util.h"
> +
> +char *gitstrchrnul(const char *s, int c)
> +{
> +	while (*s && *s != c)
> +		s++;
> +	return (char *)s;
> +}

This is so short, I think it is even better to inline it.

Ciao,
Dscho

^ permalink raw reply

* [PATCH] Remove non-existing commands from git(7) and delete their manpages
From: Jonas Fonseca @ 2007-11-09  0:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Andreas Ericsson, Johannes Schindelin, git
In-Reply-To: <7vzlxo1mga.fsf@gitster.siamese.dyndns.org>

Signed-off-by: Jonas Fonseca <fonseca@diku.dk>
---
 Documentation/cmd-list.perl       |    3 --
 Documentation/git-local-fetch.txt |   66 -------------------------------------
 Documentation/git-ssh-fetch.txt   |   52 -----------------------------
 Documentation/git-ssh-upload.txt  |   48 ---------------------------
 4 files changed, 0 insertions(+), 169 deletions(-)
 delete mode 100644 Documentation/git-local-fetch.txt
 delete mode 100644 Documentation/git-ssh-fetch.txt
 delete mode 100644 Documentation/git-ssh-upload.txt

 Junio C Hamano <gitster@pobox.com> wrote Thu, Nov 08, 2007:
 > Thanks.
 > 
 > But lost-found is merely deprecated but not removed yet, so I
 > think it should be kept in the list cmd-list.perl generates.
 
 I don't understand why you want to still advertise commands that have
 been deprecated, but here is a sliced out part of my previous patch,
 which I hope is acceptible.
 
 > We may want a mechanism to mark it deprecated in the list as
 > well, though.  Perhaps ...

 It might also make sense to put this kind of information in the manpage
 document header:

 	git-lost-found(1)
	=================
	:Deprecated:	Use git-fsck --lost-found instead.

 And then modify asciidoc.conf to put in a warnings a few places.

diff --git a/Documentation/cmd-list.perl b/Documentation/cmd-list.perl
index 8d21d42..8a0679a 100755
--- a/Documentation/cmd-list.perl
+++ b/Documentation/cmd-list.perl
@@ -124,7 +124,6 @@ git-index-pack                          plumbingmanipulators
 git-init                                mainporcelain
 git-instaweb                            ancillaryinterrogators
 gitk                                    mainporcelain
-git-local-fetch                         synchingrepositories
 git-log                                 mainporcelain
 git-lost-found                          ancillarymanipulators
 git-ls-files                            plumbinginterrogators
@@ -178,8 +177,6 @@ git-show-branch                         ancillaryinterrogators
 git-show-index                          plumbinginterrogators
 git-show-ref                            plumbinginterrogators
 git-sh-setup                            purehelpers
-git-ssh-fetch                           synchingrepositories
-git-ssh-upload                          synchingrepositories
 git-stash                               mainporcelain
 git-status                              mainporcelain
 git-stripspace                          purehelpers
diff --git a/Documentation/git-local-fetch.txt b/Documentation/git-local-fetch.txt
deleted file mode 100644
index e830dee..0000000
--- a/Documentation/git-local-fetch.txt
+++ /dev/null
@@ -1,66 +0,0 @@
-git-local-fetch(1)
-==================
-
-NAME
-----
-git-local-fetch - Duplicate another git repository on a local system
-
-
-SYNOPSIS
---------
-[verse]
-'git-local-fetch' [-c] [-t] [-a] [-d] [-v] [-w filename] [--recover] [-l] [-s] [-n]
-                  commit-id path
-
-DESCRIPTION
------------
-THIS COMMAND IS DEPRECATED.
-
-Duplicates another git repository on a local system.
-
-OPTIONS
--------
--c::
-	Get the commit objects.
--t::
-	Get trees associated with the commit objects.
--a::
-	Get all the objects.
--v::
-	Report what is downloaded.
--s::
-	Instead of regular file-to-file copying use symbolic links to the objects
-	in the remote repository.
--l::
-	Before attempting symlinks (if -s is specified) or file-to-file copying the
-	remote objects, try to hardlink the remote objects into the local
-	repository.
--n::
-	Never attempt to file-to-file copy remote objects.  Only useful with
-	-s or -l command-line options.
-
--w <filename>::
-        Writes the commit-id into the filename under $GIT_DIR/refs/<filename> on
-        the local end after the transfer is complete.
-
---stdin::
-	Instead of a commit id on the command line (which is not expected in this
-	case), 'git-local-fetch' expects lines on stdin in the format
-
-		<commit-id>['\t'<filename-as-in--w>]
-
---recover::
-	Verify that everything reachable from target is fetched.  Used after
-	an earlier fetch is interrupted.
-
-Author
-------
-Written by Junio C Hamano <junkio@cox.net>
-
-Documentation
---------------
-Documentation by David Greaves, Junio C Hamano and the git-list <git@vger.kernel.org>.
-
-GIT
----
-Part of the gitlink:git[7] suite
diff --git a/Documentation/git-ssh-fetch.txt b/Documentation/git-ssh-fetch.txt
deleted file mode 100644
index 8d3e2ff..0000000
--- a/Documentation/git-ssh-fetch.txt
+++ /dev/null
@@ -1,52 +0,0 @@
-git-ssh-fetch(1)
-================
-
-NAME
-----
-git-ssh-fetch - Fetch from a remote repository over ssh connection
-
-
-
-SYNOPSIS
---------
-'git-ssh-fetch' [-c] [-t] [-a] [-d] [-v] [-w filename] [--recover] commit-id url
-
-DESCRIPTION
------------
-THIS COMMAND IS DEPRECATED.
-
-Pulls from a remote repository over ssh connection, invoking
-git-ssh-upload on the other end. It functions identically to
-git-ssh-upload, aside from which end you run it on.
-
-
-OPTIONS
--------
-commit-id::
-        Either the hash or the filename under [URL]/refs/ to
-        pull.
-
--c::
-	Get the commit objects.
--t::
-	Get trees associated with the commit objects.
--a::
-	Get all the objects.
--v::
-	Report what is downloaded.
--w::
-        Writes the commit-id into the filename under $GIT_DIR/refs/ on
-        the local end after the transfer is complete.
-
-
-Author
-------
-Written by Daniel Barkalow <barkalow@iabervon.org>
-
-Documentation
---------------
-Documentation by David Greaves, Junio C Hamano and the git-list <git@vger.kernel.org>.
-
-GIT
----
-Part of the gitlink:git[7] suite
diff --git a/Documentation/git-ssh-upload.txt b/Documentation/git-ssh-upload.txt
deleted file mode 100644
index 5e2ca8d..0000000
--- a/Documentation/git-ssh-upload.txt
+++ /dev/null
@@ -1,48 +0,0 @@
-git-ssh-upload(1)
-=================
-
-NAME
-----
-git-ssh-upload - Push to a remote repository over ssh connection
-
-
-SYNOPSIS
---------
-'git-ssh-upload' [-c] [-t] [-a] [-d] [-v] [-w filename] [--recover] commit-id url
-
-DESCRIPTION
------------
-THIS COMMAND IS DEPRECATED.
-
-Pushes from a remote repository over ssh connection, invoking
-git-ssh-fetch on the other end. It functions identically to
-git-ssh-fetch, aside from which end you run it on.
-
-OPTIONS
--------
-commit-id::
-        Id of commit to push.
-
--c::
-        Get the commit objects.
--t::
-        Get tree associated with the requested commit object.
--a::
-        Get all the objects.
--v::
-        Report what is uploaded.
--w::
-        Writes the commit-id into the filename under [URL]/refs/ on
-        the remote end after the transfer is complete.
-
-Author
-------
-Written by Daniel Barkalow <barkalow@iabervon.org>
-
-Documentation
---------------
-Documentation by Daniel Barkalow
-
-GIT
----
-Part of the gitlink:git[7] suite
-- 
1.5.3.5.1623.gabaff-dirty

-- 
Jonas Fonseca

^ permalink raw reply related

* [BUG] git-rebase fails when a commit message contains a diff
From: Jonas Fonseca @ 2007-11-09  1:12 UTC (permalink / raw)
  To: git

I have concocted a test that illustrates the problem: patch below.  The problem
is that the patches in .dotest are not properly formatted to make it easy to
distinguish commit message from commit diff.

When running the test git-rebase fails with:

	Applying Test message with diff in it
	error: config: does not exist in index
	fatal: sha1 information is lacking or useless (config).
	Repository lacks necessary blobs to fall back on 3-way merge.
	Cannot fall back to three-way merge.
	Patch failed at 0004.

By modifying the test below, a similar problem related with handling of the
end-of-message marker (---\n) can be exposed. A commit message that contains
this marker and text afterwards ends up only having the text preceeding the
marker.

diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 95e33b5..75babdb 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -83,4 +83,25 @@ test_expect_success 'rebase a single mode change' '
      GIT_TRACE=1 git rebase master
 '
 
+cat >commitdiff-message <<EOF
+Test message with diff in it
+
+--- .git/config	2007-11-02 22:44:55.000000000 +0100
++++ .git/config+	2007-11-09 01:40:30.000000000 +0100
+@@ -1,3 +1,4 @@
++# Broken
+ [core]
+ 	repositoryformatversion = 0
+ 	filemode = true
+EOF
+
+test_expect_success 'rebase commit with diff in the commit message' \
+'
+     git checkout -b diff-msg upstream-merged-nonlinear &&
+     echo 1 > W &&
+     git add W &&
+     git commit -F commitdiff-message &&
+     git rebase master
+'
+
 test_done

-- 
Jonas Fonseca

^ permalink raw reply related

* Re: [PATCH PARSEOPT 1/4] parse-options new features.
From: Pierre Habouzit @ 2007-11-09  0:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vbqa4z3ts.fsf@gitster.siamese.dyndns.org>

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

On Thu, Nov 08, 2007 at 11:59:27PM +0000, Junio C Hamano wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
> 
> > options flags:
> > ~~~~~~~~~~~~~
> >   PARSE_OPT_NONEG allow the caller to disallow the negated option to exists.
> 
> Good addition; writing OPT_CALLBACK was tricky without this when
> I tried to add --with=<commit> to git-branch.

  Well, you could do the same by hand, in the callback:

    if (unset)
        return error("go away with your --no-%s", opt->long_name);

  This is correct because only long options can be negated, and if the
callback returns -1 (< 0 actually IIRC, or maybe even !0) then
parse_options assume that you dealt with the error message, and will
just print out the usage and exit(129).

  So it merely offloads something that you could already do the very
same way _when using a callback_.

  It gets more interesting when you want to use an
OPT_INT/_STRING/whatever with an argument that isn't a callback and
don't want for some reason that --no-foo works. Before that change, you
needed a callback, now you don't :)
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

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

^ permalink raw reply

* Re: git rebase --skip
From: Björn Steinbrink @ 2007-11-09  1:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Andreas Ericsson, Mike Hommey, git
In-Reply-To: <7vir4cz45z.fsf@gitster.siamese.dyndns.org>

On 2007.11.08 15:52:08 -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
> 
> > Personally, I don't see the point of a --force option; it turns your work
> > flow from:
> >
> >   1. git-rebase --skip
> >   2. Oops, I guess I have to reset.
> >   3. git-reset --hard; git-rebase --skip
> >
> > to:
> >
> >   1. same as above
> >   2. same as above
> >   3. git-rebase --force --skip
> 
> I do not see it as improvement, either, for the same reason you
> state.
> 
> > AIUI, Andreas's proposal is not so much DWIM as "do the obvious thing,
> > but include a safety valve to prevent throwing away work." Is there
> > actually a case where it would not have the desired effect?
> 
> The user is explicitly saying --skip, so I do not think it is
> dangerous even if we unconditionally did "reset --hard" at that
> point.

The user _must_ say --skip in the case I outlined. And I'm pretty sure
that the first thing I'll (accidently) do once --skip implies "reset
--hard" is to forget to commit. Murphy has never let me down.

How about adding that --amend option that someone mentioned? Or even
just letting --continue act like --skip when there's nothing to commit.
That way, you're no longer forced to use --skip.

Björn

^ permalink raw reply

* [PATCH] git-checkout: Support relative paths containing "..".
From: David Symonds @ 2007-11-09  0:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Johannes Schindelin, Andreas Ericsson, David Symonds

Signed-off-by: David Symonds <dsymonds@gmail.com>
---
 git-checkout.sh |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-checkout.sh b/git-checkout.sh
index c00cedd..aa724ac 100755
--- a/git-checkout.sh
+++ b/git-checkout.sh
@@ -133,9 +133,9 @@ Did you intend to checkout '$@' which can not be resolved as commit?"
 	fi
 
 	# Make sure the request is about existing paths.
-	git ls-files --error-unmatch -- "$@" >/dev/null || exit
-	git ls-files -- "$@" |
-	git checkout-index -f -u --stdin
+	git ls-files --full-name --error-unmatch -- "$@" >/dev/null || exit
+	git ls-files --full-name -- "$@" |
+		(cd_to_toplevel && git checkout-index -f -u --stdin)
 
 	# Run a post-checkout hook -- the HEAD does not change so the
 	# current HEAD is passed in for both args
-- 
1.5.3.1

^ permalink raw reply related

* [PATCH] git-checkout: Test for relative path use.
From: David Symonds @ 2007-11-09  0:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Johannes Schindelin, Andreas Ericsson, David Symonds
In-Reply-To: <11945685673280-git-send-email-dsymonds@gmail.com>

Signed-off-by: David Symonds <dsymonds@gmail.com>
---
	Test 5 in this series fails because of a bug in git-ls-files, where
		git-ls-files t/../
	(with or without --full-name) returns no files.

 t/t2008-checkout-subdir.sh |   79 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 79 insertions(+), 0 deletions(-)
 create mode 100755 t/t2008-checkout-subdir.sh

diff --git a/t/t2008-checkout-subdir.sh b/t/t2008-checkout-subdir.sh
new file mode 100755
index 0000000..f226511
--- /dev/null
+++ b/t/t2008-checkout-subdir.sh
@@ -0,0 +1,79 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 David Symonds
+
+test_description='git checkout from subdirectories'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+
+	echo "base" > file0 &&
+	git add file0 &&
+	mkdir dir1 &&
+	echo "hello" > dir1/file1 &&
+	git add dir1/file1 &&
+	mkdir dir2 &&
+	echo "bonjour" > dir2/file2 &&
+	git add dir2/file2 &&
+	test_tick &&
+	git commit -m "populate tree"
+
+'
+
+test_expect_success 'remove and restore with relative path' '
+
+	cd dir1 &&
+	rm ../file0 &&
+	git checkout HEAD -- ../file0 &&
+	test "base" = "$(cat ../file0)" &&
+	rm ../dir2/file2 &&
+	git checkout HEAD -- ../dir2/file2 &&
+	test "bonjour" = "$(cat ../dir2/file2)" &&
+	rm ../file0 ./file1 &&
+	git checkout HEAD -- .. &&
+	test "base" = "$(cat ../file0)" &&
+	test "hello" = "$(cat file1)" &&
+	cd -
+
+'
+
+test_expect_success 'checkout with empty prefix' '
+
+	rm file0 &&
+	git checkout HEAD -- file0 &&
+	test "base" = "$(cat file0)"
+
+'
+
+test_expect_success 'checkout with simple prefix' '
+
+	rm dir1/file1 &&
+	git checkout HEAD -- dir1 &&
+	test "hello" = "$(cat dir1/file1)" &&
+	rm dir1/file1 &&
+	git checkout HEAD -- dir1/file1 &&
+	test "hello" = "$(cat dir1/file1)"
+
+'
+
+test_expect_success 'checkout with complex relative path' '
+
+	rm file1 &&
+	git checkout HEAD -- ../dir1/../dir1/file1 && test -f ./file1
+
+'
+
+test_expect_failure 'relative path outside tree should fail' \
+	'git checkout HEAD -- ../../Makefile'
+
+test_expect_failure 'incorrect relative path to file should fail (1)' \
+	'git checkout HEAD -- ../file0'
+
+test_expect_failure 'incorrect relative path should fail (2)' \
+	'cd dir1 && git checkout HEAD -- ./file0'
+
+test_expect_failure 'incorrect relative path should fail (3)' \
+	'cd dir1 && git checkout HEAD -- ../../file0'
+
+test_done
-- 
1.5.3.1

^ permalink raw reply related

* [PATCH 2/2] --pretty=format: on-demand format expansion
From: René Scharfe @ 2007-11-09  0:49 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Paul Mackerras, Git Mailing List, Pierre Habouzit,
	Johannes Schindelin

Some of the --pretty=format placeholders expansions are expensive to
calculate.  This is made worse by the current code's use of
interpolate(), which requires _all_ placeholders are to be prepared
up front.

One way to speed this up is to check which placeholders are present
in the format string and to prepare only the expansions that are
needed.  That still leaves the allocation overhead of interpolate().

Another way is to use a callback based approach together with the
strbuf library to keep allocations to a minimum and avoid string
copies.  That's what this patch does.  It introduces a new strbuf
function, strbuf_expand().

The function takes a format string, list of placeholder strings,
a user supplied function 'fn', and an opaque pointer 'context'
to tell 'fn' what thingy to operate on.

The function 'fn' is expected to accept a strbuf, a parsed
placeholder string and the 'context' pointer, and append the
interpolated value for the 'context' thingy, according to the
format specified by the placeholder.

Thanks to Pierre Habouzit for his suggestion to use strchrnul() and
the code surrounding its callsite.  And thanks to Junio for most of
this commit message. :)

Here my measurements of most of Paul Mackerras' test cases that
highlighted the performance problem (best of three runs):
    
(master)
$ time git log --pretty=oneline >/dev/null

real    0m0.390s
user    0m0.340s
sys     0m0.040s

(master)
$ time git log --pretty=raw >/dev/null

real    0m0.434s
user    0m0.408s
sys     0m0.016s

(master)
$ time git log --pretty="format:%H {%P} %ct" >/dev/null

real    0m1.347s
user    0m0.080s
sys     0m1.256s

(interp_find_active -- Dscho)
$ time ./git log --pretty="format:%H {%P} %ct" >/dev/null

real    0m0.694s
user    0m0.020s
sys     0m0.672s

(strbuf_expand -- this patch)
$ time ./git log --pretty="format:%H {%P} %ct" >/dev/null

real    0m0.395s
user    0m0.352s
sys     0m0.028s

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
This version fixes a bug in the first one, which would eat
percent signs that were not followed by a placeholder from the
list.  That may be a valid thing to do for a brand-new parser,
but it regressed on what interpolate() did.

 strbuf.c |   24 ++++++
 strbuf.h |    3 +
 pretty.c |  276 ++++++++++++++++++++++++++++++++++----------------------------
 3 files changed, 180 insertions(+), 123 deletions(-)

diff --git a/strbuf.c b/strbuf.c
index f4201e1..536b432 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -129,6 +129,30 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...)
 	strbuf_setlen(sb, sb->len + len);
 }
 
+void strbuf_expand(struct strbuf *sb, const char *format,
+                   const char **placeholders, expand_fn_t fn, void *context)
+{
+	for (;;) {
+		const char *percent, **p;
+
+		percent = strchrnul(format, '%');
+		strbuf_add(sb, format, percent - format);
+		if (!*percent)
+			break;
+		format = percent + 1;
+
+		for (p = placeholders; *p; p++) {
+			if (!prefixcmp(format, *p))
+				break;
+		}
+		if (*p) {
+			fn(sb, *p, context);
+			format += strlen(*p);
+		} else
+			strbuf_addch(sb, '%');
+	}
+}
+
 size_t strbuf_fread(struct strbuf *sb, size_t size, FILE *f)
 {
 	size_t res;
diff --git a/strbuf.h b/strbuf.h
index cd7f295..21d8944 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -102,6 +102,9 @@ static inline void strbuf_addbuf(struct strbuf *sb, struct strbuf *sb2) {
 	strbuf_add(sb, sb2->buf, sb2->len);
 }
 
+typedef void (*expand_fn_t) (struct strbuf *sb, const char *placeholder, void *context);
+extern void strbuf_expand(struct strbuf *sb, const char *format, const char **placeholders, expand_fn_t fn, void *context);
+
 __attribute__((format(printf,2,3)))
 extern void strbuf_addf(struct strbuf *sb, const char *fmt, ...);
 
diff --git a/pretty.c b/pretty.c
index 490cede..9fbd73f 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1,6 +1,5 @@
 #include "cache.h"
 #include "commit.h"
-#include "interpolate.h"
 #include "utf8.h"
 #include "diff.h"
 #include "revision.h"
@@ -283,7 +282,8 @@ static char *logmsg_reencode(const struct commit *commit,
 	return out;
 }
 
-static void fill_person(struct interp *table, const char *msg, int len)
+static void format_person_part(struct strbuf *sb, char part,
+                               const char *msg, int len)
 {
 	int start, end, tz = 0;
 	unsigned long date;
@@ -295,7 +295,10 @@ static void fill_person(struct interp *table, const char *msg, int len)
 	start = end + 1;
 	while (end > 0 && isspace(msg[end - 1]))
 		end--;
-	table[0].value = xmemdupz(msg, end);
+	if (part == 'n') {	/* name */
+		strbuf_add(sb, msg, end);
+		return;
+	}
 
 	if (start >= len)
 		return;
@@ -307,7 +310,10 @@ static void fill_person(struct interp *table, const char *msg, int len)
 	if (end >= len)
 		return;
 
-	table[1].value = xmemdupz(msg + start, end - start);
+	if (part == 'e') {	/* email */
+		strbuf_add(sb, msg + start, end - start);
+		return;
+	}
 
 	/* parse date */
 	for (start = end + 1; start < len && isspace(msg[start]); start++)
@@ -318,7 +324,10 @@ static void fill_person(struct interp *table, const char *msg, int len)
 	if (msg + start == ep)
 		return;
 
-	table[5].value = xmemdupz(msg + start, ep - (msg + start));
+	if (part == 't') {	/* date, UNIX timestamp */
+		strbuf_add(sb, msg + start, ep - (msg + start));
+		return;
+	}
 
 	/* parse tz */
 	for (start = ep - msg + 1; start < len && isspace(msg[start]); start++)
@@ -329,123 +338,107 @@ static void fill_person(struct interp *table, const char *msg, int len)
 			tz = -tz;
 	}
 
-	interp_set_entry(table, 2, show_date(date, tz, DATE_NORMAL));
-	interp_set_entry(table, 3, show_date(date, tz, DATE_RFC2822));
-	interp_set_entry(table, 4, show_date(date, tz, DATE_RELATIVE));
-	interp_set_entry(table, 6, show_date(date, tz, DATE_ISO8601));
+	switch (part) {
+	case 'd':	/* date */
+		strbuf_addstr(sb, show_date(date, tz, DATE_NORMAL));
+		return;
+	case 'D':	/* date, RFC2822 style */
+		strbuf_addstr(sb, show_date(date, tz, DATE_RFC2822));
+		return;
+	case 'r':	/* date, relative */
+		strbuf_addstr(sb, show_date(date, tz, DATE_RELATIVE));
+		return;
+	case 'i':	/* date, ISO 8601 */
+		strbuf_addstr(sb, show_date(date, tz, DATE_ISO8601));
+		return;
+	}
 }
 
-void format_commit_message(const struct commit *commit,
-                           const void *format, struct strbuf *sb)
+static void format_commit_item(struct strbuf *sb, const char *placeholder,
+                               void *context)
 {
-	struct interp table[] = {
-		{ "%H" },	/* commit hash */
-		{ "%h" },	/* abbreviated commit hash */
-		{ "%T" },	/* tree hash */
-		{ "%t" },	/* abbreviated tree hash */
-		{ "%P" },	/* parent hashes */
-		{ "%p" },	/* abbreviated parent hashes */
-		{ "%an" },	/* author name */
-		{ "%ae" },	/* author email */
-		{ "%ad" },	/* author date */
-		{ "%aD" },	/* author date, RFC2822 style */
-		{ "%ar" },	/* author date, relative */
-		{ "%at" },	/* author date, UNIX timestamp */
-		{ "%ai" },	/* author date, ISO 8601 */
-		{ "%cn" },	/* committer name */
-		{ "%ce" },	/* committer email */
-		{ "%cd" },	/* committer date */
-		{ "%cD" },	/* committer date, RFC2822 style */
-		{ "%cr" },	/* committer date, relative */
-		{ "%ct" },	/* committer date, UNIX timestamp */
-		{ "%ci" },	/* committer date, ISO 8601 */
-		{ "%e" },	/* encoding */
-		{ "%s" },	/* subject */
-		{ "%b" },	/* body */
-		{ "%Cred" },	/* red */
-		{ "%Cgreen" },	/* green */
-		{ "%Cblue" },	/* blue */
-		{ "%Creset" },	/* reset color */
-		{ "%n" },	/* newline */
-		{ "%m" },	/* left/right/bottom */
-	};
-	enum interp_index {
-		IHASH = 0, IHASH_ABBREV,
-		ITREE, ITREE_ABBREV,
-		IPARENTS, IPARENTS_ABBREV,
-		IAUTHOR_NAME, IAUTHOR_EMAIL,
-		IAUTHOR_DATE, IAUTHOR_DATE_RFC2822, IAUTHOR_DATE_RELATIVE,
-		IAUTHOR_TIMESTAMP, IAUTHOR_ISO8601,
-		ICOMMITTER_NAME, ICOMMITTER_EMAIL,
-		ICOMMITTER_DATE, ICOMMITTER_DATE_RFC2822,
-		ICOMMITTER_DATE_RELATIVE, ICOMMITTER_TIMESTAMP,
-		ICOMMITTER_ISO8601,
-		IENCODING,
-		ISUBJECT,
-		IBODY,
-		IRED, IGREEN, IBLUE, IRESET_COLOR,
-		INEWLINE,
-		ILEFT_RIGHT,
-	};
+	const struct commit *commit = context;
 	struct commit_list *p;
-	char parents[1024];
-	unsigned long len;
 	int i;
 	enum { HEADER, SUBJECT, BODY } state;
 	const char *msg = commit->buffer;
 
-	if (ILEFT_RIGHT + 1 != ARRAY_SIZE(table))
-		die("invalid interp table!");
-
 	/* these are independent of the commit */
-	interp_set_entry(table, IRED, "\033[31m");
-	interp_set_entry(table, IGREEN, "\033[32m");
-	interp_set_entry(table, IBLUE, "\033[34m");
-	interp_set_entry(table, IRESET_COLOR, "\033[m");
-	interp_set_entry(table, INEWLINE, "\n");
+	switch (placeholder[0]) {
+	case 'C':
+		switch (placeholder[3]) {
+		case 'd':	/* red */
+			strbuf_addstr(sb, "\033[31m");
+			return;
+		case 'e':	/* green */
+			strbuf_addstr(sb, "\033[32m");
+			return;
+		case 'u':	/* blue */
+			strbuf_addstr(sb, "\033[34m");
+			return;
+		case 's':	/* reset color */
+			strbuf_addstr(sb, "\033[m");
+			return;
+		}
+	case 'n':		/* newline */
+		strbuf_addch(sb, '\n');
+		return;
+	}
 
 	/* these depend on the commit */
 	if (!commit->object.parsed)
 		parse_object(commit->object.sha1);
-	interp_set_entry(table, IHASH, sha1_to_hex(commit->object.sha1));
-	interp_set_entry(table, IHASH_ABBREV,
-			find_unique_abbrev(commit->object.sha1,
-				DEFAULT_ABBREV));
-	interp_set_entry(table, ITREE, sha1_to_hex(commit->tree->object.sha1));
-	interp_set_entry(table, ITREE_ABBREV,
-			find_unique_abbrev(commit->tree->object.sha1,
-				DEFAULT_ABBREV));
-	interp_set_entry(table, ILEFT_RIGHT,
-			 (commit->object.flags & BOUNDARY)
-			 ? "-"
-			 : (commit->object.flags & SYMMETRIC_LEFT)
-			 ? "<"
-			 : ">");
-
-	parents[1] = 0;
-	for (i = 0, p = commit->parents;
-			p && i < sizeof(parents) - 1;
-			p = p->next)
-		i += snprintf(parents + i, sizeof(parents) - i - 1, " %s",
-			sha1_to_hex(p->item->object.sha1));
-	interp_set_entry(table, IPARENTS, parents + 1);
-
-	parents[1] = 0;
-	for (i = 0, p = commit->parents;
-			p && i < sizeof(parents) - 1;
-			p = p->next)
-		i += snprintf(parents + i, sizeof(parents) - i - 1, " %s",
-			find_unique_abbrev(p->item->object.sha1,
-				DEFAULT_ABBREV));
-	interp_set_entry(table, IPARENTS_ABBREV, parents + 1);
 
+	switch (placeholder[0]) {
+	case 'H':		/* commit hash */
+		strbuf_addstr(sb, sha1_to_hex(commit->object.sha1));
+		return;
+	case 'h':		/* abbreviated commit hash */
+		strbuf_addstr(sb, find_unique_abbrev(commit->object.sha1,
+		                                     DEFAULT_ABBREV));
+		return;
+	case 'T':		/* tree hash */
+		strbuf_addstr(sb, sha1_to_hex(commit->tree->object.sha1));
+		return;
+	case 't':		/* abbreviated tree hash */
+		strbuf_addstr(sb, find_unique_abbrev(commit->tree->object.sha1,
+		                                     DEFAULT_ABBREV));
+		return;
+	case 'P':		/* parent hashes */
+		for (p = commit->parents; p; p = p->next) {
+			if (p != commit->parents)
+				strbuf_addch(sb, ' ');
+			strbuf_addstr(sb, sha1_to_hex(p->item->object.sha1));
+		}
+		return;
+	case 'p':		/* abbreviated parent hashes */
+		for (p = commit->parents; p; p = p->next) {
+			if (p != commit->parents)
+				strbuf_addch(sb, ' ');
+			strbuf_addstr(sb, find_unique_abbrev(
+					p->item->object.sha1, DEFAULT_ABBREV));
+		}
+		return;
+	case 'm':		/* left/right/bottom */
+		strbuf_addch(sb, (commit->object.flags & BOUNDARY)
+		                 ? '-'
+		                 : (commit->object.flags & SYMMETRIC_LEFT)
+		                 ? '<'
+		                 : '>');
+		return;
+	}
+
+	/* For the rest we have to parse the commit header. */
 	for (i = 0, state = HEADER; msg[i] && state < BODY; i++) {
 		int eol;
 		for (eol = i; msg[eol] && msg[eol] != '\n'; eol++)
 			; /* do nothing */
 
 		if (state == SUBJECT) {
-			table[ISUBJECT].value = xmemdupz(msg + i, eol - i);
+			if (placeholder[0] == 's') {
+				strbuf_add(sb, msg + i, eol - i);
+				return;
+			}
 			i = eol;
 		}
 		if (i == eol) {
@@ -453,29 +446,66 @@ void format_commit_message(const struct commit *commit,
 			/* strip empty lines */
 			while (msg[eol + 1] == '\n')
 				eol++;
-		} else if (!prefixcmp(msg + i, "author "))
-			fill_person(table + IAUTHOR_NAME,
-					msg + i + 7, eol - i - 7);
-		else if (!prefixcmp(msg + i, "committer "))
-			fill_person(table + ICOMMITTER_NAME,
-					msg + i + 10, eol - i - 10);
-		else if (!prefixcmp(msg + i, "encoding "))
-			table[IENCODING].value =
-				xmemdupz(msg + i + 9, eol - i - 9);
+		} else if (!prefixcmp(msg + i, "author ")) {
+			if (placeholder[0] == 'a') {
+				format_person_part(sb, placeholder[1],
+				                   msg + i + 7, eol - i - 7);
+				return;
+			}
+		} else if (!prefixcmp(msg + i, "committer ")) {
+			if (placeholder[0] == 'c') {
+				format_person_part(sb, placeholder[1],
+				                   msg + i + 10, eol - i - 10);
+				return;
+			}
+		} else if (!prefixcmp(msg + i, "encoding ")) {
+			if (placeholder[0] == 'e') {
+				strbuf_add(sb, msg + i + 9, eol - i - 9);
+				return;
+			}
+		}
 		i = eol;
 	}
-	if (msg[i])
-		table[IBODY].value = xstrdup(msg + i);
-
-	len = interpolate(sb->buf + sb->len, strbuf_avail(sb),
-				format, table, ARRAY_SIZE(table));
-	if (len > strbuf_avail(sb)) {
-		strbuf_grow(sb, len);
-		interpolate(sb->buf + sb->len, strbuf_avail(sb) + 1,
-					format, table, ARRAY_SIZE(table));
-	}
-	strbuf_setlen(sb, sb->len + len);
-	interp_clear_table(table, ARRAY_SIZE(table));
+	if (msg[i] && placeholder[0] == 'b')	/* body */
+		strbuf_addstr(sb, msg + i);
+}
+
+void format_commit_message(const struct commit *commit,
+                           const void *format, struct strbuf *sb)
+{
+	const char *placeholders[] = {
+		"H",		/* commit hash */
+		"h",		/* abbreviated commit hash */
+		"T",		/* tree hash */
+		"t",		/* abbreviated tree hash */
+		"P",		/* parent hashes */
+		"p",		/* abbreviated parent hashes */
+		"an",		/* author name */
+		"ae",		/* author email */
+		"ad",		/* author date */
+		"aD",		/* author date, RFC2822 style */
+		"ar",		/* author date, relative */
+		"at",		/* author date, UNIX timestamp */
+		"ai",		/* author date, ISO 8601 */
+		"cn",		/* committer name */
+		"ce",		/* committer email */
+		"cd",		/* committer date */
+		"cD",		/* committer date, RFC2822 style */
+		"cr",		/* committer date, relative */
+		"ct",		/* committer date, UNIX timestamp */
+		"ci",		/* committer date, ISO 8601 */
+		"e",		/* encoding */
+		"s",		/* subject */
+		"b",		/* body */
+		"Cred",		/* red */
+		"Cgreen",	/* green */
+		"Cblue",	/* blue */
+		"Creset",	/* reset color */
+		"n",		/* newline */
+		"m",		/* left/right/bottom */
+		NULL
+	};
+	strbuf_expand(sb, format, placeholders, format_commit_item, (void *)commit);
 }
 
 static void pp_header(enum cmit_fmt fmt,
 

^ permalink raw reply related

* Re: [PATCH 3/3] pretty=format: Avoid some expensive calculations when not needed
From: René Scharfe @ 2007-11-09  0:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vhcjxaire.fsf@gitster.siamese.dyndns.org>

Junio C Hamano schrieb:
> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> 
>> I haven't seen any comments on strbuf_expand.  Is it too far out?
>> Here it is again, adjusted for current master and with the changes
>> to strbuf.[ch] coming first:
> 
> Numbers talk ;-).

:-)  I just hope someone else has compared the times, too..

> In your previous round, you alluded that the strbuf_expand()
> interface could allow caching of the return value of fn(), but I
> do not think strbuf_expand() in this patch has anything to
> directly support that notion.
> 
> Nor I would expect to --- fn() could keep the really expensive
> information cached, keyed with context value, if it wanted to,
> but in practice for the purpose of format_commit_item() I do not
> offhand see anything cacheable and reusable, unless the user did
> stupid things (e.g. use more than one %h in the format string).

Yes, that's what I arrived at, too, when I actually wrote and used
the function.

> I added a few paragraphs to describe the API in the commit log
> message, and rewrote "# master" to "(master)" etc.

Thanks!  I'll send a slightly enhanced version in a two-patch
series -- with an actual commit message, including your API
desciption -- in just a few seconds.

René

^ permalink raw reply

* [PATCH 1/2] Add strchrnul()
From: René Scharfe @ 2007-11-09  0:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Pierre Habouzit, Git Mailing List, Johannes Schindelin

As suggested by Pierre Habouzit, add strchrnul().  It's a useful GNU
extension and can simplify string parser code.  There are several
places in git that can be converted to strchrnul(); as a trivial
example, this patch introduces its usage to builtin-fetch--tool.c.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---

 Makefile              |   13 +++++++++++++
 builtin-fetch--tool.c |    8 ++------
 compat/strchrnul.c    |    8 ++++++++
 git-compat-util.h     |    5 +++++
 4 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index 0d5590f..578c999 100644
--- a/Makefile
+++ b/Makefile
@@ -30,6 +30,8 @@ all::
 #
 # Define NO_MEMMEM if you don't have memmem.
 #
+# Define NO_STRCHRNUL if you don't have strchrnul.
+#
 # Define NO_STRLCPY if you don't have strlcpy.
 #
 # Define NO_STRTOUMAX if you don't have strtoumax in the C library.
@@ -406,6 +408,7 @@ ifeq ($(uname_S),Darwin)
 	OLD_ICONV = UnfortunatelyYes
 	NO_STRLCPY = YesPlease
 	NO_MEMMEM = YesPlease
+	NO_STRCHRNUL = YesPlease
 endif
 ifeq ($(uname_S),SunOS)
 	NEEDS_SOCKET = YesPlease
@@ -413,6 +416,7 @@ ifeq ($(uname_S),SunOS)
 	SHELL_PATH = /bin/bash
 	NO_STRCASESTR = YesPlease
 	NO_MEMMEM = YesPlease
+	NO_STRCHRNUL = YesPlease
 	NO_HSTRERROR = YesPlease
 	ifeq ($(uname_R),5.8)
 		NEEDS_LIBICONV = YesPlease
@@ -438,6 +442,7 @@ ifeq ($(uname_O),Cygwin)
 	NO_D_INO_IN_DIRENT = YesPlease
 	NO_STRCASESTR = YesPlease
 	NO_MEMMEM = YesPlease
+	NO_STRCHRNUL = YesPlease
 	NO_SYMLINK_HEAD = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
@@ -452,12 +457,14 @@ endif
 ifeq ($(uname_S),FreeBSD)
 	NEEDS_LIBICONV = YesPlease
 	NO_MEMMEM = YesPlease
+	NO_STRCHRNUL = YesPlease
 	BASIC_CFLAGS += -I/usr/local/include
 	BASIC_LDFLAGS += -L/usr/local/lib
 endif
 ifeq ($(uname_S),OpenBSD)
 	NO_STRCASESTR = YesPlease
 	NO_MEMMEM = YesPlease
+	NO_STRCHRNUL = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	BASIC_CFLAGS += -I/usr/local/include
 	BASIC_LDFLAGS += -L/usr/local/lib
@@ -473,6 +480,7 @@ endif
 ifeq ($(uname_S),AIX)
 	NO_STRCASESTR=YesPlease
 	NO_MEMMEM = YesPlease
+	NO_STRCHRNUL = YesPlease
 	NO_STRLCPY = YesPlease
 	NEEDS_LIBICONV=YesPlease
 endif
@@ -485,6 +493,7 @@ ifeq ($(uname_S),IRIX64)
 	NO_SETENV=YesPlease
 	NO_STRCASESTR=YesPlease
 	NO_MEMMEM = YesPlease
+	NO_STRCHRNUL = YesPlease
 	NO_STRLCPY = YesPlease
 	NO_SOCKADDR_STORAGE=YesPlease
 	SHELL_PATH=/usr/gnu/bin/bash
@@ -695,6 +704,10 @@ ifdef NO_MEMMEM
 	COMPAT_CFLAGS += -DNO_MEMMEM
 	COMPAT_OBJS += compat/memmem.o
 endif
+ifdef NO_STRCHRNUL
+	COMPAT_CFLAGS += -DNO_STRCHRNUL
+	COMPAT_OBJS += compat/strchrnul.o
+endif
 
 ifdef THREADED_DELTA_SEARCH
 	BASIC_CFLAGS += -DTHREADED_DELTA_SEARCH
diff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c
index 6a78517..ed60847 100644
--- a/builtin-fetch--tool.c
+++ b/builtin-fetch--tool.c
@@ -435,9 +435,7 @@ static int pick_rref(int sha1_only, const char *rref, const char *ls_remote_resu
 				cp++;
 			if (!*cp)
 				break;
-			np = strchr(cp, '\n');
-			if (!np)
-				np = cp + strlen(cp);
+			np = strchrnul(cp, '\n');
 			if (pass) {
 				lrr_list[i].line = cp;
 				lrr_list[i].name = cp + 41;
@@ -461,9 +459,7 @@ static int pick_rref(int sha1_only, const char *rref, const char *ls_remote_resu
 			rref++;
 		if (!*rref)
 			break;
-		next = strchr(rref, '\n');
-		if (!next)
-			next = rref + strlen(rref);
+		next = strchrnul(rref, '\n');
 		rreflen = next - rref;
 
 		for (i = 0; i < lrr_count; i++) {
diff --git a/compat/strchrnul.c b/compat/strchrnul.c
new file mode 100644
index 0000000..51839fe
--- /dev/null
+++ b/compat/strchrnul.c
@@ -0,0 +1,8 @@
+#include "../git-compat-util.h"
+
+char *gitstrchrnul(const char *s, int c)
+{
+	while (*s && *s != c)
+		s++;
+	return (char *)s;
+}
diff --git a/git-compat-util.h b/git-compat-util.h
index 7b29d1b..e72654b 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -183,6 +183,11 @@ void *gitmemmem(const void *haystack, size_t haystacklen,
                 const void *needle, size_t needlelen);
 #endif
 
+#ifdef NO_STRCHRNUL
+#define strchrnul gitstrchrnul
+char *gitstrchrnul(const char *s, int c);
+#endif
+
 extern void release_pack_memory(size_t, int);
 
 static inline char* xstrdup(const char *str)

^ permalink raw reply related

* Re: [PATCH] Port git commit to C.
From: Junio C Hamano @ 2007-11-08 23:47 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Kristian Høgsberg, git
In-Reply-To: <20071108232751.GC4899@steel.home>

Alex Riesen <raa.lkml@gmail.com> writes:

> Kristian Høgsberg, Thu, Nov 08, 2007 17:59:00 +0100:
>> This makes git commit a builtin and moves git-commit.sh to
>> contrib/examples.  This also removes the git-runstatus
>> helper, which was mostly just a git-status.sh implementation detail.
>
> Applied instead of 00c8febf563da on Junio's pu it breaks t1400:

Don't worry.  It will pass with two of the Dscho's patches.

The racy-git issue is still there, though.

^ permalink raw reply

* corrupt object on git-gc
From: Yossi Leybovich @ 2007-11-08 23:59 UTC (permalink / raw)
  To: git
In-Reply-To: <458BC6B0F287034F92FE78908BD01CE814472B3D@mtlexch01.mtl.com>

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

Hi

I wonder if someone can help in this error
I tried to do git-gc and got error on corrupted object. 

I do the following:

$ git-gc
Generating pack...
Done counting 3037 objects.
Deltifying 3037 objects...
error: corrupt loose object '4b9458b3786228369c63936db65827de3cc06200'
fatal: object 4b9458b3786228369c63936db65827de3cc06200 cannot be read
error: failed to run repack

sleybo@SLEYBO-LT /w/work/EMC/ib.071030.001/ib
$ cd .git/objects/4b/

sleybo@SLEYBO-LT /w/work/EMC/ib.071030.001/ib/.git/objects/4b
$ git-fsck-objects.exe 9458b3786228369c63936db65827de3cc06200
error: corrupt loose object '4b9458b3786228369c63936db65827de3cc06200'
error: 4b9458b3786228369c63936db65827de3cc06200: object corrupt or
missing
error: invalid parameter: expected sha1, got
'9458b3786228369c63936db65827de3cc0
6200'
missing blob 4b9458b3786228369c63936db65827de3cc06200

Unfortunately I can't get this object from backup directories as advise
in the FAQ.
Can this object manually fixed by any tool? (the object is attached) I
don't even know to which file/tree/commit it belong how can I know that
?

Thanks
Yossi

[-- Attachment #2: 9458b3786228369c63936db65827de3cc06200 --]
[-- Type: application/octet-stream, Size: 7661 bytes --]

^ permalink raw reply

* Re: [PATCH] Port git commit to C.
From: Junio C Hamano @ 2007-11-08 23:14 UTC (permalink / raw)
  To: Kristian Høgsberg; +Cc: git
In-Reply-To: <1194541140-3062-1-git-send-email-krh@redhat.com>

A huge diff.

I'll quote the difference between this version and the version
previously parked on 'pu', and comment, after re-reverting what
you reverted from the parked version (which has minor tweaks
such as removing git-runstatus from .gitignore I made
previously).

>  builtin-commit.c |   88 +++++++++++++++++++++++++++---------------------------
>  1 files changed, 44 insertions(+), 44 deletions(-)
> 
> diff --git a/builtin-commit.c b/builtin-commit.c
> index f108e90..669cc6b 100644
> --- a/builtin-commit.c
> +++ b/builtin-commit.c
> @@ -70,25 +70,22 @@ static char *prepare_index(const char **files, const char *prefix)
>  	struct tree *tree;
>  	struct lock_file *next_index_lock;
>  
> +	if (interactive) {
> +		interactive_add();
> +		return get_index_file();
> +	}
> +

So interactive bypasses everything else, which makes sense.

What happens when the user aborts interactive_add, wishing to
abort the whole commit process?

> @@ -267,36 +264,41 @@ static int message_is_empty(struct strbuf *sb, int start)
>  
>  static void determine_author_info(struct strbuf *sb)
>  {
> -	char *p, *eol;
> -	char *name = NULL, *email = NULL;
> +	char *name, *email, *date;
>  
> -	if (force_author) {
> -		const char *eoname = strstr(force_author, " <");
> -		const char *eomail = strchr(force_author, '>');
> +	name = getenv("GIT_AUTHOR_NAME");
> +	email = getenv("GIT_AUTHOR_EMAIL");
> +	date = getenv("GIT_AUTHOR_DATE");
>  
> -		if (!eoname || !eomail)
> -			die("malformed --author parameter\n");
> -		name = xstrndup(force_author, eoname - force_author);
> -		email = xstrndup(eoname + 2, eomail - eoname - 2);
> -		/* REVIEW: drops author date from amended commit on --amend --author=<author> */
> -		strbuf_addf(sb, "author %s\n",
> -			    fmt_ident(name, email,
> -				      getenv("GIT_AUTHOR_DATE"), 1));
> -		free(name);
> -		free(email);
> -	} else if (use_message) {
> -		p = strstr(use_message_buffer, "\nauthor");
> -		if (!p)
> +	if (use_message) {
> +		const char *a, *lb, *rb, *eol;
> +
> +		a = strstr(use_message_buffer, "\nauthor ");
> +		if (!a)
>  			die("invalid commit: %s\n", use_message);

You know in what sense it is invalid here and ...

> -		p++;
> -		eol = strchr(p, '\n');
> -		if (!eol)
> +
> +		lb = strstr(a + 8, " <");
> +		rb = strstr(a + 8, "> ");
> +		eol = strchr(a + 8, '\n');
> +		if (!lb || !rb || !eol)
>  			die("invalid commit: %s\n", use_message);

... here.  Would it be more helpful to say "No author line", and
"Cannot parse author line", I wonder.  Probably too much.

> -		strbuf_add(sb, p, eol + 1 - p);
> -	} else {
> -		strbuf_addf(sb, "author %s\n", git_author_info(1));
> +		name = xstrndup(a + 8, lb - (a + 8));
> +		email = xstrndup(lb + 2, rb - (lb + 2));
> +		date = xstrndup(rb + 2, eol - (rb + 2));
>  	}
> +
> +	if (force_author) {
> +		const char *lb = strstr(force_author, " <");
> +		const char *rb = strchr(force_author, '>');
> +
> +		if (!lb || !rb)
> +			die("malformed --author parameter\n");
> +		name = xstrndup(force_author, lb - force_author);
> +		email = xstrndup(lb + 2, rb - (lb + 2));
> +	}
> +
> +	strbuf_addf(sb, "author %s\n", fmt_ident(name, email, date, 1));

I see a slight leak here of name/email/date depending on how
they are obtained.  Probably it is too much hassle to deal with
for too little gain.

^ permalink raw reply

* Re: [PATCH 1/3] Refactor working tree setup
From: Junio C Hamano @ 2007-11-08 23:34 UTC (permalink / raw)
  To: Miles Bader; +Cc: Mike Hommey, git
In-Reply-To: <87bqa4gy2h.fsf@catnip.gol.com>

Miles Bader <miles@gnu.org> writes:

> Mike Hommey <mh@glandium.org> writes:
>> Create a setup_work_tree() that can be used from any command requiring
>> a working tree conditionally.
> ...
>> +void setup_work_tree(void) {
>> +	const char *work_tree = get_git_work_tree();
>
> Hi, could you please not use this "function begin brace at EOL" style?

Hmm.  I should have been more careful.

-- >8 --
[PATCH] Style: place opening brace of a function definition at column 1

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 bundle.c    |    3 ++-
 daemon.c    |    6 ++++--
 dir.c       |    3 ++-
 help.c      |    3 ++-
 quote.c     |    3 ++-
 setup.c     |    3 ++-
 transport.c |    6 ++++--
 utf8.c      |    3 ++-
 8 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/bundle.c b/bundle.c
index 0869fcf..e4d60cd 100644
--- a/bundle.c
+++ b/bundle.c
@@ -23,7 +23,8 @@ static void add_to_ref_list(const unsigned char *sha1, const char *name,
 }
 
 /* returns an fd */
-int read_bundle_header(const char *path, struct bundle_header *header) {
+int read_bundle_header(const char *path, struct bundle_header *header)
+{
 	char buffer[1024];
 	int fd;
 	long fpos;
diff --git a/daemon.c b/daemon.c
index b8df980..41a60af 100644
--- a/daemon.c
+++ b/daemon.c
@@ -406,7 +406,8 @@ static struct daemon_service daemon_service[] = {
 	{ "receive-pack", "receivepack", receive_pack, 0, 1 },
 };
 
-static void enable_service(const char *name, int ena) {
+static void enable_service(const char *name, int ena)
+{
 	int i;
 	for (i = 0; i < ARRAY_SIZE(daemon_service); i++) {
 		if (!strcmp(daemon_service[i].name, name)) {
@@ -417,7 +418,8 @@ static void enable_service(const char *name, int ena) {
 	die("No such service %s", name);
 }
 
-static void make_service_overridable(const char *name, int ena) {
+static void make_service_overridable(const char *name, int ena)
+{
 	int i;
 	for (i = 0; i < ARRAY_SIZE(daemon_service); i++) {
 		if (!strcmp(daemon_service[i].name, name)) {
diff --git a/dir.c b/dir.c
index 5bcc764..01790ab 100644
--- a/dir.c
+++ b/dir.c
@@ -298,7 +298,8 @@ int excluded(struct dir_struct *dir, const char *pathname)
 	return 0;
 }
 
-static struct dir_entry *dir_entry_new(const char *pathname, int len) {
+static struct dir_entry *dir_entry_new(const char *pathname, int len)
+{
 	struct dir_entry *ent;
 
 	ent = xmalloc(sizeof(*ent) + len + 1);
diff --git a/help.c b/help.c
index 855aeef..8217d97 100644
--- a/help.c
+++ b/help.c
@@ -79,7 +79,8 @@ static void uniq(struct cmdnames *cmds)
 	cmds->cnt = j;
 }
 
-static void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes) {
+static void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes)
+{
 	int ci, cj, ei;
 	int cmp;
 
diff --git a/quote.c b/quote.c
index 919d092..0455783 100644
--- a/quote.c
+++ b/quote.c
@@ -131,7 +131,8 @@ static signed char const sq_lookup[256] = {
 	/* 0x80 */ /* set to 0 */
 };
 
-static inline int sq_must_quote(char c) {
+static inline int sq_must_quote(char c)
+{
 	return sq_lookup[(unsigned char)c] + quote_path_fully > 0;
 }
 
diff --git a/setup.c b/setup.c
index df006d9..1e2c55d 100644
--- a/setup.c
+++ b/setup.c
@@ -206,7 +206,8 @@ static const char *set_work_tree(const char *dir)
 	return NULL;
 }
 
-void setup_work_tree(void) {
+void setup_work_tree(void)
+{
 	const char *work_tree = get_git_work_tree();
 	const char *git_dir = get_git_dir();
 	if (!is_absolute_path(git_dir))
diff --git a/transport.c b/transport.c
index d44fe7c..fa5cfbb 100644
--- a/transport.c
+++ b/transport.c
@@ -380,7 +380,8 @@ static int disconnect_walker(struct transport *transport)
 }
 
 #ifndef NO_CURL
-static int curl_transport_push(struct transport *transport, int refspec_nr, const char **refspec, int flags) {
+static int curl_transport_push(struct transport *transport, int refspec_nr, const char **refspec, int flags)
+{
 	const char **argv;
 	int argc;
 	int err;
@@ -646,7 +647,8 @@ static int fetch_refs_via_pack(struct transport *transport,
 	return 0;
 }
 
-static int git_transport_push(struct transport *transport, int refspec_nr, const char **refspec, int flags) {
+static int git_transport_push(struct transport *transport, int refspec_nr, const char **refspec, int flags)
+{
 	struct git_transport_data *data = transport->data;
 	const char **argv;
 	char *rem;
diff --git a/utf8.c b/utf8.c
index 4efef6f..8095a71 100644
--- a/utf8.c
+++ b/utf8.c
@@ -11,7 +11,8 @@ struct interval {
 };
 
 /* auxiliary function for binary search in interval table */
-static int bisearch(ucs_char_t ucs, const struct interval *table, int max) {
+static int bisearch(ucs_char_t ucs, const struct interval *table, int max)
+{
 	int min = 0;
 	int mid;
 

^ permalink raw reply related

* RE: GIT_DIR/--git-dir question
From: Joakim Tjernlund @ 2007-11-08 23:24 UTC (permalink / raw)
  To: 'Junio C Hamano'; +Cc: git
In-Reply-To: <7vfxzg1jpb.fsf@gitster.siamese.dyndns.org>

> From: Junio C Hamano [mailto:gitster@pobox.com] 
> Sent: den 8 november 2007 23:01
> To: Joakim Tjernlund
> Cc: git@vger.kernel.org
> Subject: Re: GIT_DIR/--git-dir question
> 
> "Joakim Tjernlund" <joakim.tjernlund@transmode.se> writes:
> 
> > I just started to look at having multiple work trees, each 
> working on a separate branch,
> > using the same git repo.
> 
> I suspect contrib/workdir is what you are looking for not
> GIT_DIR.

Yes, it does look like what I am looking for. But now I am 
a bit confused as to what GIT_DIR/--git-dir is supposed to be
used for.

 Jocke

^ permalink raw reply

* Re: [PATCH] Port git commit to C.
From: Alex Riesen @ 2007-11-08 23:27 UTC (permalink / raw)
  To: Kristian Høgsberg; +Cc: Junio C Hamano, git
In-Reply-To: <1194541140-3062-1-git-send-email-krh@redhat.com>

Kristian Høgsberg, Thu, Nov 08, 2007 17:59:00 +0100:
> This makes git commit a builtin and moves git-commit.sh to
> contrib/examples.  This also removes the git-runstatus
> helper, which was mostly just a git-status.sh implementation detail.

Applied instead of 00c8febf563da on Junio's pu it breaks t1400:

* expecting success: echo TEST >F &&
     git add F &&
         GIT_AUTHOR_DATE="2005-05-26 23:30" \
         GIT_COMMITTER_DATE="2005-05-26 23:30" git-commit -m add -a &&
         h_TEST=$(git rev-parse --verify HEAD)
         echo The other day this did not work. >M &&
         echo And then Bob told me how to fix it. >>M &&
         echo OTHER >F &&
         GIT_AUTHOR_DATE="2005-05-26 23:41" \
         GIT_COMMITTER_DATE="2005-05-26 23:41" git-commit -F M -a &&
         h_OTHER=$(git rev-parse --verify HEAD) &&
         GIT_AUTHOR_DATE="2005-05-26 23:44" \
         GIT_COMMITTER_DATE="2005-05-26 23:44" git-commit --amend &&
         h_FIXED=$(git rev-parse --verify HEAD) &&
         echo Merged initial commit and a later commit. >M &&
         echo $h_TEST >.git/MERGE_HEAD &&
         GIT_AUTHOR_DATE="2005-05-26 23:45" \
         GIT_COMMITTER_DATE="2005-05-26 23:45" git-commit -F M &&
         h_MERGED=$(git rev-parse --verify HEAD)
         rm -f M
Created initial commit 2bc82dd: add
 1 files changed, 1 insertions(+), 0 deletions(-)
 create mode 100644 F
Created commit d244b72: The other day this did not work.
 1 files changed, 1 insertions(+), 1 deletions(-)
launching editor, log (null)
fatal: * no commit message?  aborting commit.
*   ok 28: creating initial files

* expecting success: diff expect .git/logs/refs/heads/master
3,4d2
< d244b725ca2c5f8aaacd9df2468b890499380862  C O Mitter <committer@example.com> 1117151040 +0000 commit (amend): The other day this did not work.
<   C O Mitter <committer@example.com> 1117151100 +0000 commit (merge): Merged initial commit and a later commit.
* FAIL 29: git-commit logged updates
        diff expect .git/logs/refs/heads/master


Which is not the test actually failed. The failed one is 28, but the
last "rm -f M" killed the error because of missed "&&" before it.

I believe you need something like this to fix the test:

diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index ce045b2..a90824b 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -205,7 +205,7 @@ test_expect_success \
 	 echo $h_TEST >.git/MERGE_HEAD &&
 	 GIT_AUTHOR_DATE="2005-05-26 23:45" \
 	 GIT_COMMITTER_DATE="2005-05-26 23:45" git-commit -F M &&
-	 h_MERGED=$(git rev-parse --verify HEAD)
+	 h_MERGED=$(git rev-parse --verify HEAD) &&
 	 rm -f M'
 
 cat >expect <<EOF

and something like this to refill the strbuf of commit message with
the text read from the amended commit (the failed test was a
"git commit --amend"). The patch is on top of yours "Export
launch_editor() and make it accept ':' as a no-op editor":

diff --git a/builtin-tag.c b/builtin-tag.c
index c3b76da..8ca9ffb 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -42,17 +42,17 @@ void launch_editor(const char *path, struct strbuf *buffer)
 	if (!editor)
 		editor = "vi";
 
-	if (!strcmp(editor, ":"))
-		return;
-
-	memset(&child, 0, sizeof(child));
-	child.argv = args;
-	args[0] = editor;
-	args[1] = path;
-	args[2] = NULL;
-
-	if (run_command(&child))
-		die("There was a problem with the editor %s.", editor);
+	if (strcmp(editor, ":"))
+	{
+		memset(&child, 0, sizeof(child));
+		child.argv = args;
+		args[0] = editor;
+		args[1] = path;
+		args[2] = NULL;
+
+		if (run_command(&child))
+			die("There was a problem with the editor %s.", editor);
+	}
 
 	if (strbuf_read_file(buffer, path, 0) < 0)
 		die("could not read message file '%s': %s",

^ permalink raw reply related

* Re: git rebase --skip
From: Jeff King @ 2007-11-08 23:16 UTC (permalink / raw)
  To: Björn Steinbrink; +Cc: Andreas Ericsson, Mike Hommey, git
In-Reply-To: <20071108104403.GB31187@atjola.homenet>

On Thu, Nov 08, 2007 at 11:44:03AM +0100, Björn Steinbrink wrote:

> > How about if the state to skip was stashed, the patch reapplied and the
> > differences compared. If they were identical, go ahead and force the
> > reset --hard, otherwise abort. That way, --skip will dwim only when
> > it's safe, and all the lost work can be automagically created by
> > just re-applying the patch again?
> 
> I'd prefer the --force option suggested in some other mail. Maybe I'm
> just not manly enough, but messing up a rebase can mean lots of
> duplicated work, so I'm rather happy with no dwim at all. Maybe for the
> real manly users out there, add a rebase.alwaysForce option so they can
> laugh at me for not using that ;-)

Personally, I don't see the point of a --force option; it turns your work
flow from:

  1. git-rebase --skip
  2. Oops, I guess I have to reset.
  3. git-reset --hard; git-rebase --skip

to:

  1. same as above
  2. same as above
  3. git-rebase --force --skip

I guess it's a little bit easier to explain to new users, but it in no
way eliminates the annoyance of "I expected this to work, and it
didn't, so now I have to think about what happened and enter another
command."

AIUI, Andreas's proposal is not so much DWIM as "do the obvious thing,
but include a safety valve to prevent throwing away work." Is there
actually a case where it would not have the desired effect?

-Peff

^ permalink raw reply

* Re: [NEW REPLACEMENT PATCH] git-checkout: Add a test case for relative paths use.
From: Johannes Schindelin @ 2007-11-08 23:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: David Symonds, git, Andreas Ericsson
In-Reply-To: <7v7iks31lm.fsf@gitster.siamese.dyndns.org>

Hi,

On Thu, 8 Nov 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> +	mkdir dir2 &&
> >> +	echo bonjour > dir2/file2 &&
> >> +	git add dir2/file2 &&
> >> +	git commit -m "populate tree"
> >> +
> >> +'
> >
> > Please lose the empty line before the closing quote.  (This applies to all 
> > tests.)
> 
> I personaly find the extra blank lines before and after the
> indented test body easier to read. 

Personally, I don't.  But you are the maintainer.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 1/3] Refactor working tree setup
From: Miles Bader @ 2007-11-08 22:41 UTC (permalink / raw)
  To: Mike Hommey; +Cc: git, Junio C Hamano
In-Reply-To: <1194088993-25692-1-git-send-email-mh@glandium.org>

Mike Hommey <mh@glandium.org> writes:
> Create a setup_work_tree() that can be used from any command requiring
> a working tree conditionally.
...
> +void setup_work_tree(void) {
> +	const char *work_tree = get_git_work_tree();

Hi, could you please not use this "function begin brace at EOL" style?

It's inconsistent with the rest of the source, makes the code harder to
read, and confuses Emacs in some cases[1].  Also it's an abomination
unto God, but I imagine it's the first of these reasons that you'll care
about the most... :-)

Thanks,

-Miles


[1] Until quite recently, Emacs c-mode couldn't find the beginning of
such functions (recent versions of c-mode seem OK though).

-- 
The secret to creativity is knowing how to hide your sources.
  --Albert Einstein

^ permalink raw reply

* Re: [PATCH 3/3] git-fetch: avoid local fetching from alternate (again)
From: Shawn O. Pearce @ 2007-11-08 22:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20071108112254.GN14735@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> wrote:
> Junio C Hamano <gitster@pobox.com> wrote:
> > "Shawn O. Pearce" <spearce@spearce.org> writes:
> > 
> > > I'm starting to suspect heap corruption again in builtin-fetch.
> > > This patch alters the malloc() calls we are doing and may be shifting
> > > something around just enough in memory to cause a data overwrite or
> > > something and that's why this tag just drops out of the linked list?
> > > But then why does that happen in the test suite but not outside.
> > > Maybe because the test suite is setting environment variables that
> > > I'm not and the impact of those combined with these additional
> > > mallocs is what is breaking it?  *sigh*
> 
> Found it.  This ain't pretty.  Remember, what's failing in the test
> suite is we aren't getting "tag-three-file" automatically followed
> during fetch.  This is an annotated tag referring to a blob.

Here's one possible way of fixing this.  I think what we want is to
include "--not --all" when we run the object listing immediately
after the fetch so we mark *only* those objects we just fetched
and ignore the ones we already had reachable through existing refs.
This makes the walking cost proportional to the size of the fetch
and not the size of the object database.

Unfortunately when you insert "--not" in front of "--all" in the args
array below the test vectors all fail again.  Apparently we already
have the blob that "tag-three-file" refers to so its not something
we walk over during this process and the tag isn't followed.
That happens because the test repository we are fetching into
already has a number of objects through its refs/remotes/origin/*
namespace created by git-clone and the blob is already considered
to be reachable.

I'm not formatting this as a real patch because I'm not yet sure this
is the right way to fix the automatic tag following code.  Or the
right way to abuse the revision walking machinary within git-fetch
to implement such a feature.  Comments would be most appreciated.  :)


--8>--
diff --git a/builtin-fetch.c b/builtin-fetch.c
index 847db73..77f1901 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -8,6 +8,9 @@
 #include "path-list.h"
 #include "remote.h"
 #include "transport.h"
+#include "diff.h"
+#include "list-objects.h"
+#include "revision.h"
 
 static const char fetch_usage[] = "git-fetch [-a | --append] [--upload-pack <upload-pack>] [-f | --force] [--no-tags] [-t | --tags] [-k | --keep] [-u | --update-head-ok] [--depth <depth>] [-v | --verbose] [<repository> <refspec>...]";
 
@@ -335,11 +338,43 @@ static void store_updated_refs(const char *url, struct ref *ref_map)
 	fclose(fp);
 }
 
+static void mark_just_fetched_commit(struct commit *commit)
+{
+}
+
+static void mark_just_fetched_object(struct object_array_entry *p)
+{
+	if (p->item->type == OBJ_BLOB && !has_sha1_file(p->item->sha1))
+		die("missing blob object '%s'", sha1_to_hex(p->item->sha1));
+}
+
 static int fetch_refs(struct transport *transport, struct ref *ref_map)
 {
 	int ret = transport_fetch_refs(transport, ref_map);
-	if (!ret)
+	if (!ret) {
+		const char *args[] = { "rev-list", "--objects", "--all", NULL};
+		struct rev_info revs;
+		struct ref *ref;
+
+		init_revisions(&revs, NULL);
+		setup_revisions(ARRAY_SIZE(args) - 1, args, &revs, NULL);
+		save_commit_buffer = 0;
+		track_object_refs = 0;
+
+		for (ref = ref_map; ref; ref = ref->next) {
+			struct object *o = parse_object(ref->old_sha1);
+			if (o)
+				add_pending_object(&revs, o, "(just-fetched)");
+		}
+
+		prepare_revision_walk(&revs);
+		mark_edges_uninteresting(revs.commits, &revs, NULL);
+		traverse_commit_list(&revs,
+			mark_just_fetched_commit,
+			mark_just_fetched_object);
+
 		store_updated_refs(transport->url, ref_map);
+	}
 	transport_unlock_pack(transport);
 	return ret;
 }
@@ -365,6 +400,7 @@ static struct ref *find_non_local_tags(struct transport *transport,
 	struct ref *ref_map = NULL;
 	struct ref **tail = &ref_map;
 	const struct ref *ref;
+	struct object *obj;
 
 	for_each_ref(add_existing, &existing_refs);
 	for (ref = transport_get_remote_refs(transport); ref; ref = ref->next) {
@@ -389,7 +425,8 @@ static struct ref *find_non_local_tags(struct transport *transport,
 
 		if (!path_list_has_path(&existing_refs, ref_name) &&
 		    !path_list_has_path(&new_refs, ref_name) &&
-		    lookup_object(ref->old_sha1)) {
+		    (obj = lookup_object(ref->old_sha1)) &&
+		    obj->flags & SEEN) {
 			path_list_insert(ref_name, &new_refs);
 
 			rm = alloc_ref(strlen(ref_name) + 1);
diff --git a/list-objects.c b/list-objects.c
index e5c88c2..78bf04c 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -170,4 +170,5 @@ void traverse_commit_list(struct rev_info *revs,
 	}
 	for (i = 0; i < objects.nr; i++)
 		show_object(&objects.objects[i]);
+	free(objects.objects);
 }

-- 
Shawn.

^ permalink raw reply related

* Re: [PATCH 2/3] Fix sed script to work with AIX and BSD sed.
From: Benoit Sigoure @ 2007-11-08 22:15 UTC (permalink / raw)
  To: Ralf Wildenhues; +Cc: git
In-Reply-To: <20071108214824.GH31439@ins.uni-bonn.de>

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

On Nov 8, 2007, at 10:48 PM, Ralf Wildenhues wrote:

> diff --git a/git-bisect.sh b/git-bisect.sh
> index c18bd32..3aac816 100755
> --- a/git-bisect.sh
> +++ b/git-bisect.sh
> @@ -276,7 +276,8 @@ exit_if_skipped_commits () {
>  	if expr "$_tried" : ".*[|].*" > /dev/null ; then
>  		echo "There are only 'skip'ped commit left to test."
>  		echo "The first bad commit could be any of:"
> -		echo "$_tried" | sed -e 's/[|]/\n/g'
> +		echo "$_tried" | sed -e 's/[|]/\

Just out of curiosity, is there any valid reason to use `[' and `]'  
in this RE?  By default sed does not use extended RE (at least none  
of the implementation I know of) so why not just use sed 's/|/...' ?

> +/g'
>  		echo "We cannot bisect more!"
>  		exit 2
>  	fi

-- 
Benoit Sigoure aka Tsuna
EPITA Research and Development Laboratory



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 186 bytes --]

^ permalink raw reply

* Re: [PATCH] hooks--update: fix test for properly set up project description file
From: Junio C Hamano @ 2007-11-08 22:13 UTC (permalink / raw)
  To: Gerrit Pape; +Cc: Andreas Ericsson, git
In-Reply-To: <20071108140200.24902.qmail@f23d7e396b1523.315fe32.mid.smarden.org>

Gerrit Pape <pape@smarden.org> writes:

> The update hook template intends to abort if the project description file
> hasn't been adjusted or is empty.  This patch fixes the check for 'being
> adjusted'.
>
> Signed-off-by: Gerrit Pape <pape@smarden.org>
> ---
>
> On Tue, Nov 06, 2007 at 02:55:55PM +0100, Andreas Ericsson wrote:
>> Gerrit Pape wrote:
>> > # check for no description
>> >-projectdesc=$(sed -e '1p' "$GIT_DIR/description")
>> >-if [ -z "$projectdesc" -o "$projectdesc" = "Unnamed repository; edit
>> >this
>> >file to name it for gitweb" ]; then
>> >+projectdesc=$(sed -ne '1p' "$GIT_DIR/description")
>>
>> Write this as
>>       projectdesc=$(sed -e 1q "$GIT_DIR/description")
>> instead. It's a little shorter, a little faster and slightly more
>> portable.

I even suspect that the original 'p' is indeed a typo of 'q'.

^ permalink raw reply

* Re: GIT_DIR/--git-dir question
From: Junio C Hamano @ 2007-11-08 22:00 UTC (permalink / raw)
  To: Joakim Tjernlund; +Cc: git
In-Reply-To: <007c01c8223a$6af2e550$5267a8c0@Jocke>

"Joakim Tjernlund" <joakim.tjernlund@transmode.se> writes:

> I just started to look at having multiple work trees, each working on a separate branch,
> using the same git repo.

I suspect contrib/workdir is what you are looking for not
GIT_DIR.

^ permalink raw reply

* Re: git push mirror mode
From: Junio C Hamano @ 2007-11-08 21:53 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Andy Whitcroft, git
In-Reply-To: <Pine.LNX.4.64.0711081218090.4362@racer.site>

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

> On Thu, 8 Nov 2007, Andy Whitcroft wrote:
>
>> Ok, sometime back Junio sent out a proof-of-concept change to
>> send-pack allowing a mirror mode.
>
> You added/left his sign-off, but did not attribute the patches to him.  

No big deal; I do not think much of my changes remain in the
result.  Mentioning "inspired by" would be nice as courtesy, but
I think this is mostly Andy's work.

As I haven't seen _his_ part of the change before he posted this
updated patch, copying my S-o-b line wasn't necessary either.

^ 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