Git development
 help / color / mirror / Atom feed
* [PATCH] git-svn: SVN 1.1.x library compatibility
From: Eric Wong @ 2006-06-28 10:07 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7virml4pqg.fsf@assigned-by-dhcp.cox.net>

Tested on a plain Ubuntu Hoary installation
using subversion 1.1.1-2ubuntu3

1.1.x issues I had to deal with:

* Avoid the noisy command-line client compatibility check if we
  use the libraries.

* get_log() arguments differ (now using a nice wrapper from
  Junio's suggestion)

* get_file() is picky about what kind of file handles it gets,
  so I ended up redirecting STDOUT.  I'm probably overflushing
  my file handles, but that's the safest thing to do...

* BDB kept segfaulting on me during tests, so svnadmin will use FSFS
  whenever we can.

* If somebody used an expanded CVS $Id$ line inside a file, then
  propsetting it to use svn:keywords will cause the original CVS
  $Id$ to be retained when asked for the original file.  As far as
  I can see, this is a server-side issue.  We won't care in the
  test anymore, as long as it's not expanded by SVN, a static
  CVS $Id$ line is fine.

While we're at making ourselves more compatible, avoid grep
along with the -q flag, which is GNU-specific. (grep avoidance
tip from Junio, too)

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 Junio C Hamano <junkio@cox.net> wrote:
 > Eric Wong <normalperson@yhbt.net> writes:
 > > * get_log() arguments differ.
 > 
 > Maybe you would want to have a single wrapper sub {} for this,
 > instead of repeating it all over, perhaps like this:
 > 
 > 	sub log_get {
 >         	my ($SVN_LOG, @args) = @_;
 >         	if ($SVN::CORE::VERSION ge '1.2.0') {
 >                 	splice(@args, 3, 0, 0);
 >                 }
 >                 $SVN_LOG->get_log(@args);
 > 	}

 I actually did it backwards (le) in case I want create something that
 could take advantage of the limit arg for newer versions but degrade
 gracefully for 1.1.x

 contrib/git-svn/git-svn.perl                     |   30 +++++++++++++++++-----
 contrib/git-svn/t/lib-git-svn.sh                 |    8 +++++-
 contrib/git-svn/t/t0000-contrib-git-svn.sh       |   14 +++++++++-
 contrib/git-svn/t/t0001-contrib-git-svn-props.sh |    4 +--
 4 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/contrib/git-svn/git-svn.perl b/contrib/git-svn/git-svn.perl
index 08c3010..f026b24 100755
--- a/contrib/git-svn/git-svn.perl
+++ b/contrib/git-svn/git-svn.perl
@@ -134,7 +134,7 @@ usage(1) unless defined $cmd;
 init_vars();
 load_authors() if $_authors;
 load_all_refs() if $_branch_all_refs;
-svn_compat_check();
+svn_compat_check() unless $_use_lib;
 migration_check() unless $cmd =~ /^(?:init|rebuild|multi-init)$/;
 $cmd{$cmd}->[0]->(@ARGV);
 exit 0;
@@ -379,7 +379,8 @@ sub fetch_lib {
 			# performance sucks with it enabled, so it's much
 			# faster to fetch revision ranges instead of relying
 			# on the limiter.
-			$SVN_LOG->get_log( '/'.$SVN_PATH, $min, $max, 0, 1, 1,
+			libsvn_get_log($SVN_LOG, '/'.$SVN_PATH,
+					$min, $max, 0, 1, 1,
 				sub {
 					my $log_msg;
 					if ($last_commit) {
@@ -924,7 +925,7 @@ sub graft_file_copy_lib {
 	$SVN::Error::handler = \&libsvn_skip_unknown_revs;
 	while (1) {
 		my $pool = SVN::Pool->new;
-		$SVN_LOG->get_log( "/$path", $min, $max, 0, 1, 1,
+		libsvn_get_log($SVN_LOG, "/$path", $min, $max, 0, 1, 1,
 			sub {
 				libsvn_graft_file_copies($grafts, $tree_paths,
 							$path, @_);
@@ -2358,8 +2359,8 @@ sub libsvn_load {
 	return unless $_use_lib;
 	$_use_lib = eval {
 		require SVN::Core;
-		if ($SVN::Core::VERSION lt '1.2.1') {
-			die "Need SVN::Core 1.2.1 or better ",
+		if ($SVN::Core::VERSION lt '1.1.0') {
+			die "Need SVN::Core 1.1.0 or better ",
 					"(got $SVN::Core::VERSION) ",
 					"Falling back to command-line svn\n";
 		}
@@ -2392,9 +2393,15 @@ sub libsvn_get_file {
 	my $pool = SVN::Pool->new;
 	defined($pid = open3($in, $out, '>&STDERR',
 				qw/git-hash-object -w --stdin/)) or croak $!;
-	my ($r, $props) = $SVN->get_file($f, $rev, $in, $pool);
+	# redirect STDOUT for SVN 1.1.x compatibility
+	open my $stdout, '>&', \*STDOUT or croak $!;
+	open STDOUT, '>&', $in or croak $!;
+	$| = 1; # not sure if this is necessary, better safe than sorry...
+	my ($r, $props) = $SVN->get_file($f, $rev, \*STDOUT, $pool);
 	$in->flush == 0 or croak $!;
+	open STDOUT, '>&', $stdout or croak $!;
 	close $in or croak $!;
+	close $stdout or croak $!;
 	$pool->clear;
 	chomp($hash = do { local $/; <$out> });
 	close $out or croak $!;
@@ -2566,7 +2573,8 @@ sub revisions_eq {
 	if ($_use_lib) {
 		# should be OK to use Pool here (r1 - r0) should be small
 		my $pool = SVN::Pool->new;
-		$SVN->get_log("/$path", $r0, $r1, 0, 1, 1, sub {$nr++},$pool);
+		libsvn_get_log($SVN, "/$path", $r0, $r1,
+				0, 1, 1, sub {$nr++}, $pool);
 		$pool->clear;
 	} else {
 		my ($url, undef) = repo_path_split($SVN_URL);
@@ -2606,6 +2614,14 @@ sub libsvn_find_parent_branch {
 	return undef;
 }
 
+sub libsvn_get_log {
+	my ($ra, @args) = @_;
+	if ($SVN::Core::VERSION le '1.2.0') {
+		splice(@args, 3, 1);
+	}
+	$ra->get_log(@args);
+}
+
 sub libsvn_new_tree {
 	if (my $log_entry = libsvn_find_parent_branch(@_)) {
 		return $log_entry;
diff --git a/contrib/git-svn/t/lib-git-svn.sh b/contrib/git-svn/t/lib-git-svn.sh
index 2843258..d7f972a 100644
--- a/contrib/git-svn/t/lib-git-svn.sh
+++ b/contrib/git-svn/t/lib-git-svn.sh
@@ -33,7 +33,13 @@ svnrepo=$PWD/svnrepo
 
 set -e
 
-svnadmin create $svnrepo
+if svnadmin create --help | grep fs-type >/dev/null
+then
+	svnadmin create --fs-type fsfs "$svnrepo"
+else
+	svnadmin create "$svnrepo"
+fi
+
 svnrepo="file://$svnrepo/test-git-svn"
 
 
diff --git a/contrib/git-svn/t/t0000-contrib-git-svn.sh b/contrib/git-svn/t/t0000-contrib-git-svn.sh
index 443d518..b482bb6 100644
--- a/contrib/git-svn/t/t0000-contrib-git-svn.sh
+++ b/contrib/git-svn/t/t0000-contrib-git-svn.sh
@@ -5,6 +5,16 @@ #
 
 test_description='git-svn tests'
 GIT_SVN_LC_ALL=$LC_ALL
+
+case "$LC_ALL" in
+*.UTF-8)
+	have_utf8=t
+	;;
+*)
+	have_utf8=
+	;;
+esac
+
 . ./lib-git-svn.sh
 
 mkdir import
@@ -173,7 +183,7 @@ then
 fi
 
 
-if test -n "$GIT_SVN_LC_ALL" && echo $GIT_SVN_LC_ALL | grep -q '\.UTF-8$'
+if test "$have_utf8" = t
 then
 	name="commit with UTF-8 message: locale: $GIT_SVN_LC_ALL"
 	echo '# hello' >> exec-2.sh
@@ -203,7 +213,7 @@ fi
 
 name='check imported tree checksums expected tree checksums'
 rm -f expected
-if test -n "$GIT_SVN_LC_ALL" && echo $GIT_SVN_LC_ALL | grep -q '\.UTF-8$'
+if test "$have_utf8" = t
 then
 	echo tree f735671b89a7eb30cab1d8597de35bd4271ab813 > expected
 fi
diff --git a/contrib/git-svn/t/t0001-contrib-git-svn-props.sh b/contrib/git-svn/t/t0001-contrib-git-svn-props.sh
index 54e0ed7..a5a235f 100644
--- a/contrib/git-svn/t/t0001-contrib-git-svn-props.sh
+++ b/contrib/git-svn/t/t0001-contrib-git-svn-props.sh
@@ -21,8 +21,8 @@ a_empty_crlf=
 
 cd import
 	cat >> kw.c <<\EOF
-/* Make it look like somebody copied a file from CVS into SVN: */
-/* $Id: kw.c,v 1.1.1.1 1994/03/06 00:00:00 eric Exp $ */
+/* Somebody prematurely put a keyword into this file */
+/* $Id$ */
 EOF
 
 	printf "Hello\r\nWorld\r\n" > crlf
-- 
1.4.1.rc1.gc7c5

^ permalink raw reply related

* [PATCH] cvsimport - cleanup of the multi-indexes handling
From: Martin Langhoff @ 2006-06-28 10:13 UTC (permalink / raw)
  To: git, junkio, Johannes.Schindelin; +Cc: Martin Langhoff

Indexes are only needed when we are about preparing to commit. Prime them
inside commit() when we have all the info we need, and remove all the
redundant index setups.

While we are at it, make sure that index handling is correct when opening
new branches, and on initial import.

Signed-off-by: Martin Langhoff <martin@catalyst.net.nz>

---

This has passed some light testing. Applies on top of master, and seems to
do the right thing with tidier code . It even makes sense ;-)

---
 git-cvsimport.perl |   62 ++++++++++++++++++++--------------------------------
 1 files changed, 24 insertions(+), 38 deletions(-)

diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 50f5d96..9fa598a 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -467,11 +467,6 @@ my $orig_git_index;
 $orig_git_index = $ENV{GIT_INDEX_FILE} if exists $ENV{GIT_INDEX_FILE};
 
 my %index; # holds filenames of one index per branch
-$index{$opt_o} = tmpnam();
-
-$ENV{GIT_INDEX_FILE} = $index{$opt_o};
-system("git-read-tree", $opt_o);
-die "read-tree failed: $?\n" if $?;
 
 unless(-d $git_dir) {
 	system("git-init-db");
@@ -499,14 +494,6 @@ unless(-d $git_dir) {
 	$orig_branch = $last_branch;
 	$tip_at_start = `git-rev-parse --verify HEAD`;
 
-	# populate index
-	unless ($index{$last_branch}) {
-	    $index{$last_branch} = tmpnam();
-	}
-	$ENV{GIT_INDEX_FILE} = $index{$last_branch};
-	system('git-read-tree', $last_branch);
-	die "read-tree failed: $?\n" if $?;
-
 	# Get the last import timestamps
 	opendir(D,"$git_dir/refs/heads");
 	while(defined(my $head = readdir(D))) {
@@ -623,6 +610,27 @@ # commits that cvsps cannot place anywhe
 $ignorebranch{'#CVSPS_NO_BRANCH'} = 1;
 
 sub commit {
+	if ($branch eq $opt_o && !$index{branch} && !get_headref($branch, $git_dir)) { 
+	    # looks like an initial commit
+	    # use the index primed by git-init-db
+	    $ENV{GIT_INDEX_FILE} = '.git/index';
+	    $index{$branch} = '.git/index';
+	} else {
+	    # use an index per branch to speed up
+	    # imports of projects with many branches
+	    unless ($index{$branch}) {
+		$index{$branch} = tmpnam();
+		$ENV{GIT_INDEX_FILE} = $index{$branch};
+		if ($ancestor) {
+		    system("git-read-tree", $ancestor);
+		} else {
+		    system("git-read-tree", $branch);
+		}
+		die "read-tree failed: $?\n" if $?;
+	    }
+	}
+        $ENV{GIT_INDEX_FILE} = $index{$branch};
+
 	update_index(@old, @new);
 	@old = @new = ();
 	my $tree = write_tree();
@@ -811,30 +819,6 @@ while(<CVS>) {
 			close(H)
 				or die "Could not write branch $branch: $!";
 		}
-		if(($ancestor || $branch) ne $last_branch) {
-			print "Switching from $last_branch to $branch\n" if $opt_v;
-			unless ($index{$branch}) {
-			    $index{$branch} = tmpnam();
-			    $ENV{GIT_INDEX_FILE} = $index{$branch};
-			    system("git-read-tree", $branch);
-			    die "read-tree failed: $?\n" if $?;
-			}
-			# just in case
-			$ENV{GIT_INDEX_FILE} = $index{$branch};
-			if ($ancestor) {
-			    print "have ancestor $ancestor" if $opt_v;
-			    system("git-read-tree", $ancestor);
-			    die "read-tree failed: $?\n" if $?;
-			}
-		} else {
-			# just in case
-			unless ($index{$branch}) {
-			    $index{$branch} = tmpnam();
-			    $ENV{GIT_INDEX_FILE} = $index{$branch};
-			    system("git-read-tree", $branch);
-			    die "read-tree failed: $?\n" if $?;
-			}
-		}
 		$last_branch = $branch if $branch ne $last_branch;
 		$state = 9;
 	} elsif($state == 8) {
@@ -898,7 +882,9 @@ #	VERSION:1.96->1.96.2.1
 commit() if $branch and $state != 11;
 
 foreach my $git_index (values %index) {
-    unlink($git_index);
+    if ($git_index ne '.git/index') {
+	unlink($git_index);
+    }
 }
 
 if (defined $orig_git_index) {
-- 
1.4.1.rc1.g1ef9

^ permalink raw reply related

* Re: Quick merge status updates.
From: Junio C Hamano @ 2006-06-28 10:14 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: git
In-Reply-To: <1151489103.28036.6.camel@dv>

Pavel Roskin <proski@gnu.org> writes:

> I think my Perl 5.8.8 is "too new".  "man perlfunc" says about "use":
> ...
> I think the BEGIN block has priority over other statements.  My solution
> was to put the @INC change in the BEGIN block as well.

Actually I do use 5.8.8 and everything you quoted makes perfect
sense, -- in fact now I do not know *why* it did _not_ break for
me without the BEGIN {} block, especially I do not have any
PERL* environment variable to point at anywhere under $HOME.

Thanks for the fix.

^ permalink raw reply

* [PATCH (fixed) 1/2] rebase: check for errors from git-commit
From: Eric Wong @ 2006-06-28 10:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

commit does not always succeed, so we'll have to check for
it in the absence of set -e.  This fixes a regression
introduced in 9e4bc7dd1bb9d92491c475cec55147fa0b3f954d

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-rebase.sh |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 9ad1c44..28860fc 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -59,8 +59,13 @@ continue_merge () {
 
 	if test -n "`git-diff-index HEAD`"
 	then
+		if ! git-commit -C "`cat $dotest/current`"
+		then
+			echo "Commit failed, please do not call \"git commit\""
+			echo "directly, but instead do one of the following: "
+			die "$RESOLVEMSG"
+		fi
 		printf "Committed: %0${prec}d" $msgnum
-		git-commit -C "`cat $dotest/current`"
 	else
 		printf "Already applied: %0${prec}d" $msgnum
 	fi
-- 
1.4.1.rc1.gc7c5

^ permalink raw reply related

* Re: [PATCH] git.c: Re-introduce sane error messages on missing commands.
From: Johannes Schindelin @ 2006-06-28 10:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Andreas Ericsson, git
In-Reply-To: <7vr71938t4.fsf@assigned-by-dhcp.cox.net>

Hi,

On Wed, 28 Jun 2006, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Try this:
> >
> > $ mkdir 5
> > $ cd 5
> > $ git-init-db
> > $ rm .git/config # yes, really.
> > $ git abc
> 
> Thanks for trying to help, but not really.

Okay. Does not happen with 'next' here, too. I have some changes in my 
private repo (which eventually should culminate in the big mmap()ed sooper 
config parsing / writing thingie), which make it break. The following 
patch fixes this (and potentially Andreas' problem, too).

-- cut here --

[PATCH] save errno in handle_alias()

git.c:main() relies on the value of errno being set by the last attempt to 
execute the command. However, if something goes awry in handle_alias(), 
that assumption is wrong. So restore errno before returning from 
handle_alias().

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

---

diff --git a/git.c b/git.c
index 94e9a4a..7c7106e 100644
--- a/git.c
+++ b/git.c
@@ -99,7 +99,7 @@ static int split_cmdline(char *cmdline, 
 
 static int handle_alias(int *argcp, const char ***argv)
 {
-	int nongit = 0, ret = 0;
+	int nongit = 0, ret = 0, saved_errno = errno;
 	const char *subdir;
 
 	subdir = setup_git_directory_gently(&nongit);
@@ -137,6 +137,8 @@ static int handle_alias(int *argcp, cons
 	if (subdir)
 		chdir(subdir);
 
+	errno = saved_errno;
+
 	return ret;
 }
 

^ permalink raw reply related

* Re: [PATCH] cvsimport - cleanup of the multi-indexes handling
From: Junio C Hamano @ 2006-06-28 11:10 UTC (permalink / raw)
  To: Martin Langhoff; +Cc: git, junkio, Johannes.Schindelin
In-Reply-To: <11514896033560-git-send-email-martin@catalyst.net.nz>

Martin Langhoff <martin@catalyst.net.nz> writes:

> Indexes are only needed when we are about preparing to commit. Prime them
> inside commit() when we have all the info we need, and remove all the
> redundant index setups.

Obviously makes sense although I admit I do not have to interact
with CVS these days anymore (lucky me ;-).

^ permalink raw reply

* Re: CFT: merge-recursive in C (updated)
From: Alex Riesen @ 2006-06-28 11:35 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Junio C Hamano, Fredrik Kuivinen, Linus Torvalds
In-Reply-To: <Pine.LNX.4.63.0606281202360.29667@wbgn013.biozentrum.uni-wuerzburg.de>

On 6/28/06, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > > > > - use a pipe to "git-update-index --index-info" instead of using command
> > > > > line
> >
> > ...and to take it a step further, a patch (0002) to avoid too many calls to
> > git-write-tree and to git-update-index.
>
> ... and introduces a lot more lines doing debug output.

well, that's how I found out about what to fix. Was really impressed
when saw the difference :)
What stands out next is getRenames (to be renamed into get_renames),
a little profiling shows that the renames lists are the culprit this time too.

I actually expected these problems, but decided to postpone the
optimization for later: linked lists are comfortable to work with.
I didn't had much time for this project, and the first commit is
dated 7th June - it was a very slow progress.

> However, the change is good, but I would not call it "FILE *fp". IMHO
> "FILE *update_index_pipe" would explain better what you do there.

just update_index would be enough. It can't possibly mean anything else,
being FILE* in that context.

By the way, is it safe to use "git-update-index --index-info"?
AFAICS it was designed for this kind of use, but the most
visible user of it (git-update-recursive.py) didn't use --index-info
this way! Was there any specific reasons?

^ permalink raw reply

* Re: [PATCH] git.c: Re-introduce sane error messages on missing commands.
From: Andreas Ericsson @ 2006-06-28 11:53 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.63.0606281240480.29667@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin wrote:
> Hi,
> 
> On Wed, 28 Jun 2006, Junio C Hamano wrote:
> 
> 
>>Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>
>>
>>>Try this:
>>>
>>>$ mkdir 5
>>>$ cd 5
>>>$ git-init-db
>>>$ rm .git/config # yes, really.
>>>$ git abc
>>
>>Thanks for trying to help, but not really.
> 
> 
> Okay. Does not happen with 'next' here, too. I have some changes in my 
> private repo (which eventually should culminate in the big mmap()ed sooper 
> config parsing / writing thingie), which make it break. The following 
> patch fixes this (and potentially Andreas' problem, too).
> 

It should, although the command it tried to execute will still be empty 
if it fails for some other reason (file not executable / permission 
denied), since it only does the right thing on ENOENT.

This is also, imo, a bit worse than preserving the errno from the 
execve() call in the caller, since errno is sometimes a macro (yes, only 
in threaded apps atm, but still...), and it will be easy to forget to 
look in handle_alias() if other things change in main() that makes this 
bug resurface.

Oh, and the part of my patch removing the git_command variable from 
git.c:main() still has to be applied for arbitrary error-messages to 
look sane.

$ grep -B1 git_command git.c
         char *slash = strrchr(cmd, '/');
         char git_command[PATH_MAX + 1];
--
         fprintf(stderr, "Failed to run command '%s': %s\n",
                 git_command, strerror(errno));


Btw, Junio, did you try this with 'master' as of yesterday morning (git 
version 1.4.1.rc1.g1ef9)? It's reproducible on every machine I've tried 
so far (well, only five, but still), so it seems odd that you don't see it.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: [PATCH] git.c: Re-introduce sane error messages on missing commands.
From: Marco Roeland @ 2006-06-28 12:00 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, Andreas Ericsson, git
In-Reply-To: <Pine.LNX.4.63.0606281240480.29667@wbgn013.biozentrum.uni-wuerzburg.de>

On Wednesday June 28th 2006 Johannes Schindelin wrote:

> [PATCH] save errno in handle_alias()
> 
> git.c:main() relies on the value of errno being set by the last attempt to 
> execute the command. However, if something goes awry in handle_alias(), 
> that assumption is wrong. So restore errno before returning from 
> handle_alias().

If we rely on the value of errno we should always immediately store it's
value anyway. On some neolithic systems like the "MSVCRT.DLL" C runtime
library on Windows (used by e.g. the Mingw compiler, don't know about
Cygwin) a lot of runtime functions actually even reset the value of
errno to 0 on success!
-- 
Marco Roeland

^ permalink raw reply

* Re: CFT: merge-recursive in C (updated)
From: Johannes Schindelin @ 2006-06-28 12:55 UTC (permalink / raw)
  To: Alex Riesen; +Cc: git
In-Reply-To: <81b0412b0606280435t70ac9957jae2c4d1c10b7681d@mail.gmail.com>

Hi,

On Wed, 28 Jun 2006, Alex Riesen wrote:

> What stands out next is getRenames (to be renamed into get_renames), a 
> little profiling shows that the renames lists are the culprit this time 
> too.

In my attempts, the path_list did not contain paths, but structs 
containing a path and a void pointer. I think I will resurrect this idea 
for the renames.

Ciao,
Dscho

^ permalink raw reply

* Re: CFT: merge-recursive in C (updated)
From: Alex Riesen @ 2006-06-28 14:27 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0606281342290.29667@wbgn013.biozentrum.uni-wuerzburg.de>

On 6/28/06, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
> > What stands out next is getRenames (to be renamed into get_renames), a
> > little profiling shows that the renames lists are the culprit this time
> > too.
>
> In my attempts, the path_list did not contain paths, but structs
> containing a path and a void pointer. I think I will resurrect this idea
> for the renames.
>

What was the pointer for?

^ permalink raw reply

* Re: [PATCH] git.c: Re-introduce sane error messages on missing commands.
From: Christopher Faylor @ 2006-06-28 14:59 UTC (permalink / raw)
  To: Andreas Ericsson, Junio C Hamano, Johannes Schindelin,
	Marco Roeland, git
In-Reply-To: <20060628120044.GA3228@fiberbit.xs4all.nl>

On Wed, Jun 28, 2006 at 02:00:44PM +0200, Marco Roeland wrote:
>On Wednesday June 28th 2006 Johannes Schindelin wrote:
>
>> [PATCH] save errno in handle_alias()
>> 
>> git.c:main() relies on the value of errno being set by the last attempt to 
>> execute the command. However, if something goes awry in handle_alias(), 
>> that assumption is wrong. So restore errno before returning from 
>> handle_alias().
>
>If we rely on the value of errno we should always immediately store it's
>value anyway. On some neolithic systems like the "MSVCRT.DLL" C runtime
>library on Windows (used by e.g. the Mingw compiler, don't know about
>Cygwin) a lot of runtime functions actually even reset the value of
>errno to 0 on success!

Cygwin does not use MSVCRT.DLL and tries to be careful about spurious
resetting of errno.

cgf

^ permalink raw reply

* Re: CFT: merge-recursive in C
From: Christopher Faylor @ 2006-06-28 15:06 UTC (permalink / raw)
  To: Junio C Hamano, Fredrik Kuivinen, Alex Riesen, git
In-Reply-To: <20060626233838.GA3121@steel.home>

On Tue, Jun 27, 2006 at 01:38:38AM +0200, Alex Riesen wrote:
>It still uses some calls to git programs (git-update-index,
>git-hash-object, git-diff-tree and git-write-tree), and merge(1) has
>the labels (-L) missing - I was unsure how to tackle this on windows -
>it has only argv[1].

Actually, Windows should behave the same as Linux wrt argv handling.
You can use argv[1] ... argv[n] modulo any differences in command line
quoting.

On Windows the arguments are broken into individual components by the
runtime, e.g., MSVCRT.dll or Cygwin1.dll.

cgf

^ permalink raw reply

* Re: CFT: merge-recursive in C (updated)
From: Johannes Schindelin @ 2006-06-28 15:09 UTC (permalink / raw)
  To: Alex Riesen; +Cc: git
In-Reply-To: <81b0412b0606280727k2f4d5c26m5d37f2835821c93b@mail.gmail.com>

Hi,

On Wed, 28 Jun 2006, Alex Riesen wrote:

> On 6/28/06, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > 
> > > What stands out next is getRenames (to be renamed into get_renames), a
> > > little profiling shows that the renames lists are the culprit this time
> > > too.
> > 
> > In my attempts, the path_list did not contain paths, but structs
> > containing a path and a void pointer. I think I will resurrect this idea
> > for the renames.
> > 
> 
> What was the pointer for?

We want to emulate the set with a sorted list. The pointer is for the 
payload.

Ciao,
Dscho

^ permalink raw reply

* Discover the latest Lots of pleasure waits for you right after you take this medicine
From: Coleman @ 2006-06-28 16:31 UTC (permalink / raw)
  To: glenda

How are you? 


 Prolong your ssex. You have smaall peniis? Gain an extra 20% in thikness! Come on in here: http://magpills.com/gal/ms 


 Never look a gift horse in the mouth Blessings Are Not Valued Until They Are Gone Experience is a wonderful thing It enables you to recognize a mistake when you make it again  First Impressions are the Most Lasting Abstinence makes the heart go wander. Plant the crab tree where you will, it will never bear pippins 

^ permalink raw reply

* connect.c and errno
From: Morten Welinder @ 2006-06-28 16:56 UTC (permalink / raw)
  To: GIT Mailing List

It looks like connect.c waits too long before it uses errno in both copies
of git_tcp_connect_sock.  Both close and freeaddrinfo can poke any
non-zero value in there.

M.

^ permalink raw reply

* Prepare "git-merge-tree" for future work
From: Linus Torvalds @ 2006-06-28 18:18 UTC (permalink / raw)
  To: Junio C Hamano, Git Mailing List


This changes how "git-merge-tree" works in two ways:

 - instead of printing things out as we walk the trees, we save the 
   results in memory
 - when we've walked the tree fully, we print out the results in a more 
   explicit way, describing the data.

This is basically preparatory work for extending the git-merge-tree 
functionality in interestign directions.

In particular, git-merge-tree is also how you would create a diff between 
two trees _without_ necessarily creating the merge commit itself. In other 
words, if you were to just wonder what another branch adds, you should be 
able to (eventually) just do

	git merge-tree -p $base HEAD $otherbranch

to generate a diff of what the merge would look like. The current merge 
tree already basically has all the smarts for this, and the explanation of 
the results just means that hopefully somebody else than me could do the 
boring work.

(You'd basically be able to do the above diff by just changing the 
printout format for the explanation, and making the "changed in both" 
first do a three-way merge before it diffs the result).

The other thing that the in-memory format allows is rename detection 
(which the current code does not do). That's the basic reason why we don't 
want to just explain the differences as we go along - because we want to 
be able to look at the _other_ differences to see whether the reason an 
entry got deleted in either branch was perhaps because it got added in 
another place..

Rename detection should be a fairly trivial pass in between the tree 
diffing and the explanation.

In the meantime, this doesn't actually do anything new, it just outputs 
the information in a more verbose manner. 

For an example merge, commit 5ab2c0a47574c92f92ea3709b23ca35d96319edd in 
the git tree works well and shows renames, along with true removals and 
additions and files that got changed in both branches. Too see that as a 
tree merge, do:

	git-merge-tree 64e86c57 c5c23745 928e47e3

where the two last ones are the tips that got merged, and the first one is 
the merge base.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---
diff --git a/merge-tree.c b/merge-tree.c
index 9dcaab7..fd0c211 100644
--- a/merge-tree.c
+++ b/merge-tree.c
@@ -1,11 +1,79 @@
 #include "cache.h"
 #include "tree-walk.h"
+#include "blob.h"
 
 static const char merge_tree_usage[] = "git-merge-tree <base-tree> <branch1> <branch2>";
 static int resolve_directories = 1;
 
+struct merge_list {
+	struct merge_list *next;
+	struct merge_list *link;	/* other stages for this object */
+
+	unsigned int stage : 2,
+		     flags : 30;
+	unsigned int mode;
+	const char *path;
+	struct blob *blob;
+};
+
+static struct merge_list *merge_result, **merge_result_end = &merge_result;
+
+static void add_merge_entry(struct merge_list *entry)
+{
+	*merge_result_end = entry;
+	merge_result_end = &entry->next;
+}
+
 static void merge_trees(struct tree_desc t[3], const char *base);
 
+static const char *explanation(struct merge_list *entry)
+{
+	switch (entry->stage) {
+	case 0:
+		return "merged";
+	case 3:
+		return "added in remote";
+	case 2:
+		if (entry->link)
+			return "added in both";
+		return "added in local";
+	}
+
+	/* Existed in base */
+	entry = entry->link;
+	if (!entry)
+		return "removed in both";
+
+	if (entry->link)
+		return "changed in both";
+
+	if (entry->stage == 3)
+		return "removed in local";
+	return "removed in remote";
+}
+
+static void show_result_list(struct merge_list *entry)
+{
+	printf("%s\n", explanation(entry));
+	do {
+		struct merge_list *link = entry->link;
+		static const char *desc[4] = { "result", "base", "our", "their" };
+		printf("  %-6s %o %s %s\n", desc[entry->stage], entry->mode, sha1_to_hex(entry->blob->object.sha1), entry->path);
+		entry = link;
+	} while (entry);
+}
+
+static void show_result(void)
+{
+	struct merge_list *walk;
+
+	walk = merge_result;
+	while (walk) {
+		show_result_list(walk);
+		walk = walk->next;
+	}
+}
+
 /* An empty entry never compares same, not even to another empty entry */
 static int same_entry(struct name_entry *a, struct name_entry *b)
 {
@@ -15,24 +83,34 @@ static int same_entry(struct name_entry 
 		a->mode == b->mode;
 }
 
-static const char *sha1_to_hex_zero(const unsigned char *sha1)
+static struct merge_list *create_entry(unsigned stage, unsigned mode, const unsigned char *sha1, const char *path)
 {
-	if (sha1)
-		return sha1_to_hex(sha1);
-	return "0000000000000000000000000000000000000000";
+	struct merge_list *res = xmalloc(sizeof(*res));
+
+	memset(res, 0, sizeof(*res));
+	res->stage = stage;
+	res->path = path;
+	res->mode = mode;
+	res->blob = lookup_blob(sha1);
+	return res;
 }
 
 static void resolve(const char *base, struct name_entry *branch1, struct name_entry *result)
 {
+	struct merge_list *orig, *final;
+	const char *path;
+
 	/* If it's already branch1, don't bother showing it */
 	if (!branch1)
 		return;
 
-	printf("0 %06o->%06o %s->%s %s%s\n",
-		branch1->mode, result->mode,
-		sha1_to_hex_zero(branch1->sha1),
-		sha1_to_hex_zero(result->sha1),
-		base, result->path);
+	path = strdup(mkpath("%s%s", base, result->path));
+	orig = create_entry(2, branch1->mode, branch1->sha1, path);
+	final = create_entry(0, result->mode, result->sha1, path);
+
+	final->link = orig;
+
+	add_merge_entry(final);
 }
 
 static int unresolved_directory(const char *base, struct name_entry n[3])
@@ -71,16 +149,40 @@ static int unresolved_directory(const ch
 	return 1;
 }
 
+
+static struct merge_list *link_entry(unsigned stage, const char *base, struct name_entry *n, struct merge_list *entry)
+{
+	const char *path;
+	struct merge_list *link;
+
+	if (!n->mode)
+		return entry;
+	if (entry)
+		path = entry->path;
+	else
+		path = strdup(mkpath("%s%s", base, n->path));
+	link = create_entry(stage, n->mode, n->sha1, path);
+	link->link = entry;
+	return link;
+}
+
 static void unresolved(const char *base, struct name_entry n[3])
 {
+	struct merge_list *entry = NULL;
+
 	if (unresolved_directory(base, n))
 		return;
-	if (n[0].sha1)
-		printf("1 %06o %s %s%s\n", n[0].mode, sha1_to_hex(n[0].sha1), base, n[0].path);
-	if (n[1].sha1)
-		printf("2 %06o %s %s%s\n", n[1].mode, sha1_to_hex(n[1].sha1), base, n[1].path);
-	if (n[2].sha1)
-		printf("3 %06o %s %s%s\n", n[2].mode, sha1_to_hex(n[2].sha1), base, n[2].path);
+
+	/*
+	 * Do them in reverse order so that the resulting link
+	 * list has the stages in order - link_entry adds new
+	 * links at the front.
+	 */
+	entry = link_entry(3, base, n + 2, entry);
+	entry = link_entry(2, base, n + 1, entry);
+	entry = link_entry(1, base, n + 0, entry);
+
+	add_merge_entry(entry);
 }
 
 /*
@@ -172,5 +274,7 @@ int main(int argc, char **argv)
 	free(buf1);
 	free(buf2);
 	free(buf3);
+
+	show_result();
 	return 0;
 }

^ permalink raw reply related

* Re: [PATCH] GIT_TRACE: show which built-in/external commands are executed
From: Matthias Lederhofer @ 2006-06-28 18:22 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0606260129410.29667@wbgn013.biozentrum.uni-wuerzburg.de>

> That still leaves my problem: GIT_TRACE=1 on scripts is incomplete.
Ok, I see what you mean.

This actually affects all scripts which are called as git-foo instead
of git foo (but built-in commands show up anyway). So I'd add a
warning to the documentation in which cases GIT_TRACE will not show
that a command is executed.
In the git repository I found 47 shell scripts (git-* without the
header file) which would be affected. Searching for all occurences of
git-(one of those shell-scripts) I found 50 lines of code which use
it. If there is any interest in changing this I can try to change
this.

^ permalink raw reply

* [PATCH] Makefile: set USE_PIC on Linux x86_64 for linking with Git.pm
From: Marco Roeland @ 2006-06-28 18:35 UTC (permalink / raw)
  To: Git Mailing List

In commit 6294a10 it was noted that "on x86-64 it seems that Git.xs does
not link without compiling the main git objects with -fPIC". Set it
therefore automatically on this platform.

Signed-off-by: Marco Roeland <marco.roeland@xs4all.nl>
---
At the moment this is 'pu' stuff.
---
 Makefile |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index 2df5bd4..0f0e25a 100644
--- a/Makefile
+++ b/Makefile
@@ -254,6 +254,9 @@ # we had "elif" things would have been m
 
 ifeq ($(uname_S),Linux)
 	NO_STRLCPY = YesPlease
+	ifneq (,$(findstring x86_64,$(uname_M)))
+		USE_PIC = YesPlease
+	endif
 endif
 ifeq ($(uname_S),Darwin)
 	NEEDS_SSL_WITH_CRYPTO = YesPlease
-- 
1.4.1.rc1.g3550-dirty

^ permalink raw reply related

* Re: [PATCH] Makefile: set USE_PIC on Linux x86_64 for linking with Git.pm
From: Junio C Hamano @ 2006-06-28 18:53 UTC (permalink / raw)
  To: Marco Roeland; +Cc: git
In-Reply-To: <20060628183557.GA5713@fiberbit.xs4all.nl>

Marco Roeland <marco.roeland@xs4all.nl> writes:

> In commit 6294a10 it was noted that "on x86-64 it seems that Git.xs does
> not link without compiling the main git objects with -fPIC". Set it
> therefore automatically on this platform.

I agree with it in principle but was too lazy to do that myself.
I wonder it should be inside Linux, though.

>  ifeq ($(uname_S),Linux)
>  	NO_STRLCPY = YesPlease
> +	ifneq (,$(findstring x86_64,$(uname_M)))
> +		USE_PIC = YesPlease
> +	endif
>  endif

^ permalink raw reply

* Re: [PATCH] Makefile: set USE_PIC on Linux x86_64 for linking with Git.pm
From: Junio C Hamano @ 2006-06-28 18:59 UTC (permalink / raw)
  To: Marco Roeland; +Cc: git
In-Reply-To: <7vr719159v.fsf@assigned-by-dhcp.cox.net>

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

> Marco Roeland <marco.roeland@xs4all.nl> writes:
>
>> In commit 6294a10 it was noted that "on x86-64 it seems that Git.xs does
>> not link without compiling the main git objects with -fPIC". Set it
>> therefore automatically on this platform.
>
> I agree with it in principle but was too lazy to do that myself.
> I wonder it should be inside Linux, though.
>
>>  ifeq ($(uname_S),Linux)
>>  	NO_STRLCPY = YesPlease
>> +	ifneq (,$(findstring x86_64,$(uname_M)))
>> +		USE_PIC = YesPlease
>> +	endif
>>  endif

In other words, I am wondering why you did not do this more
obvious one:

        ifeq ($(uname_M),x86_64)
                USE_PIC = YesPlease
        endif

My suspicion is that you protected that in Linux on purpose
because you know that my version would break for somebody else,
or because you are trying to be cautious not to break other
platforms you do not have access to, and I cannot tell which.

^ permalink raw reply

* Re: [PATCH] Makefile: set USE_PIC on Linux x86_64 for linking with Git.pm
From: Marco Roeland @ 2006-06-28 19:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Marco Roeland, git
In-Reply-To: <7vr719159v.fsf@assigned-by-dhcp.cox.net>

On Wednesday June 28th 2006 Junio C Hamano wrote:

> I agree with it in principle but was too lazy to do that myself.
> I wonder it should be inside Linux, though.
> 
> >  ifeq ($(uname_S),Linux)
> >  	NO_STRLCPY = YesPlease
> > +	ifneq (,$(findstring x86_64,$(uname_M)))
> > +		USE_PIC = YesPlease
> > +	endif
> >  endif

Yes, I wondered about that myself. I chose to be on the safe side: I
can and have tested this myself on Linux x86-64 but am not sure if it's
needed on the BSD's for example.

Even for Linux someone mentioned that probably i386 is the exception in
_not_ needing the -fPIC linkage. It might even be specific to the Perl
"xs" implementation specifics?

So I should have added "Works for me (TM)"! ;-)
-- 
Marco Roeland

^ permalink raw reply

* Re: [PATCH] Makefile: set USE_PIC on Linux x86_64 for linking with Git.pm
From: Marco Roeland @ 2006-06-28 19:21 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Marco Roeland, git
In-Reply-To: <7virml14za.fsf@assigned-by-dhcp.cox.net>

On Wednesday June 28th 2006 Junio C Hamano wrote:

> >>  ifeq ($(uname_S),Linux)
> >>  	NO_STRLCPY = YesPlease
> >> +	ifneq (,$(findstring x86_64,$(uname_M)))
> >> +		USE_PIC = YesPlease
> >> +	endif
> >>  endif
> 
> In other words, I am wondering why you did not do this more
> obvious one:
> 
>         ifeq ($(uname_M),x86_64)
>                 USE_PIC = YesPlease
>         endif
> 
> My suspicion is that you protected that in Linux on purpose
> because you know that my version would break for somebody else,
> or because you are trying to be cautious not to break other
> platforms you do not have access to, and I cannot tell which.

Sorry for the confusion. Yes that construct is much more readable. I
copy and pasted it from another section in the Makefile and adapted it
to this use. I tested it and it worked so I decided no to change it
anymore. So that clears up the syntactical issue.

I certainly do not know cases outside Linux where this might break on
x86-64. I just tried to limit it to the case I could test. But perhaps
someone with an x86-64 BSD or Solaris might try it?

To paraphrase Dave Jones: I type 'make', it fails. Some 'git log' later
I realise I have to manually define 'USE_PIC'. Hey, why doesn't it work
automagically? Some 'git grep' and I spot a construct for specific
(sub)platforms. Monkey see, monkey do. I type 'make', it works and
monkey sends patch! Thats it! No subtleties involved;-)
-- 
Marco Roeland

^ permalink raw reply

* [PATCH] Makefile: set USE_PIC on Linux x86_64 for linking with Git.xs
From: Marco Roeland @ 2006-06-28 19:55 UTC (permalink / raw)
  To: Marco Roeland; +Cc: Junio C Hamano, git
In-Reply-To: <20060628192145.GD5713@fiberbit.xs4all.nl>

In commit 6294a10 it was noted that "on x86-64 it seems that Git.xs does
not link without compiling the main git objects with -fPIC". Set it
therefore automatically on this platform.

This patch does this only for _Linux_ x86-64, as that is the only x86-64
platform I have access to. But it might very well make sense on other
x86-64 platforms, please test and report if you have such a platform.

Signed-off-by: Marco Roeland <marco.roeland@xs4all.nl>
---
This applies to 'pu'. It is an amended version from an earlier one
with a simplification from Junio and a clarification why it is Linux
specific at the moment. The title was also slightly improved.
---
 Makefile |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index 2df5bd4..88cfe2b 100644
--- a/Makefile
+++ b/Makefile
@@ -254,6 +254,9 @@ # we had "elif" things would have been m
 
 ifeq ($(uname_S),Linux)
 	NO_STRLCPY = YesPlease
+	ifeq ($(uname_M),x86_64)
+		USE_PIC = YesPlease
+	endif
 endif
 ifeq ($(uname_S),Darwin)
 	NEEDS_SSL_WITH_CRYPTO = YesPlease
-- 
1.4.1.rc1.g3550-dirty

^ permalink raw reply related

* Problem with GITK
From: Paolo Ciarrocchi @ 2006-06-28 20:40 UTC (permalink / raw)
  To: Git Mailing List

Hi all,
there is a strange orange line in the following screenshot from gitk:
http://paolo.ciarrocchi.googlepages.com/Screenshotgit.png

paolo@Italia:~/linux-2.6$ git version
git version 1.4.1.rc1.g47e5

Ciao,

-- 
Paolo
http://paolociarrocchi.googlepages.com
http://picasaweb.google.com/paolo.ciarrocchi

^ 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