Git development
 help / color / mirror / Atom feed
* Re: [PATCH] git-svn: fix dcommit losing changes when out-of-date from svn
From: Seth Falcon @ 2006-11-09 17:42 UTC (permalink / raw)
  To: git
In-Reply-To: <7vfyctkki5.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> writes:
> Eric Wong <normalperson@yhbt.net> writes:
>
>> There was a bug in dcommit (and commit-diff) which caused deltas
>> to be generated against the latest version of the changed file
>> in a repository, and not the revision we are diffing (the tree)
>> against locally.
>>
>> This bug can cause recent changes to the svn repository to be
>> silently clobbered by git-svn if our repository is out-of-date.

Eric, with this patch, is a dcommit operation as safe as a regular svn
commit from an svn working copy?  That is, the commit will abort if
the svn repository has changes that your git-svn/git repo hasn't yet
seen?  I'm pretty sure the answer is yes, but I'd like to be sure :-)

> Steven, I do not interact with real svn repository myself so I
> can only judge from the test in this patch and Steven's test
> case, so it would be more assuring for me if you can confirm it
> fixes the issue for you.

I'm not Steven, but I was reproducing the bug and with Eric's patch, I
get a nice error/abort when I try to dcommit when the svn repos has
relevant changes that I have not fetched yet.

Best,


^ permalink raw reply

* [PATCH] Nicer error messages in case saving an object to db goes wrong
From: Petr Baudis @ 2006-11-09 12:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Currently the error e.g. when pushing to a read-only repository is quite
confusing, this attempts to clean it up, unifies error reporting between
various object writers and uses error() on couple more places.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 sha1_file.c |   21 ++++++++++++---------
 1 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 6ea59b5..09456d2 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1455,8 +1455,7 @@ int move_temp_to_file(const char *tmpfil
 	unlink(tmpfile);
 	if (ret) {
 		if (ret != EEXIST) {
-			fprintf(stderr, "unable to write sha1 filename %s: %s\n", filename, strerror(ret));
-			return -1;
+			return error("unable to write sha1 filename %s: %s\n", filename, strerror(ret));
 		}
 		/* FIXME!!! Collision check here ? */
 	}
@@ -1566,16 +1565,17 @@ int write_sha1_file(void *buf, unsigned
 	}
 
 	if (errno != ENOENT) {
-		fprintf(stderr, "sha1 file %s: %s\n", filename, strerror(errno));
-		return -1;
+		return error("sha1 file %s: %s\n", filename, strerror(errno));
 	}
 
 	snprintf(tmpfile, sizeof(tmpfile), "%s/obj_XXXXXX", get_object_directory());
 
 	fd = mkstemp(tmpfile);
 	if (fd < 0) {
-		fprintf(stderr, "unable to create temporary sha1 filename %s: %s\n", tmpfile, strerror(errno));
-		return -1;
+		if (errno == EPERM)
+			return error("insufficient permission for adding an object to repository database %s\n", get_object_directory());
+		else
+			return error("unable to create temporary sha1 filename %s: %s\n", tmpfile, strerror(errno));
 	}
 
 	/* Set it up */
@@ -1690,9 +1690,12 @@ int write_sha1_from_fd(const unsigned ch
 	snprintf(tmpfile, sizeof(tmpfile), "%s/obj_XXXXXX", get_object_directory());
 
 	local = mkstemp(tmpfile);
-	if (local < 0)
-		return error("Couldn't open %s for %s",
-			     tmpfile, sha1_to_hex(sha1));
+	if (local < 0) {
+		if (errno == EPERM)
+			return error("insufficient permission for adding an object to repository database %s\n", get_object_directory());
+		else
+			return error("unable to create temporary sha1 filename %s: %s\n", tmpfile, strerror(errno));
+	}
 
 	memset(&stream, 0, sizeof(stream));

^ permalink raw reply related

* Non-ASCII paths and git-cvsserver
From: sf @ 2006-11-09 11:11 UTC (permalink / raw)
  To: git

Hello,

I want to access a git repository via git-cvsserver. The problem is that 
the repository contains paths with umlauts. These paths come out quoted 
and escaped when checked out with cvs.

Test case:

#! /bin/sh

set -e -u -x

WORK='/tmp/gittest'
FILE=$'\303\244'

mkdir "${WORK}"
mkdir "${WORK}/git"

#trap 'rm -r "${WORK}"' EXIT

cd "${WORK}/git"

git init-db
git repo-config gitcvs.enabled 1
git repo-config gitcvs.logfile "${WORK}/git/.git/cvslog.txt"

touch "${FILE}"
git add "${FILE}"
git commit -a -mx

cd "${WORK}"

CVS_SERVER='git-cvsserver'
export CVS_SERVER

cvs -d ":fork:${WORK}/git/.git" co master

ls master

### end


This is what I get:

+ WORK=/tmp/gittest
+ FILE=$'\303\244'
+ mkdir /tmp/gittest
+ mkdir /tmp/gittest/git
+ cd /tmp/gittest/git
+ git init-db
defaulting to local storage area
+ git repo-config gitcvs.enabled 1
+ git repo-config gitcvs.logfile /tmp/gittest/git/.git/cvslog.txt
+ touch $'\303\244'
+ git add $'\303\244'
+ git commit -a -mx
Committing initial tree 23d6145738bba135994775c19d6e8ae707d399ee
+ cd /tmp/gittest
+ CVS_SERVER=git-cvsserver
+ export CVS_SERVER
+ cvs -d :fork:/tmp/gittest/git/.git co master
cvs checkout: Updating master
U master/"\303\244"
+ ls master
"\303\244"  CVS


I do not speak perl so can anyone help?

Regards

Stephan

^ permalink raw reply

* Re: Deleting binary file
From: Jakub Narebski @ 2006-11-09 11:10 UTC (permalink / raw)
  To: git
In-Reply-To: <1d592d70611090147p6f10f921s740e3cc19cb67c09@mail.gmail.com>

Kirill Shutemov wrote:

>$ git-applymbox 0001-test.txt

Does using "git am --binary 0001-test.txt" help?
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git


^ permalink raw reply

* Re: [PATCH] gitweb: protect blob and diff output lines from controls.
From: Jakub Narebski @ 2006-11-09 10:41 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8xiklxh1.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>> Junio C Hamano wrote:
>>> [...]  There is another
>>> thing I noticed while testing it with an artifitial test that I
>>> haven't fixed, but I think you already know about it (when the
>>> commitdiff is completely empty except mode changes, we end up
>>> with unbalanced div).  My test's tip can be found at
>>> 'gitweb-test-funny-char' branch temporarily in the git.git
>>> repository.
>>
>> Damn. I thought I corrected this on resend...
> 
> I think you need this, otherwise when the last filepair changes
> only metainfo you fail to close the extended header div.

True, I forgot about the situation where empty patch is _last_
in the patchset (which includes the situation when it is _only_
patch).

> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 1a757cc..e54a29e 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -2338,6 +2338,8 @@ sub git_patchset_body {
>  
>  		print format_diff_line($patch_line);
>  	}
> +	print "</div>\n" if $in_header; # extended header
I would write '# class="diff extended_header"' here instead.
> +
>  	print "</div>\n" if $patch_found; # class="patch"
>  
>  	print "</div>\n"; # class="patchset"

Looks good. Ack.

-- 
Jakub Narebski

^ permalink raw reply

* Re: [PATCH] gitweb: protect blob and diff output lines from controls.
From: Junio C Hamano @ 2006-11-09 10:34 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <200611091102.56565.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

> Junio C Hamano wrote:
>> [...]  There is another
>> thing I noticed while testing it with an artifitial test that I
>> haven't fixed, but I think you already know about it (when the
>> commitdiff is completely empty except mode changes, we end up
>> with unbalanced div).  My test's tip can be found at
>> 'gitweb-test-funny-char' branch temporarily in the git.git
>> repository.
>
> Damn. I thought I corrected this on resend...

I think you need this, otherwise when the last filepair changes
only metainfo you fail to close the extended header div.

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 1a757cc..e54a29e 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2338,6 +2338,8 @@ sub git_patchset_body {
 
 		print format_diff_line($patch_line);
 	}
+	print "</div>\n" if $in_header; # extended header
+
 	print "</div>\n" if $patch_found; # class="patch"
 
 	print "</div>\n"; # class="patchset"

^ permalink raw reply related

* Re: Command for reverting all patches below particular commit
From: Jakub Narebski @ 2006-11-09 10:27 UTC (permalink / raw)
  To: git
In-Reply-To: <4552FE43.7020804@calsoftinc.com>

Umesh Deshpande wrote:

> Hi,
> I want to revert a particular commit, but because of dependency issues I 
> am not able to do so. The revert operation fails.

Cannot you resolve conflicts during revert, and commit resolved result
as revert of a commit?
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git


^ permalink raw reply

* Command for reverting all patches below particular commit
From: Umesh Deshpande @ 2006-11-09 10:09 UTC (permalink / raw)
  To: git

Hi,
I want to revert a particular commit, but because of dependency issues I 
am not able to do so. The revert operation fails.
I want to revert all the commits below the specified commit so that I 
will be able to remove that particular commit.
Is there any command to do this?

Thanks

Regards

^ permalink raw reply

* Re: [PATCH] gitweb: protect blob and diff output lines from controls.
From: Jakub Narebski @ 2006-11-09 10:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vlkmlkkq8.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> [...]  There is another
> thing I noticed while testing it with an artifitial test that I
> haven't fixed, but I think you already know about it (when the
> commitdiff is completely empty except mode changes, we end up
> with unbalanced div).  My test's tip can be found at
> 'gitweb-test-funny-char' branch temporarily in the git.git
> repository.

Damn. I thought I corrected this on resend...

	if ($patch_found) {
		# close extended header for previous empty patch
		if ($in_header) {
			print "</div>\n" # class="diff extended_header"
		}
		# close previous patch
		print "</div>\n"; # class="patch"
	}
-- 
Jakub Narebski

^ permalink raw reply

* Re: [PATCH] git-svn: fix dcommit losing changes when out-of-date from svn
From: Junio C Hamano @ 2006-11-09 10:00 UTC (permalink / raw)
  To: Eric Wong, Steven Grimm; +Cc: git
In-Reply-To: <20061109091937.GA22853@localdomain>

Eric Wong <normalperson@yhbt.net> writes:

> There was a bug in dcommit (and commit-diff) which caused deltas
> to be generated against the latest version of the changed file
> in a repository, and not the revision we are diffing (the tree)
> against locally.
>
> This bug can cause recent changes to the svn repository to be
> silently clobbered by git-svn if our repository is out-of-date.
>
> Thanks to Steven Grimm for noticing the bug.
>
> The (few) people using the commit-diff command are now required
> to use the -r/--revision argument.  dcommit usage is unchanged.
>
> Signed-off-by: Eric Wong <normalperson@yhbt.net>

Thanks both for your clear problem report and quick resolution
of the issue.

Steven, I do not interact with real svn repository myself so I
can only judge from the test in this patch and Steven's test
case, so it would be more assuring for me if you can confirm it
fixes the issue for you.

^ permalink raw reply

* Re: [PATCH] gitweb: protect blob and diff output lines from controls.
From: Junio C Hamano @ 2006-11-09  9:55 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git, Petr Baudis, Luben Tuikov
In-Reply-To: <200611091024.35019.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

> Jakub Narebski wrote:
>> I'm not sure what quoting to choose for esc_attr, but there we could
>> use even --no-control-chars quoting (replacing any control character
>> by '?');  but perhaps in some cases like git_print_page_path
>> subroutine CEC is better.

To be honest, I do not have strong preference between the
escaping style.  If the gitweb cabal feel it is more natural to
see "^L" in blobs and "\f" in path, I will very happily accept
such a patch.

> I'm rambling. esc_attr is special case, because CGI does escapeHTML
> (and I hope also to_utf8) for us. Using <span class="cntrl">...</span>
> has also no sense. So there should be separate esc_attr_path subroutine
> I think.

Yes.  It is unfortunate that there needs different types of
quoting.  I think the first step would be to stop calling
esc_html in esc_path.  I think it was a mistake, and I did not
correct it when I started touching it.

Somehow I ended up spending sizeable part of my git day this
week on fixing up blob/blame/tag/commit message view regarding
this "make controls visible and safe" issues on the 'master'
branch, but I have been consciously staying out of gitweb/ part
of the system, primarily because there are many other people who
are more interested and qualified in it than myself.

I'll step aside and try not to get in the way.  There is another
thing I noticed while testing it with an artifitial test that I
haven't fixed, but I think you already know about it (when the
commitdiff is completely empty except mode changes, we end up
with unbalanced div).  My test's tip can be found at
'gitweb-test-funny-char' branch temporarily in the git.git
repository.

^ permalink raw reply

* Re: Deleting binary file
From: Jakub Narebski @ 2006-11-09  9:52 UTC (permalink / raw)
  To: git
In-Reply-To: <1d592d70611090147p6f10f921s740e3cc19cb67c09@mail.gmail.com>

Kirill Shutemov wrote:

> Why can't I delete binary file via patch?
> 
>$ git-init-db
> defaulting to local storage area
>$ cp /bin/sh .
>$ git-add sh
>$ git-commit -m 'test'
> Committing initial tree efa3b1f9fdc4cdb8aab5bf869f580664a52ac04c
>$ git-rm sh
> rm 'sh'
>$ rm sh
>$ git-commit -m 'test'
>$ git-format-patch HEAD^

What happens if you use --binary option here?

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git


^ permalink raw reply

* Deleting binary file
From: Kirill Shutemov @ 2006-11-09  9:47 UTC (permalink / raw)
  To: git

Why can't I delete binary file via patch?

> git-init-db
defaulting to local storage area
> cp /bin/sh .
> git-add sh
> git-commit -m 'test'
Committing initial tree efa3b1f9fdc4cdb8aab5bf869f580664a52ac04c
> git-rm sh
rm 'sh'
> rm sh
> git-commit -m 'test'
> git-format-patch HEAD^
0001-test.txt
> git-reset --hard HEAD^1
> git-applymbox 0001-test.txt
1 patch(es) to process.

Applying 'test'

error: No changes
> cat 0001-test.txt
From 9fd9a46e5404e7c389cfb48026b68d46faa47081 Mon Sep 17 00:00:00 2001
From: Kirill A. Shutemov <k.shutemov@gmail.com>
Date: Wed, 8 Nov 2006 11:19:35 +0200
Subject: [PATCH] test

---
sh |  Bin
1 files changed, 0 insertions(+), 0 deletions(-)

diff --git a/sh b/sh
deleted file mode 100755
index a57a92a..0000000
Binary files a/sh and /dev/null differ
-- 

^ permalink raw reply

* Re: [PATCH] gitweb: protect blob and diff output lines from controls.
From: Jakub Narebski @ 2006-11-09  9:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwt65pgqs.fsf@assigned-by-dhcp.cox.net>

Dnia czwartek 9. listopada 2006 02:10, Junio C Hamano napisał:
> Jakub Narebski <jnareb@gmail.com> writes:

>> Well, the pathname has the limit that it must be in single line
>> after quoting. The "blob" output is multipage.
> 
> I honestly have _no_ idea what distincition you are seeing
> here.  Both blob and diff output are processed one line at a
> time and its result would be on a single line too.

I was thinking about _conceptual_ difference, not technical.
(Perhaps I should make it more clear.) 

Pathname is item (or part of item in the case of page_path)
which is contained (and must be contained) in single line.
It is also expected (although if we follow this expectation
is up to us) that the pathname would quote special characters
similarly to how shell/operating system quotes pathnames
(e.g. in ls output).

"Blob" output on the other hand ("blob" view and patch part of
"blobdiff" and "commitdiff" views) is [a part of] larger, multiline
whole. One could also expect that special characters would be
quoted like editor quotes special characters. (Of course question
is: which editor?)

This of course is complicate by single line output like subject
or authorship, or signoff, which is not pathname.


All this discussion shows that gitweb quoting is more complicated
that I thought.
-- 
Jakub Narebski

^ permalink raw reply

* Re: [PATCH] gitweb: protect blob and diff output lines from controls.
From: Jakub Narebski @ 2006-11-09  9:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <200611090104.32247.jnareb@gmail.com>

Jakub Narebski wrote:
> I'm not sure what quoting to choose for esc_attr, but there we could
> use even --no-control-chars quoting (replacing any control character
> by '?');  but perhaps in some cases like git_print_page_path
> subroutine CEC is better.

I'm rambling. esc_attr is special case, because CGI does escapeHTML
(and I hope also to_utf8) for us. Using <span class="cntrl">...</span>
has also no sense. So there should be separate esc_attr_path subroutine
I think.

Even if we decide that esc_html and esc_path should give identical
output (the difference that _might_ be here is that in esc_html we
don't need to escape whitespace control characters valid in HTML,
like tab (HT, TAB) or newline (LF); on the other hand thanks to
line-by-line processing we should never get newline in "blob", and
thanks to untabify we should never get tab in "blob") I think it would
be prudent to have esc_path, even as thin wrapper just caling esc_html.

We might decide to use different style for control characters in
different views, but that I think can be done using pure CSS.
-- 
Jakub Narebski

^ permalink raw reply

* [PATCH] git-svn: fix dcommit losing changes when out-of-date from svn
From: Eric Wong @ 2006-11-09  9:19 UTC (permalink / raw)
  To: Steven Grimm; +Cc: git, Junio C Hamano
In-Reply-To: <455277A6.2000404@midwinter.com>

There was a bug in dcommit (and commit-diff) which caused deltas
to be generated against the latest version of the changed file
in a repository, and not the revision we are diffing (the tree)
against locally.

This bug can cause recent changes to the svn repository to be
silently clobbered by git-svn if our repository is out-of-date.

Thanks to Steven Grimm for noticing the bug.

The (few) people using the commit-diff command are now required
to use the -r/--revision argument.  dcommit usage is unchanged.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 Documentation/git-svn.txt              |    1 +
 git-svn.perl                           |   29 +++++++++++-
 t/t9105-git-svn-commit-diff.sh         |    2 +-
 t/t9106-git-svn-commit-diff-clobber.sh |   74 ++++++++++++++++++++++++++++++++
 4 files changed, 102 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 450ff1f..a764d1f 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -120,6 +120,7 @@ manually joining branches on commit.
 	URL of the target Subversion repository.  The final argument
 	(URL) may be omitted if you are working from a git-svn-aware
 	repository (that has been init-ed with git-svn).
+	The -r<revision> option is required for this.
 
 'graft-branches'::
 	This command attempts to detect merges/branches from already
diff --git a/git-svn.perl b/git-svn.perl
index 4a56f18..80b7b87 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -134,6 +134,7 @@ my %cmd = (
 	'commit-diff' => [ \&commit_diff, 'Commit a diff between two trees',
 			{ 'message|m=s' => \$_message,
 			  'file|F=s' => \$_file,
+			  'revision|r=s' => \$_revision,
 			%cmt_opts } ],
 	dcommit => [ \&dcommit, 'Commit several diffs to merge with upstream',
 			{ 'merge|m|M' => \$_merge,
@@ -586,11 +587,21 @@ sub commit_lib {
 sub dcommit {
 	my $gs = "refs/remotes/$GIT_SVN";
 	chomp(my @refs = safe_qx(qw/git-rev-list --no-merges/, "$gs..HEAD"));
+	my $last_rev;
 	foreach my $d (reverse @refs) {
+		unless (defined $last_rev) {
+			(undef, $last_rev, undef) = cmt_metadata("$d~1");
+			unless (defined $last_rev) {
+				die "Unable to extract revision information ",
+				    "from commit $d~1\n";
+			}
+		}
 		if ($_dry_run) {
 			print "diff-tree $d~1 $d\n";
 		} else {
-			commit_diff("$d~1", $d);
+			if (my $r = commit_diff("$d~1", $d, undef, $last_rev)) {
+				$last_rev = $r;
+			} # else: no changes, same $last_rev
 		}
 	}
 	return if $_dry_run;
@@ -814,6 +825,8 @@ sub commit_diff {
 		print STDERR "Needed URL or usable git-svn id command-line\n";
 		commit_diff_usage();
 	}
+	my $r = shift || $_revision;
+	die "-r|--revision is a required argument\n" unless (defined $r);
 	if (defined $_message && defined $_file) {
 		print STDERR "Both --message/-m and --file/-F specified ",
 				"for the commit message.\n",
@@ -830,13 +843,22 @@ sub commit_diff {
 	($repo, $SVN_PATH) = repo_path_split($SVN_URL);
 	$SVN_LOG ||= libsvn_connect($repo);
 	$SVN ||= libsvn_connect($repo);
+	if ($r eq 'HEAD') {
+		$r = $SVN->get_latest_revnum;
+	} elsif ($r !~ /^\d+$/) {
+		die "revision argument: $r not understood by git-svn\n";
+	}
 	my @lock = $SVN::Core::VERSION ge '1.2.0' ? (undef, 0) : ();
-	my $ed = SVN::Git::Editor->new({	r => $SVN->get_latest_revnum,
+	my $rev_committed;
+	my $ed = SVN::Git::Editor->new({	r => $r,
 						ra => $SVN_LOG, c => $tb,
 						svn_path => $SVN_PATH
 					},
 				$SVN->get_commit_editor($_message,
-					sub {print "Committed $_[0]\n"},@lock)
+					sub {
+						$rev_committed = $_[0];
+						print "Committed $_[0]\n";
+					}, @lock)
 				);
 	my $mods = libsvn_checkout_tree($ta, $tb, $ed);
 	if (@$mods == 0) {
@@ -846,6 +868,7 @@ sub commit_diff {
 		$ed->close_edit;
 	}
 	$_message = $_file = undef;
+	return $rev_committed;
 }
 
 ########################### utility functions #########################
diff --git a/t/t9105-git-svn-commit-diff.sh b/t/t9105-git-svn-commit-diff.sh
index f994b72..746c827 100755
--- a/t/t9105-git-svn-commit-diff.sh
+++ b/t/t9105-git-svn-commit-diff.sh
@@ -33,7 +33,7 @@ prev=`git rev-parse --verify HEAD^1`
 
 test_expect_success 'test the commit-diff command' "
 	test -n '$prev' && test -n '$head' &&
-	git-svn commit-diff '$prev' '$head' '$svnrepo' &&
+	git-svn commit-diff -r1 '$prev' '$head' '$svnrepo' &&
 	svn co $svnrepo wc &&
 	cmp readme wc/readme
 	"
diff --git a/t/t9106-git-svn-commit-diff-clobber.sh b/t/t9106-git-svn-commit-diff-clobber.sh
new file mode 100755
index 0000000..58698b3
--- /dev/null
+++ b/t/t9106-git-svn-commit-diff-clobber.sh
@@ -0,0 +1,74 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Eric Wong
+test_description='git-svn commit-diff clobber'
+. ./lib-git-svn.sh
+
+if test -n "$GIT_SVN_NO_LIB" && test "$GIT_SVN_NO_LIB" -ne 0
+then
+	echo 'Skipping: commit-diff clobber needs SVN libraries'
+	test_done
+	exit 0
+fi
+
+test_expect_success 'initialize repo' "
+	mkdir import &&
+	cd import &&
+	echo initial > file &&
+	svn import -m 'initial' . $svnrepo &&
+	cd .. &&
+	echo initial > file &&
+	git update-index --add file &&
+	git commit -a -m 'initial'
+	"
+test_expect_success 'commit change from svn side' "
+	svn co $svnrepo t.svn &&
+	cd t.svn &&
+	echo second line from svn >> file &&
+	svn commit -m 'second line from svn' &&
+	cd .. &&
+	rm -rf t.svn
+	"
+
+test_expect_failure 'commit conflicting change from git' "
+	echo second line from git >> file &&
+	git commit -a -m 'second line from git' &&
+	git-svn commit-diff -r1 HEAD~1 HEAD $svnrepo
+	" || true
+
+test_expect_success 'commit complementing change from git' "
+	git reset --hard HEAD~1 &&
+	echo second line from svn >> file &&
+	git commit -a -m 'second line from svn' &&
+	echo third line from git >> file &&
+	git commit -a -m 'third line from git' &&
+	git-svn commit-diff -r2 HEAD~1 HEAD $svnrepo
+	"
+
+test_expect_failure 'dcommit fails to commit because of conflict' "
+	git-svn init $svnrepo &&
+	git-svn fetch &&
+	git reset --hard refs/remotes/git-svn &&
+	svn co $svnrepo t.svn &&
+	cd t.svn &&
+	echo fourth line from svn >> file &&
+	svn commit -m 'fourth line from svn' &&
+	cd .. &&
+	rm -rf t.svn &&
+	echo 'fourth line from git' >> file &&
+	git commit -a -m 'fourth line from git' &&
+	git-svn dcommit
+	" || true
+
+test_expect_success 'dcommit does the svn equivalent of an index merge' "
+	git reset --hard refs/remotes/git-svn &&
+	echo 'index merge' > file2 &&
+	git update-index --add file2 &&
+	git commit -a -m 'index merge' &&
+	echo 'more changes' >> file2 &&
+	git update-index file2 &&
+	git commit -a -m 'more changes' &&
+	git-svn dcommit
+	"
+
+test_done
-- 
1.4.4.rc1.g40e94

-- 

^ permalink raw reply related

* Re: git-svn can lose changes silently
From: Seth Falcon @ 2006-11-09  7:33 UTC (permalink / raw)
  To: git
In-Reply-To: <455277A6.2000404@midwinter.com>

Steven Grimm <koreth@midwinter.com> writes:

> git-svn is happy to overwrite changes in the svn repository with no
> warnings. Didn't seem to be known behavior when I mentioned it in
> #git, so here's an example, starting completely from scratch to make
> it easier to reproduce. I'm using git-svn 1.4.3 and svn 1.2.3 on OS
> X.

I can reproduce your example and also see the same thing when
accessing an svn repository via https [I've never used the local
file:// mode and was leary of it].

> It is probably not a feature that you can lose changes without knowing
> about it! Even if I'd run git-svn fetch before that commit, it still
> wouldn't help if the svn version of the file changed between the time
> I ran fetch and the time I ran dcommit, totally possible with a busy
> svn repository.
>
> Opinions? Suggestions on fixing it? Do other people agree this is a
> bug?

I think this is a pretty big deal -- at least in terms of user
expectation.  The man page for 'git-svn commit' warns that upstream
changes will be lost.  But in suggesting to use 'git-svn dcommit'
instead, one might expect (hope?) this overwritting upstream changes
to not occur.

I'm embarassed to admit that I've been using git-svn for a few weeks
now and didn't test out these details.  

Here is a patch that I think improves matters a bit:

diff --git a/git-svn.perl b/git-svn.perl
index 4a56f18..daa1764 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -830,6 +830,17 @@ sub commit_diff {
        ($repo, $SVN_PATH) = repo_path_split($SVN_URL);
        $SVN_LOG ||= libsvn_connect($repo);
        $SVN ||= libsvn_connect($repo);
+
+        my ($r_last, $cmt_last) = svn_grab_base_rev();
+        my $fetched = fetch();
+       if ($r_last != $fetched->{revision}) {
+               print STDERR "There are new revisions that were fetched ",
+                               "and need to be merged (or acknowledged) ",
+                               "before committing.\n",
+                               "last rev: $r_last\n",
+                               " current: $fetched->{revision}\n";
+               exit 1;
+       }
        my @lock = $SVN::Core::VERSION ge '1.2.0' ? (undef, 0) : ();
        my $ed = SVN::Git::Editor->new({        r => $SVN->get_latest_revnum,
                                                ra => $SVN_LOG, c => $tb,

But for a possibly busy svn repository, the only real solution is one
that uses the svn libs in a way that behaves like the svn client in
terms of atomicity.


^ permalink raw reply related

* Re: Shallow clone
From: Aneesh Kumar @ 2006-11-09  4:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vac31p8om.fsf@assigned-by-dhcp.cox.net>

On 11/9/06, Junio C Hamano <junkio@cox.net> wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@gmail.com> writes:
>
> >
> But it seems to need some more work.  I just tried to clone
> git.git with --depth=1 and it cauterizes each branch with two
> commits (I think that is what depth=1 means -- the latest and
> one behind it), but it pulled almost the whole repository
> anyway, and it turns out that "git log v1.4.3-rc1" gives me the
> full history leading to it.
>
> Subsequent "git fetch --depth 99999" makes the branches
> connected to the root commit, and I am reasonably sure we do not
> have that many commits, but .git/shallow did not become empty.
>

I tried a local clone with --depth 2 and it showed two commits
in all the branches/DAG. But then doing a  pull --depth 4 listed more
commits than 4. I didn't look much into this.


^ permalink raw reply

* Re: Shallow clone
From: Junio C Hamano @ 2006-11-09  4:04 UTC (permalink / raw)
  To: Aneesh Kumar K.V; +Cc: git
In-Reply-To: <45521AE9.7050902@gmail.com>

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

>> I am trying to test this feature. Is there a documentation
>> .git/shallow some where. Atleast what those entries
>> mean ? I know in the mail johannes mentioned only core git will
>> touch this file. But it should be ok to be descriptive like other
>> files. (FETCH_HEAD)
>
> diff --git a/Documentation/repository-layout.txt b/Documentation/repository-layout.txt
> index 275d18b..03a6f77 100644
> --- a/Documentation/repository-layout.txt
> +++ b/Documentation/repository-layout.txt
> @@ -141,3 +141,9 @@ logs/refs/heads/`name`::
>  
>  logs/refs/tags/`name`::
>  	Records all changes made to the tag named `name`.
> +
> +shallow::
> +	Records the sha1 of the commits which is marked to have no
> +	parents to represent a shallow repository.The commit object
> +	will have the parent information present. It carry one
> +	record per line.

I would drop the second sentence which is just confusing but
otherwise it is correct, I think (I just started trying it
out).

But it seems to need some more work.  I just tried to clone
git.git with --depth=1 and it cauterizes each branch with two
commits (I think that is what depth=1 means -- the latest and
one behind it), but it pulled almost the whole repository
anyway, and it turns out that "git log v1.4.3-rc1" gives me the
full history leading to it.

Subsequent "git fetch --depth 99999" makes the branches
connected to the root commit, and I am reasonably sure we do not
have that many commits, but .git/shallow did not become empty.

I haven't followed the code closely enough to tell if these are
just minor details needing more polish, or something more
fundamental in the design.



^ permalink raw reply

* shallow clone failed git pull
From: Aneesh Kumar K.V @ 2006-11-09  4:02 UTC (permalink / raw)
  To: git

I was using the pu branch i tried to update the git repository and i got this error.

alk 9e950efa20dc8037c27509666cba6999da9368e8
walk 3b6a792f6ace33584897d1af08630c9acc0ce221
* refs/heads/origin: fast forward to branch 'master' of http://repo.or.cz/r/linux-2.6
  old..new: 3d42488..088406b
Auto-following refs/tags/v2.6.19-rc5
shallow clone with http not supported


This repository was not cloned with -depth. I only updated the git tools using the pu branch


^ permalink raw reply

* Re: What's in git.git
From: Dave Dillow @ 2006-11-09  3:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Horst H. von Brand, git
In-Reply-To: <7vslgtpbx0.fsf@assigned-by-dhcp.cox.net>

On Wed, 2006-11-08 at 18:54 -0800, Junio C Hamano wrote:
> "Horst H. von Brand" <vonbrand@laptop13.inf.utfsm.cl> writes:
> 
> > David Lang <dlang@digitalinsight.com> wrote:
> >> On Tue, 7 Nov 2006, Junio C Hamano wrote:
> >> 
> >> > [pu]
> >> >
> >> >  Johannes's shallow clone work now should rebase cleanly on top
> >> >  of 'master' although I haven't done so yet.  As he said
> >> >  himself the series is waiting for people who have needs for
> >> >  such a feature to raise hands.
> >> 
> >> I haven't been watching this recently, but if this is what I
> >> understand it to be (the ability to get a partial repository from
> >> upstream and work normally from there with the result of data-mineing
> >> tools sometimes reporting 'that's part of the truncated history' if
> >> they hit the cutoff) consider my hand raised.
> >
> > +1
> 
> What does that plus one mean?  I do not know where people picked
> up this annoying plus or minus one business, but could you all
> stop that?

Horst can speak for himself, but I'd wager he's using the Apache voting
conventions:


^ permalink raw reply

* Re: What's in git.git
From: Junio C Hamano @ 2006-11-09  3:04 UTC (permalink / raw)
  To: Horst H. von Brand; +Cc: git
In-Reply-To: <7vslgtpbx0.fsf@assigned-by-dhcp.cox.net>

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

> "Horst H. von Brand" <vonbrand@laptop13.inf.utfsm.cl> writes:
>
>>> On Tue, 7 Nov 2006, Junio C Hamano wrote:
>>> 
>>> > [pu]
>>> >
>>> >  Johannes's shallow clone work now should rebase cleanly on top
>>> >  of 'master' although I haven't done so yet.  As he said
>>> >  himself the series is waiting for people who have needs for
>>> >  such a feature to raise hands.
>>
>> +1
>
> What does that plus one mean?  I do not know where people picked
> up this annoying plus or minus one business, but could you all
> stop that?
>
> If you are volunteering to help debugging and feeding bugfixes
> that is very much welcome and appreciated.

Sorry, I sent the message without finishing.  What should have
followed is...

  On the other hand, if you are saying "yes, shallow would fit
  my workflow and would be useful to have, but I do not have
  time nor inclination to help hacking on it", that is also
  fine.

  But "+1" does not help me tell which one you really mean.

> Thanks.

^ permalink raw reply

* Re: What's in git.git
From: Junio C Hamano @ 2006-11-09  2:54 UTC (permalink / raw)
  To: Horst H. von Brand; +Cc: git
In-Reply-To: <200611090228.kA92SMqw006666@laptop13.inf.utfsm.cl>

"Horst H. von Brand" <vonbrand@laptop13.inf.utfsm.cl> writes:

> David Lang <dlang@digitalinsight.com> wrote:
>> On Tue, 7 Nov 2006, Junio C Hamano wrote:
>> 
>> > [pu]
>> >
>> >  Johannes's shallow clone work now should rebase cleanly on top
>> >  of 'master' although I haven't done so yet.  As he said
>> >  himself the series is waiting for people who have needs for
>> >  such a feature to raise hands.
>> 
>> I haven't been watching this recently, but if this is what I
>> understand it to be (the ability to get a partial repository from
>> upstream and work normally from there with the result of data-mineing
>> tools sometimes reporting 'that's part of the truncated history' if
>> they hit the cutoff) consider my hand raised.
>
> +1

What does that plus one mean?  I do not know where people picked
up this annoying plus or minus one business, but could you all
stop that?

If you are volunteering to help debugging and feeding bugfixes
that is very much welcome and appreciated.

Thanks.

^ permalink raw reply

* Re: What's in git.git
From: Horst H. von Brand @ 2006-11-09  2:28 UTC (permalink / raw)
  To: David Lang; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.63.0611072009220.2601@qynat.qvtvafvgr.pbz>

David Lang <dlang@digitalinsight.com> wrote:
> On Tue, 7 Nov 2006, Junio C Hamano wrote:
> 
> > [pu]
> >
> >  Johannes's shallow clone work now should rebase cleanly on top
> >  of 'master' although I haven't done so yet.  As he said
> >  himself the series is waiting for people who have needs for
> >  such a feature to raise hands.
> 
> I haven't been watching this recently, but if this is what I
> understand it to be (the ability to get a partial repository from
> upstream and work normally from there with the result of data-mineing
> tools sometimes reporting 'that's part of the truncated history' if
> they hit the cutoff) consider my hand raised.

+1
-- 
Dr. Horst H. von Brand                   User #22616 counter.li.org
Departamento de Informatica                    Fono: +56 32 2654431
Universidad Tecnica Federico Santa Maria             +56 32 2654239
Casilla 110-V, Valparaiso, Chile               Fax:  +56 32 2797513

^ permalink raw reply

* Re: [PATCH] clear error message for clone a gitweb URL
From: Liu Yubao @ 2006-11-09  1:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7iy5sma9.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> Liu Yubao <yubao.liu@gmail.com> writes:
> 
>> When clone a gitweb URL, git reports "Can't lock ref", it's not clear for users,
>> this patch adds clear error message for this case.
>>
>> diff --git a/fetch.c b/fetch.c
>> index c426c04..40c5183 100644
>> --- a/fetch.c
>> +++ b/fetch.c
>> @@ -266,6 +266,14 @@ int pull(int targets, char **target, con
>>  		if (!write_ref || !write_ref[i])
>>  			continue;
>>  
>> +		if (*write_ref[i] == '\0') {
>> +			if (strncmp(write_ref_log_details, "http", 4) == 0)
>> +				error("Can't feed empty ref, seems you are fetching from a gitweb URL, "
>> +				      "check it in web browser for git URL.");
>> +			else
>> +				error("Can't feed empty ref");
>> +			goto unlock_and_fail;
> 
> You might have got that error by feeding an URL for gitweb, but
> I do not think the code, even with your additions, knows enough
> to tell that the user's mistake isn't other kinds of errors.
> 
> I am afraid that it would cause the user to waste time going
> wild goose chase if you say "seems you are...".  The phrasing
> makes it sound as if the tool _knows_ with some certainty that
> it is more plausible cause of the error than other kinds, while
> it certainly doesn't.
> 

I agree with you, it's not a fault of fetch.c.

> I think the reason it does not notice the breakage much earlier
> is that git-clone does not notice that gitweb URL gives nonsense
> to requests to "http://host/gitweb.cgi/$project/info/refs", so
> your patch to git-clone.sh is probably touching the right place,
> but I still feel the wording is a bit too strong and definitive
> than it should be.
> 
> Perhaps...
> 
> diff --git a/git-clone.sh b/git-clone.sh
> index 3f006d1..7ae69d9 100755
> --- a/git-clone.sh
> +++ b/git-clone.sh
> @@ -46,15 +46,18 @@ Perhaps git-update-server-info needs to
>  	do
>  		name=`expr "z$refname" : 'zrefs/\(.*\)'` &&
>  		case "$name" in
> -		*^*)	continue;;
> -		esac
> +		*^*)	continue ;;
> +		'')	false ;;
> +		esac &&
>  		if test -n "$use_separate_remote" &&
>  		   branch_name=`expr "z$name" : 'zheads/\(.*\)'`
>  		then
>  			tname="remotes/$origin/$branch_name"
>  		else
>  			tname=$name
> -		fi
> +		fi || {
> +			die "info/refs has nonsense $sha1 $refname, are you pulling from the right repository URL?"
> +		}
>  		git-http-fetch -v -a -w "$tname" "$name" "$1/" || exit 1
>  	done <"$clone_tmp/refs"
>  	rm -fr "$clone_tmp"
> 
It works well. Maybe it's better to say "info/refs has nonsense sha1($sha1) 

^ 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