Git development
 help / color / mirror / Atom feed
* Re: git branch --set-upstream regression in master
From: Junio C Hamano @ 2011-09-16 23:14 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git, conrad.irwin
In-Reply-To: <7v7h58dri4.fsf@alter.siamese.dyndns.org>

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

> I took a brief look at --set-upstream codepath, and I have to say that the
> implementation is totally broken with respect to an existing branch.
>
> Given
>
> 	$ git branch master --set-upstream origin/master
>
> it passes the exact same codepath as
>
> 	$ git branch master origin/master
>
> uses, only with a different "track" flag, no?  That is, it calls a
> function that is meant to _create_ branch "master" from given branch point
> "origin/master", namely create_branch().  And then create_branch(),
> contrary to its name, is littered with "dont_change_ref" special case to
> work it around, depending on the value of "track".

So here is a quick-and-dirty patch, which may or may not compile or pass
tests.

 branch.c           |   21 ++++++++++++---------
 branch.h           |   12 +++++++++++-
 builtin/branch.c   |    2 +-
 builtin/checkout.c |    3 ++-
 4 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/branch.c b/branch.c
index 478d825..fecedd3 100644
--- a/branch.c
+++ b/branch.c
@@ -135,23 +135,25 @@ static int setup_tracking(const char *new_ref, const char *orig_ref,
 	return 0;
 }
 
-int validate_new_branchname(const char *name, struct strbuf *ref, int force)
+int validate_new_branchname(const char *name, struct strbuf *ref,
+			    int force, int attr_only)
 {
-	const char *head;
-	unsigned char sha1[20];
-
 	if (strbuf_check_branch_ref(ref, name))
 		die("'%s' is not a valid branch name.", name);
 
 	if (!ref_exists(ref->buf))
 		return 0;
-	else if (!force)
+	else if (!force && !attr_only)
 		die("A branch named '%s' already exists.", ref->buf + strlen("refs/heads/"));
 
-	head = resolve_ref("HEAD", sha1, 0, NULL);
-	if (!is_bare_repository() && head && !strcmp(head, ref->buf))
-		die("Cannot force update the current branch.");
+	if (!attr_only) {
+		const char *head;
+		unsigned char sha1[20];
 
+		head = resolve_ref("HEAD", sha1, 0, NULL);
+		if (!is_bare_repository() && head && !strcmp(head, ref->buf))
+			die("Cannot force update the current branch.");
+	}
 	return 1;
 }
 
@@ -171,7 +173,8 @@ void create_branch(const char *head,
 	if (track == BRANCH_TRACK_EXPLICIT || track == BRANCH_TRACK_OVERRIDE)
 		explicit_tracking = 1;
 
-	if (validate_new_branchname(name, &ref, force || track == BRANCH_TRACK_OVERRIDE)) {
+	if (validate_new_branchname(name, &ref, force,
+				    track == BRANCH_TRACK_OVERRIDE)) {
 		if (!force)
 			dont_change_ref = 1;
 		else
diff --git a/branch.h b/branch.h
index 01544e2..1285158 100644
--- a/branch.h
+++ b/branch.h
@@ -20,8 +20,18 @@ void create_branch(const char *head, const char *name, const char *start_name,
  * interpreted ref in ref, force indicates whether (non-head) branches
  * may be overwritten. A non-zero return value indicates that the force
  * parameter was non-zero and the branch already exists.
+ *
+ * Contrary to all of the above, when attr_only is 1, the caller is
+ * not interested in verifying if it is Ok to update the named
+ * branch to point at a potentially different commit. It is merely
+ * asking if it is OK to change some attribute for the named branch
+ * (e.g. tracking upstream).
+ *
+ * NEEDSWORK: This needs to be split into two separate functions in the
+ * longer run for sanity.
+ *
  */
-int validate_new_branchname(const char *name, struct strbuf *ref, int force);
+int validate_new_branchname(const char *name, struct strbuf *ref, int force, int attr_only);
 
 /*
  * Remove information about the state of working on the current
diff --git a/builtin/branch.c b/builtin/branch.c
index aa705a0..f49596f 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -566,7 +566,7 @@ static void rename_branch(const char *oldname, const char *newname, int force)
 			die(_("Invalid branch name: '%s'"), oldname);
 	}
 
-	validate_new_branchname(newname, &newref, force);
+	validate_new_branchname(newname, &newref, force, 0);
 
 	strbuf_addf(&logmsg, "Branch: renamed %s to %s",
 		 oldref.buf, newref.buf);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 3bb6525..5e356a6 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1073,7 +1073,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	if (opts.new_branch) {
 		struct strbuf buf = STRBUF_INIT;
 
-		opts.branch_exists = validate_new_branchname(opts.new_branch, &buf, !!opts.new_branch_force);
+		opts.branch_exists = validate_new_branchname(opts.new_branch, &buf,
+							     !!opts.new_branch_force, 0);
 
 		strbuf_release(&buf);
 	}

^ permalink raw reply related

* Re: git branch --set-upstream regression in master
From: Junio C Hamano @ 2011-09-16 22:58 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git, conrad.irwin
In-Reply-To: <CAG+J_DyxNpPevwfrJVkv3GBmv0tEXgW2LZtdHgarFoXb9Qqghw@mail.gmail.com>

Jay Soffian <jaysoffian@gmail.com> writes:

> This used to be possible on the checked out branch:
>
>   $ git branch master --set-upstream origin/master
>
> Now it gives "fatal: Cannot force update the current branch." which is
> broken. You should be able to setup/change the tracking information on
> the checked out branch.
>
> It's apparently due to ci/forbid-unwanted-current-branch-update.

Does

	git branch --set-upstream master origin/master

work? If so then I wouldn't worry too much about it (your "arg then
option" should be forbidden in the longer term anyway). If not, we would
need to patch it.

> (BTW, --set-upstream still needs to be fixed so that these mean the same
> thing:
>
>   $ git branch master --set-upstream origin/master
>   $ git branch --set-upstream origin/master master

If we are doing anythning, I think it needs to be fixed not to allow the
former, period.

> .. to just allow:
>
>   $ git branch --set-upstream origin/master
>
> w/o having to specify the checked-out branch.

That may be a nice feature enhancement, post 1.7.7 release.

I took a brief look at --set-upstream codepath, and I have to say that the
implementation is totally broken with respect to an existing branch.

Given

	$ git branch master --set-upstream origin/master

it passes the exact same codepath as

	$ git branch master origin/master

uses, only with a different "track" flag, no?  That is, it calls a
function that is meant to _create_ branch "master" from given branch point
"origin/master", namely create_branch().  And then create_branch(),
contrary to its name, is littered with "dont_change_ref" special case to
work it around, depending on the value of "track".

^ permalink raw reply

* Re: zealous git convert determined to set up git server
From: Jakub Narebski @ 2011-09-16 22:39 UTC (permalink / raw)
  To: Joshua Stoutenburg; +Cc: Git List
In-Reply-To: <CAOZxsTqtW=DD7zFwQLjknJR8g0nnh0WPUPna6_np4bVoGnSntQ@mail.gmail.com>

Joshua Stoutenburg wrote:
> 2011/9/15 Jakub Narebski <jnareb@gmail.com>:

> > I think that either "Pro Git" book, or "The Git Community Book"
> > would be a best source to learn about setting-up git server.
> >
> > I think the simplest solution for git hosting management would be to
> > use gitolite (there are other git repository management software:
> > Gitosis, SCM Manager, Gitblit).
> >
> > If you want to host something like GitHub, there are open source
> > solutions too: Gitorious, InDefero, Girocco + gitweb,...
> >
> > HTH
> > --
> > Jakub Narębski
> >
> >
> 
> I totally didn't see "The Git Community Book".  There's no link for it
> where I was looking: http://git-scm.com/documentation

I think that link to "Git Community Book" (http://book.git-scm.com) on Git
Documentation page (http://git-scm.com/documentation) got replaced by the
link to "Pro Git" book; I guess becaue the former is not finished and it
doesn't look like it would be finished soon.

[...]
> Question 2: It seems gitolite is the popular choice for git user
> management.  Any reason why?

From Gitosis and Gitolite, both git repository management tools, Gitosis
requires setuptools beside Python, and looks like it is not developed
anymore, while Gitolite (which started as rewrite of Gitosis in Perl)
requires only Perl and is actively developed.

Nb. even Gitosis author recommends Gitolite:
http://scie.nti.st/2007/11/14/hosting-git-repositories-the-easy-and-secure-way

  Update (12-12-2010): For additional features not present in gitosis,
  check out gitolite.
 
> Question 3: So, Gitorious is more than just a repository hosting
> website?  It's also an open source repository hosting platform, which
> powers the Gitorious website?  That's pretty cool.

Yes, GitHub:FI (this one proprietary), Gitorious (powering gitorious.org),
InDefero, Gitblit and Girocco + gitweb (the last one powering http://repo.or.cz)
are all full-fledged git hosting sites, with web interface to view and
manage repositories.

-- 
Jakub Narebski
Poland

^ permalink raw reply

* [PATCH] send-email: Honor multi-part email messages
From: Alexey Shumkin @ 2011-09-16 22:32 UTC (permalink / raw)
  To: git; +Cc: Alexey Shumkin

"git format-patch --attach/--inline" generates multi-part messages.
Every part of such messages can contain non-ASCII characters with its own
"Content-Type" and "Content-Transfer-Encoding" headers.
But git-send-mail script interprets a patch-file as one-part message
and does not recognize multi-part messages.
So already quoted printable email subject may be encoded as quoted printable
again. Due to this bug email subject looks corrupted in email clients.

Signed-off-by: Alexey Shumkin <zapped@mail.ru>
---
 git-send-email.perl   |    5 +++
 t/t9001-send-email.sh |   66 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+), 0 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 98ab33a..1abf4a4 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1403,12 +1403,17 @@ sub file_has_nonascii {
 
 sub body_or_subject_has_nonascii {
 	my $fn = shift;
+	my $multipart = 0;
 	open(my $fh, '<', $fn)
 		or die "unable to open $fn: $!\n";
 	while (my $line = <$fh>) {
 		last if $line =~ /^$/;
+		if ($line =~ /^Content-Type:\s*multipart\/mixed.*$/) {
+			$multipart = 1;
+		}
 		return 1 if $line =~ /^Subject.*[^[:ascii:]]/;
 	}
+	return 0 if $multipart;
 	while (my $line = <$fh>) {
 		return 1 if $line =~ /[^[:ascii:]]/;
 	}
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 579ddb7..151ad35 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1168,4 +1168,70 @@ test_expect_success $PREREQ '--force sends cover letter template anyway' '
 	test -n "$(ls msgtxt*)"
 '
 
+test_expect_success $PREREQ 'setup multi-part message' '
+cat >multi-part-email-using-8bit <<EOF
+From fe6ecc66ece37198fe5db91fa2fc41d9f4fe5cc4 Mon Sep 17 00:00:00 2001
+Message-Id: <bogus-message-id@example.com>
+From: author@example.com
+Date: Sat, 12 Jun 2010 15:53:58 +0200
+Subject: [PATCH] =?UTF-8?q?=D0=94=D0=BE=D0=B1=D0=B0=D0=B2=D0=BB=D0=B5=D0=BD=20?=
+ =?UTF-8?q?=D1=84=D0=B0=D0=B9=D0=BB?=
+MIME-Version: 1.0
+Content-Type: multipart/mixed; boundary="------------123"
+
+This is a multi-part message in MIME format.
+--------------1.7.6.3.4.gf71f
+Content-Type: text/plain; charset=UTF-8; format=fixed
+Content-Transfer-Encoding: 8bit
+
+This is a message created with "git format-patch --attach=123"
+---
+ master   |    1 +
+ файл |    1 +
+ 2 files changed, 2 insertions(+), 0 deletions(-)
+ create mode 100644 master
+ create mode 100644 файл
+
+
+--------------123
+Content-Type: text/x-patch; name="0001-.patch"
+Content-Transfer-Encoding: 8bit
+Content-Disposition: attachment; filename="0001-.patch"
+
+diff --git a/master b/master
+new file mode 100644
+index 0000000..1f7391f
+--- /dev/null
++++ b/master
+@@ -0,0 +1 @@
++master
+diff --git a/файл b/файл
+new file mode 100644
+index 0000000..44e5cfe
+--- /dev/null
++++ b/файл
+@@ -0,0 +1 @@
++содержимое файла
+
+--------------123--
+EOF
+'
+
+test_expect_success $PREREQ 'setup expect' '
+cat >expected <<EOF
+Subject: [PATCH] =?UTF-8?q?=D0=94=D0=BE=D0=B1=D0=B0=D0=B2=D0=BB=D0=B5=D0=BD=20?= =?UTF-8?q?=D1=84=D0=B0=D0=B9=D0=BB?=
+EOF
+'
+
+test_expect_success $PREREQ '--attach/--inline also treats subject' '
+	clean_fake_sendmail &&
+	echo bogus |
+	git send-email --from=author@example.com --to=nobody@example.com \
+			--smtp-server="$(pwd)/fake.sendmail" \
+			--8bit-encoding=UTF-8 \
+			multi-part-email-using-8bit >stdout &&
+	grep "Subject" msgtxt1 >actual &&
+	test_cmp expected actual
+'
+
 test_done
-- 
1.7.6.3.4.gf71f

^ permalink raw reply related

* Re: zealous git convert determined to set up git server
From: Ilari Liusvaara @ 2011-09-16 22:22 UTC (permalink / raw)
  To: Andreas Krey; +Cc: Sitaram Chamarty, Git List
In-Reply-To: <20110916204032.GA13922@inner.h.iocl.org>

On Fri, Sep 16, 2011 at 10:40:32PM +0200, Andreas Krey wrote:
> 
> It well looks so, but I have a question: It seems that it assumes a
> flat set of *.git repos. Unfortunately my current setup has the repos
> in a hierarchy, like area/sub/repo.git, and I don't want everyone to
> change their local repo configs. Is it possible to keep it like that
> (and consequently have 'repo area/sub/repo' lines) when putting it
> under gitolite control?

It is possible to have '/' in repository name in Gitolite. Heck,
most repos I have in Gitolite have '/' in their names...

-Ilari

^ permalink raw reply

* git branch --set-upstream regression in master
From: Jay Soffian @ 2011-09-16 21:43 UTC (permalink / raw)
  To: git, Junio C Hamano, conrad.irwin

This used to be possible on the checked out branch:

  $ git branch master --set-upstream origin/master

Now it gives "fatal: Cannot force update the current branch." which is
broken. You should be able to setup/change the tracking information on
the checked out branch.

It's apparently due to ci/forbid-unwanted-current-branch-update.

Sorry I don't have time to contribute a patch at the moment.

(BTW, --set-upstream still needs to be fixed so that these mean the same thing:

  $ git branch master --set-upstream origin/master
  $ git branch --set-upstream origin/master master

and to just allow:

  $ git branch --set-upstream origin/master

w/o having to specify the checked-out branch.)

j.

^ permalink raw reply

* Re: [PATCH] Disambiguate duplicate t9160* tests
From: Frederic Heitzmann @ 2011-09-16 21:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, bjacobs, rchen
In-Reply-To: <7viposdwos.fsf@alter.siamese.dyndns.org>

Le 16/09/2011 23:06, Junio C Hamano a écrit :
> Junio C Hamano<gitster@pobox.com>  writes:
>
>> Thanks.
>
> Heh, I spoke too early. It still refers to contents of t/t9160/ directory.
> We would need this squashed into your patch (no need to resend).
>
>   t/t9161-git-svn-mergeinfo-push.sh |    2 +-
>   t/{t9160 =>  t9161}/branches.dump  |    0
>   2 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/t/t9161-git-svn-mergeinfo-push.sh b/t/t9161-git-svn-mergeinfo-push.sh
> index 216f3d7..6ef0c0b 100755
> --- a/t/t9161-git-svn-mergeinfo-push.sh
> +++ b/t/t9161-git-svn-mergeinfo-push.sh
> @@ -10,7 +10,7 @@ test_description='git-svn svn mergeinfo propagation'
>
>   test_expect_success 'load svn dump' "
>   	svnadmin load -q '$rawsvnrepo' \
> -	<  '$TEST_DIRECTORY/t9160/branches.dump'&&
> +	<  '$TEST_DIRECTORY/t9161/branches.dump'&&
>   	git svn init --minimize-url -R svnmerge \
>   	  -T trunk -b branches '$svnrepo'&&
>   	git svn fetch --all
> diff --git a/t/t9160/branches.dump b/t/t9161/branches.dump
> similarity index 100%
> rename from t/t9160/branches.dump
> rename to t/t9161/branches.dump

Ooops ! I checked 'make test', but missed the most obvious change.
Sorry for this.

--
Fred

^ permalink raw reply

* Re: [PATCH v3] git svn dcommit: new option --interactive.
From: Junio C Hamano @ 2011-09-16 21:08 UTC (permalink / raw)
  To: Frédéric Heitzmann; +Cc: git, normalperson
In-Reply-To: <1316206921-29311-1-git-send-email-frederic.heitzmann@gmail.com>

I am not accepting any new features at this point in the release cycle, so
please do not Cc me unless it is a patch to fix regression or minor
documentation.

Thanks.

^ permalink raw reply

* Re: [PATCH] Disambiguate duplicate t9160* tests
From: Junio C Hamano @ 2011-09-16 21:06 UTC (permalink / raw)
  To: Frédéric Heitzmann; +Cc: git, bjacobs, rchen
In-Reply-To: <7vty8cdxun.fsf@alter.siamese.dyndns.org>

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

> Thanks.

Heh, I spoke too early. It still refers to contents of t/t9160/ directory.
We would need this squashed into your patch (no need to resend).

 t/t9161-git-svn-mergeinfo-push.sh |    2 +-
 t/{t9160 => t9161}/branches.dump  |    0
 2 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/t/t9161-git-svn-mergeinfo-push.sh b/t/t9161-git-svn-mergeinfo-push.sh
index 216f3d7..6ef0c0b 100755
--- a/t/t9161-git-svn-mergeinfo-push.sh
+++ b/t/t9161-git-svn-mergeinfo-push.sh
@@ -10,7 +10,7 @@ test_description='git-svn svn mergeinfo propagation'
 
 test_expect_success 'load svn dump' "
 	svnadmin load -q '$rawsvnrepo' \
-	  < '$TEST_DIRECTORY/t9160/branches.dump' &&
+	  < '$TEST_DIRECTORY/t9161/branches.dump' &&
 	git svn init --minimize-url -R svnmerge \
 	  -T trunk -b branches '$svnrepo' &&
 	git svn fetch --all
diff --git a/t/t9160/branches.dump b/t/t9161/branches.dump
similarity index 100%
rename from t/t9160/branches.dump
rename to t/t9161/branches.dump

^ permalink raw reply related

* [PATCH v3] git svn dcommit: new option --interactive.
From: Frédéric Heitzmann @ 2011-09-16 21:02 UTC (permalink / raw)
  To: gitster; +Cc: git, normalperson, Frédéric Heitzmann

Allow the user to check the patch set before it is commited to SNV. It is
then possible to accept/discard one patch, accept all, or quit.

This interactive mode is similar with 'git send email' behaviour. However,
'git svn dcommit' returns as soon as one patch is discarded.
Part of the code was taken from git-send-email.perl (see 'ask' function)

Tests several combinations of potential answers to
'git svn dcommit --interactive'. For each of them, test whether patches
were commited to SVN or not.

Thanks-to Eric Wong <normalperson@yhbt.net> for the initial idea.

Reviewed-by: Eric Wong <normalperson@yhbt.net>
Signed-off-by: Frédéric Heitzmann <frederic.heitzmann@gmail.com>
---
 Minor change from v2 : rename t9160... to t9162... to avoid name
 collision
 ref: <1316202903-5085-1-git-send-email-frederic.heitzmann@gmail.com>

 Documentation/git-svn.txt              |    8 +++
 git-svn.perl                           |   76 +++++++++++++++++++++++++++++++-
 t/t9162-git-svn-dcommit-interactive.sh |   64 +++++++++++++++++++++++++++
 3 files changed, 147 insertions(+), 1 deletions(-)
 create mode 100644 t/t9162-git-svn-dcommit-interactive.sh

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 08cad6d..c8f0883 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -234,6 +234,14 @@ svn:mergeinfo property in the SVN repository when possible. Currently, this can
 only be done when dcommitting non-fast-forward merges where all parents but the
 first have already been pushed into SVN.
 
+--interactive;;
+	Ask the user to confirm that a patch set should actually be sent to SVN.
+	For each patch, one may answer "yes" (accept this patch), "no" (discard this
+	patch), "all" (accept all patches), or "quit".
+	+
+	'git svn dcommit' returns immediately if answer if "no" or "quit", without
+	commiting anything to SVN.
+
 'branch'::
 	Create a branch in the SVN repository.
 
diff --git a/git-svn.perl b/git-svn.perl
index 351e743..121332d 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -87,7 +87,7 @@ my ($_stdin, $_help, $_edit,
 	$_version, $_fetch_all, $_no_rebase, $_fetch_parent,
 	$_merge, $_strategy, $_dry_run, $_local,
 	$_prefix, $_no_checkout, $_url, $_verbose,
-	$_git_format, $_commit_url, $_tag, $_merge_info);
+	$_git_format, $_commit_url, $_tag, $_merge_info, $_interactive);
 $Git::SVN::_follow_parent = 1;
 $SVN::Git::Fetcher::_placeholder_filename = ".gitignore";
 $_q ||= 0;
@@ -163,6 +163,7 @@ my %cmd = (
 			  'revision|r=i' => \$_revision,
 			  'no-rebase' => \$_no_rebase,
 			  'mergeinfo=s' => \$_merge_info,
+			  'interactive|i' => \$_interactive,
 			%cmt_opts, %fc_opts } ],
 	branch => [ \&cmd_branch,
 	            'Create a branch in the SVN repository',
@@ -256,6 +257,27 @@ my %cmd = (
 		{} ],
 );
 
+use Term::ReadLine;
+package FakeTerm;
+sub new {
+	my ($class, $reason) = @_;
+	return bless \$reason, shift;
+}
+sub readline {
+	my $self = shift;
+	die "Cannot use readline on FakeTerm: $$self";
+}
+package main;
+
+my $term = eval {
+	$ENV{"GIT_SVN_NOTTY"}
+		? new Term::ReadLine 'git-svn', \*STDIN, \*STDOUT
+		: new Term::ReadLine 'git-svn';
+};
+if ($@) {
+	$term = new FakeTerm "$@: going non-interactive";
+}
+
 my $cmd;
 for (my $i = 0; $i < @ARGV; $i++) {
 	if (defined $cmd{$ARGV[$i]}) {
@@ -366,6 +388,36 @@ sub version {
 	exit 0;
 }
 
+sub ask {
+	my ($prompt, %arg) = @_;
+	my $valid_re = $arg{valid_re};
+	my $default = $arg{default};
+	my $resp;
+	my $i = 0;
+
+	if ( !( defined($term->IN)
+            && defined( fileno($term->IN) )
+            && defined( $term->OUT )
+            && defined( fileno($term->OUT) ) ) ){
+		return defined($default) ? $default : undef;
+	}
+
+	while ($i++ < 10) {
+		$resp = $term->readline($prompt);
+		if (!defined $resp) { # EOF
+			print "\n";
+			return defined $default ? $default : undef;
+		}
+		if ($resp eq '' and defined $default) {
+			return $default;
+		}
+		if (!defined $valid_re or $resp =~ /$valid_re/) {
+			return $resp;
+		}
+	}
+	return undef;
+}
+
 sub do_git_init_db {
 	unless (-d $ENV{GIT_DIR}) {
 		my @init_db = ('init');
@@ -746,6 +798,28 @@ sub cmd_dcommit {
 		     "If these changes depend on each other, re-running ",
 		     "without --no-rebase may be required."
 	}
+
+	if (defined $_interactive){
+		my $ask_default = "y";
+		foreach my $d (@$linear_refs){
+			print "debug : d = $d\n";
+			my ($fh, $ctx) = command_output_pipe(qw(show --summary), "$d");
+			while (<$fh>){
+				print $_;
+			}
+			command_close_pipe($fh, $ctx);
+			$_ = ask("Commit this patch to SVN? ([y]es (default)|[n]o|[q]uit|[a]ll): ",
+			         valid_re => qr/^(?:yes|y|no|n|quit|q|all|a)/i,
+			         default => $ask_default);
+			die "Commit this patch reply required" unless defined $_;
+			if (/^[nq]/i) {
+				exit(0);
+			} elsif (/^a/i) {
+				last;
+			}
+		}
+	}
+
 	my $expect_url = $url;
 
 	my $push_merge_info = eval {
diff --git a/t/t9162-git-svn-dcommit-interactive.sh b/t/t9162-git-svn-dcommit-interactive.sh
new file mode 100644
index 0000000..e38d9fa
--- /dev/null
+++ b/t/t9162-git-svn-dcommit-interactive.sh
@@ -0,0 +1,64 @@
+#!/bin/sh
+#
+# Copyright (c) 2011 Frédéric Heitzmann
+
+test_description='git svn dcommit --interactive series'
+. ./lib-git-svn.sh
+
+test_expect_success 'initialize repo' '
+	svn_cmd mkdir -m"mkdir test-interactive" "$svnrepo/test-interactive" &&
+	git svn clone "$svnrepo/test-interactive" test-interactive &&
+	cd test-interactive &&
+	touch foo && git add foo && git commit -m"foo: first commit" &&
+	git svn dcommit
+	'
+
+test_expect_success 'answers: y [\n] yes' '
+	(
+		echo "change #1" >> foo && git commit -a -m"change #1" &&
+		echo "change #2" >> foo && git commit -a -m"change #2" &&
+		echo "change #3" >> foo && git commit -a -m"change #3" &&
+		( echo "y
+
+y" | GIT_SVN_NOTTY=1 git svn dcommit --interactive ) &&
+		test $(git rev-parse HEAD) = $(git rev-parse remotes/git-svn)
+	)
+	'
+
+test_expect_success 'answers: yes yes no' '
+	(
+		echo "change #1" >> foo && git commit -a -m"change #1" &&
+		echo "change #2" >> foo && git commit -a -m"change #2" &&
+		echo "change #3" >> foo && git commit -a -m"change #3" &&
+		( echo "yes
+yes
+no" | GIT_SVN_NOTTY=1 git svn dcommit --interactive ) &&
+		test $(git rev-parse HEAD^^^) = $(git rev-parse remotes/git-svn) &&
+		git reset --hard remotes/git-svn
+	)
+	'
+
+test_expect_success 'answers: yes quit' '
+	(
+		echo "change #1" >> foo && git commit -a -m"change #1" &&
+		echo "change #2" >> foo && git commit -a -m"change #2" &&
+		echo "change #3" >> foo && git commit -a -m"change #3" &&
+		( echo "yes
+quit" | GIT_SVN_NOTTY=1 git svn dcommit --interactive ) &&
+		test $(git rev-parse HEAD^^^) = $(git rev-parse remotes/git-svn) &&
+		git reset --hard remotes/git-svn
+	)
+	'
+
+test_expect_success 'answers: all' '
+	(
+		echo "change #1" >> foo && git commit -a -m"change #1" &&
+		echo "change #2" >> foo && git commit -a -m"change #2" &&
+		echo "change #3" >> foo && git commit -a -m"change #3" &&
+		( echo "all" | GIT_SVN_NOTTY=1 git svn dcommit --interactive ) &&
+		test $(git rev-parse HEAD) = $(git rev-parse remotes/git-svn) &&
+		git reset --hard remotes/git-svn
+	)
+	'
+
+test_done
-- 
1.7.7.rc0.200.g2f9e2e

^ permalink raw reply related

* Re: [PATCH] ls-remote: a lone "-h" is asking for help
From: Junio C Hamano @ 2011-09-16 20:53 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: git
In-Reply-To: <CAGdFq_jc4YDaD+NL6_+buCrOt2yAK+-_MDOJQU5qnS13P65CzQ@mail.gmail.com>

Sverre Rabbelier <srabbelier@gmail.com> writes:

> ... Should we have some way to queue patches like
> this, or would someone have to resend after the appropriate release?

For this particular case, I do not think it is worth _my_ time to keep
maintaining that patch.

It is understandable that a new person will be hurt if

	$ git ls-remote -h

does not give a short-help message and instead try to contact and show the
origin repository which may not even exist, and that is why I sent a fix.

But would anybody gets hurt if

	$ git ls-remote -h origin
        $ git ls-remote -h git://git.kernel.org/pub/git/git.git

kept working as they do today, given that we do not advertise -h as
a synonym in "git ls-remote -h" output?

That is why I said "I am not opposed to", and not "I'd volunteer to do
that". It's not worth my time, but since you brought it up, you may care
more about it.

^ permalink raw reply

* Re: [PATCH/RFC] bash: add --word-diff option to diff [AND --set-upstream TO push] auto-completion
From: Jonathan Nieder @ 2011-09-16 20:47 UTC (permalink / raw)
  To: Rodrigo Rosenfeld Rosas; +Cc: SZEDER Gábor, Thomas Rast, git
In-Reply-To: <4E737199.1000107@yahoo.com.br>

Rodrigo Rosenfeld Rosas wrote:

> While on the topic, it would also be interesting to add "--set-upstream" to
> "git push" autocompletion. Don't you agree?

Yes, of course.

^ permalink raw reply

* Re: [PATCH] Disambiguate duplicate t9160* tests
From: Junio C Hamano @ 2011-09-16 20:41 UTC (permalink / raw)
  To: Frédéric Heitzmann; +Cc: git, bjacobs, rchen
In-Reply-To: <1316202903-5085-1-git-send-email-frederic.heitzmann@gmail.com>

Thanks.

^ permalink raw reply

* Re: zealous git convert determined to set up git server
From: Andreas Krey @ 2011-09-16 20:40 UTC (permalink / raw)
  To: Sitaram Chamarty; +Cc: Git List
In-Reply-To: <CAMK1S_jK2w8v4ushsZztQ0QY-eZq8axso-DpmCCvA=Gp7iXkBg@mail.gmail.com>

On Fri, 16 Sep 2011 22:30:35 +0000, Sitaram Chamarty wrote:
...
> Well it *is* pretty darn powerful (I'm the author; allow me some
> preening!) but I believe the real reason is that it is the most
> *transparent* solution.

It well looks so, but I have a question: It seems that it assumes a
flat set of *.git repos. Unfortunately my current setup has the repos
in a hierarchy, like area/sub/repo.git, and I don't want everyone to
change their local repo configs. Is it possible to keep it like that
(and consequently have 'repo area/sub/repo' lines) when putting it
under gitolite control?

Andreas

^ permalink raw reply

* Re: [PATCH] ls-remote: a lone "-h" is asking for help
From: Sverre Rabbelier @ 2011-09-16 20:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v1uvgfcur.fsf@alter.siamese.dyndns.org>

Heya,

On Fri, Sep 16, 2011 at 22:31, Junio C Hamano <gitster@pobox.com> wrote:
> I am not opposed to. We should do the usual "start from warning and then
> deprecate" dance, but I do not think we would want to have a "I want the
> old behaviour, please keep it" configuration, especially if we are talking
> about a big version bump like 2.0.
>
> The first step would look something like this, on top of the previous
> patch.

Makes sense.

I remember some sort of "this is for post 1.7.x" section in what's
cooking at some point. Should we have some way to queue patches like
this, or would someone have to resend after the appropriate release?

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCH] ls-remote: a lone "-h" is asking for help
From: Junio C Hamano @ 2011-09-16 20:31 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: git
In-Reply-To: <CAGdFq_hug3zNwvDZ3c8iG-F8jJSuxsuFghMWtWTmUTdfTrWiqg@mail.gmail.com>

Sverre Rabbelier <srabbelier@gmail.com> writes:

> On Fri, Sep 16, 2011 at 21:35, Junio C Hamano <gitster@pobox.com> wrote:
>> Sverre Rabbelier <srabbelier@gmail.com> writes:
>>> Should we really have "-h" as a short for anything other than "--help"
>>> in the first place?
> ...
> Does git 2.0 count?

I am not opposed to. We should do the usual "start from warning and then
deprecate" dance, but I do not think we would want to have a "I want the
old behaviour, please keep it" configuration, especially if we are talking
about a big version bump like 2.0.

The first step would look something like this, on top of the previous
patch.

 builtin/ls-remote.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 41c88a9..dabe21e 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -28,6 +28,12 @@ static int tail_match(const char **pattern, const char *path)
 	return 0;
 }
 
+static void warn_h_deprecation(void)
+{
+	warning("Using -h as synonym for --heads is deprecated");
+	warning("and will be removed in future versions of Git.");
+}
+
 int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 {
 	int i;
@@ -64,6 +70,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 			}
 			if (!strcmp("--heads", arg) || !strcmp("-h", arg)) {
 				flags |= REF_HEADS;
+				if (!arg[2])
+					warn_h_deprecation();
 				continue;
 			}
 			if (!strcmp("--refs", arg)) {

^ permalink raw reply related

* Re: [PATCH] gitweb: Strip non-printable characters from syntax highlighter output
From: Junio C Hamano @ 2011-09-16 20:24 UTC (permalink / raw)
  To: Jakub Narebski
  Cc: Christopher M. Fuhrman, git, Christopher Wilson, Sylvain Rabot
In-Reply-To: <201109162058.51132.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

> So actually now I see that while this line is good to have in esc_html(),
> it is not really necessary in sanitize().
>
> But anyway we don't want to replace undef with an empty string; undef is
> (usually) an error, and we want to catch it, not to hide it.

Heh, get off your high horse---whoever wrote such a caller that calls the
subroutine and uses its result without checking it against undef is not
qualified to make such a statement. I do not think letting "perl -w"
notice and complain about an attempt to concatenate undef with string
counts as "catching" it.

^ permalink raw reply

* Re: [PATCH] mergetool: Use args as pathspec to unmerged files
From: Junio C Hamano @ 2011-09-16 20:17 UTC (permalink / raw)
  To: Jonathon Mah; +Cc: git, Dan McGee, David Aguilar
In-Reply-To: <C5AD8BFC-DA48-4CE9-B821-D0076825F33C@JonathonMah.com>

Jonathon Mah <me@JonathonMah.com> writes:

> Mergetool now treats its path arguments as a pathspec (like other git
> subcommands), restricting action to the given files and directories.
> Files matching the pathspec are filtered so mergetool only acts on
> unmerged paths; previously it would assume each path argument was in an
> unresolved state, and get confused when it couldn't check out their
> other stages.
>
> Running "git mergetool subdir" will prompt to resolve all conflicted
> blobs under subdir.
>
> Signed-off-by: Jonathon Mah <me@JonathonMah.com>

It looks like this simplifies the code quote a bit and make the result
easier to follow ;-)  Nicely done.

As nobody reads from a pipe in while loop and runs merge_file or prompt
inside, there no longer is a reason to redirect the original standard
input and make it available, hence we could perhaps add this patch on top
of your change.

Ack from mergetool/difftool folks?

Thanks.

 git-mergetool.sh |   10 ++++------
 1 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index 83551c7..0a06bde 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -362,20 +362,18 @@ if test -z "$files" ; then
     exit 0
 fi
 
-# Save original stdin
-exec 3<&0
-
 printf "Merging:\n"
 printf "$files\n"
 
 IFS='
-'; for i in $files
+'
+for i in $files
 do
     if test $last_status -ne 0; then
-	prompt_after_failed_merge <&3 || exit 1
+	prompt_after_failed_merge || exit 1
     fi
     printf "\n"
-    merge_file "$i" <&3
+    merge_file "$i"
     last_status=$?
     if test $last_status -ne 0; then
 	rollup_status=1

^ permalink raw reply related

* [PATCH] Disambiguate duplicate t9160* tests
From: Frédéric Heitzmann @ 2011-09-16 19:55 UTC (permalink / raw)
  To: git; +Cc: gitster, bjacobs, rchen, Frédéric Heitzmann

1e5814f created t9160-git-svn-mergeinfo-push.sh on 11/9/7
40a1530 createds t9160-git-svn-preserve-empty-dirs.sh on 11/7/20
The former test script is renumbered to t9161.

Signed-off-by: Frédéric Heitzmann <frederic.heitzmann@gmail.com>
---

  I did not find any explicit objection in t/README but it looks odd.

 ...nfo-push.sh => t9161-git-svn-mergeinfo-push.sh} |    0
 1 files changed, 0 insertions(+), 0 deletions(-)
 rename t/{t9160-git-svn-mergeinfo-push.sh => t9161-git-svn-mergeinfo-push.sh} (100%)

diff --git a/t/t9160-git-svn-mergeinfo-push.sh b/t/t9161-git-svn-mergeinfo-push.sh
similarity index 100%
rename from t/t9160-git-svn-mergeinfo-push.sh
rename to t/t9161-git-svn-mergeinfo-push.sh
-- 
1.7.7.rc0.200.g2f9e2e

^ permalink raw reply

* Re: [PATCH] ls-remote: a lone "-h" is asking for help
From: Sverre Rabbelier @ 2011-09-16 19:44 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vehzgfffw.fsf@alter.siamese.dyndns.org>

Heya,

On Fri, Sep 16, 2011 at 21:35, Junio C Hamano <gitster@pobox.com> wrote:
> Sverre Rabbelier <srabbelier@gmail.com> writes:
>> Should we really have "-h" as a short for anything other than "--help"
>> in the first place?
>
> You have been here long enough to know the answer to that question, no?

Yeah, you're right :).

> The answer would be different if you are starting a new project from
> scratch and if you are talking about a project with existing userbase.

Does git 2.0 count?

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCH] ls-remote: a lone "-h" is asking for help
From: Junio C Hamano @ 2011-09-16 19:35 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: git
In-Reply-To: <CAGdFq_h474OrLzP+CHj_eSdSp53n8x7jz1ORT16dOhvRdQMP+g@mail.gmail.com>

Sverre Rabbelier <srabbelier@gmail.com> writes:

> Should we really have "-h" as a short for anything other than "--help"
> in the first place?

You have been here long enough to know the answer to that question, no?

The answer would be different if you are starting a new project from
scratch and if you are talking about a project with existing userbase.

^ permalink raw reply

* Re: [PATCH] ls-remote: a lone "-h" is asking for help
From: Sverre Rabbelier @ 2011-09-16 19:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vobykfj7g.fsf@alter.siamese.dyndns.org>

Heya,

On Fri, Sep 16, 2011 at 20:14, Junio C Hamano <gitster@pobox.com> wrote:
> It does not give a short-help for the command. Instead because "-h" is a
> synonym for "--heads", it runs "git ls-remote --heads", and because there
> is no remote specified on the command line, we run it against the default
> "origin" remote, hence end up doing the same as

Should we really have "-h" as a short for anything other than "--help"
in the first place?

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* [PATCH v3] request-pull: state what commit to expect
From: Junio C Hamano @ 2011-09-16 19:04 UTC (permalink / raw)
  To: git; +Cc: Linus Torvalds
In-Reply-To: <7vobynui8a.fsf@alter.siamese.dyndns.org>

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

> Linus Torvalds <torvalds@linux-foundation.org> writes:
>
>> I think that would probably be a good idea, although I'd actually
>> prefer you to be more verbose, and more human-friendly, and actually
>> talk about the commit in a readable way. Get rid of the *horrible*
>> BRANCH-NOT-VERIFIED message...
>>
>>  Top commit 1f51b001cccf: "Merge branches 'cns3xxx/fixes',
>>  'omap/fixes' and 'davinci/fixes' into fixes"
>>
>>  and at *that* point you might have a "UNVERIFIED" notice for people
>> to check if they forgot to push.
>
> That UNVERIFIED thing was neither my favorite nor my idea, and I'd happily
> rip it out in any second ;-)

So this is the third round.

-- >8 --
The message gives a detailed explanation of the commit the requester based
the changes on, but lacks information that is necessary for the person who
performs a fetch & merge in order to verify that the correct branch was
fetched when responding to the pull request.

Add a few more lines to describe the commit at the tip expected to be
fetched to the same level of detail as the base commit.

Also update the warning message slightly when the script notices that the
commit may not have been pushed.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

A UI wart that we cannot fix without breaking backward compatibility is
that the "end" parameter (which defaults to HEAD and is assigned to $head
variable in the script) the requestor uses from the command line names a
commit (often the name of a local branch), but for the purpose of telling
which ref to pull from the public repository, that is a _wrong_ thing to
give to the recipient.

Because the act of generating a request-pull message and the act of
pushing to the public repository are not linked in any way, the script
does not know _how_ the requestor caused (or intends to cause) the commit
to sit at the tip of which branch. There is no guarantee that a lazy "git
push" that relies on the configured refspec will be (or have been) used,
so even parsing the output from "git push -n --porcelain -v $there" would
not tell the script which branch the commit to be pulled is to be pushed
out to, or if the branch is consistent with the request message.

The use of "git ls-remote" in the script and picking one of the refs that
matches the commit object at random from its output is unsatisfactory, but
that is unfortunately the best this script could do without correcting the
design mistake and redefining what the "end" parameter means.

If we can break the backward compatibility and redefine that the "end"
parameter now means the name of the branch at the public repository, it
would make the operation a lot more robust.  We could then:

 - $branch is what is given by the end user (it is an error not to give
   the "end" parameter);
 - run "git ls-remote $url $head" to find $headrev;
 - generate the message and shortlog using the information obtained from
   $url; and
 - get rid of "did you forget to push" message.

We could allow adding yet another argument which names a commit object
locally, and make sure if the $headrev observed by ls-remote does not
match it.

---
 git-request-pull.sh     |   34 +++++++++++++++++++---------------
 t/t5150-request-pull.sh |    6 ++++++
 2 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/git-request-pull.sh b/git-request-pull.sh
index afb75e8..438e7eb 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -35,7 +35,7 @@ do
 	shift
 done
 
-base=$1 url=$2 head=${3-HEAD}
+base=$1 url=$2 head=${3-HEAD} status=0
 
 test -n "$base" && test -n "$url" || usage
 baserev=$(git rev-parse --verify "$base"^0) &&
@@ -51,25 +51,29 @@ find_matching_branch="/^$headrev	"'refs\/heads\//{
 }'
 branch=$(git ls-remote "$url" | sed -n -e "$find_matching_branch")
 url=$(git ls-remote --get-url "$url")
-if test -z "$branch"
-then
-	echo "warn: No branch of $url is at:" >&2
-	git log --max-count=1 --pretty='tformat:warn:   %h: %s' $headrev >&2
-	echo "warn: Are you sure you pushed $head there?" >&2
-	echo >&2
-	echo >&2
-	branch=..BRANCH.NOT.VERIFIED..
-	status=1
-fi
 
 git show -s --format='The following changes since commit %H:
 
   %s (%ci)
 
-are available in the git repository at:' $baserev &&
-echo "  $url $branch" &&
-echo &&
+are available in the git repository at:
+' $baserev &&
+echo "  $url${branch+ $branch}" &&
+git show -s --format='
+for you to fetch changes up to %H:
+
+  %s (%ci)
+
+----------------------------------------------------------------' $headrev &&
 
 git shortlog ^$baserev $headrev &&
-git diff -M --stat --summary $patch $merge_base..$headrev || exit
+git diff -M --stat --summary $patch $merge_base..$headrev || status=1
+
+if test -z "$branch"
+then
+	echo "warn: No branch of $url is at:" >&2
+	git show -s --format='warn:   %h: %s' $headrev >&2
+	echo "warn: Are you sure you pushed '$head' there?" >&2
+	status=1
+fi
 exit $status
diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh
index 9cc0a42..5bd1682 100755
--- a/t/t5150-request-pull.sh
+++ b/t/t5150-request-pull.sh
@@ -193,8 +193,14 @@ test_expect_success 'pull request format' '
 	  SUBJECT (DATE)
 
 	are available in the git repository at:
+
 	  URL BRANCH
 
+	for you to fetch changes up to OBJECT_NAME:
+
+	  SUBJECT (DATE)
+
+	----------------------------------------------------------------
 	SHORTLOG
 
 	DIFFSTAT
-- 
1.7.7.rc1.3.g559357

^ permalink raw reply related

* Re: [PATCH] gitweb: Strip non-printable characters from syntax highlighter output
From: Jakub Narebski @ 2011-09-16 18:58 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Christopher M. Fuhrman, git, Christopher Wilson, Sylvain Rabot
In-Reply-To: <7vwrd8fnxr.fsf@alter.siamese.dyndns.org>

On Fri, 16 Sep 2011, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:

> Micronit:
> 
> > +# Sanitize for use in XHTML + application/xml+xhtm (valid XML 1.0)
> > +sub sanitize {
> > +	my $str = shift;
> > +
> > +	return undef unless defined $str;
> 
> Given that the _whole_ point of this subroutine is to make $str safe for
> printing, wouldn't you want to either (1) die, declaring that feeding an
> undef to this subroutine is a programming error, or (2) return an empty
> string?

Well, that

	return undef unless defined $str;

line is copy'n'paste (as is most of sanitize() body) from esc_html().
This line was added in 1df4876 (gitweb: Protect escaping functions against
calling on undef, 2010-02-07) with the following explanation

    This is a bit of future-proofing esc_html and friends: when called
    with undefined value they would now would return undef... which would
    probably mean that error would still occur, but closer to the source
    of problem.
    
    This means that we can safely use
      esc_html(shift) || "Internal Server Error"
    in die_error() instead of
      esc_html(shift || "Internal Server Error")

So actually now I see that while this line is good to have in esc_html(),
it is not really necessary in sanitize().

But anyway we don't want to replace undef with an empty string; undef is
(usually) an error, and we want to catch it, not to hide it.
 
> Given that the input to this function is from the result of feeding $line
> to untabify, which relies on $line being defined, and that $line comes
> from "while (my $line = <$fd>)" (and then chomp $line), it may be Ok for
> this subroutine to make the same assumption as untabify makes.

Right.

Passing undef to sanitize() is usually an error, and we don't want to hide
it.  We want for gitweb test to detect it.

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH 2/2] grep --no-index: don't use git standard exclusions
From: Bert Wesarg @ 2011-09-16 18:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmxe5pp4n.fsf@alter.siamese.dyndns.org>

On Thu, Sep 15, 2011 at 21:44, Junio C Hamano <gitster@pobox.com> wrote:
> Bert Wesarg <bert.wesarg@googlemail.com> writes:
>
>> On Wed, Jul 20, 2011 at 22:57, Junio C Hamano <gitster@pobox.com> wrote:
>>>  - Since 3081623 (grep --no-index: allow use of "git grep" outside a git
>>>   repository, 2010-01-15) and 59332d1 (Resurrect "git grep --no-index",
>>>   2010-02-06), "grep --no-index" incorrectly paid attention to the
>>>   exclude patterns. We shouldn't have, and we'd fix that bug.
>>
>> Fix this bug.
>
> On a busy list like this, it is brutal to withhold the better clues you
> certainly had when you wrote this message that would help people to locate
> the original message you are quoting, and instead forcing everybody to go
> back 5000 messages in the archive to find it. E.g.
>
>    http://article.gmane.org/gmane.comp.version-control.git/177548
>    http://mid.gmane.org/7vzkk86577.fsf@alter.siamese.dyndns.org
>
> Or perhaps have
>
>    References: <7vzkk86577.fsf@alter.siamese.dyndns.org>
>
> in the header.

Sorry for this patch with insufficient references and context. I
realized it too late, and the time was short.

>
> As to the patch, I think this addresses only one fourth of the issue
> identified in that discussion (it is a good starting point, though).
>

I thought to split the bug fixing from the new features. I already
implemented --exclude-standard, including --exclude=<patter>,
--exclude-from=<file> and --exclude-per-directory=<file>. But it's not
ready because of missing tests and documentation. So I just spilled
this bug fix patch out and will now work on the next part.

> With this change, it would now make sense to teach --[no-]exclude-standard
> to "git grep", and "--exclude-standard" is immediately useful when used
> with "--no-index". When we add "git grep --untracked-too" (which lets us
> search in the working tree), people can add "--no-exclude-standard" to the
> command line to say "I want to find the needle even from an ignored file".

Would '--untracked-too' only be a synonym for '--no-index
--exclude-standard', i.e. the current behavior?

Bert

>
> Thanks.
>

^ 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