Git development
 help / color / mirror / Atom feed
* The lifecycle of a patch and the maintainer involvement
From: Junio C Hamano @ 2009-01-25 20:35 UTC (permalink / raw)
  To: Brent Goodrick; +Cc: Johannes Schindelin, Daniel Barkalow, Mike Ralphson, git
In-Reply-To: <18811.32772.728276.923430@hungover.brentg.com>

Brent Goodrick <bgoodr@gmail.com> writes:

> While I'm at it, what is the standard procedure for submitting git
> patches for review once I've cooked up and validated it on my end? I'm
> guessing posting the patch into this mailing list is part of the
> answer to that question.

Yes, a guideline is in Documentation/SubmittingPatches for the initial
submission.  After that, the lifecycle of a patch submitted on the list
goes like this:

 (1) A patch is shown to the list participants.

 (2) People may like it, or may have issues with it, and responds with
     their comments describing problems, suggestions for improvements,
     etc.  People who are not interested in the topic may stay silent.

 (3) The original author responds with updated patch.  Sometimes people
     who commented on in step 2 may even send "here is how I would do this
     one; don't you think this is better?", and the original author may
     say "Yeah, let's use yours instead".

 (4) After steps 2 and 3 repeats zero or more times, the latest patch may
     become one that everyone likes, or at least nobody has trouble with
     inclusion.  The author sends such a patch saying "this is meant for
     inclusion based on discussion and refinements in these threads...".

 (5) The maintainer picks it up when it looks polished enough.

Your patch may appear in the periodical "What's cooking" or "What's in"
summary with zero iteration of steps 2 and 3 if it is obvious enough.

I act as just one of the list participant during steps 1-3.  I may stay
silent during this period but that only means the topic is not interesting
to me and nothing more.  It does not mean that the topic has no chance of
getting included.

I act as the maintainer for steps 4 and 5.  If you do not hear from me
after step 4, then I am either being lazy, busy, or sick, or the patch got
lost in the noise and I need a reminder.  Note that I may reject or ask
further refinement at step 4 to ensure overall quality throughout the
system even in areas I am not interested in and didn't say anything during
steps 1-3.

^ permalink raw reply

* Re: [PATCH next] t1505: remove debugging cruft
From: Junio C Hamano @ 2009-01-25 20:35 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, Johannes Schindelin
In-Reply-To: <1232835794-22176-1-git-send-email-trast@student.ethz.ch>

Thanks.

^ permalink raw reply

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

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.

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

^ permalink raw reply

* Re: [PATCH] http-push: refactor request url creation
From: Junio C Hamano @ 2009-01-25 20:35 UTC (permalink / raw)
  To: Ray Chuan; +Cc: git, Johannes Schindelin
In-Reply-To: <be6fef0d0901242208p635264e5jc1f95d784cd51450@mail.gmail.com>

Ray Chuan <rctay89@gmail.com> writes:

> Currently, functions that deal with objects on the remote repository
> have to allocate and do strcpys to generate the URL.
>
> This patch saves them this trouble, by providing a function that
> returns a URL: either the object's 2-digit hex directory (eg.
> /objects/a1/) or the complete object location (eg. /objects/a1/b2).
>
> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>

If you wrote it, and then Johannes vetted it, then these two should be in
the opposite order.

> +static char *get_remote_object_url(const char *url, const char *hex,
> int only_two_digit_prefix) {

Linewrapped.

> +	struct strbuf buf = STRBUF_INIT;
> +
> +	strbuf_addf(&buf, "%sobjects/%.*s/", url, 2, hex);
> +	if(!only_two_digit_prefix)

"if (..."

> @@ -304,17 +312,7 @@ static void start_fetch_loose(struct
> transfer_request *request)
>
>  	git_SHA1_Init(&request->c);
>
> -	url = xmalloc(strlen(remote->url) + 50);
> ...
> -	strcpy(request->url, url);
> +	request->url = get_remote_object_url(remote->url, hex, 0);
> ...
> -	curl_easy_setopt(slot->curl, CURLOPT_URL, url);
> +	curl_easy_setopt(slot->curl, CURLOPT_URL, request->url);

The original code gave a separate "url" to setop() but this gives the same
string.  Does curl_easy_setop() copies the given string away?  IOW is this
change safe?

> @@ -550,21 +540,11 @@ static void start_put(struct transfer_request *request)
>
>  	request->buffer.buf.len = stream.total_out;
>
> -	request->url = xmalloc(strlen(remote->url) +
> ...
> -	strcpy(posn, request->lock->token);
> +	strbuf_addf(&url_buf, "Destination: %s",
> get_remote_object_url(remote->url, hex, 0));

Linewrapped.

The return value from get_remote_object_url() leaks here.

> +	request->dest = strbuf_detach(&url_buf, NULL);
> +
> +	strbuf_addf(&url_buf, "%s_%s", get_remote_object_url(remote->url,
> hex, 0), request->lock->token);

The return value from get_remote_object_url() leaks here, too.

^ permalink raw reply

* Re: [PATCH 1/2] user-manual: Simplify the user configuration.
From: Junio C Hamano @ 2009-01-25 20:34 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Johannes Schindelin, Hannu Koivisto, git
In-Reply-To: <94a0d4530901240604o5ae0d321h17dc6aabeefe9d53@mail.gmail.com>

Felipe Contreras <felipe.contreras@gmail.com> writes:

> It's much easier for everyone to just use git config --global than
> explain how create and edit the .gitconfig file. If this is explained
> it shouldn't be in the "Telling git your name" section.

I think it is Ok to have, and it probably is helpful to have, a document
that accompanies the generic manual, "Platform supplement", to explain
things like "On this platform, what is referred to as $HOME in the generic
manual is implemented as %HOMEDRIVE%%HOMEPATH%", to cover peculiarities of
each platform.

But I think it is going backwards to butcher the description in the main
manual in order to make the platform supplement shorter.  Even Windows is
not a good enough reason to break the logical ordering of how things
should be best taught to the readers of the manual, which is not specific
to a platform.

^ permalink raw reply

* Re: [PATCH v1 1/3] Introduce config variable "diff.primer"
From: Junio C Hamano @ 2009-01-25 20:34 UTC (permalink / raw)
  To: Keith Cascio; +Cc: Jeff King, Johannes Schindelin, git
In-Reply-To: <1232904657-31831-2-git-send-email-keith@cs.ucla.edu>

Keith Cascio <keith@cs.ucla.edu> writes:

> Introduce config variable "diff.primer".
> Allows user to specify arbitrary options
> to pass to diff on every invocation,
> including internal invocations from other
> programs, e.g. git-gui.
> Introduce diff command-line options:
> --no-primer, --machine-friendly
> Protect git-format-patch, git-apply,
> git-am, git-rebase, git-gui and gitk
> from inapplicable options.
>
> Signed-off-by: Keith Cascio <keith@cs.ucla.edu>

Your Subject is good; in a shortlog output that will be taken 3 months
down the road, it will still tell us what this patch was about, among 100
other patches that are about different topics.

The proposed commit log message describes what the patch does, but it does
not explain what problem it solves, nor why the approach the patch takes
to solve that problem is good.  Lines that are too short and too dense
without paragraph breaks do not help readability either.

> ---
>  Documentation/config.txt       |   14 +++++++
>  Documentation/diff-options.txt |   13 ++++++
>  Makefile                       |    2 +
>  builtin-log.c                  |    1 +
>  diff.c                         |   83 +++++++++++++++++++++++++++++++++++-----
>  diff.h                         |   15 ++++++-
>  git-gui/lib/diff.tcl           |    8 +++-
>  gitk-git/gitk                  |   16 ++++----
>  8 files changed, 129 insertions(+), 23 deletions(-)

You can work around the backward incompatibility you are introducing for
known users that you broke with your patch, by including updates to them,
and that is what your patches to git-gui and gitk are, but that is a sure
sign that the approach is flawed.

The point of lowlevel plumbing (e.g. diff-{files,index,tree}) is to give
people's scripts an interface that they can rely on.  It is not about
giving a magic interface that all the users are somehow magically upgraded
without change, when the underlying git is upgraded.

If a script X does not use "ignore whitespace" without an explicit request
from the end user when it runs diff-tree internally, installing a new
version of diff-tree should *NOT* magically make script X to run it with
"ignore whitespace", because you do not know what the script X uses
diff-tree output for and how, even when the end user sets diff.primer to
get "ignore whitespace" applied to his command line invocation of "git
diff" Porcelain.  Imagine a case where the operation of that script X
relies on seeing at least two context lines around the hunk in order to
correctly parse textual diff output from "diff-index -p", and the user
sets "-U1" in diff.primer --- you would break the script and it is not
fair to blame the script for not explicitly passing -U3 and relying on the
default.

Scriptability by definition means you do not know how scripts written by
people around plumbing use the output; I do not think you can sensibly say
"this should not be turned on in a machine friendly output, but this is
safe to use".

I would not be opposed to an enhancement to the plumbing that the scripts
can use to say "I am willing to take any option (or perhaps "these
options") given to me via diff.primer".  Some scripts may want to be just
a pass-thru of whatever the underlying git-diff-* command outputs, and it
may be a handy way to magically upgrade them to allow their invocation of
lowlevel plumbing to be affected by what the end-user configured.  But
that magic upgrade has to be an opt/in process.

There are funny indentation to align the same variable names on two
adjacent lines and such; please don't.

^ permalink raw reply

* Re: [PATCH v1 1/3] Introduce config variable "diff.primer"
From: Keith Cascio @ 2009-01-25 20:14 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git, Shawn O Pearce
In-Reply-To: <alpine.DEB.1.00.0901252016590.14855@racer>

On Sun, 25 Jan 2009, Johannes Schindelin wrote:

> What about my scripts I have here locally?  Do you want to change them, 
> too?

Someone who already wrote local scripts probably won't use the diff.primer 
feature, or if he does, he'll be thoughtful about the consequences and modify 
his scripts.  diff.primer has no effect unless you use it.  It is totally 
personal.

> > I fact, by introducing the cpp macro DIFF_MACHINE_FRIENDLY() and the 
> > command-line options "--machine-friendly" and "--no-primer", I made such 
> > protection declarative.  Don't you find it preferable that existing programs 
> > and scripts would explicitly declare their desire for machine-friendly 
> > output?

> No.  We made a promise long time ago that plumbing (and "git diff" is 
> pretty much plumbing, except for the configurable colorness) would not 
> change behind scripts' backs.
> 
> And since Shawn uses plumbing for that very reason, your diff.primer patch 
> would not be allowed to make a difference.  Ever.

I fixed up every single place where gitk and git-gui internally call diff.  
Before I ever got there, Shawn already protected himself by passing --no-color 
to "git diff" (git-gui/lib/diff.tcl line 279).  My changes are in a good spirit 
because they enhance the declarativeness and explicitness of the code.  I added 
meaningful information.

> Now, if you would have changed only the UI diff things (i.e. git diff, but 
> not git diff-files), I could have accepted the diff.primer patch for 
> different applications than "git gui", but from cursory reading of your 
> patch it does not appear so.
> 
> Speaking of appearance (or for that matter, explaining why it was only a 
> cursory reading): did it not occur to you that your coding style is 
> utterly different from the surrounding code?
> 
> Just to number a few things that would definitely prohibit this patch from 
> being applied:
> 
> - space instead of tabs,
> - horrible lengths of spaces within the line,
> - no space after if, but after the parenthesis.

You are right, I should mimic the existing conventions.  If we reach consensus 
about the functionality of the patch, I would be happy to redo all the white 
space to match what's already there.  In case you are curious, I put whitespace 
in lines in the name of readability, to make tokens line up with each other.  
Again, it is right to mimic local convention and I have no problem making that 
revision.

> Besides, it seems you did a lot of "fixes" on the side that I do not like at 
> all.  Simple example: if the original code cleared the DIRSTAT_CUMULATIVE 
> flag, it is not acceptable for you to introduce an unnecessary if(), testing 
> if the CUMULATIVE flag was set to begin with.

I appreciate that you noticed this little detail.  You are extremely thorough 
and I take your criticism very seriously.  The reason for that little if() is 
that, with the enhanced declarativeness of the code in diff.h, calling the cpp 
macro DIFF_OPT_SET() is now more meaningful that just flipping a bit.  It is now 
the equivalent of saying "I intend to set this bit, and I want my intention 
recorded and honored later."  The purpose of the piece of code where I 
introduced the if() is only to setup the right defaults, not to declare user 
intention.  Therefore, IOW, if the default is already in place, do nothing.

c is only one of many languages I write.  Designers of newer languages emphasize 
meangingfulness, explicitness, declarativeness of code, e.g. the DRY principle, 
etc.  c is very flexible, so it does not prevent us from following these 
beneficial practices in c.  After all, the first c++ compilers simply emitted c 
and then called a c compiler, right?

With thanks,
Keith

^ permalink raw reply

* [PATCH 2/3] git-svn: documented --ignore-paths
From: Vitaly "_Vi" Shukela @ 2009-01-25 19:49 UTC (permalink / raw)
  To: trast; +Cc: git, Vitaly "_Vi" Shukela
In-Reply-To: <1232912944-27076-1-git-send-email-public_vi@tut.by>


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.6.1.288.gff3b4

^ permalink raw reply related

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


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.6.1.288.gff3b4

^ permalink raw reply related

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


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.6.1.288.gff3b4

^ permalink raw reply related

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

Hi,

On Sun, 25 Jan 2009, Keith Cascio wrote:

> On Sun, 25 Jan 2009, Johannes Schindelin wrote:
> 
> > That would break existing scripts using "git diff" rather badly.  We 
> > already did not allow something like "git config alias.diff ..." from 
> > changing the behavior of "git diff", so I cannot find a reason why we 
> > should let diff.primer (a misnomer BTW) override the behavior.
> 
> I took special care to protect all core scripts from the effects.

What about my scripts I have here locally?  Do you want to change them, 
too?

> I fact, by introducing the cpp macro DIFF_MACHINE_FRIENDLY() and the 
> command-line options "--machine-friendly" and "--no-primer", I made such 
> protection declarative.  Don't you find it preferable that existing 
> programs and scripts would explicitly declare their desire for 
> machine-friendly output?

No.  We made a promise long time ago that plumbing (and "git diff" is 
pretty much plumbing, except for the configurable colorness) would not 
change behind scripts' backs.

And since Shawn uses plumbing for that very reason, your diff.primer patch 
would not be allowed to make a difference.  Ever.

Now, if you would have changed only the UI diff things (i.e. git diff, but 
not git diff-files), I could have accepted the diff.primer patch for 
different applications than "git gui", but from cursory reading of your 
patch it does not appear so.

Speaking of appearance (or for that matter, explaining why it was only a 
cursory reading): did it not occur to you that your coding style is 
utterly different from the surrounding code?

Just to number a few things that would definitely prohibit this patch from 
being applied:

- space instead of tabs,

- horrible lengths of spaces within the line,

- no space after if, but after the parenthesis.

Now, this could be good explanation why you need the patch (to ignore 
white-space), but that is not a reason of letting us suffer, too.

Besides, it seems you did a lot of "fixes" on the side that I do not like 
at all.  Simple example: if the original code cleared the 
DIRSTAT_CUMULATIVE flag, it is not acceptable for you to introduce an 
unnecessary if(), testing if the CUMULATIVE flag was set to begin with.

Ciao,
Dscho

^ permalink raw reply

* [PATCH] Makefile: Make 'configure --with-expat=path' actually work
From: Serge van den Boom @ 2009-01-25 19:24 UTC (permalink / raw)
  To: git

The prefix specified with the --with-expat option of configure was not
actually used.

Signed-off-by: Serge van den Boom <svdb@stack.nl>
---
diff --git a/Makefile b/Makefile
index b4d9cb4..e7218cb 100644
--- a/Makefile
+++ b/Makefile
@@ -849,7 +849,12 @@ else
  		endif
  	endif
  	ifndef NO_EXPAT
-		EXPAT_LIBEXPAT = -lexpat
+		ifdef EXPATDIR
+			BASIC_CFLAGS += -I$(EXPATDIR)/include
+			EXPAT_LIBEXPAT = -L$(EXPATDIR)/$(lib) $(CC_LD_DYNPATH)$(EXPATDIR)/$(lib) -lexpat
+		else
+			EXPAT_LIBEXPAT = -lexpat
+		endif
  	endif
  endif

^ permalink raw reply related

* Re: [PATCH v1 3/3] git-gui hooks for new config variable "diff.primer"
From: Keith Cascio @ 2009-01-25 18:58 UTC (permalink / raw)
  To: Johannes Schindelin, Shawn O Pearce
  Cc: Junio C Hamano, Jeff King, Teemu Likonen, git
In-Reply-To: <alpine.DEB.1.00.0901251918230.14855@racer>

On Sun, 25 Jan 2009, Johannes Schindelin wrote:

> Rather than storing the information about how to call "git diff" as 
> diff.primer, why don't you store that information in a config variable 
> gui.whiteSpaceMode and teach "git gui" to call "git diff" accordingly?
> 
> That would have the further advantage of not breaking other people's 
> setups...

This wasn't just for the GUI.  Using Git for a project I imported from CVS, I 
wanted --ignore-space-at-eol to be in effect at all times on the command line.  
Of course, the "alias.dff" approach suggested yesterday by Teemu would work for 
that.  But I got the feeling this is a more general need.  I'll name my primary 
inspiration: ExamDiff (a Windows program).  ExamDiff lets you specify a wide 
range of options that remain in effect over all invocations.  Seems like 
something a lot of users would find natural.  Please comment.

> Please submit git-gui patches without the git-gui prefix, as it makes it 
> harder on the maintainer of git-gui, Shawn (who you did not Cc: BTW).

Sorry Shawn, I should have Cc'd you.  Please let me know if I can improve the 
git-gui code in any way.

                                        -- Keith

^ permalink raw reply

* Re: Translations in Git release?
From: Jeff King @ 2009-01-25 18:54 UTC (permalink / raw)
  To: Dill; +Cc: git
In-Reply-To: <60646ee10901250941s34f7accem1b74fc201e895a41@mail.gmail.com>

On Sun, Jan 25, 2009 at 10:41:02AM -0700, Dill wrote:

> Is there a plan to include translations of the Documentation within
> Git or should they exist outside of the project?

I don't recall seeing any discussion on documentation translations
previously. But there is some precedent for having translations in the
tree, as git-gui's messages are translated into quite a few languages.

-Peff

^ permalink raw reply

* [EGIT PATCH 4/4] Retain Eclipse Git preferences page hierarchy without warnings in plugin.xml
From: Robin Rosenberg @ 2009-01-25 18:52 UTC (permalink / raw)
  To: spearce; +Cc: git, Robin Rosenberg
In-Reply-To: <1232909538-3279-3-git-send-email-robin.rosenberg@dewire.com>

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 org.spearce.egit.ui/plugin.xml |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/org.spearce.egit.ui/plugin.xml b/org.spearce.egit.ui/plugin.xml
index c2b82fd..869108c 100644
--- a/org.spearce.egit.ui/plugin.xml
+++ b/org.spearce.egit.ui/plugin.xml
@@ -185,7 +185,7 @@
             id="org.spearce.egit.ui.GitPreferences" />
 
       <page name="%GitPreferences_HistoryPreferencePage_name"
-            category="org.eclipse.team.ui.TeamPreferences/org.spearce.egit.ui.GitPreferences"
+	    category="org.spearce.egit.ui.GitPreferences"
             class="org.spearce.egit.ui.internal.preferences.HistoryPreferencePage"
 	    id="org.spearce.egit.ui.internal.preferences.HistoryPreferencePage" >
 	    <keywordReference
@@ -193,7 +193,7 @@
 	    </keywordReference>
 	    </page>
       <page name="%GitPreferences_WindowCachePreferencePage_name"
-            category="org.eclipse.team.ui.TeamPreferences/org.spearce.egit.ui.GitPreferences"
+	    category="org.spearce.egit.ui.GitPreferences"
             class="org.spearce.egit.ui.internal.preferences.WindowCachePreferencePage"
 	    id="org.spearce.egit.ui.internal.preferences.WindowCachePreferencePage" >
 	    <keywordReference
-- 
1.6.1.285.g35d8b

^ permalink raw reply related

* [EGIT PATCH 3/4] Add the keyword "Git" to the git preferences pages.
From: Robin Rosenberg @ 2009-01-25 18:52 UTC (permalink / raw)
  To: spearce; +Cc: git, Robin Rosenberg
In-Reply-To: <1232909538-3279-2-git-send-email-robin.rosenberg@dewire.com>

If the users searches for "Git" in the workspaces preferences, all
git related preferences pages will now be found.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 org.spearce.egit.ui/plugin.xml |   20 ++++++++++++++++++--
 1 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/org.spearce.egit.ui/plugin.xml b/org.spearce.egit.ui/plugin.xml
index e5c81e5..c2b82fd 100644
--- a/org.spearce.egit.ui/plugin.xml
+++ b/org.spearce.egit.ui/plugin.xml
@@ -187,11 +187,19 @@
       <page name="%GitPreferences_HistoryPreferencePage_name"
             category="org.eclipse.team.ui.TeamPreferences/org.spearce.egit.ui.GitPreferences"
             class="org.spearce.egit.ui.internal.preferences.HistoryPreferencePage"
-            id="org.spearce.egit.ui.internal.preferences.HistoryPreferencePage" />
+	    id="org.spearce.egit.ui.internal.preferences.HistoryPreferencePage" >
+	    <keywordReference
+		  id="org.spearce.egit.ui.keyword.git">
+	    </keywordReference>
+	    </page>
       <page name="%GitPreferences_WindowCachePreferencePage_name"
             category="org.eclipse.team.ui.TeamPreferences/org.spearce.egit.ui.GitPreferences"
             class="org.spearce.egit.ui.internal.preferences.WindowCachePreferencePage"
-            id="org.spearce.egit.ui.internal.preferences.WindowCachePreferencePage" />
+	    id="org.spearce.egit.ui.internal.preferences.WindowCachePreferencePage" >
+	    <keywordReference
+		  id="org.spearce.egit.ui.keyword.git">
+	    </keywordReference>
+      </page>
    </extension>
 
    <extension point="org.eclipse.ui.propertyPages">
@@ -212,6 +220,14 @@
          </filter>
       </page>
    </extension>
+
+   <extension point="org.eclipse.ui.keywords">
+	  <keyword
+			id="org.spearce.egit.ui.keyword.git"
+			label="git">
+	  </keyword>
+   </extension>
+
    <extension point="org.eclipse.ui.decorators">
       <decorator
             lightweight="true"
-- 
1.6.1.285.g35d8b

^ permalink raw reply related

* [EGIT PATCH 1/4] Cleanup ui plugin.xml, refer to interface IResource, not Resource
From: Robin Rosenberg @ 2009-01-25 18:52 UTC (permalink / raw)
  To: spearce; +Cc: git, Robin Rosenberg

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 org.spearce.egit.ui/plugin.xml |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/org.spearce.egit.ui/plugin.xml b/org.spearce.egit.ui/plugin.xml
index e6e3762..0b18f15 100644
--- a/org.spearce.egit.ui/plugin.xml
+++ b/org.spearce.egit.ui/plugin.xml
@@ -170,7 +170,7 @@
             </adapter>
          </factory>
          <factory
-               adaptableType="org.eclipse.core.internal.resources.Resource"
+	       adaptableType="org.eclipse.core.resources.IResource"
                class="org.spearce.egit.ui.internal.factories.GitAdapterFactory">
             <adapter
                   type="org.eclipse.team.ui.history.IHistoryPageSource">
-- 
1.6.1.285.g35d8b

^ permalink raw reply related

* [EGIT PATCH 2/4] Cleanup ui plugin.xml, replace deprecated elements and attributes
From: Robin Rosenberg @ 2009-01-25 18:52 UTC (permalink / raw)
  To: spearce; +Cc: git, Robin Rosenberg
In-Reply-To: <1232909538-3279-1-git-send-email-robin.rosenberg@dewire.com>

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 org.spearce.egit.ui/plugin.xml |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/org.spearce.egit.ui/plugin.xml b/org.spearce.egit.ui/plugin.xml
index 0b18f15..e5c81e5 100644
--- a/org.spearce.egit.ui/plugin.xml
+++ b/org.spearce.egit.ui/plugin.xml
@@ -196,11 +196,16 @@
 
    <extension point="org.eclipse.ui.propertyPages">
       <page
-            objectClass="org.eclipse.core.resources.IProject"
-            adaptable="true"
             name="%Git"
             class="org.spearce.egit.ui.internal.preferences.GitProjectPropertyPage"
             id="org.spearce.egit.ui.preferences.GitProjectPropertyPage">
+	    <enabledWhen>
+		<adapt type="org.eclipse.core.resources.IProject">
+			<test
+		    property="projectPersistentProperty org.eclipse.team.core.repository" value="org.spearce.egit.core.GitProvider">
+			</test>
+		</adapt>
+	    </enabledWhen>
          <filter
                name="projectPersistentProperty"
                value="org.eclipse.team.core.repository=org.spearce.egit.core.GitProvider">
-- 
1.6.1.285.g35d8b

^ permalink raw reply related

* Re: What's cooking in git.git (Jan 2009, #06; Sat, 24)
From: Jeff King @ 2009-01-25 18:51 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0901251308430.14855@racer>

On Sun, Jan 25, 2009 at 01:17:33PM +0100, Johannes Schindelin wrote:

> BTW a test run on my machine resulted in a few koku of valgrind errors; 
> This was done in my personal tree which contains dozens of extra patches, 
> so I want to repeat the exercize with 'next' first, but I think we will 
> get quite some patches due to the valgrind support...

Hmm. I hope not. "master" was testing clean on valgrind a few months ago
(and took only a handful of cleanups to get that way after years of
development), so either you are getting false positives, my original
work was getting false negatives, or the code quality has dropped
dramatically in the last 3 months. :)

-Peff

^ permalink raw reply

* Re: CR codes from git commands
From: Brent Goodrick @ 2009-01-25 18:47 UTC (permalink / raw)
  To: Boyd Stephen Smith Jr.; +Cc: Brent Goodrick, git
In-Reply-To: <200901250319.05665.bss@iguanasuicide.net>


Boyd Stephen Smith Jr. writes:
 > On Saturday 24 January 2009, Brent Goodrick <bgoodr@gmail.com> wrote 
 > about 'Re: CR codes from git commands':
 > >While I'm at it, what is the standard procedure for submitting git
 > >patches for review once I've cooked up and validated it on my end? I'm
 > >guessing posting the patch into this mailing list is part of the
 > >answer to that question.
 > 
 > If you've got a patch, I assume you've got a checkout.  Look in 
 > Documentation/SubmittingPatches.

Thanks I see that now. No, I don't have a patch yet, was struggling to
find that basic info that really should be front and center somewhere
on the wiki (and also access to the wiki is very slow).

bg

^ permalink raw reply

* Re: [PATCH v1 1/3] Introduce config variable "diff.primer"
From: Keith Cascio @ 2009-01-25 18:44 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, Jeff King, git
In-Reply-To: <alpine.DEB.1.00.0901251916010.14855@racer>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1197 bytes --]

On Sun, 25 Jan 2009, Johannes Schindelin wrote:

> That would break existing scripts using "git diff" rather badly.  We already 
> did not allow something like "git config alias.diff ..." from changing the 
> behavior of "git diff", so I cannot find a reason why we should let 
> diff.primer (a misnomer BTW) override the behavior.

I took special care to protect all core scripts from the effects.  Quote from 
patch 1/3:
> Protect git-format-patch, git-apply,
> git-am, git-rebase, git-gui and gitk
> from inapplicable options.

I fact, by introducing the cpp macro DIFF_MACHINE_FRIENDLY() and the 
command-line options "--machine-friendly" and "--no-primer", I made such 
protection declarative.  Don't you find it preferable that existing programs and 
scripts would explicitly declare their desire for machine-friendly output?

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>

Thanks for your input.  More input welcome.
                                    -- Keith

^ permalink raw reply

* Re: [PATCH] Allow cloning an empty repository
From: Sverre Rabbelier @ 2009-01-25 18:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, Git Mailinglist, Miklos Vajna
In-Reply-To: <7vy6x0j7py.fsf@gitster.siamese.dyndns.org>

On Sun, Jan 25, 2009 at 06:05, Junio C Hamano <gitster@pobox.com> wrote:
> Yeah, it is on 'next' and I've pushed the results out last night, but I
> got sick and didn't manage to send out "What's cooking".

Ok, awesome! :).

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCH v1 3/3] git-gui hooks for new config variable "diff.primer"
From: Johannes Schindelin @ 2009-01-25 18:22 UTC (permalink / raw)
  To: Keith Cascio; +Cc: Junio C Hamano, Jeff King, git
In-Reply-To: <1232904657-31831-4-git-send-email-keith@cs.ucla.edu>

Hi,

On Sun, 25 Jan 2009, Keith Cascio wrote:

> git-gui hooks for new config variable "diff.primer".
> Add three checkboxes to both sides of
> options panel (local/global).
> Add a sub-menu named "White Space" to
> diff-panel right-click context menu, with
> three checkboxes.

Rather than storing the information about how to call "git diff" as 
diff.primer, why don't you store that information in a config variable 
gui.whiteSpaceMode and teach "git gui" to call "git diff" accordingly?

That would have the further advantage of not breaking other people's 
setups...

>  git-gui/git-gui.sh     |   51 ++++++++++++++++++++++++++++++++++++++++++
>  git-gui/lib/option.tcl |   57 +++++++++++++++++++++++++++++++++++++++++++----

Please submit git-gui patches without the git-gui prefix, as it makes it 
harder on the maintainer of git-gui, Shawn (who you did not Cc: BTW).

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH v1 1/3] Introduce config variable "diff.primer"
From: Johannes Schindelin @ 2009-01-25 18:17 UTC (permalink / raw)
  To: Keith Cascio; +Cc: Junio C Hamano, Jeff King, git
In-Reply-To: <1232904657-31831-2-git-send-email-keith@cs.ucla.edu>

Hi,

On Sun, 25 Jan 2009, Keith Cascio wrote:

> Introduce config variable "diff.primer".
> Allows user to specify arbitrary options
> to pass to diff on every invocation,
> including internal invocations from other
> programs, e.g. git-gui.

That would break existing scripts using "git diff" rather badly.  We 
already did not allow something like "git config alias.diff ..." from 
changing the behavior of "git diff", so I cannot find a reason why we 
should let diff.primer (a misnomer BTW) override the behavior.

Ciao,
Dscho

^ permalink raw reply

* [PATCH] Add new testcase for git-merge-tree
From: Miklos Vajna @ 2009-01-25 18:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
 t/t6034-merge-tree.sh |   40 ++++++++++++++++++++++++++++++++++++++++
 1 files changed, 40 insertions(+), 0 deletions(-)
 create mode 100755 t/t6034-merge-tree.sh

diff --git a/t/t6034-merge-tree.sh b/t/t6034-merge-tree.sh
new file mode 100755
index 0000000..42bd5e7
--- /dev/null
+++ b/t/t6034-merge-tree.sh
@@ -0,0 +1,40 @@
+#!/bin/sh
+
+test_description='git merge-tree
+
+Testing merging two trees without touching the index.'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	echo c0 > c0.c &&
+	git add c0.c &&
+	git commit -m c0 &&
+	git tag c0 &&
+	echo c1 > c1.c &&
+	git add c1.c &&
+	git commit -m c1 &&
+	git tag c1 &&
+	c1=$(git hash-object c1.c)
+	git reset --hard c0 &&
+	echo c2 > c2.c &&
+	git add c2.c &&
+	git commit -m c2 &&
+	git tag c2
+	c2=$(git hash-object c2.c)
+'
+
+test_expect_success 'merge c1 with c2' '
+	cat <<EOF >expected &&
+added in local
+  our    100644 $c1 c1.c
+added in remote
+  their  100644 $c2 c2.c
+@@ -0,0 +1 @@
++c2
+EOF
+	git merge-tree c0 c1 c2 > actual &&
+	test_cmp expected actual
+'
+
+test_done
-- 
1.6.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox