Git development
 help / color / mirror / Atom feed
* Re: Questions about git-fast-import for cvs2svn
From: Shawn O. Pearce @ 2007-07-16  6:19 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Michael Haggerty, git
In-Reply-To: <alpine.LFD.0.999.0707151119120.20061@woody.linux-foundation.org>

Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Sun, 15 Jul 2007, Michael Haggerty wrote:
> > 2. It appears that author/committer require an email address.  How
> > important is a valid email address here?
> 
> Git itself doesn't really care, and many CVS conversions have just 
> converted the username into "user <user>", but from a QoI standpoint it's 
> much nicer if you at least were to allow the kind of conversion that 
> allows user-name to be associated with an email.
> 
> Maybe git-fast-import could be taught to do the kind of user name 
> conversion that we already do for CVS imports.. Shawn?

It could, but I'm not sure I want to implement it.  ;-)

I pretty much view source->Git translation as the business/policy
of the frontend, not of fast-import.  But we have three frontends
that all share the same file format (git-cvsimport, git-svnimport,
git-svn), and are all independent implementations.  Maybe pushing
it down into a tool like fast-import would benefit a lot of users,
and thus should be done.

I'll put it on my todo list.  Which is much longer than I have
time for.
 
> > 5. Is there any significance at all to the order that commits are output
> > to git-fast-import?  Obviously, blobs have to be defined before they are
> > used, and '<committish>'s have to be defined before they are referenced.
> >  But is there any other significance to the order of commits?
> 
> Not afaik. Git internally very fundamentally simply doesn't care (there 
> simply _is_ no object ordering, there is just objects that point to other 
> objects), and I don't think git-fast-import could possibly care either. 
> You do need to be "topologically" sorted (since you cannot even point to 
> commits without having their SHA1's), but that should be it.

Linus is completely correct here.  The only requirement on data
ordering is that all parent commits (from/merge lines) must
come before any child that depends on them.  But that's a pretty
reasonable request, as almost all VCS systems want data to come in
at least that order, if not something even more strict.

In theory marks could be used to stub in commits and let you feed
them out of order, but to make that work fast-import would need to
buffer them until it saw everything it needed to produce a SHA-1.
Not exactly a good idea.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] vc-git: support asynchronous annotations, and improve versioning.
From: Junio C Hamano @ 2007-07-16  6:14 UTC (permalink / raw)
  To: David Kastrup; +Cc: git
In-Reply-To: <85k5t0or0d.fsf@lola.goethe.zz>

David Kastrup <dak@gnu.org> writes:

> How do I go about resubmitting with better comments?  Make an extra
> branch and redo the part in artificial new commits?

The original was not even numbered so I cannot judge how the
development proceeded, but I think the series is somewhat ill
organized and the problem is not just commit log messages.

For example, "vc-git.el: various improvements." removes NIL
check for 'commit' and then later "Make several improvements"
reinstates that check, perhaps because you realized the earlier
change to remove it was a mistake (without the explanation in
the log I can only guess the reason).  Same for 'async to 0
change which is made by "Make synchronous for now".

Preserving a honest, true-to-reality, history is one thing
during the development, but when the overall improvements and
enhancements reached a usable state, it is much more pleasant to
review if the series is presented as if you knew what necessary
changes are and what pitfalls to avoid from the beginning, and
did your development in logical sequence, building one commit on
top of the previous enhancements without "oops, that was a
mistake so fix it while we are improving something else".

My cursory review suggests that:

* contrib/emacs/Makefile: Also install .el files.

This is a good patch, independent from anything else.

* contrib/emacs/vc-git.el: various improvements.
* Make several improvements and get annotations to work (Emacs support pending).

These two try to improve the same area, but the latter contains
bugfixes for the former, not necessarily a logical succession
(if it were a logical sequence to build and enhance, the first
should be usable by itself, without a later bugfix).

If these were to be made more than one logically independent
patches, then probably the first would be to simplify
vc-git-symbolic-commit implementation and update its callers
(without having to say "oops, I needed the check for NIL commit
after all" later), and without changing what the callers do.
The second one would be "prev and next can be made much
simpler".  The third one would be "annotate can take 'rev' and
extract-revision-at-line is now usable".  But judging from how
closely these changes are tied together (all of them depend on
the external interface to vc-git-symbolic-commit) and how small
each change is, I would probably make them a single commit that
you do not have to say "oops" anywhere.

* vc-git: support asynchronous annotations, and improve versioning.
* (vc-git-annotate-command):  Make synchronous for now.

The same discussion here.  The first one goes in the right
direction, but then the moment of "oops" comes --- async does
not necessarily work.  Probably the right separation, if these
two were to become two separate commits, would be to add async
(but leave it synchronous, with a big code comment at the
calling site of vc-do-command why you pass 0 instead of async),
and then make "and improve versioning" part (whatever that is --
I cannot figure out which part of the patch "improve"-ment
refers to) a separate commit.

I would end this message with a very useful URL:

    http://article.gmane.org/gmane.comp.version-control.git/10894

I am with Linus 100%, regarding "honest" vs "disgusting"
history.

^ permalink raw reply

* Re: [WIP PATCH 6/6] notes: add notes-index for a substantial speedup.
From: Shawn O. Pearce @ 2007-07-16  6:01 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Alberto Bertogli, git, gitster, Johan Herland
In-Reply-To: <Pine.LNX.4.64.0707160025480.14781@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> 
> Actually, this commit adds two methods for a notes index:
> 
> - a sorted list with a fan out to help binary search, and
> - a modified hash table.
> 
> It also adds a test which is used to determine the best algorithm.

I know this is a nice backwards compatible way to organize notes,
and to make them reasonably efficiently found, but I'd almost
rather just see them crammed into the packfile alongside of the
commit it annotates, so that the packfile reader can quickly find
the annotation at the same time it finds the commit.

aka packv4.

Ok, enough dreaming for today.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] Fix whitespace in "Format of STDIN stream" of fast-import
From: Shawn O. Pearce @ 2007-07-16  5:39 UTC (permalink / raw)
  To: Alex Riesen; +Cc: git, Junio C Hamano
In-Reply-To: <20070715085740.GB2568@steel.home>

Alex Riesen <raa.lkml@gmail.com> wrote:
> Something probably assumed that HT indentation is 4 characters.

Yea, my f*!&!(@*& text editor.  Thanks for the fix.  Applied.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] Fix git-p4 on Windows to not use the Posix sysconf
From: Shawn O. Pearce @ 2007-07-16  5:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Simon Hausmann, Alex Riesen, Marius Storm-Olsen, git
In-Reply-To: <7vmyxydwld.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
> > Thanks.  Marius' patch was whitespace damaged in the context lines,
> > but it was easily repaired.  I've got a couple of other small items
> > in my fastimport repository that I'm going to ask Junio to include
> > in 1.5.3 shortly.
> 
> Thanks for taking care of this.  I have pulled gfi master into
> 'master'.  Perhaps we would want to tag -rc2 this weekend, run
> with it for a week or so and see if we need -rc3 before the
> final.

I'm not entirely sure how we're going to handle the git-p4 patches;
I see there's already another set available to use '-x -' to avoid
command line length problems.

I'm more than happy to play patch monkey and ship them through the
fastimport repository, but since I'm not a p4 user that offers little
value to the process, other than perhaps to save you a little time.

Simon suggested he might setup a git fork on repo.or.cz himself, at
which point you could pull the patches for git-p4 directly from him.

Simon?

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] vc-git: support asynchronous annotations, and improve versioning.
From: David Kastrup @ 2007-07-16  5:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v3azpavae.fsf@assigned-by-dhcp.cox.net>

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

> David Kastrup <dak@gnu.org> writes:
>
>> (vc-git-symbolic-commit): Allow nil to pass through.
>> (vc-git-previous-version): Use explicit parent argument.
>> (vc-git-next-version): Simplify.
>> (vc-git-annotate-command): Use `vc-do-command'.
>> (vc-git-annotate-extract-revision-at-line): Rename from
>> `vc-annotate-extract-revision-at-line'.
>> (vc-git-checkout): Make nicer way of ensuring encoding.
>>
>
> These do not seem to match what the patch does at all.
> I give up.

The previous changelog entry crept in as well.

> Will apply only the "install .el, too" change from the confusing
> series for now.

Fair enough.

How do I go about resubmitting with better comments?  Make an extra
branch and redo the part in artificial new commits?

I have no experience.  Any good pointers how one does this sort of
propose, rewind, propose stuff most conveniently?

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply

* Re: [PATCH] contrib/emacs/vc-git.el: various improvements.
From: David Kastrup @ 2007-07-16  5:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vejj9avaf.fsf@assigned-by-dhcp.cox.net>

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

> David Kastrup <dak@gnu.org> writes:
>
>> (vc-git-previous-version): Simplify.
>> (vc-git-next-version): Simplify and make more efficient.
>
> If you make the result symbolic using vc-git-symbolic-commit, do you
> need to add --abbrev/--abbrev-commit to these functions?  These
> options have very small but still non zero cost.

vc-git-symbolic-commit may not always find a name (at least it did not
while I still used --tags).  The abbrev provides a fallback in that
case.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply

* Re: [PATCH 4/6] Add a test script for "git notes"
From: Junio C Hamano @ 2007-07-16  5:11 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Alberto Bertogli, git, gitster, Johan Herland
In-Reply-To: <Pine.LNX.4.64.0707160024060.14781@racer.site>

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

> diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
> new file mode 100755
> index 0000000..eb50191
> --- /dev/null
> +++ b/t/t3301-notes.sh
> @@ -0,0 +1,63 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2007 Johannes E. Schindelin
> +#
> +
> +test_description='Test commit notes'
> +
> +. ./test-lib.sh
> +
> +test_expect_success setup '
> +	: > a1 &&
> +	git add a1 &&
> +	test_tick &&
> +	git commit -m 1st &&
> +	: > a2 &&
> +	git add a2 &&
> +	test_tick &&
> +	git commit -m 2nd
> +'

Does not test the failure mode of not having a HEAD yet.

> +cat > fake_editor.sh << EOF
> +echo "\$MSG" > "\$1"
> +echo "\$MSG" >& 2
> +EOF

You can avoid all these backslashes by saying:

	cat >fake_editor.sh <<\EOF
        echo "$MSG" >"$1"
        echo "$MSG" >&2
	EOF

> +chmod a+x fake_editor.sh
> +VISUAL="$(pwd)"/fake_editor.sh
> +export VISUAL

Not that it hurts anybody, but do you really need that $(pwd),
instead of "./fake_editor.sh"?

> +
> +test_expect_success 'need notes ref' '
> +	! MSG=1 git notes edit &&
> +	! MSG=2 git notes show
> +'
> +
> +test_expect_success 'create notes' '
> +	git config core.notesRef refs/notes/commits &&
> +	MSG=b1 git notes edit &&
> +cat .git/new-notes &&
> +test b1 = "$(cat .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^
> +'

Is there particular reason for that (lack of) indentation for
the two lines among them?

I think it is a bug to leave ".git/new-notes" and friends
behind.

> +
> +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

Hmph.  This makes the reader wonder why this is not optional,
perhaps linked to --decorate option somehow.

^ permalink raw reply

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

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

> 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 |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 63 insertions(+), 1 deletions(-)
>  create mode 100755 git-notes.sh
>
> diff --git a/git-notes.sh b/git-notes.sh
> new file mode 100755
> index 0000000..e0ad0b9
> --- /dev/null
> +++ b/git-notes.sh
> @@ -0,0 +1,61 @@
> +#!/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" &&
> +	die "No notes ref set."

	test -n "${GIT_NOTES_REF=$(git config core.notesref)}" || die

> +COMMIT=$(git rev-parse --verify --default HEAD "$2")

This silently annotates the HEAD commit if $2 is misspelled, I
suspect.  Also if HEAD does not exist, COMMIT will be empty and
this whole command will exit with non-zero status, which you
would want to catch here...

> +NAME=$(echo $COMMIT | sed "s/^../&\//")

... or here.

> +case "$1" in
> +edit)
> +	MESSAGE="$GIT_DIR"/new-notes
> +	GIT_NOTES_REF= git log -1 $COMMIT | sed "s/^/#/" > "$MESSAGE"

$MESSAGE and its associated temporary file needs to be cleaned
up upon command exit; perhaps a trap is in order.

> +	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 $OLDTIP"
> +		git read-tree $GIT_NOTES_REF || die "Could not read index"
> +		git cat-file blob :$NAME >> "$MESSAGE" 2> /dev/null
> +	fi

I take that OLDTIP is a typo.

	if CURRENT_HEAD=$(git show-ref -s "$GIT_NOTES_REF")
        then
		PARENT="-p $CURRENT_HEAD"
                ...
	else
        	PARENT=
	fi

> +
> +	${VISUAL:-${EDITOR:-vi}} "$MESSAGE"
> +
> +	grep -v ^# < "$MESSAGE" | git stripspace > "$MESSAGE".processed

Makes us wonder if we would want to teach hash-stripping to
git-stripspace, doesn't it?

> +	mv "$MESSAGE".processed "$MESSAGE"
> +	if [ -z "$(cat "$MESSAGE")" ]; then

Make this 'if test -s "$MESSAGE"' and swap then/else clause
around; no reason to slurp the value into your shell.

> +		test -z "$CURRENT_HEAD" &&
> +			die "Will not initialise with empty tree"
> +		git update-index --force-remove $NAME ||
> +			die "Could not update index"
> +	else
> +		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"
> +	fi
> +
> +	TREE=$(git write-tree) || die "Could not write tree"
> +	NEW_HEAD=$(: | git commit-tree $TREE $PARENT) ||
> +		die "Could not annotate"

Hmph.  How about "echo Annotate $COMMIT | git commit-tree..."?

> +	case "$CURRENT_HEAD" in
> +	'') git update-ref $GIT_NOTES_REF $NEW_HEAD ;;
> +	*) git update-ref $GIT_NOTES_REF $NEW_HEAD $CURRENT_HEAD;;
> +	esac
> +;;

There are some places that have "$GIT_NOTES_REF" in dq and some
places you don't.  I think GIT_NOTES_REF begins with refs/ and
consists only of valid refname characters, so unless the user
wants to shoot himself in the foot it should be Ok, but we
probably would want to quote it.

Also, as unquoted $CURRENT_HEAD will not even count as a
parameter to update-ref, you do not have to do that case/esac,
but simply do:

	git update-ref "$GIT_NOTES_REF" $NEW_HEAD $CURRENT_HEAD

Would we have reflog for this ref?  What would we want to see as
the message if we do?

^ permalink raw reply

* Re: [PATCH 2/6] Introduce commit notes
From: Junio C Hamano @ 2007-07-16  5:11 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Alberto Bertogli, git, gitster, Johan Herland
In-Reply-To: <Pine.LNX.4.64.0707160022560.14781@racer.site>

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

> +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.
> +

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}"?

> diff --git a/commit.c b/commit.c
> index 0c350bc..3529b6a 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);
> +
>  	buf[offset] = '\0';
>  	free(reencoded);
>  	return offset;

This makes me wonder if there are cases where "notes" need to be
reencoded to honor log_output_encoding.

Since more and more people live in UTF-8 only world, and this is
a _new_ feature anyway, we could declare that "notes" blobs MUST
be encoded in UTF-8 upfront, but even if we did so we would need
reencoding to log_output_encoding, I suspect.

> diff --git a/notes.c b/notes.c
> new file mode 100644
> index 0000000..5d1bb1a
> --- /dev/null
> +++ b/notes.c
> @@ -0,0 +1,64 @@
> +#include "cache.h"
> +#include "commit.h"
> +#include "notes.h"
> +#include "refs.h"
> +
> +static int initialized;
> +
> +void get_commit_notes(const struct commit *commit,
> +		char **buf_p, unsigned long *offset_p, unsigned long *space_p)
> +{
> +	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);
> +		if (env) {
> +			if (notes_ref_name)
> +				free(notes_ref_name);
> +			notes_ref_name = xstrdup(getenv(GIT_NOTES_REF));

	xstrdup(env)?

> +		} else if (!notes_ref_name)
> +			notes_ref_name = xstrdup("refs/notes/commits");

We would probably want to give another preprocessor constant for
this hardcoded string, next to GIT_NOTES_REF definition in cache.h.

> +		if (notes_ref_name && read_ref(notes_ref_name, sha1)) {
> +			free(notes_ref_name);
> +			notes_ref_name = NULL;
> +		}
> +		initialized = 1;
> +	}
> +	if (!notes_ref_name)
> +		return;
> +
> +	hex = sha1_to_hex(commit->object.sha1);
> +	snprintf(name, sizeof(name), "%s:%.*s/%.*s",
> +			notes_ref_name, 2, hex, 38, hex + 2);

Too long a notes_ref_name and it won't overrun the buffer but
the failure is not detected, and ...

> +	if (get_sha1(name, sha1))
> +		return;

... this would fail silently, leaving the user scratching his head.

> +
> +	if (!(msg = read_sha1_file(sha1, &type, &msglen)) || !msglen)
> +		return;

What's in "type" at this point?  Having a tree there is not an
error?

> +	/* we will end the annotation by a newline anyway. */
> +	if (msg[msglen - 1] == '\n')
> +		msglen--;
> +	ALLOC_GROW(*buf_p, *offset_p + 14 + msglen, *space_p);
> +	*offset_p += sprintf(*buf_p + *offset_p, "\nNotes:\n");

Fourteen is because...

> +
> +	for (msgoffset = 0; msgoffset < msglen;) {
> +		int linelen = get_line_length(msg + msgoffset, msglen);
> +
> +		ALLOC_GROW(*buf_p, *offset_p + linelen + 6, *space_p);
> +		*offset_p += sprintf(*buf_p + *offset_p,
> +				"    %.*s", linelen, msg + msgoffset);

Six is because...

> +		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..aed80e7
> --- /dev/null
> +++ b/notes.h
> @@ -0,0 +1,9 @@
> +#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);
> +
> +#define GIT_NOTES_REF "GIT_NOTES_REF"

Judging from the existing entries in cache.h, it seems that
GIT_NOTES_REF_ENVIRONMENT would be more appropriate preprocessor
symbol for this.  Also let's have this in cache.h next to
GIT_DIR_ENVIRONMENT and friends, with another definition for
"refs/notes/commits".

^ permalink raw reply

* [PATCH] git-svn: fix commiting renames over DAV with funky file names
From: Eric Wong @ 2007-07-16  4:53 UTC (permalink / raw)
  To: Rogan Dawes, Junio C Hamano; +Cc: git
In-Reply-To: <20070712090635.GA18155@mayonaise>

Renaming files with non-URI friendly characters caused
breakage when committing to DAV repositories (over http(s)).

Even if I try leaving out the $self->{url} from the return value
of url_path(), a partial (without host), unescaped path name
does not work.

Filenames for DAV repos need to be URI-encoded before being
passed to the library.  Since this bug did not affect file://
and svn:// repos, the git-svn test library needed to be expanded
to include support for starting Apache with mod_dav_svn enabled.

This new test is not enabled by default, but can be enabled by
setting SVN_HTTPD_PORT to any available TCP/IP port on
127.0.0.1.

Additionally, for running this test, the following variables
(with defaults shown) can be changed for the suitable system.
The default values are set for Debian systems:

  SVN_HTTPD_MODULE_PATH=/usr/lib/apache2/modules
  SVN_HTTPD_PATH=/usr/sbin/apache2

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---

 Rogan: the patch should help, a single space anywhere in the
 path causes SVN to screw up the file names.  Of course I'm at a
 loss as to why only DAV repositories need it and why the SVN
 libraries don't abstract that away from me.

 git-svn.perl                             |    3 +
 t/lib-git-svn.sh                         |   34 ++++++++++
 t/t9115-git-svn-dcommit-funky-renames.sh |   54 +++++++++++++++
 t/t9115/funky-names.dump                 |  105 ++++++++++++++++++++++++++++++
 4 files changed, 196 insertions(+), 0 deletions(-)
 create mode 100755 t/t9115-git-svn-dcommit-funky-renames.sh
 create mode 100644 t/t9115/funky-names.dump

diff --git a/git-svn.perl b/git-svn.perl
index 299b40f..01c3904 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -2724,6 +2724,9 @@ sub repo_path {
 
 sub url_path {
 	my ($self, $path) = @_;
+	if ($self->{url} =~ m#^https?://#) {
+		$path =~ s/([^a-zA-Z0-9_.-])/uc sprintf("%%%02x",ord($1))/eg;
+	}
 	$self->{url} . '/' . $self->repo_path($path);
 }
 
diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
index f6fe78c..8d4a447 100644
--- a/t/lib-git-svn.sh
+++ b/t/lib-git-svn.sh
@@ -48,3 +48,37 @@ svnrepo="file://$svnrepo"
 poke() {
 	test-chmtime +1 "$1"
 }
+
+SVN_HTTPD_MODULE_PATH=${SVN_HTTPD_MODULE_PATH-'/usr/lib/apache2/modules'}
+SVN_HTTPD_PATH=${SVN_HTTPD_PATH-'/usr/sbin/apache2'}
+
+start_httpd () {
+	if test -z "$SVN_HTTPD_PORT"
+	then
+		echo >&2 'SVN_HTTPD_PORT is not defined!'
+		return
+	fi
+
+	mkdir "$GIT_DIR"/logs
+
+	cat > "$GIT_DIR/httpd.conf" <<EOF
+ServerName "git-svn test"
+ServerRoot "$GIT_DIR"
+DocumentRoot "$GIT_DIR"
+PidFile "$GIT_DIR/httpd.pid"
+Listen 127.0.0.1:$SVN_HTTPD_PORT
+LoadModule dav_module $SVN_HTTPD_MODULE_PATH/mod_dav.so
+LoadModule dav_svn_module $SVN_HTTPD_MODULE_PATH/mod_dav_svn.so
+<Location /svn>
+	DAV svn
+	SVNPath $rawsvnrepo
+</Location>
+EOF
+	"$SVN_HTTPD_PATH" -f "$GIT_DIR"/httpd.conf -k start
+	svnrepo=http://127.0.0.1:$SVN_HTTPD_PORT/svn
+}
+
+stop_httpd () {
+	test -z "$SVN_HTTPD_PORT" && return
+	"$SVN_HTTPD_PATH" -f "$GIT_DIR"/httpd.conf -k stop
+}
diff --git a/t/t9115-git-svn-dcommit-funky-renames.sh b/t/t9115-git-svn-dcommit-funky-renames.sh
new file mode 100755
index 0000000..182299c
--- /dev/null
+++ b/t/t9115-git-svn-dcommit-funky-renames.sh
@@ -0,0 +1,54 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Eric Wong
+
+
+test_description='git-svn dcommit can commit renames of files with ugly names'
+
+. ./lib-git-svn.sh
+
+test_expect_success 'load repository with strange names' "
+	svnadmin load -q $rawsvnrepo < ../t9115/funky-names.dump &&
+	start_httpd
+	"
+
+test_expect_success 'init and fetch repository' "
+	git svn init $svnrepo &&
+	git svn fetch &&
+	git reset --hard git-svn
+	"
+
+test_expect_success 'create file in existing ugly and empty dir' '
+	mkdir "#{bad_directory_name}" &&
+	echo hi > "#{bad_directory_name}/ foo" &&
+	git update-index --add "#{bad_directory_name}/ foo" &&
+	git commit -m "new file in ugly parent" &&
+	git svn dcommit
+	'
+
+test_expect_success 'rename ugly file' '
+	git mv "#{bad_directory_name}/ foo" "file name with feces" &&
+	git commit -m "rename ugly file" &&
+	git svn dcommit
+	'
+
+test_expect_success 'rename pretty file' '
+	echo :x > pretty &&
+	git update-index --add pretty &&
+	git commit -m "pretty :x" &&
+	git svn dcommit &&
+	mkdir regular_dir_name &&
+	git mv pretty regular_dir_name/pretty &&
+	git commit -m "moved pretty file" &&
+	git svn dcommit
+	'
+
+test_expect_success 'rename pretty file into ugly one' '
+	git mv regular_dir_name/pretty "#{bad_directory_name}/ booboo" &&
+	git commit -m booboo &&
+	git svn dcommit
+	'
+
+stop_httpd
+
+test_done
diff --git a/t/t9115/funky-names.dump b/t/t9115/funky-names.dump
new file mode 100644
index 0000000..da0440a
--- /dev/null
+++ b/t/t9115/funky-names.dump
@@ -0,0 +1,105 @@
+SVN-fs-dump-format-version: 2
+
+UUID: 819c44fe-2bcc-4066-88e4-985e2bc0b418
+
+Revision-number: 0
+Prop-content-length: 56
+Content-length: 56
+
+K 8
+svn:date
+V 27
+2007-07-12T07:54:26.062914Z
+PROPS-END
+
+Revision-number: 1
+Prop-content-length: 152
+Content-length: 152
+
+K 7
+svn:log
+V 44
+what will those wacky people think of next?
+
+K 10
+svn:author
+V 12
+normalperson
+K 8
+svn:date
+V 27
+2007-07-12T08:00:05.011573Z
+PROPS-END
+
+Node-path:  leading space
+Node-kind: dir
+Node-action: add
+Prop-content-length: 10
+Content-length: 10
+
+PROPS-END
+
+
+Node-path:  leading space file
+Node-kind: file
+Node-action: add
+Prop-content-length: 10
+Text-content-length: 5
+Text-content-md5: e4fa20c67542cdc21271e08d329397ab
+Content-length: 15
+
+PROPS-END
+ugly
+
+
+Node-path: #{bad_directory_name}
+Node-kind: dir
+Node-action: add
+Prop-content-length: 10
+Content-length: 10
+
+PROPS-END
+
+
+Node-path: #{cool_name}
+Node-kind: file
+Node-action: add
+Prop-content-length: 10
+Text-content-length: 18
+Text-content-md5: 87dac40ca337dfa3dcc8911388c3ddda
+Content-length: 28
+
+PROPS-END
+strange name here
+
+
+Node-path: dir name with spaces
+Node-kind: dir
+Node-action: add
+Prop-content-length: 10
+Content-length: 10
+
+PROPS-END
+
+
+Node-path: file name with spaces
+Node-kind: file
+Node-action: add
+Prop-content-length: 10
+Text-content-length: 7
+Text-content-md5: c1f10cfd640618484a2a475c11410fd3
+Content-length: 17
+
+PROPS-END
+spaces
+
+
+Node-path: regular_dir_name
+Node-kind: dir
+Node-action: add
+Prop-content-length: 10
+Content-length: 10
+
+PROPS-END
+
+
-- 
1.5.3.rc2.1.g4e223

-- 
Eric Wong

^ permalink raw reply related

* [PATCH 2/2] git-p4: input to "p4 files" by stdin instead of arguments
From: Scott Lamb @ 2007-07-16  3:58 UTC (permalink / raw)
  To: Simon Hausmann; +Cc: git, Scott Lamb
In-Reply-To: <11845582912155-git-send-email-slamb@slamb.org>

This approach, suggested by Alex Riesen, bypasses the need for xargs-style
argument list handling. The handling in question looks broken in a corner
case with SC_ARG_MAX=4096 and final argument over 96 characters.

Signed-off-by: Scott Lamb <slamb@slamb.org>
---
 contrib/fast-import/git-p4 |   28 +++++++---------------------
 1 files changed, 7 insertions(+), 21 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index d93e656..54053e3 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -725,27 +725,13 @@ class P4Sync(Command):
         if not files:
             return
 
-        # We cannot put all the files on the command line
-        # OS have limitations on the max lenght of arguments
-        # POSIX says it's 4096 bytes, default for Linux seems to be 130 K.
-        # and all OS from the table below seems to be higher than POSIX.
-        # See http://www.in-ulm.de/~mascheck/various/argmax/
-        if (self.isWindows):
-            argmax = 2000
-        else:
-            argmax = min(4000, os.sysconf('SC_ARG_MAX'))
-
-        chunk = ''
-        filedata = []
-        for i in xrange(len(files)):
-            f = files[i]
-            chunk += '"%s#%s" ' % (f['path'], f['rev'])
-            if len(chunk) > argmax or i == len(files)-1:
-                data = p4CmdList('print %s' % chunk)
-                if "p4ExitCode" in data[0]:
-                    die("Problems executing p4. Error: [%d]." % (data[0]['p4ExitCode']));
-                filedata.extend(data)
-                chunk = ''
+        filedata = p4CmdList('-x - print',
+                             stdin='\n'.join(['%s#%s' % (f['path'], f['rev'])
+                                              for f in files]),
+                             stdin_mode='w+')
+        if "p4ExitCode" in filedata[0]:
+            die("Problems executing p4. Error: [%d]."
+                % (filedata[0]['p4ExitCode']));
 
         j = 0;
         contents = {}
-- 
1.5.2.2.238.g7cbf2f2-dirty

^ permalink raw reply related

* [PATCH 1/2] git-p4: use subprocess in p4CmdList
From: Scott Lamb @ 2007-07-16  3:58 UTC (permalink / raw)
  To: Simon Hausmann; +Cc: git, Scott Lamb

This allows bidirectional piping - useful for "-x -" to avoid commandline
arguments - and is a step toward bypassing the shell.

Signed-off-by: Scott Lamb <slamb@slamb.org>
---
 contrib/fast-import/git-p4 |   23 ++++++++++++++++++-----
 1 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index d877150..d93e656 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -63,21 +63,34 @@ def system(cmd):
     if os.system(cmd) != 0:
         die("command failed: %s" % cmd)
 
-def p4CmdList(cmd):
+def p4CmdList(cmd, stdin=None, stdin_mode='w+b'):
     cmd = "p4 -G %s" % cmd
     if verbose:
         sys.stderr.write("Opening pipe: %s\n" % cmd)
-    pipe = os.popen(cmd, "rb")
+
+    # Use a temporary file to avoid deadlocks without
+    # subprocess.communicate(), which would put another copy
+    # of stdout into memory.
+    stdin_file = None
+    if stdin is not None:
+        stdin_file = tempfile.TemporaryFile(prefix='p4-stdin', mode=stdin_mode)
+        stdin_file.write(stdin)
+        stdin_file.flush()
+        stdin_file.seek(0)
+
+    p4 = subprocess.Popen(cmd, shell=True,
+                          stdin=stdin_file,
+                          stdout=subprocess.PIPE)
 
     result = []
     try:
         while True:
-            entry = marshal.load(pipe)
+            entry = marshal.load(p4.stdout)
             result.append(entry)
     except EOFError:
         pass
-    exitCode = pipe.close()
-    if exitCode != None:
+    exitCode = p4.wait()
+    if exitCode != 0:
         entry = {}
         entry["p4ExitCode"] = exitCode
         result.append(entry)
-- 
1.5.2.2.238.g7cbf2f2-dirty

^ permalink raw reply related

* Re: git proxy issue
From: Aubrey Li @ 2007-07-16  3:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk5t1avcn.fsf@assigned-by-dhcp.cox.net>

On 7/16/07, Junio C Hamano <gitster@pobox.com> wrote:
> "Aubrey Li" <aubreylee@gmail.com> writes:
>
> > From which release version git proxy is supported?
>
> v0.99.9k (Nov 25 2005) is the first tagged release (the patch
> itself was from Nov 4 2005).
>
>
oh, so how to config it?
Thanks,
-Aubrey

^ permalink raw reply

* Re: Questions about git-fast-import for cvs2svn
From: Eric Wong @ 2007-07-16  3:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Sean, Michael Haggerty, Shawn O. Pearce, git
In-Reply-To: <7vd4ytebsw.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <gitster@pobox.com> wrote:
> Sean <seanlkml@sympatico.ca> writes:
> >> 2. It appears that author/committer require an email address.  How
> >> important is a valid email address here?
> >
> > It's not necessary for the operation of Git itself; it's up to you to
> > decide how important the information is to your project.  You should
> > be able to set an empty email address for author or committer in
> > git fast-import as "name <>".
> 
> Don't do this; git-cvsimport and git-svn uses "name <name>"
> which is a saner compromise.  This way, you can add .mailmap to
> help later "git shortlog" to map using "<name>" part to more
> human friendly name.  Mapping at conversion time would also be
> good and git-cvsimport knows about it (I do not know about
> git-svn).

git-svn can do this, too.

I don't use it myself, but I remember the file format is the
same as the one git-svnimport and git-cvsimport use.

-- 
Eric Wong

^ permalink raw reply

* Re: [PATCH] translate bad characters in refnames during git-svn fetch
From: Eric Wong @ 2007-07-16  3:30 UTC (permalink / raw)
  To: git discussion list
In-Reply-To: <20070715130548.GA6144@piper.oerlikon.madduck.net>

martin f krafft <madduck@madduck.net> wrote:
> Hi,
> 
> I am trying to track/convert the Debian pkg-mdadm repository with
> git-svn:
> 
>   svn://svn.debian.org/svn/pkg-mdadm/mdadm/trunk
> 
> My problem is that the fetching fails:
> 
>   fatal: refs/remotes/tags/2.6.1-1~exp.1: cannot lock the ref
>   update-ref -m r311 refs/remotes/tags/2.6.1-1~exp.1
>   c6e351ea25dc90714048e33693099595c2d5dab8: command returned error:
>   128
> 
> This is because the ~ character is an invalid character for
> a refname (it's used to specify the nth parent).
> 
> So I figured that the best way to deal with this is to introduce
> a conversion filter to git-svn, but I cannot figure out where it has
> to go. My perl is rusty and even after an hour now with the code,
> I could not find the right spot.
> 
> The following patch works, but I can't really explain why. Moreover,
> it does not change the STDERR output, so you'll still get stuff like 
> 
>   r340 = 0dc5693471af9dfdb712c1342071ba1040af8963
>   (tags/2.6.1-1~exp.3)
> 
> which makes me think that it's translating the refname too late.
> However, the end result looks sane.
> 
> Comments welcome,

The major issue with this is that it doesn't handle odd cases
where a refname is sanitized into something
(say "1234~2" sanitizes to "1234=2"), and then another branch
is created named "1234=2".

git-svn should at least keep track of what it got sanitized to, to
avoid clobbering branches.

I started working on this a while back but haven't gotten around
to revisiting it:
http://thread.gmane.org/gmane.comp.version-control.git/45651

> ---
> git-check-ref-format(1) documents which characters may be contained in
> a refname. Since Subversion has different rules, an import can result in
> problems, such as:
> 
>   fatal: refs/remotes/tags/2.6.1-1~exp.1: cannot lock the ref
>   update-ref -m r311 refs/remotes/tags/2.6.1-1~exp.1
>   c6e351ea25dc90714048e33693099595c2d5dab8: command returned error: 128
> 
> This patch translates bad characters to valid substitutes to enable imports of
> tags/branches/whatever using characters that git does not allow in refnames.
> 
> Signed-off-by: martin f. krafft <madduck@piper.oerlikon.madduck.net>
> ---
>  git-svn.perl |   24 +++++++++++++++++++++++-
>  1 files changed, 23 insertions(+), 1 deletions(-)
> 
> diff --git a/git-svn.perl b/git-svn.perl
> index 299b40f..de43697 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -1239,7 +1239,29 @@ sub new {
>  	$self;
>  }
>  
> -sub refname { "refs/remotes/$_[0]->{ref_id}" }
> +sub refname {
> +	my ($refname) = $_[0]->{ref_id};
> +	## transform the refname as per rules in git-check-ref-format(1):
> +	# no slash-separated omponent can begin with a dot .
> +	# /.* becomes /,*
> +	$refname =~ s|/\.|/,|g;
> +	# It cannot have two consecutive dots .. anywhere
> +	# .. becomes ,,
> +	$refname =~ s|\.\.|,,|g;
> +	# It cannot have ASCII control character space, tilde ~, caret ^,
> +	# colon :, question-mark ?, asterisk *, or open bracket[ anywhere
> +	# <space> becomes _
> +	# ~ becomes =
> +	# ^ becomes @
> +	# : becomes %
> +	# ? becomes $
> +	# * becomes +
> +	# [ becomes (
> +	$refname =~ y| ~^:?*[|_=@%\$+(|;
> +	# It cannot end with a slash /
> +	$refname =~ s|/$||g;
> +	"refs/remotes/$refname";
> +}
>  
>  sub svm_uuid {
>  	my ($self) = @_;
> -- 

-- 
Eric Wong

^ permalink raw reply

* Re: [PATCH] vc-git: support asynchronous annotations, and improve versioning.
From: Junio C Hamano @ 2007-07-16  3:20 UTC (permalink / raw)
  To: David Kastrup; +Cc: git
In-Reply-To: <cfa5ed80635135dd7544f2b4c6df521a4353e90d.1184548803.git.dak@gnu.org>

David Kastrup <dak@gnu.org> writes:

> (vc-git-symbolic-commit): Allow nil to pass through.
> (vc-git-previous-version): Use explicit parent argument.
> (vc-git-next-version): Simplify.
> (vc-git-annotate-command): Use `vc-do-command'.
> (vc-git-annotate-extract-revision-at-line): Rename from
> `vc-annotate-extract-revision-at-line'.
> (vc-git-checkout): Make nicer way of ensuring encoding.
>

These do not seem to match what the patch does at all.
I give up.

Will apply only the "install .el, too" change from the confusing
series for now.

> diff --git a/contrib/emacs/vc-git.el b/contrib/emacs/vc-git.el
> index 2a0a0fe..d7ad314 100644
> --- a/contrib/emacs/vc-git.el
> +++ b/contrib/emacs/vc-git.el
> @@ -173,23 +173,22 @@ Returns nil if not possible."
> ...
>  (defun vc-git-annotate-command (file buffer &optional version)
>    "Execute \"git blame\" on FILE, inserting the contents in BUFFER.
>  Optional arg VERSION is a version to annotate from."
> -  (vc-do-command buffer
> -		 'async
> -		 "git" file "blame"
> -		 (or version "HEAD")
> -		 "--"))
> +  (let ((coding-system-for-read git-commits-coding-system))
> +    (vc-do-command buffer
> +		   'async
> +		   "git" file "blame"
> +		   (or version "HEAD")
> +		   "--")))
>  
>  ;;(defun vc-git-annotate-command (file buf &optional rev)
>  ;;  (let ((name (file-relative-name file)))
> -- 
> 1.4.4.2

^ permalink raw reply

* Re: [PATCH] contrib/emacs/Makefile: Also install .el files.
From: Junio C Hamano @ 2007-07-16  3:20 UTC (permalink / raw)
  To: David Kastrup; +Cc: git
In-Reply-To: <09ccbeffdf2ca787844d20fdc3e93d323da206f7.1184548803.git.dak@gnu.org>

David Kastrup <dak@gnu.org> writes:

> Signed-off-by: David Kastrup <dak@gnu.org>
>  install: all
>  	$(INSTALL) -d $(DESTDIR)$(emacsdir)
> -	$(INSTALL_ELC) $(ELC) $(DESTDIR)$(emacsdir)
> +	$(INSTALL_ELC) $(ELC:.elc=.el) $(ELC) $(DESTDIR)$(emacsdir)

Ok, thanks.

^ permalink raw reply

* Re: [PATCH] contrib/emacs/vc-git.el: various improvements.
From: Junio C Hamano @ 2007-07-16  3:20 UTC (permalink / raw)
  To: David Kastrup; +Cc: git
In-Reply-To: <403842ba71506c7b194812cd9a4f669c847eb7bc.1184548803.git.dak@gnu.org>

David Kastrup <dak@gnu.org> writes:

> (vc-git-symbolic-commit): Simplify and make it return
> something useful in almost all cases.

It would be easier to review and understand if "something
useful" were more specific.

It appears that the previous version of this function tried not
to barf if it got nil or false as input but now it does not seem
to worry about that case.  Also the option given to name-rev is
different which would lead to different behaviour -- arguably a
better one, but that needs to be documented in the log message.

> (vc-git-previous-version): Simplify.
> (vc-git-next-version): Simplify and make more efficient.

If you make the result symbolic using vc-git-symbolic-commit, do
you need to add --abbrev/--abbrev-commit to these functions?
These options have very small but still non zero cost.

> (vc-git-annotate-command): heed REV argument.
> (vc-annotate-extract-revision-at-line): Activate.

Ok.

^ permalink raw reply

* Re: git proxy issue
From: Junio C Hamano @ 2007-07-16  3:19 UTC (permalink / raw)
  To: Aubrey Li; +Cc: git
In-Reply-To: <6d6a94c50707151921h7f2a65fes65c94c3c1090937a@mail.gmail.com>

"Aubrey Li" <aubreylee@gmail.com> writes:

> From which release version git proxy is supported?

v0.99.9k (Nov 25 2005) is the first tagged release (the patch
itself was from Nov 4 2005).

^ permalink raw reply

* git proxy issue
From: Aubrey Li @ 2007-07-16  2:21 UTC (permalink / raw)
  To: git

Hi list,

I saw someone post a patch to make git support proxy, has it been applied?
From which release version git proxy is supported?
If not, how to do it?

Thanks,
-Aubrey

^ permalink raw reply

* [PATCH] contrib/emacs/vc-git.el: various improvements.
From: David Kastrup @ 2007-07-16  1:24 UTC (permalink / raw)
  To: git

(vc-git-symbolic-commit): Simplify and make it return
something useful in almost all cases.
(vc-git-previous-version): Simplify.
(vc-git-next-version): Simplify and make more efficient.
(vc-git-annotate-command): heed REV argument.
(vc-annotate-extract-revision-at-line): Activate.

Signed-off-by: David Kastrup <dak@gnu.org>
---
 contrib/emacs/vc-git.el |  110 ++++++++++++++++++++---------------------------
 1 files changed, 47 insertions(+), 63 deletions(-)

diff --git a/contrib/emacs/vc-git.el b/contrib/emacs/vc-git.el
index b8f6be5..3637c8a 100644
--- a/contrib/emacs/vc-git.el
+++ b/contrib/emacs/vc-git.el
@@ -84,67 +84,51 @@
 (defun vc-git-symbolic-commit (commit)
   "Translate COMMIT string into symbolic form.
 Returns nil if not possible."
-  (and commit
-       (with-temp-buffer
-	 (and
-	  (zerop
-	   (call-process "git" nil '(t nil) nil "name-rev"
-			 "--name-only" "--tags"
-			 commit))
-	  (goto-char (point-min))
-	  (= (forward-line 2) 1)
-	  (bolp)
-	  (buffer-substring-no-properties (point-min) (1- (point-max)))))))
+  (with-temp-buffer
+    (and
+     (zerop
+      (call-process "git" nil '(t nil) nil "name-rev"
+		    commit))
+     (goto-char (point-max))
+     (bolp)
+     (zerop (forward-line -1))
+     (bobp)
+     (progn
+       (search-forward " " nil t)
+       (not (eolp)))
+     (buffer-substring-no-properties (point) (1- (point-max))))))
 
 (defun vc-git-previous-version (file rev)
   "git-specific version of `vc-previous-version'."
-  (let ((default-directory (file-name-directory (expand-file-name file)))
-	(file (file-name-nondirectory file)))
-    (vc-git-symbolic-commit
-     (with-temp-buffer
-       (and
-	(zerop
-	 (call-process "git" nil '(t nil) nil "rev-list"
-		       "-2" rev "--" file))
-	(goto-char (point-max))
-	(bolp)
-	(zerop (forward-line -1))
-	(not (bobp))
-	(buffer-substring-no-properties
-	   (point)
-	   (1- (point-max))))))))
+  (vc-git-symbolic-commit
+   (with-temp-buffer
+     (and
+      (zerop
+       (call-process "git" nil '(t nil) nil "rev-list" "--abbrev"
+		     "--abbrev-commit" "-2" rev "--" (file-relative-name file)))
+      (goto-char (point-max))
+      (bolp)
+      (zerop (forward-line -1))
+      (not (bobp))
+      (buffer-substring-no-properties
+       (point)
+       (1- (point-max)))))))
 
 (defun vc-git-next-version (file rev)
   "git-specific version of `vc-next-version'."
-  (let* ((default-directory (file-name-directory
-			     (expand-file-name file)))
-	(file (file-name-nondirectory file))
-	(current-rev
-	 (with-temp-buffer
-	   (and
-	    (zerop
-	     (call-process "git" nil '(t nil) nil "rev-list"
-			   "-1" rev "--" file))
-	    (goto-char (point-max))
-	    (bolp)
-	    (zerop (forward-line -1))
-	    (bobp)
-	    (buffer-substring-no-properties
-	     (point)
-	     (1- (point-max)))))))
-    (and current-rev
-	 (vc-git-symbolic-commit
-	  (with-temp-buffer
-	    (and
-	     (zerop
-	      (call-process "git" nil '(t nil) nil "rev-list"
-			    "HEAD" "--" file))
-	     (goto-char (point-min))
-	     (search-forward current-rev nil t)
-	     (zerop (forward-line -1))
-	     (buffer-substring-no-properties
-	      (point)
-	      (progn (forward-line 1) (1- (point))))))))))
+  (vc-git-symbolic-commit
+   (with-temp-buffer
+     (and
+      (zerop
+       (call-process "git" nil '(t nil) nil "rev-list" "--abbrev"
+		     "--abbrev-commit"
+		     "HEAD" "--not" rev "--" (file-relative-name file)))
+      (goto-char (point-max))
+      (bolp)
+      (zerop (forward-line -1))
+      (buffer-substring-no-properties
+       (point)
+       (1- (point-max)))))))
 
 (defun vc-git-revert (file &optional contents-done)
   "Revert FILE to the version stored in the git repository."
@@ -197,20 +181,20 @@ Returns nil if not possible."
     (vc-git--run-command file "checkout" (or rev "HEAD"))))
 
 (defun vc-git-annotate-command (file buf &optional rev)
-  ; FIXME: rev is ignored
   (let ((name (file-relative-name file)))
-    (call-process "git" nil buf nil "blame" name)))
+    (if rev
+	(call-process "git" nil buf nil "blame" rev "--" name)
+      (call-process "git" nil buf nil "blame" "--" name))))
 
 (defun vc-git-annotate-time ()
   (and (re-search-forward "[0-9a-f]+ (.* \\([0-9]+\\)-\\([0-9]+\\)-\\([0-9]+\\) \\([0-9]+\\):\\([0-9]+\\):\\([0-9]+\\) \\([-+0-9]+\\) +[0-9]+)" nil t)
        (vc-annotate-convert-time
         (apply #'encode-time (mapcar (lambda (match) (string-to-number (match-string match))) '(6 5 4 3 2 1 7))))))
 
-;; Not really useful since we can't do anything with the revision yet
-;;(defun vc-annotate-extract-revision-at-line ()
-;;  (save-excursion
-;;    (move-beginning-of-line 1)
-;;    (and (looking-at "[0-9a-f]+")
-;;         (buffer-substring (match-beginning 0) (match-end 0)))))
+(defun vc-annotate-extract-revision-at-line ()
+  (save-excursion
+    (move-beginning-of-line 1)
+    (and (looking-at "[0-9a-f]+")
+         (buffer-substring (match-beginning 0) (match-end 0)))))
 
 (provide 'vc-git)
-- 
1.4.4.2

^ permalink raw reply related

* [PATCH] vc-git: support asynchronous annotations, and improve versioning.
From: David Kastrup @ 2007-07-15 23:53 UTC (permalink / raw)
  To: git
In-Reply-To: <403842ba71506c7b194812cd9a4f669c847eb7bc.1184548803.git.dak@gnu.org>

(vc-git-symbolic-commit): Allow nil to pass through.
(vc-git-previous-version): Use explicit parent argument.
(vc-git-next-version): Simplify.
(vc-git-annotate-command): Use `vc-do-command'.
(vc-git-annotate-extract-revision-at-line): Rename from
`vc-annotate-extract-revision-at-line'.
(vc-git-checkout): Make nicer way of ensuring encoding.

Signed-off-by: David Kastrup <dak@gnu.org>
---
 contrib/emacs/vc-git.el |   21 ++++++++++-----------
 1 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/contrib/emacs/vc-git.el b/contrib/emacs/vc-git.el
index 2a0a0fe..d7ad314 100644
--- a/contrib/emacs/vc-git.el
+++ b/contrib/emacs/vc-git.el
@@ -173,23 +173,22 @@ Returns nil if not possible."
       (let ((fullname (substring
                        (vc-git--run-command-string file "ls-files" "-z" "--full-name" "--")
                        0 -1))
-            (coding-system-for-read 'no-conversion)
-            coding-system-for-write)
+            (coding-system-for-read 'no-conversion))
         (with-temp-file destfile
-	  (prog1
-	      (eq 0 (call-process "git" nil t nil "cat-file" "blob"
-				  (concat (or rev "HEAD") ":" fullname)))
-	    (setq coding-system-for-write 'no-conversion))))
+	  (setq buffer-file-coding-system 'no-conversion)
+	  (eq 0 (call-process "git" nil t nil "cat-file" "blob"
+			      (concat (or rev "HEAD") ":" fullname)))))
     (vc-git--run-command file "checkout" (or rev "HEAD"))))
 
 (defun vc-git-annotate-command (file buffer &optional version)
   "Execute \"git blame\" on FILE, inserting the contents in BUFFER.
 Optional arg VERSION is a version to annotate from."
-  (vc-do-command buffer
-		 'async
-		 "git" file "blame"
-		 (or version "HEAD")
-		 "--"))
+  (let ((coding-system-for-read git-commits-coding-system))
+    (vc-do-command buffer
+		   'async
+		   "git" file "blame"
+		   (or version "HEAD")
+		   "--")))
 
 ;;(defun vc-git-annotate-command (file buf &optional rev)
 ;;  (let ((name (file-relative-name file)))
-- 
1.4.4.2

^ permalink raw reply related

* [PATCH] (vc-git-annotate-command):  Make synchronous for now.
From: David Kastrup @ 2007-07-16  0:05 UTC (permalink / raw)
  To: git
In-Reply-To: <403842ba71506c7b194812cd9a4f669c847eb7bc.1184548803.git.dak@gnu.org>

We need a test to figure out whether Emacs supports asynchronous
annotations (not yet available in CVS).

Signed-off-by: David Kastrup <dak@gnu.org>
---
 contrib/emacs/vc-git.el |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/contrib/emacs/vc-git.el b/contrib/emacs/vc-git.el
index d7ad314..c3f30ad 100644
--- a/contrib/emacs/vc-git.el
+++ b/contrib/emacs/vc-git.el
@@ -185,7 +185,7 @@ Returns nil if not possible."
 Optional arg VERSION is a version to annotate from."
   (let ((coding-system-for-read git-commits-coding-system))
     (vc-do-command buffer
-		   'async
+		   0
 		   "git" file "blame"
 		   (or version "HEAD")
 		   "--")))
-- 
1.4.4.2

^ permalink raw reply related

* Re: [PATCH] contrib/emacs...
From: David Kastrup @ 2007-07-16  1:27 UTC (permalink / raw)
  To: git
In-Reply-To: <09ccbeffdf2ca787844d20fdc3e93d323da206f7.1184548803.git.dak@gnu.org>


I apologize for the somewhat messy state of the patches and commit
messages: I am trying to get a hang of the tools right now.

If people have suggestions how to read up on cleaning up and
reordering commits before submission, I'd be greatful.

-- 
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