Git development
 help / color / mirror / Atom feed
* Re: [PATCH v1 0/3] Introduce config variable "diff.primer"
From: Jeff King @ 2009-01-25 22:07 UTC (permalink / raw)
  To: Keith Cascio; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.GSO.2.00.0901251239000.12651@kiwi.cs.ucla.edu>

On Sun, Jan 25, 2009 at 12:41:21PM -0800, Keith Cascio wrote:

> > I am puzzled.
> > 
> > The gitattributes mechanism is about per-path settings, but I do not
> > think a mnemonicprefix that is per-path makes much sense.
> 
> That was just an example (perhaps poorly chosen).  What I meant to
> suggest is making gitattributes consistent with gitconfig WRT at least
> the [diff] section.  But maybe that's not appropriate.  Thanks for the
> insight.

I don't think you want it entirely consistent. What would
diff.renamelimit mean in the context of a gitattribute? But I do think
it makes sense for some (like specific diff options such as whitespace
handling).

Also, if you're going to have options that apply to gitattributes diff
drivers _and_ as a general fallback, I think we need to define when the
fallback kicks in. That is, let's say I have a gitattributes file like
this:

   *.c diff=c

and my config says:

  [diff]
    opt1 = val1_default
    opt2 = val2_default

  [diff "c"]
    opt1 = val1_c

Now obviously if I want to use opt1 for my C files, it should be val1_c.
But if I want to use opt2, what should it use? There are two reasonable
choices, I think:

  1. You use val2_default. The rationale is that the "c" diff driver did
     not define an opt2, so you fall back to the global default.

  2. It is unset. The rationale is that you are using the "c" diff
     driver, and it has left the value unset. The default then means "if
     you have no diff driver setup".

I suspect "1" is what people would want most of the time, but "2" is
actually more flexible (since there is otherwise no way to say "I
explicitly left diff.c.opt2 unset").

If (2) is desired, I think it makes more sense to put such "default"
options into their own diff driver section. Like:

  [diff "default"]
    opt2 = whatever

And then it is more clear that once you have selected the "c" diff
driver, the values in the other "default" are not relevant.

I don't think this is a huge issue overall, but it occurs to me that we
have just added diff.wordRegex and diff.*.wordRegex. So it makes sense
to think for a minute which behavior we want before it ships and we are
stuck with backwards compatibility forever.

-Peff

^ permalink raw reply

* Re: [PATCH v1 1/3] Introduce config variable "diff.primer"
From: Jeff King @ 2009-01-25 22:11 UTC (permalink / raw)
  To: Keith Cascio; +Cc: Johannes Schindelin, Junio C Hamano, git
In-Reply-To: <alpine.GSO.2.00.0901251033160.12651@kiwi.cs.ucla.edu>

On Sun, Jan 25, 2009 at 10:44:25AM -0800, Keith Cascio wrote:

> The name "primer" is open to discussion, of course.  But I like it.
> From Merriam-Webster:
> primer n 1: a device for priming 2: material used in priming a surface
> prime vb 1: fill, load 2: to prepare for firing 3: to apply the first color, coating or preparation to <~ a wall>

FWIW, I found it very confusing. I would have expected "diff.options" or
"diff.defaults". There is also some precedent in the form of
GIT_DIFF_OPTS, but I believe it _only_ handles --unified and -u, so it
is not necessarily a useful model.

-Peff

^ permalink raw reply

* [PATCH] git-svn: add --ignore-paths option for fetching
From: Vitaly "_Vi" Shukela @ 2009-01-25 22:21 UTC (permalink / raw)
  To: git; +Cc: Vitaly "_Vi" Shukela

This will be useful when somebody want to checkout something partially from
repository with some non-standart layout or exclude some files from it.
Example: repository has structure /module-{a,b,c}/{trunk,branches,tags}/...
Modules are interdependent, and you want it to be single repostory (to commit
to all modules simultaneously and view complete history), but do not want
branches and tags be checked out into working copy.
Other use case is excluding some large blobs.

The quirk for now is that user must specify this option every fetch/rebase;
in other case he may get extra files or "file not found" errors. It may be
will be resolved by adding regular expression to .git/config into
[svn-remote ...] to make it persistent.

Signed-off-by: Vitaly "_Vi" Shukela <public_vi@tut.by>
---
 git-svn.perl |   43 +++++++++++++++++++++++++++----------------
 1 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index d4cb538..40b0e9e 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -70,7 +70,8 @@ my ($_stdin, $_help, $_edit,
 $Git::SVN::_follow_parent = 1;
 my %remote_opts = ( 'username=s' => \$Git::SVN::Prompt::_username,
                     'config-dir=s' => \$Git::SVN::Ra::config_dir,
-                    'no-auth-cache' => \$Git::SVN::Prompt::_no_auth_cache );
+                    'no-auth-cache' => \$Git::SVN::Prompt::_no_auth_cache,
+                    'ignore-paths=s' => \$SVN::Git::Fetcher::ignore_regex );
 my %fc_opts = ( 'follow-parent|follow!' => \$Git::SVN::_follow_parent,
 		'authors-file|A=s' => \$_authors,
 		'repack:i' => \$Git::SVN::_repack,
@@ -3245,6 +3246,21 @@ use warnings;
 use Carp qw/croak/;
 use File::Temp qw/tempfile/;
 use IO::File qw//;
+use vars qw/ $ignore_regex/;
+
+# returns true if a given path is inside a ".git" directory
+sub in_dot_git($) {
+	$_[0] =~ m{(?:^|/)\.git(?:/|$)};
+}
+
+# 0 -- don't ignore, 1 -- ignore
+sub is_path_ignored($) {
+    my ($path) = @_;
+    return 1 if in_dot_git($path);
+    return 0 unless defined($ignore_regex);
+    return 1 if $path =~ m!$ignore_regex!o;
+    return 0;
+}
 
 # file baton members: path, mode_a, mode_b, pool, fh, blob, base
 sub new {
@@ -3292,11 +3308,6 @@ sub _mark_empty_symlinks {
 	\%ret;
 }
 
-# returns true if a given path is inside a ".git" directory
-sub in_dot_git {
-	$_[0] =~ m{(?:^|/)\.git(?:/|$)};
-}
-
 sub set_path_strip {
 	my ($self, $path) = @_;
 	$self->{path_strip} = qr/^\Q$path\E(\/|$)/ if length $path;
@@ -3322,7 +3333,7 @@ sub git_path {
 
 sub delete_entry {
 	my ($self, $path, $rev, $pb) = @_;
-	return undef if in_dot_git($path);
+	return undef if is_path_ignored($path);
 
 	my $gpath = $self->git_path($path);
 	return undef if ($gpath eq '');
@@ -3352,7 +3363,7 @@ sub open_file {
 	my ($self, $path, $pb, $rev) = @_;
 	my ($mode, $blob);
 
-	goto out if in_dot_git($path);
+	goto out if is_path_ignored($path);
 
 	my $gpath = $self->git_path($path);
 	($mode, $blob) = (command('ls-tree', $self->{c}, '--', $gpath)
@@ -3372,7 +3383,7 @@ sub add_file {
 	my ($self, $path, $pb, $cp_path, $cp_rev) = @_;
 	my $mode;
 
-	if (!in_dot_git($path)) {
+	if (!is_path_ignored($path)) {
 		my ($dir, $file) = ($path =~ m#^(.*?)/?([^/]+)$#);
 		delete $self->{empty}->{$dir};
 		$mode = '100644';
@@ -3383,7 +3394,7 @@ sub add_file {
 
 sub add_directory {
 	my ($self, $path, $cp_path, $cp_rev) = @_;
-	goto out if in_dot_git($path);
+	goto out if is_path_ignored($path);
 	my $gpath = $self->git_path($path);
 	if ($gpath eq '') {
 		my ($ls, $ctx) = command_output_pipe(qw/ls-tree
@@ -3407,7 +3418,7 @@ out:
 
 sub change_dir_prop {
 	my ($self, $db, $prop, $value) = @_;
-	return undef if in_dot_git($db->{path});
+	return undef if is_path_ignored($db->{path});
 	$self->{dir_prop}->{$db->{path}} ||= {};
 	$self->{dir_prop}->{$db->{path}}->{$prop} = $value;
 	undef;
@@ -3415,7 +3426,7 @@ sub change_dir_prop {
 
 sub absent_directory {
 	my ($self, $path, $pb) = @_;
-	return undef if in_dot_git($pb->{path});
+	return undef if is_path_ignored($path);
 	$self->{absent_dir}->{$pb->{path}} ||= [];
 	push @{$self->{absent_dir}->{$pb->{path}}}, $path;
 	undef;
@@ -3423,7 +3434,7 @@ sub absent_directory {
 
 sub absent_file {
 	my ($self, $path, $pb) = @_;
-	return undef if in_dot_git($pb->{path});
+	return undef if is_path_ignored($path);
 	$self->{absent_file}->{$pb->{path}} ||= [];
 	push @{$self->{absent_file}->{$pb->{path}}}, $path;
 	undef;
@@ -3431,7 +3442,7 @@ sub absent_file {
 
 sub change_file_prop {
 	my ($self, $fb, $prop, $value) = @_;
-	return undef if in_dot_git($fb->{path});
+	return undef if is_path_ignored($fb->{path});
 	if ($prop eq 'svn:executable') {
 		if ($fb->{mode_b} != 120000) {
 			$fb->{mode_b} = defined $value ? 100755 : 100644;
@@ -3447,7 +3458,7 @@ sub change_file_prop {
 
 sub apply_textdelta {
 	my ($self, $fb, $exp) = @_;
-	return undef if (in_dot_git($fb->{path}));
+	return undef if is_path_ignored($fb->{path});
 	my $fh = $::_repository->temp_acquire('svn_delta');
 	# $fh gets auto-closed() by SVN::TxDelta::apply(),
 	# (but $base does not,) so dup() it for reading in close_file
@@ -3494,7 +3505,7 @@ sub apply_textdelta {
 
 sub close_file {
 	my ($self, $fb, $exp) = @_;
-	return undef if (in_dot_git($fb->{path}));
+	return undef if is_path_ignored($fb->{path});
 
 	my $hash;
 	my $path = $self->git_path($fb->{path});
-- 
1.5.6.5

^ permalink raw reply related

* [PATCH] git-svn: Add test for --ignore-paths parameter
From: Vitaly "_Vi" Shukela @ 2009-01-25 22:21 UTC (permalink / raw)
  To: git; +Cc: Vitaly "_Vi" Shukela
In-Reply-To: <1232922102-6144-2-git-send-email-public_vi@tut.by>

Added a test for this option, similar to (and based on) t9133 about ignorance of .git directories

Signed-off-by: Vitaly "_Vi" Shukela <public_vi@tut.by>
---
 t/t9134-git-svn-ignore-paths.sh |   97 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 97 insertions(+), 0 deletions(-)
 create mode 100755 t/t9134-git-svn-ignore-paths.sh

diff --git a/t/t9134-git-svn-ignore-paths.sh b/t/t9134-git-svn-ignore-paths.sh
new file mode 100755
index 0000000..094cb6a
--- /dev/null
+++ b/t/t9134-git-svn-ignore-paths.sh
@@ -0,0 +1,97 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 Eric Wong
+#
+
+test_description='git svn property tests'
+. ./lib-git-svn.sh
+
+test_expect_success 'setup test repository' '
+	svn co "$svnrepo" s &&
+	(
+		cd s &&
+		mkdir qqq www &&
+		echo test_qqq > qqq/test_qqq.txt &&
+		echo test_www > www/test_www.txt &&
+		svn add qqq &&
+		svn add www &&
+		svn commit -m "create some files" &&
+		svn up &&
+		echo hi >> www/test_www.txt &&
+		svn commit -m "modify www/test_www.txt" &&
+		svn up
+	)
+'
+
+test_expect_success 'clone an SVN repository with ignored www directory' '
+	git svn clone --ignore-paths="^www" "$svnrepo" g &&
+	echo test_qqq > expect &&
+	for i in g/*/*.txt; do cat $i >> expect2; done &&
+	test_cmp expect expect2
+'
+
+test_expect_success 'SVN-side change outside of www' '
+	(
+		cd s &&
+		echo b >> qqq/test_qqq.txt &&
+		svn commit -m "SVN-side change outside of www" &&
+		svn up &&
+		svn log -v | fgrep "SVN-side change outside of www"
+	)
+'
+
+test_expect_success 'update git svn-cloned repo' '
+	(
+		cd g &&
+		git svn rebase --ignore-paths="^www" &&
+		echo -e "test_qqq\nb" > expect &&
+		for i in */*.txt; do cat $i >> expect2; done &&
+		test_cmp expect2 expect &&
+		rm expect expect2
+	)
+'
+
+test_expect_success 'SVN-side change inside of ignored www' '
+	(
+		cd s &&
+		echo zaq >> www/test_www.txt
+		svn commit -m "SVN-side change inside of www/test_www.txt" &&
+		svn up &&
+		svn log -v | fgrep "SVN-side change inside of www/test_www.txt"
+	)
+'
+
+test_expect_success 'update git svn-cloned repo' '
+	(
+		cd g &&
+		git svn rebase --ignore-paths="^www" &&
+		echo -e "test_qqq\nb" > expect &&
+		for i in */*.txt; do cat $i >> expect2; done &&
+		test_cmp expect2 expect &&
+		rm expect expect2
+	)
+'
+
+test_expect_success 'SVN-side change in and out of ignored www' '
+	(
+		cd s &&
+		echo cvf >> www/test_www.txt
+		echo ygg >> qqq/test_qqq.txt
+		svn commit -m "SVN-side change in and out of ignored www" &&
+		svn up &&
+		svn log -v | fgrep "SVN-side change in and out of ignored www"
+	)
+'
+
+test_expect_success 'update git svn-cloned repo again' '
+	(
+		cd g &&
+		git svn rebase --ignore-paths="^www" &&
+		echo -e "test_qqq\nb\nygg" > expect &&
+		for i in */*.txt; do cat $i >> expect2; done &&
+		test_cmp expect2 expect &&
+		rm expect expect2
+	)
+'
+
+test_done
-- 
1.5.6.5

^ permalink raw reply related

* [PATCH] git-svn: documented --ignore-paths
From: Vitaly "_Vi" Shukela @ 2009-01-25 22:21 UTC (permalink / raw)
  To: git; +Cc: Vitaly "_Vi" Shukela
In-Reply-To: <1232922102-6144-1-git-send-email-public_vi@tut.by>

Documented --ignore-paths option of git-svn to inform users about
the feature and provide some examples.

Signed-off-by: Vitaly "_Vi" Shukela <public_vi@tut.by>
---
 Documentation/git-svn.txt |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 63d2f5e..2215fcb 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -103,6 +103,19 @@ repository to be able to interoperate with someone else's local Git
 repository, either don't use this option or you should both use it in
 the same local timezone.
 
+--ignore-paths=<regex>;;
+	This allows one to specify Perl regular expression that will
+	cause skipping of all matching paths from checkout from SVN.
+	Examples: 
+
+	--ignore-paths="^doc" - skip "doc*" directory for every fetch.
+
+	--ignore-paths="^[^/]+/(?:branches|tags)" - skip "branches"
+	    and "tags" of first level directories.
+
+	Regular expression is not persistent, you should specify
+	it every time when fetching.
+
 'clone'::
 	Runs 'init' and 'fetch'.  It will automatically create a
 	directory based on the basename of the URL passed to it;
-- 
1.5.6.5

^ permalink raw reply related

* [PATCH] gitweb: last-modified time should be commiter, not author
From: Giuseppe Bilotta @ 2009-01-25 22:42 UTC (permalink / raw)
  To: git; +Cc: Jakub Narebski, Giuseppe Bilotta
In-Reply-To: <1232686121-1800-5-git-send-email-giuseppe.bilotta@gmail.com>

The last-modified time header added by RSS to increase cache hits from
readers should be set to the date the repository was last modified. The
author time in this respect is not a good guess because the last commit
might come from a oldish patch.

Use the committer time for the last-modified header to ensure a more
correct guess of the last time the repository was modified.
---
 gitweb/gitweb.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 756868a..8c49c75 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -6015,7 +6015,7 @@ sub git_feed {
 	}
 	if (defined($commitlist[0])) {
 		%latest_commit = %{$commitlist[0]};
-		%latest_date   = parse_date($latest_commit{'author_epoch'});
+		%latest_date   = parse_date($latest_commit{'committer_epoch'});
 		print $cgi->header(
 			-type => $content_type,
 			-charset => 'utf-8',
-- 
1.5.6.5

^ permalink raw reply related

* [PATCH] gitweb: check if-modified-since for feeds
From: Giuseppe Bilotta @ 2009-01-25 22:42 UTC (permalink / raw)
  To: git; +Cc: Jakub Narebski, Giuseppe Bilotta
In-Reply-To: <1232923370-4427-1-git-send-email-giuseppe.bilotta@gmail.com>

Offering Last-modified header for feeds is only half the work: we should
also check that same date against If-modified-since, and bail out early
with 304 Not Modified.
---
 gitweb/gitweb.perl |   20 +++++++++++++++++++-
 1 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 8c49c75..0a5d229 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -6015,7 +6015,25 @@ sub git_feed {
 	}
 	if (defined($commitlist[0])) {
 		%latest_commit = %{$commitlist[0]};
-		%latest_date   = parse_date($latest_commit{'committer_epoch'});
+		my $latest_epoch = $latest_commit{'committer_epoch'};
+		%latest_date   = parse_date($latest_epoch);
+		my $if_modified = $cgi->http('IF_MODIFIED_SINCE');
+		if (defined $if_modified) {
+			my $since;
+			if (eval { require HTTP::Date; 1; }) {
+				$since = HTTP::Date::str2time($if_modified);
+			} elsif (eval { require Time::ParseDate; 1; }) {
+				$since = Time::ParseDate::parsedate($if_modified, GMT => 1);
+			}
+			if (defined $since && $latest_epoch <= $since) {
+				print $cgi->header(
+					-type => $content_type,
+					-charset => 'utf-8',
+					-last_modified => $latest_date{'rfc2822'},
+					-status => 304);
+				return;
+			}
+		}
 		print $cgi->header(
 			-type => $content_type,
 			-charset => 'utf-8',
-- 
1.5.6.5

^ permalink raw reply related

* Re: [PATCH] git-svn: add --ignore-paths option for fetching
From: Eric Wong @ 2009-01-25 22:42 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Vitaly _Vi Shukela, git
In-Reply-To: <200901251521.15591.trast@student.ethz.ch>

Thomas Rast <trast@student.ethz.ch> wrote:
> Vitaly "_Vi" Shukela wrote:
> > 
> > Signed-off-by: Vitaly "_Vi" Shukela <public_vi@tut.by>
> 
> This would be a good place to explain why this is useful, and (if
> applicable) why you chose to implement it the way you did.
> 
> > --- a/Documentation/git-svn.txt
> > +++ b/Documentation/git-svn.txt
> > @@ -96,6 +96,10 @@ COMMANDS
> >  	Store Git commit times in the local timezone instead of UTC.  This
> >  	makes 'git-log' (even without --date=local) show the same times
> >  	that `svn log` would in the local timezone.
> > +--ignore-paths=<regex>;;
> > +	This allows one to specify regular expression that will
> > +	cause skipping of all matching paths from checkout from SVN.
> > +	Example: --ignore-paths='^doc'
> >  
> >  This doesn't interfere with interoperating with the Subversion
> >  repository you cloned from, but if you wish for your local Git
> 
> You put the --ignore-paths explanation in the middle of the
> --localtime documentation (the last paragraph quoted still talks about
> --localtime).
> 
> > @@ -3245,6 +3246,15 @@ use warnings;
> >  use Carp qw/croak/;
> >  use File::Temp qw/tempfile/;
> >  use IO::File qw//;
> > +use vars qw/ $ignoreRegex/;
> > +
> > +# 0 -- don't ignore, 1 -- ignore
> > +sub isPathIgnored($) {
> > +    return 0 unless defined($ignoreRegex);
> > +    my $path = shift;
> > +    return 1 if $path =~ m!^$ignoreRegex!o;
> > +    return 0;
> > +}
> 
> This is the first function in git-svn.perl using camelCase.  Consider
> sticking to the current style and spelling it is_path_ignored().

Also, indentation is always done with tabs in git-svn (and the vast
majority of git as well).

> > @@ -3372,11 +3384,14 @@ sub add_file {
> >  	my ($self, $path, $pb, $cp_path, $cp_rev) = @_;
> >  	my $mode;
> >  
> > +	goto out if isPathIgnored($path);
> > +
> >  	if (!in_dot_git($path)) {
> >  		my ($dir, $file) = ($path =~ m#^(.*?)/?([^/]+)$#);
> >  		delete $self->{empty}->{$dir};
> >  		$mode = '100644';
> >  	}
> > +out:
> >  	{ path => $path, mode_a => $mode, mode_b => $mode,
> >  	  pool => SVN::Pool->new, action => 'A' };
> >  }
> 
> You broke the symmetry here, while all other hunks just add an
> equivalent check to the existing in_dot_git().
> 
> However, the latter makes me wonder if it would be cleaner to move the
> in_dot_git() test to isPathIgnored (er, is_path_ignored) too?

Thanks for the review, Thomas.  I agree with all your suggestions.

Vitaly: thank you for the patch.  Can you also provide a testcase to
ensure this functionality doesn't break during refactorings?  Thanks.

-- 
Eric Wong

^ permalink raw reply

* Re: [PATCH] git-svn: add --ignore-paths option for fetching
From: public_vi @ 2009-01-25 22:48 UTC (permalink / raw)
  To: Eric Wong; +Cc: Thomas Rast, git
In-Reply-To: <20090125224238.GA31581@untitled>

Eric Wong wrote:
> Thomas Rast <trast@student.ethz.ch> wrote:
>   
>
> Also, indentation is always done with tabs in git-svn (and the vast
> majority of git as well).
>
>   
Whitespace is invisible for me now, I only checked according to 
Documentation/SubmittingPatches about extra lines adds or removes caused 
just by whitespace difference.
>
>
> Vitaly: thank you for the patch.  Can you also provide a testcase to
> ensure this functionality doesn't break during refactorings?  Thanks.
>
>   
Already done it with testcase. There are two new packs of patches sent 
to git@vger.kernel.org, the first is outdated too, the second is current.
(I don't yet completely know how things should be done).

^ permalink raw reply

* Re: [PATCH 1/2] user-manual: Simplify the user configuration.
From: Wincent Colaiuta @ 2009-01-25 22:55 UTC (permalink / raw)
  To: Jeff King
  Cc: Felipe Contreras, Junio C Hamano, Johannes Schindelin,
	Hannu Koivisto, git
In-Reply-To: <20090125214435.GA20173@coredump.intra.peff.net>

El 25/1/2009, a las 22:44, Jeff King escribió:

> On Sun, Jan 25, 2009 at 11:12:57PM +0200, Felipe Contreras wrote:
>
>> However, my last proposal was to have both the git config --global
>> *and* the $HOME/.gitconfig description. Is there any argument against
>> that?
>
> This is like the fifth time you have asked, and for some reason,  
> nobody
> seems to have said yes or no. So I will go ahead and say: yes, I think
> that is a fine idea.
>
> I think there should also be some explanatory text that indicates they
> are totally interchangeable for the rest of the document. Something
> like: "When we show configuration in the rest of this document, we  
> will
> use format X [I think probably "git config $VAR $VALUE"]. But you can
> use whichever method you are most comfortable with."

I already suggested something similar about 4 days ago:

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

Cheers,
Wincent

^ permalink raw reply

* Re: [PATCH v1 1/3] Introduce config variable "diff.primer"
From: Keith Cascio @ 2009-01-25 22:58 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20090125221141.GA17490@coredump.intra.peff.net>

On Sun, 25 Jan 2009, Jeff King wrote:

> FWIW, I found it very confusing. I would have expected "diff.options" or 
> "diff.defaults". There is also some precedent in the form of GIT_DIFF_OPTS, 
> but I believe it _only_ handles --unified and -u, so it is not necessarily a 
> useful model.

OK, point taken.  I wasn't trying to be idiosyncratic at all.  Just trying to be 
explicit and avoid all confusion.  Since all diff options already have default 
values, primer looks to me like the layer one step above defaults, hence the 
painting analogy.  Mercurial calls it "defaults", but that doesn't mean we 
should necessarily follow in their footsteps (see 
http://article.gmane.org/gmane.comp.version-control.git/107103).

I think being as clear as possible about what primer is, that is it NOT 
defaults, helps to feel more comfortable with its consequences, i.e. in my 
opinion, that it will not break things.

                                   -- Keith

^ permalink raw reply

* Re: [PATCH 0/2] Add submodule-support to git archive
From: Lars Hjemli @ 2009-01-25 23:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nanako Shiraishi, Johannes Schindelin, git
In-Reply-To: <7veiyrdszf.fsf@gitster.siamese.dyndns.org>

On Sun, Jan 25, 2009 at 21:35, Junio C Hamano <gitster@pobox.com> wrote:
> Lars Hjemli <hjemli@gmail.com> writes:
>
>> On Sun, Jan 25, 2009 at 05:53, Nanako Shiraishi <nanako3@lavabit.com> wrote:
>>> What would I do to try this new series? Fork a branch from Junio's master branch,
>>> apply your new patches, and merge the result to Junio's next?
>>
>> Yes, that sounds right (btw: the series is buildt on top of 5dc1308562
>> (Merge branch 'js/patience-diff') and can be pulled from
>> git://hjemli.net/pub/git/git lh/traverse-gitlinks).
>>
>> But before merging with 'next', you'll need to `git revert -m 1 bdf31cbc00`.
>
> Yuck, that is too much to ask for regular testers and users.

Sorry about that.


> Could we switch to incremental refinements once a series hits next, pretty
> please?

The problem in this particular case is that the design has changed so
much since the first iteration that we're not really talking about
incremental refinements but rather a different approach to the same
problem.

If you want me to build on top of the series in next anyways, would it
be acceptable if the first patch on top of ee306d2d59 reverts the
previous attempt? I think the rest of the series will be easier to
review that way.

--
larsh

^ permalink raw reply

* [PATCH 0/3] Valgrind support
From: Johannes Schindelin @ 2009-01-25 23:18 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0901212332030.3586@pacific.mpi-cbg.de>


I finally decided to give in on both the lock (let's see how many races
we encounter in reality...) and the searching the PATH and handling .sh
and .perl scripts, too.  The latter issue is handled by 3/3, which is up
for discussion.

Oh, and BTW, this is vs 'next', and according to my tests, valgrind finds
at least one issue.

Jeff King (1):
  valgrind: ignore ldso and more libz errors

Johannes Schindelin (2):
  Add valgrind support in test scripts
  Valgrind support: check for more than just programming errors

 t/README                |    8 +++++-
 t/test-lib.sh           |   66 +++++++++++++++++++++++++++++++++++++++++++++-
 t/valgrind/.gitignore   |    1 +
 t/valgrind/default.supp |   45 ++++++++++++++++++++++++++++++++
 t/valgrind/valgrind.sh  |   12 ++++++++
 5 files changed, 129 insertions(+), 3 deletions(-)
 create mode 100644 t/valgrind/.gitignore
 create mode 100644 t/valgrind/default.supp
 create mode 100755 t/valgrind/valgrind.sh

^ permalink raw reply

* [PATCH v3 1/3] Add valgrind support in test scripts
From: Johannes Schindelin @ 2009-01-25 23:18 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0901260014470.14855@racer>


This patch adds the ability to use valgrind's memcheck tool to
diagnose memory problems in Git while running the test scripts.

It works by creating symlinks to a valgrind script, which have the same
name as our Git binaries, and then putting that directory in front of
the test script's PATH as well as set GIT_EXEC_PATH to that directory.

Git scripts are symlinked from that directory directly.  That way, Git
binaries called by Git scripts are valgrinded, too.

Valgrind can be used by specifying "GIT_TEST_OPTS=--valgrind" in the
make invocation. Any invocation of git that finds any errors under
valgrind will exit with failure code 126. Any valgrind output will go
to the usual stderr channel for tests (i.e., /dev/null, unless -v has
been specified).

If you need to pass options to valgrind -- you might want to run
another tool than memcheck, for example -- you can set the environment
variable GIT_VALGRIND_OPTIONS.

A few default suppressions are included, since libz seems to trigger
quite a few false positives. We'll assume that libz works and that we
can ignore any errors which are reported there.

Note: it is safe to run the valgrind tests in parallel, as the links in
t/valgrind/bin/ are created using proper locking.

Initial patch and all the hard work by Jeff King.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/README                |    8 ++++++-
 t/test-lib.sh           |   54 +++++++++++++++++++++++++++++++++++++++++++++-
 t/valgrind/.gitignore   |    1 +
 t/valgrind/default.supp |   21 ++++++++++++++++++
 t/valgrind/valgrind.sh  |   12 ++++++++++
 5 files changed, 93 insertions(+), 3 deletions(-)
 create mode 100644 t/valgrind/.gitignore
 create mode 100644 t/valgrind/default.supp
 create mode 100755 t/valgrind/valgrind.sh

diff --git a/t/README b/t/README
index 8f12d48..811bc0d 100644
--- a/t/README
+++ b/t/README
@@ -39,7 +39,8 @@ this:
     * passed all 3 test(s)
 
 You can pass --verbose (or -v), --debug (or -d), and --immediate
-(or -i) command line argument to the test.
+(or -i) command line argument to the test, or by setting GIT_TEST_OPTS
+appropriately before running "make".
 
 --verbose::
 	This makes the test more verbose.  Specifically, the
@@ -58,6 +59,11 @@ You can pass --verbose (or -v), --debug (or -d), and --immediate
 	This causes additional long-running tests to be run (where
 	available), for more exhaustive testing.
 
+--valgrind::
+	Execute all Git binaries with valgrind and exit with status
+	126 on errors (just like regular tests, this will only stop
+	the test script when running under -i).  Valgrind errors
+	go to stderr, so you might want to pass the -v option, too.
 
 Skipping Tests
 --------------
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 41d5a59..67d7883 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -94,6 +94,8 @@ do
 	--no-python)
 		# noop now...
 		shift ;;
+	--va|--val|--valg|--valgr|--valgri|--valgrin|--valgrind)
+		valgrind=t; shift ;;
 	*)
 		break ;;
 	esac
@@ -467,8 +469,56 @@ test_done () {
 # Test the binaries we have just built.  The tests are kept in
 # t/ subdirectory and are run in 'trash directory' subdirectory.
 TEST_DIRECTORY=$(pwd)
-PATH=$TEST_DIRECTORY/..:$PATH
-GIT_EXEC_PATH=$(pwd)/..
+if test -z "$valgrind"
+then
+	PATH=$TEST_DIRECTORY/..:$PATH
+	GIT_EXEC_PATH=$TEST_DIRECTORY/..
+else
+	make_symlink () {
+		test -h "$2" &&
+		test "$1" = "$(readlink "$2")" || {
+			# be super paranoid
+			if mkdir "$2".lock
+			then
+				rm -f "$2" &&
+				ln -s "$1" "$2" &&
+				rm -r "$2".lock
+			else
+				while test -d "$2".lock
+				do
+					say "Waiting for lock on $2."
+					sleep 1
+				done
+			fi
+		}
+	}
+
+	# override all git executables in TEST_DIRECTORY/..
+	GIT_VALGRIND=$TEST_DIRECTORY/valgrind
+	mkdir -p "$GIT_VALGRIND"/bin
+	ls $TEST_DIRECTORY/../git* 2> /dev/null |
+	while read symlink_target
+	do
+		# handle only executables
+		test -x "$symlink_target" || continue
+
+		base=$(basename "$symlink_target")
+		# do not override scripts
+		if test ! -d "$symlink_target" &&
+		    test "#!" != "$(head -c 2 < "$symlink_target")"
+		then
+			symlink_target=../valgrind.sh
+		fi
+		# create the link, or replace it if it is out of date
+		make_symlink "$symlink_target" \
+			"$GIT_VALGRIND/bin/$base" || exit
+	done
+	PATH=$GIT_VALGRIND/bin:$PATH
+	GIT_EXEC_PATH=$GIT_VALGRIND/bin
+	export GIT_VALGRIND
+
+	make_symlink ../../templates "$GIT_VALGRIND"/templates || exit
+fi
 GIT_TEMPLATE_DIR=$(pwd)/../templates/blt
 unset GIT_CONFIG
 GIT_CONFIG_NOSYSTEM=1
diff --git a/t/valgrind/.gitignore b/t/valgrind/.gitignore
new file mode 100644
index 0000000..ae3c172
--- /dev/null
+++ b/t/valgrind/.gitignore
@@ -0,0 +1 @@
+/bin/
diff --git a/t/valgrind/default.supp b/t/valgrind/default.supp
new file mode 100644
index 0000000..2482b3b
--- /dev/null
+++ b/t/valgrind/default.supp
@@ -0,0 +1,21 @@
+{
+	ignore-zlib-errors-cond
+	Memcheck:Cond
+	obj:*libz.so*
+}
+
+{
+	ignore-zlib-errors-value4
+	Memcheck:Value4
+	obj:*libz.so*
+}
+
+{
+	writing-data-from-zlib-triggers-errors
+	Memcheck:Param
+	write(buf)
+	obj:/lib/ld-*.so
+	fun:write_in_full
+	fun:write_buffer
+	fun:write_loose_object
+}
diff --git a/t/valgrind/valgrind.sh b/t/valgrind/valgrind.sh
new file mode 100755
index 0000000..24f3a4e
--- /dev/null
+++ b/t/valgrind/valgrind.sh
@@ -0,0 +1,12 @@
+#!/bin/sh
+
+base=$(basename "$0")
+
+exec valgrind -q --error-exitcode=126 \
+	--leak-check=no \
+	--suppressions="$GIT_VALGRIND/default.supp" \
+	--gen-suppressions=all \
+	--log-fd=4 \
+	--input-fd=4 \
+	$GIT_VALGRIND_OPTIONS \
+	"$GIT_VALGRIND"/../../"$base" "$@"
-- 
1.6.1.482.g7d54be

^ permalink raw reply related

* [PATCH v3 2/3] valgrind: ignore ldso and more libz errors
From: Johannes Schindelin @ 2009-01-25 23:19 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0901260014470.14855@racer>


On some Linux systems, we get a host of Cond and Addr errors
from calls to dlopen that are caused by nss modules. We
should be able to safely ignore anything happening in
ld-*.so as "not our problem."

[Johannes: I added some more...]

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/valgrind/default.supp |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/t/valgrind/default.supp b/t/valgrind/default.supp
index 2482b3b..b2da4fd 100644
--- a/t/valgrind/default.supp
+++ b/t/valgrind/default.supp
@@ -5,12 +5,36 @@
 }
 
 {
+	ignore-zlib-errors-value8
+	Memcheck:Value8
+	obj:*libz.so*
+}
+
+{
 	ignore-zlib-errors-value4
 	Memcheck:Value4
 	obj:*libz.so*
 }
 
 {
+	ignore-ldso-cond
+	Memcheck:Cond
+	obj:*ld-*.so
+}
+
+{
+	ignore-ldso-addr8
+	Memcheck:Addr8
+	obj:*ld-*.so
+}
+
+{
+	ignore-ldso-addr4
+	Memcheck:Addr4
+	obj:*ld-*.so
+}
+
+{
 	writing-data-from-zlib-triggers-errors
 	Memcheck:Param
 	write(buf)
-- 
1.6.1.482.g7d54be

^ permalink raw reply related

* [PATCH 3/3] Valgrind support: check for more than just programming errors
From: Johannes Schindelin @ 2009-01-25 23:20 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0901260014470.14855@racer>


This patch makes --valgrind try to override _all_ Git binaries in the
PATH, and it will make calling *.sh and *.perl scripts directly an
error.

While it is not strictly necessary to look through the whole PATH to
find git binaries to override, it is in line with running an expensive
test (which valgrind is) to make extra sure that no binary is tested
that actually comes from the git.git checkout.

In the same spirit, we can test that neither our test suite nor our
scripts try to run the *.sh or *.perl scripts directly.

It's more like a "because we can" than a "this is tightly connected
to valgrind", but in the author's opinion "because we can" is "so we
should" in this case.

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

	As I said, I vividly remember chasing a bug which turned out to be 
	a Git program that was installed, but no longer in git.git, yet 
	the test suite used it.

	This would catch it.

 t/test-lib.sh |   42 +++++++++++++++++++++++++++---------------
 1 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 67d7883..bdfb30f 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -496,23 +496,35 @@ else
 	# override all git executables in TEST_DIRECTORY/..
 	GIT_VALGRIND=$TEST_DIRECTORY/valgrind
 	mkdir -p "$GIT_VALGRIND"/bin
-	ls $TEST_DIRECTORY/../git* 2> /dev/null |
-	while read symlink_target
+	OLDIFS=$IFS
+	IFS=:
+	for path in $PATH $TEST_DIRECTORY/..
 	do
-		# handle only executables
-		test -x "$symlink_target" || continue
-
-		base=$(basename "$symlink_target")
-		# do not override scripts
-		if test ! -d "$symlink_target" &&
-		    test "#!" != "$(head -c 2 < "$symlink_target")"
-		then
-			symlink_target=../valgrind.sh
-		fi
-		# create the link, or replace it if it is out of date
-		make_symlink "$symlink_target" \
-			"$GIT_VALGRIND/bin/$base" || exit
+		ls "$path"/git "$path"/git-* 2> /dev/null |
+		while read file
+		do
+			# handle only executables
+			test -x "$file" || continue
+
+			base=$(basename "$file")
+			symlink_target=$TEST_DIRECTORY/../$base
+			# do not override scripts
+			if test -x "$symlink_target" &&
+			    test ! -d "$symlink_target" &&
+			    test "#!" != "$(head -c 2 < "$symlink_target")"
+			then
+				symlink_target=../valgrind.sh
+			fi
+			case "$base" in
+			*.sh|*.perl)
+				symlink_target=../unprocessed-script
+			esac
+			# create the link, or replace it if it is out of date
+			make_symlink "$symlink_target" \
+				"$GIT_VALGRIND/bin/$base" || exit
+		done
 	done
+	IFS=$OLDIFS
 	PATH=$GIT_VALGRIND/bin:$PATH
 	GIT_EXEC_PATH=$GIT_VALGRIND/bin
 	export GIT_VALGRIND
-- 
1.6.1.482.g7d54be

^ permalink raw reply related

* Re: [PATCH 1/2] user-manual: Simplify the user configuration.
From: Jeff King @ 2009-01-25 23:20 UTC (permalink / raw)
  To: Wincent Colaiuta
  Cc: Felipe Contreras, Junio C Hamano, Johannes Schindelin,
	Hannu Koivisto, git
In-Reply-To: <190FD80D-0F10-4C76-9029-1434F362EFDB@wincent.com>

On Sun, Jan 25, 2009 at 11:55:24PM +0100, Wincent Colaiuta wrote:

>> I think there should also be some explanatory text that indicates they
>> are totally interchangeable for the rest of the document. Something
>> like: "When we show configuration in the rest of this document, we will
>> use format X [I think probably "git config $VAR $VALUE"]. But you can
>> use whichever method you are most comfortable with."
>
> I already suggested something similar about 4 days ago:
>
> http://article.gmane.org/gmane.comp.version-control.git/106673/

Oh, I think I must have mixed up reading your post and thinking it was
from Felipe. Sorry.

At any rate, I think what you wrote there is sensible.

-Peff

^ permalink raw reply

* Re: [PATCH 0/2] Add submodule-support to git archive
From: Johannes Schindelin @ 2009-01-25 23:25 UTC (permalink / raw)
  To: Lars Hjemli; +Cc: Junio C Hamano, Nanako Shiraishi, git
In-Reply-To: <8c5c35580901251512q5058dde3rdfae81979c46c36a@mail.gmail.com>

Hi,

On Mon, 26 Jan 2009, Lars Hjemli wrote:

> On Sun, Jan 25, 2009 at 21:35, Junio C Hamano <gitster@pobox.com> wrote:
> 
> > Could we switch to incremental refinements once a series hits next, 
> > pretty please?
> 
> The problem in this particular case is that the design has changed so 
> much since the first iteration that we're not really talking about 
> incremental refinements but rather a different approach to the same 
> problem.
> 
> If you want me to build on top of the series in next anyways, would it
> be acceptable if the first patch on top of ee306d2d59 reverts the
> previous attempt? I think the rest of the series will be easier to
> review that way.

I'd appreciate that.

Since 'next' will be rewound eventually, that first iteration and the 
revert will disappear at some stage.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH v1 1/3] Introduce config variable "diff.primer"
From: Jeff King @ 2009-01-25 23:25 UTC (permalink / raw)
  To: Keith Cascio; +Cc: git
In-Reply-To: <alpine.GSO.2.00.0901251446260.12651@kiwi.cs.ucla.edu>

On Sun, Jan 25, 2009 at 02:58:34PM -0800, Keith Cascio wrote:

> OK, point taken.  I wasn't trying to be idiosyncratic at all.  Just
> trying to be explicit and avoid all confusion.  Since all diff options
> already have default values, primer looks to me like the layer one
> step above defaults, hence the painting analogy.  Mercurial calls it
> "defaults", but that doesn't mean we should necessarily follow in
> their footsteps (see
> http://article.gmane.org/gmane.comp.version-control.git/107103).
> 
> I think being as clear as possible about what primer is, that is it
> NOT defaults, helps to feel more comfortable with its consequences,
> i.e. in my opinion, that it will not break things.

I'm not sure I agree that they are not new defaults, but any such
argument is going to get into the exact definition of "default" which is
not really useful to the task at hand.

I think "options" is a better word (as in, pretend like you already
specified these "options" on the command line), but I am not going to
insist on that. I mainly just wanted to point out that I found "primer"
confusing. Enough so that, even though I knew you were interested in
this topic from your previous mails, I saw the word "primer" and said to
myself: "what in the world is this patch about?"

-Peff

^ permalink raw reply

* Re: Heads up: major rebase -i -p rework coming up
From: Johannes Schindelin @ 2009-01-25 23:29 UTC (permalink / raw)
  To: Jakub Narebski, らいしななこ
  Cc: Junio C Hamano, Sverre Rabbelier, git
In-Reply-To: <200901252303.29204.jnareb@gmail.com>

Hi,

On Sun, 25 Jan 2009, Jakub Narebski wrote:

> On Sun, 25 Jan 2009, Junio C Hamano wrote:
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> > 
> > > So maybe I answered my question myself:
> > >
> > > 	merge parents $sha1 [$sha1...] original $sha1 $msg
> > 
> > When you are reparenting, how would original commit get in the 
> > picture? You wouldn't want the resulting merge to claim it merged X 
> > (which would be what's in original's commit log) when in fact it now 
> > merged Y because the user reparented it, would you?
> 
> Well, the subject part of merge (with merged branches names) shouldn't, 
> I guess, change. The summary (shortlog) part might, or perhaps even 
> should following rewrite (if it was present here).
> 
> But there is one issue I am wondering about: could we pick up _merge_ 
> _resolution_? So if you have evil merge, and the change is for example 
> splitting commits without visible final changes, or just changing some 
> commit message before merge, it would get recreated without problems?

Nanako had a script at some stage; I would prefer an subcommand to "git 
rerere" which reconstructs the whole merge in-memory, and then records the 
conflict's resolution.

However, I really think you are getting ahead of yourself.  That is by no 
means something we want to have in rebase -p.  And even then, it would 
have to be non-automatic, i.e the user has to check the resolution.

We _know_ that git rerere does a fine job most of the time, almost all of 
the exceptions to be found when working with rebase -i extensively, as you 
are prone to take different decisions during development.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH v3 1/3] Add valgrind support in test scripts
From: Jeff King @ 2009-01-25 23:29 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0901260018340.14855@racer>

On Mon, Jan 26, 2009 at 12:18:50AM +0100, Johannes Schindelin wrote:

> Note: it is safe to run the valgrind tests in parallel, as the links in
> t/valgrind/bin/ are created using proper locking.

I actually kind of liked the original atomic version over the one with
locking. But I find this one acceptable.

> Initial patch and all the hard work by Jeff King.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

I don't know that there is much of my work left in here, but feel free
to add:

  Signed-off-by: Jeff King <peff@peff.net>

-Peff

^ permalink raw reply

* [PATCH 0/2] rebase -i --root cleanups
From: Johannes Schindelin @ 2009-01-25 23:31 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, gitster


This is just the first part of my rebase revamp; it would be good to
get the trivial stuff out of the way early, so that I do not have to
send a large patch series nobody wants to read because of its size.

Thomas, these two patches happen to touch your --root realm, maybe you 
have comments.

Johannes Schindelin (2):
  rebase -i --root: simplify code
  rebase -i --root: fix check for number of arguments

 git-rebase--interactive.sh |   20 ++++++++------------
 1 files changed, 8 insertions(+), 12 deletions(-)

^ permalink raw reply

* [PATCH 1/2] rebase -i --root: simplify code
From: Johannes Schindelin @ 2009-01-25 23:31 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, gitster
In-Reply-To: <alpine.DEB.1.00.0901260029480.14855@racer>


When we rebase with --root, what we are actually picking are the commits
in $ONTO..$HEAD.  So $ONTO is really our $UPSTREAM.  Spell it out.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 git-rebase--interactive.sh |   17 ++++++-----------
 1 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 21ac20c..6e2bf25 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -581,13 +581,15 @@ first and then run 'git rebase --continue' again."
 		if test -z "$REBASE_ROOT"
 		then
 			UPSTREAM_ARG="$1"
-			UPSTREAM=$(git rev-parse --verify "$1") || die "Invalid base"
+			UPSTREAM=$(git rev-parse --verify "$1") ||
+				die "Invalid base"
 			test -z "$ONTO" && ONTO=$UPSTREAM
 			shift
 		else
 			UPSTREAM_ARG=--root
 			test -z "$ONTO" &&
 				die "You must specify --onto when using --root"
+			UPSTREAM=$ONTO
 		fi
 		run_pre_rebase_hook "$UPSTREAM_ARG" "$@"
 
@@ -648,16 +650,9 @@ first and then run 'git rebase --continue' again."
 
 		SHORTHEAD=$(git rev-parse --short $HEAD)
 		SHORTONTO=$(git rev-parse --short $ONTO)
-		if test -z "$REBASE_ROOT"
-			# this is now equivalent to ! -z "$UPSTREAM"
-		then
-			SHORTUPSTREAM=$(git rev-parse --short $UPSTREAM)
-			REVISIONS=$UPSTREAM...$HEAD
-			SHORTREVISIONS=$SHORTUPSTREAM..$SHORTHEAD
-		else
-			REVISIONS=$ONTO...$HEAD
-			SHORTREVISIONS=$SHORTHEAD
-		fi
+		SHORTUPSTREAM=$(git rev-parse --short $UPSTREAM)
+		REVISIONS=$UPSTREAM...$HEAD
+		SHORTREVISIONS=$SHORTUPSTREAM..$SHORTHEAD
 		git rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \
 			--abbrev=7 --reverse --left-right --topo-order \
 			$REVISIONS | \
-- 
1.6.1.482.g7d54be

^ permalink raw reply related

* [PATCH 2/2] rebase -i --root: fix check for number of arguments
From: Johannes Schindelin @ 2009-01-25 23:32 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, gitster
In-Reply-To: <alpine.DEB.1.00.0901260029480.14855@racer>


Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 git-rebase--interactive.sh |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 6e2bf25..5df35b2 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -571,7 +571,8 @@ first and then run 'git rebase --continue' again."
 		;;
 	--)
 		shift
-		test ! -z "$REBASE_ROOT" -o $# -eq 1 -o $# -eq 2 || usage
+		test -z "$REBASE_ROOT" -a $# -ge 1 -a $# -le 2 ||
+		test ! -z "$REBASE_ROOT" -a $# -le 1 || usage
 		test -d "$DOTEST" &&
 			die "Interactive rebase already started"
 
-- 
1.6.1.482.g7d54be

^ permalink raw reply related

* Re: [PATCH v3 2/3] valgrind: ignore ldso and more libz errors
From: Jeff King @ 2009-01-25 23:32 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0901260019000.14855@racer>

On Mon, Jan 26, 2009 at 12:19:12AM +0100, Johannes Schindelin wrote:

> 
> On some Linux systems, we get a host of Cond and Addr errors
> from calls to dlopen that are caused by nss modules. We
> should be able to safely ignore anything happening in
> ld-*.so as "not our problem."
> 
> [Johannes: I added some more...]
> 
> Signed-off-by: Jeff King <peff@peff.net>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Your 0/3 cover letter lists this me as the author of this patch, but
there is no "From:" line at the top of this email. I don't particularly
care one way or the other for this patch, but I wanted to point it out
as a potential issue with your patch-sending workflow.

-Peff

^ 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