Git development
 help / color / mirror / Atom feed
* [PATCH 2/2] completion: unite --format and --pretty for 'log' and 'show'
From: SZEDER Gábor @ 2011-10-08  1:09 UTC (permalink / raw)
  To: Junio C Hamano, Shawn O. Pearce; +Cc: Teemu Matilainen, git
In-Reply-To: <20111008010432.GA11561@goldbirke>

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |   18 ++++--------------
 1 files changed, 4 insertions(+), 14 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 98282435..b36f9e70 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1553,14 +1553,9 @@ _git_log ()
 		merge="--merge"
 	fi
 	case "$cur" in
-	--pretty=*)
+	--pretty=*|--format=*)
 		__gitcomp "$__git_log_pretty_formats $(__git_pretty_aliases)
-			" "" "${cur##--pretty=}"
-		return
-		;;
-	--format=*)
-		__gitcomp "$__git_log_pretty_formats $(__git_pretty_aliases)
-			" "" "${cur##--format=}"
+			" "" "${cur#*=}"
 		return
 		;;
 	--date=*)
@@ -2365,14 +2360,9 @@ _git_show ()
 	__git_has_doubledash && return
 
 	case "$cur" in
-	--pretty=*)
-		__gitcomp "$__git_log_pretty_formats $(__git_pretty_aliases)
-			" "" "${cur##--pretty=}"
-		return
-		;;
-	--format=*)
+	--pretty=*|--format=*)
 		__gitcomp "$__git_log_pretty_formats $(__git_pretty_aliases)
-			" "" "${cur##--format=}"
+			" "" "${cur#*=}"
 		return
 		;;
 	--*)
-- 
1.7.7.187.ga41de

^ permalink raw reply related

* [PATCH 1/2] completion: unite --reuse-message and --reedit-message for 'notes'
From: SZEDER Gábor @ 2011-10-08  1:06 UTC (permalink / raw)
  To: Junio C Hamano, Shawn O. Pearce; +Cc: Teemu Matilainen, git
In-Reply-To: <20111008010432.GA11561@goldbirke>

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 847e6e9a..98282435 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1668,11 +1668,9 @@ _git_notes ()
 			;;
 		esac
 		;;
-	add,--reuse-message=*|append,--reuse-message=*)
-		__gitcomp "$(__git_refs)" "" "${cur##--reuse-message=}"
-		;;
+	add,--reuse-message=*|append,--reuse-message=*|\
 	add,--reedit-message=*|append,--reedit-message=*)
-		__gitcomp "$(__git_refs)" "" "${cur##--reedit-message=}"
+		__gitcomp "$(__git_refs)" "" "${cur#*=}"
 		;;
 	add,--*|append,--*)
 		__gitcomp '--file= --message= --reedit-message=
-- 
1.7.7.187.ga41de

^ permalink raw reply related

* Re: [PATCH 1/3] completion: unite --reuse-message and --reedit-message handling
From: SZEDER Gábor @ 2011-10-08  1:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Teemu Matilainen, git, Shawn O. Pearce
In-Reply-To: <7vd3e9yap7.fsf@alter.siamese.dyndns.org>

Hi,

On Thu, Oct 06, 2011 at 04:14:28PM -0700, Junio C Hamano wrote:
> All three patches make sense to me. Thanks.

This is a good cleanup/deduplication; there are a few places where
similar cleanup can be done.  Since this conflicted with one of my
more intrusive WIP branches, I figured it's better to clean up those
first, too.


Best,
Gábor

^ permalink raw reply

* Re: [PATCH] fmt-merge-msg: use branch.$name.description
From: Jakub Narebski @ 2011-10-08  0:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Michael J Gruber, Jonathan Nieder, git, Jeff King
In-Reply-To: <7vobxstt4w.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
> > Alternatively, one could store the description in a blob and refer to
> > that directly, of course. I.e., have
> >
> > refs/description/foo
> >
> > point to a blob whose content is the description of the ref
> >
> > ref/foo
> >
> > That would be unversioned, and one could decide more easily which
> > descriptions to share. (A notes tree you either push or don't.)
[...]

> But it remains that any of these approaches assume branch names are
> universal. Unlike other systems, what we call branches do not have their
> own identity, so if you really want to go that route (and we _might_ need
> to in the longer term, but I am not convinced at this point yet), you
> would first need to define how that local namespace would look like, how
> people interact with it, etc. It might be just the matter of declaring a
> convention e.g. "Among people who meet at this central repository,
> everybody must map the branches identically to their local branch
> namespace, and all sharing must go through the central repository", and
> calling a tuple <central repository URL, branch name in that repository>
> with a name that cannot be confused with "branch" (so "remote branch" is
> out), such as "(development) track".

Well, git could by default imply that 'refs/heads/*:refs/remotes/foo/*'
implies 'refs/description/*:refs/remote-descriptions/foo/*'...

...one more argument for hierarchical remote-tracking refs namespace,
i.e. 'refs/remotes/foo/refs/heads/*', and not current 'refs/remotes/foo/*'

Just my 3 eurocents^W groszy.
-- 
Jakub Narębski

^ permalink raw reply

* Re: How pretty is pretty? git cat-file -p inconsistency
From: Jakub Narebski @ 2011-10-07 23:50 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <4E8F6088.8060300@drmicha.warpmail.net>

Michael J Gruber <git@drmicha.warpmail.net> writes:

[cut]
> I never knew how ugly the output of "git tag-file tree sha1" is. I guess
> it's the type of object whose format I don't know... We don't have an
> object format description in Doc/technical, do we? tree.c doesn't tell
> me much.

I had to handle this in my attempt to write "git blame <directory>" in Perl,
which was using `git cat-file --batch`, and that gives raw data and not
pretty-printed.

Tree object consist of zero or more entries.  Each item consist of mode,
filename, and sha1:

  <mode> SPC <filename> NUL <sha1>

where

1. <mode> is variable-length (!) text (!) containing mode of an
   entry. It encodes type of entry: if it is blob (including special
   case: symbolic link), tree i.e. directory, or a commit
   i.e. submodule.  Does not include leading zeros.

2. <filename> is variable-length null-terminated ("\0") name of a file
   or directory, or name of directory where submodule is attached

3. <sha1> is 40-bytes _binary_ identifier.

HTH
-- 
Jakub Narębski

^ permalink raw reply

* Re: [PATCH] git-svn: Allow certain refs to be ignored
From: Michael Olson @ 2011-10-07 23:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Eric Wong, git
In-Reply-To: <7vvcs0s7xa.fsf@alter.siamese.dyndns.org>

On Fri, Oct 7, 2011 at 4:23 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Asking Eric to comment when he has time to do so.
>
> I find these pattern matches that are not anchored on either side
> somewhat disturbing (e.g. --ignore-refs=master would ignore master2)
> but ignore-paths codepath seems to follow the same pattern, so perhaps it
> is in line with what git-svn users want. I dunno.

My own personal use of this takes a list of patterns, concatenates
them into one giant pattern, adds '^', '$', and writes it out to
.gitconfig.  So I don't really have a preference, other than to make
both options consistent.

-- 
Michael Olson  |  http://mwolson.org/

^ permalink raw reply

* Re: [PATCH] git-svn: Allow certain refs to be ignored
From: Junio C Hamano @ 2011-10-07 23:23 UTC (permalink / raw)
  To: Eric Wong; +Cc: Michael Olson, git
In-Reply-To: <CAN4ruPiSgY+LPdDgS021WQyoHMuNrJDzrqMuCt9G5qfZ=XtjoQ@mail.gmail.com>

Asking Eric to comment when he has time to do so.

I find these pattern matches that are not anchored on either side 
somewhat disturbing (e.g. --ignore-refs=master would ignore master2)
but ignore-paths codepath seems to follow the same pattern, so perhaps it
is in line with what git-svn users want. I dunno.

Michael Olson <mwolson@gnu.org> writes:

> Implement a new --ignore-refs option which specifies a regex of refs
> to ignore while importing svn history.
>
> This is a useful supplement to the --ignore-paths option, as that
> option only operates on the contents of branches and tags, not the
> branches and tags themselves.
>
> Signed-off-by: Michael Olson <mwolson@gnu.org>
> ---
> Re-sent by request of Piotr Krukowiecki.  This is against v1.7.4.1,
> and I've been using it stably for a while.
>
>  git-svn.perl |   38 +++++++++++++++++++++++++++++++++-----
>  1 files changed, 33 insertions(+), 5 deletions(-)
>
> diff --git a/git-svn.perl b/git-svn.perl
> index 177dd25..541fa2d 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -90,7 +90,8 @@ $_q ||= 0;
>  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,
> -                    'ignore-paths=s' => \$SVN::Git::Fetcher::_ignore_regex );
> +                    'ignore-paths=s' => \$SVN::Git::Fetcher::_ignore_regex,
> +                    'ignore-refs=s' => \$Git::SVN::Ra::_ignore_refs_regex );
>  my %fc_opts = ( 'follow-parent|follow!' => \$Git::SVN::_follow_parent,
>  		'authors-file|A=s' => \$_authors,
>  		'authors-prog=s' => \$_authors_prog,
> @@ -380,9 +381,12 @@ sub do_git_init_db {
>  		command_noisy('config', "$pfx.$i", $icv{$i});
>  		$set = $i;
>  	}
> -	my $ignore_regex = \$SVN::Git::Fetcher::_ignore_regex;
> -	command_noisy('config', "$pfx.ignore-paths", $$ignore_regex)
> -		if defined $$ignore_regex;
> +	my $ignore_paths_regex = \$SVN::Git::Fetcher::_ignore_regex;
> +	command_noisy('config', "$pfx.ignore-paths", $$ignore_paths_regex)
> +		if defined $$ignore_paths_regex;
> +	my $ignore_refs_regex = \$Git::SVN::Ra::_ignore_refs_regex;
> +	command_noisy('config', "$pfx.ignore-refs", $$ignore_refs_regex)
> +		if defined $$ignore_refs_regex;
>  }
>
>  sub init_subdir {
> @@ -1831,6 +1835,8 @@ sub read_all_remotes {
>  			$r->{$1}->{svm} = {};
>  		} elsif (m!^(.+)\.url=\s*(.*)\s*$!) {
>  			$r->{$1}->{url} = $2;
> +		} elsif (m!^(.+)\.ignore-refs=\s*(.*)\s*$!) {
> +			$r->{$1}->{ignore_refs_regex} = $2;
>  		} elsif (m!^(.+)\.(branches|tags)=$svn_refspec$!) {
>  			my ($remote, $t, $local_ref, $remote_ref) =
>  			                                     ($1, $2, $3, $4);
> @@ -1867,6 +1873,16 @@ sub read_all_remotes {
>  		}
>  	} keys %$r;
>
> +	foreach my $remote (keys %$r) {
> +		foreach ( grep { defined $_ }
> +			  map { $r->{$remote}->{$_} } qw(branches tags) ) {
> +			foreach my $rs ( @$_ ) {
> +				$rs->{ignore_refs_regex} =
> +				    $r->{$remote}->{ignore_refs_regex};
> +			}
> +		}
> +	}
> +
>  	$r;
>  }
>
> @@ -4876,7 +4892,7 @@ sub apply_diff {
>  }
>
>  package Git::SVN::Ra;
> -use vars qw/@ISA $config_dir $_log_window_size/;
> +use vars qw/@ISA $config_dir $_ignore_refs_regex $_log_window_size/;
>  use strict;
>  use warnings;
>  my ($ra_invalid, $can_do_switch, %ignored_err, $RA);
> @@ -5334,6 +5350,17 @@ sub get_dir_globbed {
>  	@finalents;
>  }
>
> +# return value: 0 -- don't ignore, 1 -- ignore
> +sub is_ref_ignored {
> +	my ($g, $p) = @_;
> +	my $refname = $g->{ref}->full_path($p);
> +	return 1 if defined($g->{ignore_refs_regex}) &&
> +	            $refname =~ m!$g->{ignore_refs_regex}!;
> +	return 0 unless defined($_ignore_refs_regex);
> +	return 1 if $refname =~ m!$_ignore_refs_regex!o;
> +	return 0;
> +}
> +
>  sub match_globs {
>  	my ($self, $exists, $paths, $globs, $r) = @_;
>
> @@ -5370,6 +5397,7 @@ sub match_globs {
>  			next unless /$g->{path}->{regex}/;
>  			my $p = $1;
>  			my $pathname = $g->{path}->full_path($p);
> +			next if is_ref_ignored($g, $p);
>  			next if $exists->{$pathname};
>  			next if ($self->check_path($pathname, $r) !=
>  			         $SVN::Node::dir);

^ permalink raw reply

* Re: [WIP PATCH 0/2] Be more careful when prunning
From: Junio C Hamano @ 2011-10-07 23:00 UTC (permalink / raw)
  To: Carlos Martín Nieto; +Cc: git
In-Reply-To: <1317920187-17389-1-git-send-email-cmn@elego.de>

Carlos Martín Nieto <cmn@elego.de> writes:

> Now comes the interesting part: when --tags is given, there is no
> refspec set up, fetch just sets up a global variable. What I'm
> thinking (and going to implement after dinner, unless people cry out
> against it) is this: just before calling prune_refs, add a refspec to
> the user-provided list with the refspec refs/tags/*:refs/tags/*
> similar to what fetch_one does if you gave it "tag v1.5.6". This would
> cause us to ignore the configured refspec and keep the branches. The
> lack of '+' is most certainly on purpose. Is there anything
> fundamentally wrong with that idea?

It sounds like that the approach should work and preserve the current
"fetch --tags" semantics, but with one small caveat (which is not a
downside).

As was discussed in a few separate threads last month, in the longer term
I think we should fix the semantics of "fetch --tags" to mean "in addition
to what you usually fetch with the configured refspecs, add that all
matching refs/tags/*:refs/tags/* to the refspec" to reduce confusion.
"Only fetch tags" may make sense if you have everything else, but by
itself it is somewhat a senseless thing to do.

The semantics has been kept this way only from fear of breaking backward
compatibility, but because nobody wants to only fetch tags without
branches, this forces people to say "git fetch && git fetch --tags".

We should re-evaluate the design and change it at a major version
boundary, I would think. And when that happens, we may need to rip out the
special case you discussed above.

Thanks.

^ permalink raw reply

* [PATCH 4/4] fetch: treat --tags like refs/tags/*:refs/tags/* when pruning
From: Carlos Martín Nieto @ 2011-10-07 22:51 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano, mathstuf
In-Reply-To: <1318027869-4037-1-git-send-email-cmn@elego.de>

If --tags is specified, add that refspec to the list given to
prune_refs so it knows to treat it as a filter on what refs to
should consider for prunning. This way

    git fetch --prune --tags origin

only prunes tags and doesn't delete the branch refs.

Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
---
 builtin/fetch.c  |   23 +++++++++++++++++++++--
 t/t5510-fetch.sh |    4 ++--
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 041f79e..0f8170c 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -700,10 +700,29 @@ static int do_fetch(struct transport *transport,
 		return 1;
 	}
 	if (prune) {
-		if (ref_count)
+		/* If --tags was specified, pretend the user gave us the canonical tags refspec */
+		if (tags == TAGS_SET) {
+			const char *tags_str = "refs/tags/*:refs/tags/*";
+			struct refspec *tags_refspec, *refspec;
+
+			/* Copy the refspec and add the tags to it */
+			refspec = xcalloc(ref_count + 1, sizeof(struct refspec));
+			tags_refspec = parse_fetch_refspec(1, &tags_str);
+			memcpy(refspec, refs, ref_count * sizeof(struct refspec));
+			memcpy(&refspec[ref_count], tags_refspec, sizeof(struct refspec));
+			ref_count++;
+
+			prune_refs(refspec, ref_count, ref_map);
+
+			ref_count--;
+			/* The rest of the strings belong to fetch_one */
+			free_refspec(1, tags_refspec);
+			free(refspec);
+		} else if (ref_count) {
 			prune_refs(refs, ref_count, ref_map);
-		else
+		} else {
 			prune_refs(transport->remote->fetch, transport->remote->fetch_refspec_nr, ref_map);
+		}
 	}
 	free_refs(ref_map);
 
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 581049b..e0af4c4 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -105,7 +105,7 @@ test_expect_success 'fetch --prune with a namespace keeps other namespaces' '
 	git rev-parse origin/master
 '
 
-test_expect_failure 'fetch --prune --tags does not delete the remote-tracking branches' '
+test_expect_success 'fetch --prune --tags does not delete the remote-tracking branches' '
 	cd "$D" &&
 	git clone . prune-tags &&
 	cd prune-tags &&
@@ -116,7 +116,7 @@ test_expect_failure 'fetch --prune --tags does not delete the remote-tracking br
 	test_must_fail git rev-parse somebranch
 '
 
-test_expect_failure 'fetch --prune --tags with branch does not delete other remote-tracking branches' '
+test_expect_success 'fetch --prune --tags with branch does not delete other remote-tracking branches' '
 	cd "$D" &&
 	git clone . prune-tags-branch &&
 	cd prune-tags-branch &&
-- 
1.7.5.2.354.g349bf

^ permalink raw reply related

* [PATCH 1/4] fetch: free all the additional refspecs
From: Carlos Martín Nieto @ 2011-10-07 22:51 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano, mathstuf
In-Reply-To: <1318027869-4037-1-git-send-email-cmn@elego.de>

Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
---
 builtin/fetch.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 7a4e41c..30b485e 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -883,7 +883,7 @@ static int fetch_one(struct remote *remote, int argc, const char **argv)
 	atexit(unlock_pack);
 	refspec = parse_fetch_refspec(ref_nr, refs);
 	exit_code = do_fetch(transport, refspec, ref_nr);
-	free(refspec);
+	free_refspec(ref_nr, refspec);
 	transport_disconnect(transport);
 	transport = NULL;
 	return exit_code;
-- 
1.7.5.2.354.g349bf

^ permalink raw reply related

* [PATCHv3 0/4] Be more careful when prunning
From: Carlos Martín Nieto @ 2011-10-07 22:51 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano, mathstuf

Hello,

The first patch is not that big a deal, but it's better if we're
freeing the refspecs, we might as well free all of them.

The second patch introduces expected failures for the features that
this series fixes.

The third patch changes prune_resf and get_stale_heads so the caller
has to decide which refspecs are the appropriate ones to use. For
example, running

    git fetch --prune origin refs/heads/master:refs/heads/master

doesn't remove the other branches anymore. For a more interesting (and
believable) example, let's take

    git fetch --prune origin refs/heads/b/*:refs/heads/b/*

because you want to prune the refs inside the b/ namespace
only. Currently git will delete all the refs that aren't under that
namespace. With the second patch applied, git won't remove any refs
outside the b/ namespace.

What is probably the most usual case is covered by the forth patch,
which pretends that a "refs/tags/*:refs/tags/*" refspec was given on
the command-line. That fixes the

    git fetch --prune --tags origin

case. The non-tag refs are kept now.

Cheers,
   cmn

Carlos Martín Nieto (4):
  fetch: free all the additional refspecs
  t5510: add tests for fetch --prune
  fetch: honor the user-provided refspecs when pruning refs
  fetch: treat --tags like refs/tags/*:refs/tags/* when pruning

 builtin/fetch.c  |   33 +++++++++++++++++++++++----
 builtin/remote.c |    3 +-
 remote.c         |   66 ++++++++++++++++++++++++++++++++++++++++++++++-------
 remote.h         |    2 +-
 t/t5510-fetch.sh |   50 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 138 insertions(+), 16 deletions(-)

-- 
1.7.5.2.354.g349bf

^ permalink raw reply

* [PATCH 2/4] t5510: add tests for fetch --prune
From: Carlos Martín Nieto @ 2011-10-07 22:51 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano, mathstuf
In-Reply-To: <1318027869-4037-1-git-send-email-cmn@elego.de>

The failures will be fixed in later commits.

Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
---
 t/t5510-fetch.sh |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 50 insertions(+), 0 deletions(-)

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 7e433b1..8b5e925 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -76,6 +76,56 @@ test_expect_success "fetch test for-merge" '
 	cut -f -2 .git/FETCH_HEAD >actual &&
 	test_cmp expected actual'
 
+test_expect_success 'fetch --prune on its own works as expected' '
+	cd "$D" &&
+	git clone . prune &&
+	cd prune &&
+	git fetch origin refs/heads/master:refs/remotes/origin/extrabranch &&
+
+	git fetch --prune origin &&
+	test_must_fail git rev-parse origin/extrabranch
+'
+
+test_expect_failure 'fetch --prune with a branch name keeps branches' '
+	cd "$D" &&
+	git clone . prune-branch &&
+	cd prune-branch &&
+	git fetch origin refs/heads/master:refs/remotes/origin/extrabranch &&
+
+	git fetch --prune origin master &&
+	git rev-parse origin/extrabranch
+'
+
+test_expect_failure 'fetch --prune with a namespace keeps other namespaces' '
+	cd "$D" &&
+	git clone . prune-namespace &&
+	cd prune-namespace &&
+
+	git fetch --prune origin refs/heads/a/*:refs/remotes/origin/a/* &&
+	git rev-parse origin/master
+'
+
+test_expect_failure 'fetch --prune --tags does not delete the remote-tracking branches' '
+	cd "$D" &&
+	git clone . prune-tags &&
+	cd prune-tags &&
+	git fetch origin refs/heads/master:refs/tags/sometag &&
+
+	git fetch --prune --tags origin &&
+	git rev-parse origin/master &&
+	test_must_fail git rev-parse somebranch
+'
+
+test_expect_failure 'fetch --prune --tags with branch does not delete other remote-tracking branches' '
+	cd "$D" &&
+	git clone . prune-tags-branch &&
+	cd prune-tags-branch &&
+	git fetch origin refs/heads/master:refs/remotes/origin/extrabranch &&
+
+	git fetch --prune --tags origin master &&
+	git rev-parse origin/extrabranch
+'
+
 test_expect_success 'fetch tags when there is no tags' '
 
     cd "$D" &&
-- 
1.7.5.2.354.g349bf

^ permalink raw reply related

* [PATCH 3/4] fetch: honor the user-provided refspecs when pruning refs
From: Carlos Martín Nieto @ 2011-10-07 22:51 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano, mathstuf
In-Reply-To: <1318027869-4037-1-git-send-email-cmn@elego.de>

If the user gave us refspecs on the command line, we should use those
when deciding whether to prune a ref instead of relying on the
refspecs in the config.

Previously, running

    git fetch --prune origin refs/heads/master:refs/remotes/origin/master

would delete every other ref under the origin namespace because we
were using the refspec to filter the available refs but using the
configured refspec to figure out if a ref had been deleted on the
remote. This is clearly the wrong thing to do.

Change prune_refs and get_stale_heads to simply accept a list of
references and a list of refspecs. The caller of either function needs
to decide what refspecs should be used to decide whether a ref is
stale.

Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
---
 builtin/fetch.c  |   12 ++++++---
 builtin/remote.c |    3 +-
 remote.c         |   66 ++++++++++++++++++++++++++++++++++++++++++++++-------
 remote.h         |    2 +-
 t/t5510-fetch.sh |    4 +-
 5 files changed, 70 insertions(+), 17 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 30b485e..041f79e 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -505,10 +505,10 @@ static int fetch_refs(struct transport *transport, struct ref *ref_map)
 	return ret;
 }
 
-static int prune_refs(struct transport *transport, struct ref *ref_map)
+static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)
 {
 	int result = 0;
-	struct ref *ref, *stale_refs = get_stale_heads(transport->remote, ref_map);
+	struct ref *ref, *stale_refs = get_stale_heads(ref_map, refs, ref_count);
 	const char *dangling_msg = dry_run
 		? _("   (%s will become dangling)\n")
 		: _("   (%s has become dangling)\n");
@@ -699,8 +699,12 @@ static int do_fetch(struct transport *transport,
 		free_refs(ref_map);
 		return 1;
 	}
-	if (prune)
-		prune_refs(transport, ref_map);
+	if (prune) {
+		if (ref_count)
+			prune_refs(refs, ref_count, ref_map);
+		else
+			prune_refs(transport->remote->fetch, transport->remote->fetch_refspec_nr, ref_map);
+	}
 	free_refs(ref_map);
 
 	/* if neither --no-tags nor --tags was specified, do automated tag
diff --git a/builtin/remote.c b/builtin/remote.c
index f2a9c26..79d898b 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -349,7 +349,8 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat
 		else
 			string_list_append(&states->tracked, abbrev_branch(ref->name));
 	}
-	stale_refs = get_stale_heads(states->remote, fetch_map);
+	stale_refs = get_stale_heads(fetch_map, states->remote->fetch,
+				     states->remote->fetch_refspec_nr);
 	for (ref = stale_refs; ref; ref = ref->next) {
 		struct string_list_item *item =
 			string_list_append(&states->stale, abbrev_branch(ref->name));
diff --git a/remote.c b/remote.c
index b8ecfa5..13c9153 100644
--- a/remote.c
+++ b/remote.c
@@ -1681,36 +1681,84 @@ struct ref *guess_remote_head(const struct ref *head,
 }
 
 struct stale_heads_info {
-	struct remote *remote;
 	struct string_list *ref_names;
 	struct ref **stale_refs_tail;
+	struct refspec *refs;
+	int ref_count;
 };
 
+/* Returns 0 on success, -1 if it couldn't find a match in the refspecs. */
+static int find_in_refs(struct refspec *refs, int ref_count, struct refspec *query)
+{
+	int i;
+	struct refspec *refspec;
+
+	for (i = 0; i < ref_count; ++i) {
+		refspec = &refs[i];
+
+		/* No dst means it can't be used for prunning. */
+		if (!refspec->dst)
+			continue;
+
+		/*
+		 * No '*' means that it must match exactly. If it does
+		 * have it, try to match it against the pattern. If
+		 * the refspec matches, store the ref name as it would
+		 * appear in the server in query->src.
+		 */
+		if (!strchr(refspec->dst, '*')) {
+			if (!strcmp(query->dst, refspec->dst)) {
+				query->src = xstrdup(refspec->src);
+				return 0;
+			}
+		} else if (match_name_with_pattern(refspec->dst, query->dst,
+						    refspec->src, &query->src)) {
+			return 0;
+		}
+	}
+
+	return -1;
+}
+
 static int get_stale_heads_cb(const char *refname,
 	const unsigned char *sha1, int flags, void *cb_data)
 {
 	struct stale_heads_info *info = cb_data;
 	struct refspec refspec;
+	int ret;
 	memset(&refspec, 0, sizeof(refspec));
 	refspec.dst = (char *)refname;
-	if (!remote_find_tracking(info->remote, &refspec)) {
-		if (!((flags & REF_ISSYMREF) ||
-		    string_list_has_string(info->ref_names, refspec.src))) {
-			struct ref *ref = make_linked_ref(refname, &info->stale_refs_tail);
-			hashcpy(ref->new_sha1, sha1);
-		}
+
+	ret = find_in_refs(info->refs, info->ref_count, &refspec);
+
+	/* No matches */
+	if (ret)
+		return 0;
+
+	/*
+	 * If we did find a suitable refspec and it's not a symref and
+	 * it's not in the list of refs that currently exist in that
+	 * remote we consider it to be stale.
+	 */
+	if (!((flags & REF_ISSYMREF) ||
+	      string_list_has_string(info->ref_names, refspec.src))) {
+		struct ref *ref = make_linked_ref(refname, &info->stale_refs_tail);
+		hashcpy(ref->new_sha1, sha1);
 	}
+
+	free(refspec.src);
 	return 0;
 }
 
-struct ref *get_stale_heads(struct remote *remote, struct ref *fetch_map)
+struct ref *get_stale_heads(struct ref *fetch_map, struct refspec *refs, int ref_count)
 {
 	struct ref *ref, *stale_refs = NULL;
 	struct string_list ref_names = STRING_LIST_INIT_NODUP;
 	struct stale_heads_info info;
-	info.remote = remote;
 	info.ref_names = &ref_names;
 	info.stale_refs_tail = &stale_refs;
+	info.refs = refs;
+	info.ref_count = ref_count;
 	for (ref = fetch_map; ref; ref = ref->next)
 		string_list_append(&ref_names, ref->name);
 	sort_string_list(&ref_names);
diff --git a/remote.h b/remote.h
index 9a30a9d..5d70aff 100644
--- a/remote.h
+++ b/remote.h
@@ -164,6 +164,6 @@ struct ref *guess_remote_head(const struct ref *head,
 			      int all);
 
 /* Return refs which no longer exist on remote */
-struct ref *get_stale_heads(struct remote *remote, struct ref *fetch_map);
+struct ref *get_stale_heads(struct ref *fetch_map, struct refspec *refs, int ref_count);
 
 #endif
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 8b5e925..581049b 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -86,7 +86,7 @@ test_expect_success 'fetch --prune on its own works as expected' '
 	test_must_fail git rev-parse origin/extrabranch
 '
 
-test_expect_failure 'fetch --prune with a branch name keeps branches' '
+test_expect_success 'fetch --prune with a branch name keeps branches' '
 	cd "$D" &&
 	git clone . prune-branch &&
 	cd prune-branch &&
@@ -96,7 +96,7 @@ test_expect_failure 'fetch --prune with a branch name keeps branches' '
 	git rev-parse origin/extrabranch
 '
 
-test_expect_failure 'fetch --prune with a namespace keeps other namespaces' '
+test_expect_success 'fetch --prune with a namespace keeps other namespaces' '
 	cd "$D" &&
 	git clone . prune-namespace &&
 	cd prune-namespace &&
-- 
1.7.5.2.354.g349bf

^ permalink raw reply related

* Re: [PATCH 0/7] pickaxe: plug memory leaks, deduplicate code
From: Junio C Hamano @ 2011-10-07 22:47 UTC (permalink / raw)
  To: René Scharfe; +Cc: Git Mailing List
In-Reply-To: <4E8DD065.3040607@lsrfire.ath.cx>

Impressed; very nicely done.

Thanks.

^ permalink raw reply

* Re: [PATCH 1/2] Add Git::config_path()
From: Junio C Hamano @ 2011-10-07 22:26 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Cord Seele, Matthieu Moy, git, Eric Wong, Cord Seele
In-Reply-To: <201110072344.46556.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

>   char *expand_user_path(const char *path)
>   [...]
>                 if (username_len == 0) {
>                         const char *home = getenv("HOME");
>                         if (!home)
>                                 goto return_null;
>                         strbuf_add(&user_path, home, strlen(home));
>                 } else {
>   [...]

Ahh, Ok. I was afraid getpwnam() codepath might interfere with it.

^ permalink raw reply

* Re: [PATCH v2] Teach merge the '[-e|--edit]' option
From: Junio C Hamano @ 2011-10-07 22:15 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git, Todd A. Jacobs
In-Reply-To: <1318023997-54810-1-git-send-email-jaysoffian@gmail.com>

Jay Soffian <jaysoffian@gmail.com> writes:

> Implemented internally instead of as "git merge --no-commit && git commit"
> so that "merge --edit" is otherwise consistent (hooks, etc) with "merge".
>
> Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
> ---
> On Fri, Oct 7, 2011 at 1:30 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> If we wanted to do this properly, we should update builtin/merge.c to call
>> launch_editor() before it runs commit_tree(), in a way similar to how
>
> I disagree that this is the proper way to do it. --edit is a new option, there's
> no obviously "correct" behavior. You think 'merge --edit' should behave just
> like 'merge', I think 'merge --edit' should behave like 'merge --no-commit &&
> commit'.
>
> The commit performed internally by git-merge is already wildly inconsistent with
> git-commit.

Think and look forward.

You are complaining that the "commit" does not know enough to behave as if
it were a part of the merge command workflow if you split a usual merge
into two steps "merge --no-commit; commit".

How would you make it better? Would you strip all the things usual "merge"
does, so that it would work identically to the split one, losing some hook
support and such, or would you rather make the split case work similar to
the usual merge?

I'd say between "merge" and "merge --no-commit ; commit", the latter is
what needs to be fixed. Viewed that way, why would you even consider
making the new option behave similar to the _wrong_ one?

> I didn't bother with the commit status, it's more code than I wanted
> to deal with duplicating/refactoring from commit.c.

What do you mean by "commit status"? If you mean this patch is incomplete,
it would have been nicer if it were labeled with [PATCH/RFC].

> diff --git a/builtin/merge.c b/builtin/merge.c
> index ee56974371..0dee53b7e4 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -46,6 +46,7 @@ static const char * const builtin_merge_usage[] = {
>  
>  static int show_diffstat = 1, shortlog_len, squash;
>  static int option_commit = 1, allow_fast_forward = 1;
> +static int option_edit = 0;

No need to move this into .data segment when it can be in .bss
segment. Drop the unnecessary " = 0" before ";".

> @@ -842,30 +845,54 @@ static void add_strategies(const char *string, unsigned attr)
>  
>  }
>  
> -static void write_merge_msg(void)
> +static void write_merge_msg(struct strbuf *msg)
>  {
>  	int fd = open(git_path("MERGE_MSG"), O_WRONLY | O_CREAT, 0666);
>  	if (fd < 0)
>  		die_errno(_("Could not open '%s' for writing"),
>  			  git_path("MERGE_MSG"));
> -	if (write_in_full(fd, merge_msg.buf, merge_msg.len) != merge_msg.len)
> +	if (write_in_full(fd, msg->buf, msg->len) != msg->len)
>  		die_errno(_("Could not write to '%s'"), git_path("MERGE_MSG"));
>  	close(fd);
>  }
>  
> -static void read_merge_msg(void)
> +static void read_merge_msg(struct strbuf *msg)
>  {
> -	strbuf_reset(&merge_msg);
> -	if (strbuf_read_file(&merge_msg, git_path("MERGE_MSG"), 0) < 0)
> +	strbuf_reset(msg);
> +	if (strbuf_read_file(msg, git_path("MERGE_MSG"), 0) < 0)
>  		die_errno(_("Could not read from '%s'"), git_path("MERGE_MSG"));
>  }
>  
> -static void run_prepare_commit_msg(void)
> +static void write_merge_state();

s/()/(void)/;

Thanks.

^ permalink raw reply

* Re: [PATCH] po/pl.po: Eliminate fuzzy translations
From: Jakub Narebski @ 2011-10-07 22:11 UTC (permalink / raw)
  To: Zbigniew Jędrzejewski-Szmek
  Cc: Ævar Arnfjörð Bjarmason, Git Mailing List,
	Marcin Cieślak
In-Reply-To: <4E8AC294.2060608@in.waw.pl>

On Tue, 4 Oct 2011, Zbigniew Jędrzejewski-Szmek wrote:
> On 10/03/2011 11:37 PM, Jakub Narebski wrote:
>>
>> +# Terminologia dla kluczowych terminów z Subversion:

As I probably should write in the commit message, this terminology
is taken directly from Subversion's po/pl.po file.  Subversion has
IMHO quite good Polish translation.

On the other hand no other *.po file in Git has glossary in
comments...

>> +# path - ścieżka
>> +# URL - URL
>> +# file - plik
>> +# directory - katalog
>> +# update - aktualizacja
>> +# commit - zatwierdzenie, zatwierdzenie zmian
>
> This seems kind of awkward. E.g. 'initial commit' would become 
> 'początkowe zatwierdzenie zmian' or 'pierwsze zatwierdzenie', which just 
> doesn't sound right. What about starting with the mercurial term 
> 'changeset'? Git users also use 'commit' to mean 'change', so maybe
> the polish translation of this crucial term should be 'zmiana':
> 'initial commit' -- 'początkowa zmiana' or 'pierwsza zmiana'
> 'commit message' -- 'opis zmiany'

The problem is that "commit" might be a verb ('to commit'), where I think
"zatwierdzenie zmian" is a good translation, and "commit" might also be
used as a noun ('a commit'), where I think it should be probably translated
as "wersja" (eng. version) or "zmiana" (eng. change).

Nb. it is a good idea to take into account existing Mercurial translation
into Polish.
 
>> +# version control - zarządzanie wersjami
>> +# repository - repozytorium
>> +# branch - odgałęzienie
>
> 'gałąź'? I think that's the translation which is used in informal 
> conversations.

"Odgałęzienie" is what Subversion uses, but you are right that "gałąź"
might be better.  Though in this case the glossary cannot be marked
as coming from Subversion...
 
>> +# tag - tag
>
> 'metka', 'etykieta' according to the dictionary. I'm aware of 'metka'
> being used in CS anywhere, but it is short, and pretty cool, IMO.

I think that "etykieta" (eng. label) has already established different
meaning in Polish computer terminology for a "goto label".

"Metka" might be a good solution (it is mainly used in Polish to mean
textile labels), though I wonder if using English term "tag" (which
in Polish is used in computer science to mean metadata tag or markup
language tag) wouldn't be better as it is already computer term.
 
>> +# merge - łączenie zmian
>
> 'łączenie gałęzi'?

Well, in Subversion it is about merging changes, not branches ;-)))
 
>> +# conflict - konflikt
>> +# property - atrybut
>> +# revision - wersja
>> +# log message - opis zmian
>> +# entry/item - element
>> +# ancestry - pochodzenie
>> +# ancestor - przodek
>> +# working copy - kopia robocza
>> +# working dir - bieżący katalog
>> +# usage - wykorzystanie
>
> 'użycie', 'wywołanie'?
> E.g. 'standardowe wywołanie tego programu to: prog arg'

I'd have to check how other programs translate this.  I think
at least "usage: " in '-h' output is translated as "użycie: ",
though I am not sure if it is really a good translation to recommend.
 
>> +# source - źródłowy
>> +# destination - docelowy
>> +# hook - skrypt (skrypt repozytorium)
>> +# exclude - wykluczyć
>> +# crop - obciąć
>> +# cache - pamięć podręczna
>> +# child - obiekt podrzędny
>
> Standard CS term is 'ojciec' and 'syn' for 'parent/child'.

Right.
 
>> +# obliteration - obliteracja
>
> 'wymazanie'?

Well, anyway I don't think this is needed for git translation.
 
>> +# patch - łata
>> +# notes - adnotacja

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: unexpected behavior with `git log --skip filename`
From: Jay Soffian @ 2011-10-07 21:54 UTC (permalink / raw)
  To: Andrew McNabb; +Cc: git
In-Reply-To: <20111007171503.GB16607@mcnabbs.org>

On Fri, Oct 7, 2011 at 1:15 PM, Andrew McNabb <amcnabb@mcnabbs.org> wrote:
> The "--skip" option to "git log" did not behave as I expected, but I'm
> not sure whether this was user error, unclear documentation, or a bug.
> Specifically, I ran the following, intending to find the previous
> revision of a given file:
>
> git log --skip=1 -n 1 --oneline some-filename
>
> My expectation was that this would behave the same as:
>
> git log -n 2 --oneline some-filename |tail -n 1
>
> Instead, the --skip=1 parameter seemed to be ignored.  After I tried
> several different values, it appears that the commits are skipped before
> path matching with "some-filename".

Hmm:

$ git log --oneline GIT-VERSION-GEN | head -2
7f41b6bbe3 Post 1.7.7 first wave
703f05ad58 Git 1.7.7

$ git log --oneline --skip=1 -n 1 GIT-VERSION-GEN
703f05ad58 Git 1.7.7

j.

^ permalink raw reply

* [PATCH v2] Teach merge the '[-e|--edit]' option
From: Jay Soffian @ 2011-10-07 21:46 UTC (permalink / raw)
  To: git; +Cc: Jay Soffian, Junio C Hamano, Todd A. Jacobs
In-Reply-To: <7vk48gwvyd.fsf@alter.siamese.dyndns.org>

Implemented internally instead of as "git merge --no-commit && git commit"
so that "merge --edit" is otherwise consistent (hooks, etc) with "merge".

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
On Fri, Oct 7, 2011 at 1:30 PM, Junio C Hamano <gitster@pobox.com> wrote:
> If we wanted to do this properly, we should update builtin/merge.c to call
> launch_editor() before it runs commit_tree(), in a way similar to how

I disagree that this is the proper way to do it. --edit is a new option, there's
no obviously "correct" behavior. You think 'merge --edit' should behave just
like 'merge', I think 'merge --edit' should behave like 'merge --no-commit &&
commit'.

The commit performed internally by git-merge is already wildly inconsistent with
git-commit. If anything, --edit is a perfect excuse to say "we're trying to make
git-merge perform commits more consistently with git-commit, so we've
implemented 'merge --edit' in terms of git-commit."

I didn't bother with the commit status, it's more code than I wanted
to deal with duplicating/refactoring from commit.c.

 Documentation/merge-options.txt |    6 ++
 builtin/merge.c                 |  108 +++++++++++++++++++++++++--------------
 t/t7600-merge.sh                |   15 +++++
 3 files changed, 91 insertions(+), 38 deletions(-)

diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index b613d4ed08..6bd0b041c3 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -7,6 +7,12 @@ With --no-commit perform the merge but pretend the merge
 failed and do not autocommit, to give the user a chance to
 inspect and further tweak the merge result before committing.
 
+--edit::
+-e::
++
+	Invoke editor before committing successful merge to further
+	edit the default merge message.
+
 --ff::
 --no-ff::
 	Do not generate a merge commit if the merge resolved as
diff --git a/builtin/merge.c b/builtin/merge.c
index ee56974371..0dee53b7e4 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -46,6 +46,7 @@ static const char * const builtin_merge_usage[] = {
 
 static int show_diffstat = 1, shortlog_len, squash;
 static int option_commit = 1, allow_fast_forward = 1;
+static int option_edit = 0;
 static int fast_forward_only;
 static int allow_trivial = 1, have_message;
 static struct strbuf merge_msg;
@@ -190,6 +191,8 @@ static struct option builtin_merge_options[] = {
 		"create a single commit instead of doing a merge"),
 	OPT_BOOLEAN(0, "commit", &option_commit,
 		"perform a commit if the merge succeeds (default)"),
+	OPT_BOOLEAN('e', "edit", &option_edit,
+		"edit message before committing"),
 	OPT_BOOLEAN(0, "ff", &allow_fast_forward,
 		"allow fast-forward (default)"),
 	OPT_BOOLEAN(0, "ff-only", &fast_forward_only,
@@ -842,30 +845,54 @@ static void add_strategies(const char *string, unsigned attr)
 
 }
 
-static void write_merge_msg(void)
+static void write_merge_msg(struct strbuf *msg)
 {
 	int fd = open(git_path("MERGE_MSG"), O_WRONLY | O_CREAT, 0666);
 	if (fd < 0)
 		die_errno(_("Could not open '%s' for writing"),
 			  git_path("MERGE_MSG"));
-	if (write_in_full(fd, merge_msg.buf, merge_msg.len) != merge_msg.len)
+	if (write_in_full(fd, msg->buf, msg->len) != msg->len)
 		die_errno(_("Could not write to '%s'"), git_path("MERGE_MSG"));
 	close(fd);
 }
 
-static void read_merge_msg(void)
+static void read_merge_msg(struct strbuf *msg)
 {
-	strbuf_reset(&merge_msg);
-	if (strbuf_read_file(&merge_msg, git_path("MERGE_MSG"), 0) < 0)
+	strbuf_reset(msg);
+	if (strbuf_read_file(msg, git_path("MERGE_MSG"), 0) < 0)
 		die_errno(_("Could not read from '%s'"), git_path("MERGE_MSG"));
 }
 
-static void run_prepare_commit_msg(void)
+static void write_merge_state();
+static void abort_commit(const char *err_msg)
 {
-	write_merge_msg();
+	if (err_msg)
+		error("%s", err_msg);
+	fprintf(stderr,
+		_("Not committing merge; use 'git commit' to complete the merge.\n"));
+	write_merge_state();
+	exit(1);
+}
+
+static void prepare_to_commit(void)
+{
+	struct strbuf msg = STRBUF_INIT;
+	strbuf_addbuf(&msg, &merge_msg);
+	strbuf_addch(&msg, '\n');
+	write_merge_msg(&msg);
 	run_hook(get_index_file(), "prepare-commit-msg",
 		 git_path("MERGE_MSG"), "merge", NULL, NULL);
-	read_merge_msg();
+	if (option_edit) {
+		if (launch_editor(git_path("MERGE_MSG"), NULL, NULL))
+			abort_commit(NULL);
+	}
+	read_merge_msg(&msg);
+	stripspace(&msg, option_edit);
+	if (!msg.len)
+		abort_commit(_("Empty commit message."));
+	strbuf_release(&merge_msg);
+	strbuf_addbuf(&merge_msg, &msg);
+	strbuf_release(&msg);
 }
 
 static int merge_trivial(void)
@@ -879,7 +906,7 @@ static int merge_trivial(void)
 	parent->next = xmalloc(sizeof(*parent->next));
 	parent->next->item = remoteheads->item;
 	parent->next->next = NULL;
-	run_prepare_commit_msg();
+	prepare_to_commit();
 	commit_tree(merge_msg.buf, result_tree, parent, result_commit, NULL);
 	finish(result_commit, "In-index merge");
 	drop_save();
@@ -907,9 +934,9 @@ static int finish_automerge(struct commit_list *common,
 		for (j = remoteheads; j; j = j->next)
 			pptr = &commit_list_insert(j->item, pptr)->next;
 	}
-	free_commit_list(remoteheads);
 	strbuf_addch(&merge_msg, '\n');
-	run_prepare_commit_msg();
+	prepare_to_commit();
+	free_commit_list(remoteheads);
 	commit_tree(merge_msg.buf, result_tree, parents, result_commit, NULL);
 	strbuf_addf(&buf, "Merge made by the '%s' strategy.", wt_strategy);
 	finish(result_commit, buf.buf);
@@ -1015,6 +1042,36 @@ static int setup_with_upstream(const char ***argv)
 	return i;
 }
 
+static void write_merge_state()
+{
+	int fd;
+	struct commit_list *j;
+	struct strbuf buf = STRBUF_INIT;
+
+	for (j = remoteheads; j; j = j->next)
+		strbuf_addf(&buf, "%s\n",
+			sha1_to_hex(j->item->object.sha1));
+	fd = open(git_path("MERGE_HEAD"), O_WRONLY | O_CREAT, 0666);
+	if (fd < 0)
+		die_errno(_("Could not open '%s' for writing"),
+			  git_path("MERGE_HEAD"));
+	if (write_in_full(fd, buf.buf, buf.len) != buf.len)
+		die_errno(_("Could not write to '%s'"), git_path("MERGE_HEAD"));
+	close(fd);
+	strbuf_addch(&merge_msg, '\n');
+	write_merge_msg(&merge_msg);
+	fd = open(git_path("MERGE_MODE"), O_WRONLY | O_CREAT | O_TRUNC, 0666);
+	if (fd < 0)
+		die_errno(_("Could not open '%s' for writing"),
+			  git_path("MERGE_MODE"));
+	strbuf_reset(&buf);
+	if (!allow_fast_forward)
+		strbuf_addf(&buf, "no-ff");
+	if (write_in_full(fd, buf.buf, buf.len) != buf.len)
+		die_errno(_("Could not write to '%s'"), git_path("MERGE_MODE"));
+	close(fd);
+}
+
 int cmd_merge(int argc, const char **argv, const char *prefix)
 {
 	unsigned char result_tree[20];
@@ -1418,33 +1475,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 
 	if (squash)
 		finish(NULL, NULL);
-	else {
-		int fd;
-		struct commit_list *j;
-
-		for (j = remoteheads; j; j = j->next)
-			strbuf_addf(&buf, "%s\n",
-				sha1_to_hex(j->item->object.sha1));
-		fd = open(git_path("MERGE_HEAD"), O_WRONLY | O_CREAT, 0666);
-		if (fd < 0)
-			die_errno(_("Could not open '%s' for writing"),
-				  git_path("MERGE_HEAD"));
-		if (write_in_full(fd, buf.buf, buf.len) != buf.len)
-			die_errno(_("Could not write to '%s'"), git_path("MERGE_HEAD"));
-		close(fd);
-		strbuf_addch(&merge_msg, '\n');
-		write_merge_msg();
-		fd = open(git_path("MERGE_MODE"), O_WRONLY | O_CREAT | O_TRUNC, 0666);
-		if (fd < 0)
-			die_errno(_("Could not open '%s' for writing"),
-				  git_path("MERGE_MODE"));
-		strbuf_reset(&buf);
-		if (!allow_fast_forward)
-			strbuf_addf(&buf, "no-ff");
-		if (write_in_full(fd, buf.buf, buf.len) != buf.len)
-			die_errno(_("Could not write to '%s'"), git_path("MERGE_MODE"));
-		close(fd);
-	}
+	else
+		write_merge_state();
 
 	if (merge_was_ok) {
 		fprintf(stderr, _("Automatic merge went well; "
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 87aac835a1..8c6b811718 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -643,4 +643,19 @@ test_expect_success 'amending no-ff merge commit' '
 
 test_debug 'git log --graph --decorate --oneline --all'
 
+cat >editor <<\EOF
+#!/bin/sh
+# strip comments and blank lines from end of message
+sed -e '/^#/d' < "$1" | sed -e :a -e '/^\n*$/{$d;N;ba' -e '}' > expected
+EOF
+chmod 755 editor
+
+test_expect_success 'merge --no-ff --edit' '
+	git reset --hard c0 &&
+	EDITOR=./editor git merge --no-ff --edit c1 &&
+	verify_parents $c0 $c1 &&
+	git cat-file commit HEAD | sed "1,/^$/d" > actual &&
+	test_cmp actual expected
+'
+
 test_done
-- 
1.7.7.147.g3a3ce

^ permalink raw reply related

* Re: [PATCH 1/2] Add Git::config_path()
From: Jakub Narebski @ 2011-10-07 21:44 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Cord Seele, Matthieu Moy, git, Eric Wong, Cord Seele
In-Reply-To: <7vk48gtrh9.fsf@alter.siamese.dyndns.org>

On Mon, 7 Oct 2011, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > diff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh
> > index 3787186..7558f0c 100755
> > --- a/t/t9700-perl-git.sh
> > +++ b/t/t9700-perl-git.sh
> > @@ -43,7 +43,9 @@ test_expect_success \
> >       git config --add test.booltrue true &&
> >       git config --add test.boolfalse no &&
> >       git config --add test.boolother other &&
> > -     git config --add test.int 2k
> > +     git config --add test.int 2k &&
> > +     git config --add test.path "~/foo" &&
> > +     git config --add test.pathexpanded "$HOME/foo"
> 
> Given that test-lib.sh sets up the $HOME away from unknown place to ensure
> repeatability of tests, I am not sure if this test would ever pass.

Well, it passes.

test-lib.sh sets $HOME to "$TRASH_DIRECTORY", but this value of $HOME
is then later seen by "git config --path ..." run by Git::config_path
in t9700/test.pl

  char *expand_user_path(const char *path)
  [...]
                if (username_len == 0) {
                        const char *home = getenv("HOME");
                        if (!home)
                                goto return_null;
                        strbuf_add(&user_path, home, strlen(home));
                } else {
  [...]

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH 1/2] Add Git::config_path()
From: Junio C Hamano @ 2011-10-07 21:35 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Cord Seele, Matthieu Moy, git, Eric Wong, Cord Seele
In-Reply-To: <m3vcs01r32.fsf@localhost.localdomain>

Jakub Narebski <jnareb@gmail.com> writes:

> diff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh
> index 3787186..7558f0c 100755
> --- a/t/t9700-perl-git.sh
> +++ b/t/t9700-perl-git.sh
> @@ -43,7 +43,9 @@ test_expect_success \
>       git config --add test.booltrue true &&
>       git config --add test.boolfalse no &&
>       git config --add test.boolother other &&
> -     git config --add test.int 2k
> +     git config --add test.int 2k &&
> +     git config --add test.path "~/foo" &&
> +     git config --add test.pathexpanded "$HOME/foo"

Given that test-lib.sh sets up the $HOME away from unknown place to ensure
repeatability of tests, I am not sure if this test would ever pass.

^ permalink raw reply

* [PATCH/RFC 3/2] Refactor Git::config_*
From: Jakub Narebski @ 2011-10-07 21:17 UTC (permalink / raw)
  To: Junio C Hamano, Eric Wong; +Cc: Cord Seele, Matthieu Moy, git, Cord Seele
In-Reply-To: <7voby1oesm.fsf@alter.siamese.dyndns.org>

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

There is, especially with addition of Git::config_path(), much code
repetition in the Git::config_* family of subroutines.  Refactor
common parts of Git::config(), Git::config_bool(), Git::config_int()
and Git::config_path() into _config_common() subroutine.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This is version which has fixed style to be more Perl-ish, and which
actually works (i.e. t9700 passes).

I have also moved _config_common() after commands that use it, just like
it is done with other "private" methods (methods with names starting with
'_'), and excluded this private detail of implementation from docs.

 perl/Git.pm |   85 ++++++++++++++---------------------------------------------
 1 files changed, 20 insertions(+), 65 deletions(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index c279bfb..d6df2e8 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -562,7 +562,6 @@ sub wc_chdir {
 	$self->{opts}->{WorkingSubdir} = $subdir;
 }
 
-
 =item config ( VARIABLE )
 
 Retrieve the configuration C<VARIABLE> in the same manner as C<config>
@@ -570,30 +569,11 @@ does. In scalar context requires the variable to be set only one time
 (exception is thrown otherwise), in array context returns allows the
 variable to be set multiple times and returns all the values.
 
-This currently wraps command('config') so it is not so fast.
-
 =cut
 
 sub config {
 	my ($self, $var) = _maybe_self(@_);
-
-	try {
-		my @cmd = ('config');
-		unshift @cmd, $self if $self;
-		if (wantarray) {
-			return command(@cmd, '--get-all', $var);
-		} else {
-			return command_oneline(@cmd, '--get', $var);
-		}
-	} catch Git::Error::Command with {
-		my $E = shift;
-		if ($E->value() == 1) {
-			# Key not found.
-			return;
-		} else {
-			throw $E;
-		}
-	};
+	return _config_common($self, $var);
 }
 
 
@@ -603,60 +583,24 @@ Retrieve the bool configuration C<VARIABLE>. The return value
 is usable as a boolean in perl (and C<undef> if it's not defined,
 of course).
 
-This currently wraps command('config') so it is not so fast.
-
 =cut
 
 sub config_bool {
 	my ($self, $var) = _maybe_self(@_);
-
-	try {
-		my @cmd = ('config', '--bool', '--get', $var);
-		unshift @cmd, $self if $self;
-		my $val = command_oneline(@cmd);
-		return undef unless defined $val;
-		return $val eq 'true';
-	} catch Git::Error::Command with {
-		my $E = shift;
-		if ($E->value() == 1) {
-			# Key not found.
-			return undef;
-		} else {
-			throw $E;
-		}
-	};
+	my $val = scalar _config_common($self, $var, {'kind' => '--bool'});
+	return (defined $val && $val eq 'true');
 }
 
-
 =item config_path ( VARIABLE )
 
 Retrieve the path configuration C<VARIABLE>. The return value
 is an expanded path or C<undef> if it's not defined.
 
-This currently wraps command('config') so it is not so fast.
-
 =cut
 
 sub config_path {
 	my ($self, $var) = _maybe_self(@_);
-
-	try {
-		my @cmd = ('config', '--path');
-		unshift @cmd, $self if $self;
-		if (wantarray) {
-			return command(@cmd, '--get-all', $var);
-		} else {
-			return command_oneline(@cmd, '--get', $var);
-		}
-	} catch Git::Error::Command with {
-		my $E = shift;
-		if ($E->value() == 1) {
-			# Key not found.
-			return undef;
-		} else {
-			throw $E;
-		}
-	};
+	return _config_common($self, $var, {'kind' => '--path'});
 }
 
 =item config_int ( VARIABLE )
@@ -667,28 +611,39 @@ or 'g' in the config file will cause the value to be multiplied
 by 1024, 1048576 (1024^2), or 1073741824 (1024^3) prior to output.
 It would return C<undef> if configuration variable is not defined,
 
-This currently wraps command('config') so it is not so fast.
-
 =cut
 
 sub config_int {
 	my ($self, $var) = _maybe_self(@_);
+	return scalar _config_common($self, $var, {'kind' => '--int'});
+}
+
+# Common subroutine to implement bulk of what the config* family of methods
+# do. This wraps command('config') so it is not so fast.
+sub _config_common {
+	my ($self, $var, $opts) = _maybe_self(@_);
 
 	try {
-		my @cmd = ('config', '--int', '--get', $var);
+		my @cmd = ('config', $opts->{'kind'} ? $opts->{'kind'} : ());
 		unshift @cmd, $self if $self;
-		return command_oneline(@cmd);
+		if (wantarray) {
+			return command(@cmd, '--get-all', $var);
+		} else {
+			return command_oneline(@cmd, '--get', $var);
+		}
 	} catch Git::Error::Command with {
 		my $E = shift;
 		if ($E->value() == 1) {
 			# Key not found.
-			return undef;
+			return;
 		} else {
 			throw $E;
 		}
 	};
+
 }
 
+
 =item get_colorbool ( NAME )
 
 Finds if color should be used for NAMEd operation from the configuration,
-- 
1.7.6

^ permalink raw reply related

* Re: [PATCH] fmt-merge-msg: use branch.$name.description
From: Junio C Hamano @ 2011-10-07 20:59 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Jonathan Nieder, git, Jeff King
In-Reply-To: <4E8EED39.1060607@drmicha.warpmail.net>

Michael J Gruber <git@drmicha.warpmail.net> writes:

> Alternatively, one could store the description in a blob and refer to
> that directly, of course. I.e., have
>
> refs/description/foo
>
> point to a blob whose content is the description of the ref
>
> ref/foo
>
> That would be unversioned, and one could decide more easily which
> descriptions to share. (A notes tree you either push or don't.)

If refs/descriptions/foo were to point at a commit object and its commit
log message is used to store the description you could make it versioned.

A history that records forest of descriptions where its commit contains a
tree whose leaves are branch names is slightly better than notes based
approach in that it does not have to violate "notes are for objects"
design principle, but it shares the same "branch names are local" issue as
your "lone refs/description/foo describes 'foo' and 'foo' only".

In addition, it shares with the notes based approach that exchanging a
description on a single branch will inevitably pull in descriptions of all
the other branches you happen to have in the forest, which was one of the
reasons that recording "push -s" certificates in notes tree failed whether
the v2 or v3 approach was used.

You should be able to make a few changes to jc/request-pull-show-head-4 to
move the description to such a "refs/desc/$name" from completely local
"branch.$name.description". The machinery to edit the description is
localized to edit_branch_description() introduced in b7200e8 (branch:
teach --edit-description option, 2011-09-20), and the machinery to read
the description is localized to read_branch_desc() in 6f9a332 (branch: add
read_branch_desc() helper function, 2011-09-21); the remainder of the
series could be used unmodified.

But it remains that any of these approaches assume branch names are
universal. Unlike other systems, what we call branches do not have their
own identity, so if you really want to go that route (and we _might_ need
to in the longer term, but I am not convinced at this point yet), you
would first need to define how that local namespace would look like, how
people interact with it, etc. It might be just the matter of declaring a
convention e.g. "Among people who meet at this central repository,
everybody must map the branches identically to their local branch
namespace, and all sharing must go through the central repository", and
calling a tuple <central repository URL, branch name in that repository>
with a name that cannot be confused with "branch" (so "remote branch" is
out), such as "(development) track".

^ permalink raw reply

* Re: [PATCH] fmt-merge-msg: use branch.$name.description
From: Michael J Gruber @ 2011-10-07 20:40 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jonathan Nieder, git, Jeff King
In-Reply-To: <7v1uuovahm.fsf@alter.siamese.dyndns.org>

Junio C Hamano venit, vidit, dixit 07.10.2011 21:59:
> Jonathan Nieder <jrnieder@gmail.com> writes:
> 
>> I am surprised that you seem to have missed what I meant by "branch
>> names are local"....
>> This matters, a lot, because there is no easy way to partition a
>> namespace of names descriptive for humans without a central authority
>> to negotiate conflicts.
> 
> I think we are in total agreement.  The branch names are local, but the
> users may want to annotate to describe _the history_ they intend to build
> on it.
> 
> Various ways to convey the description when the end product (i.e. the
> history built on it) is shiped were outlined in the series, so that the
> shipper can help the receiver understand the history. The information in
> the annotation (i.e. the _value_ of branch.$name.description) is something
> the shipper wants to share with the receiver, but the mapping between the
> local name of the branch the shipper used to build that history (i.e. the
> key "$name" in branch.$name.description) is immaterial in the end result.

It just seems we judge the "local" aspect completely differently. The
point that I don't get is: How to share a branch without sharing a
branch name? You cannot. Of course you may "change the name" during the
push, or during a fetch, and at that point you know both and can map the
description.

Note that I'm not tied to the notes implementation. But versioned branch
descriptions would be nice for several purposes, for example series
cover letters, or descriptions on long lived feature branches. And I
don't see how else to have versioned descriptions.

Also, for transporting descriptions via config, you have to have an
updated git on the server side, whereas anything object/ref based would
work now, right?

Michael

^ permalink raw reply

* Re: [PATCH 1/2] Add Git::config_path()
From: Jakub Narebski @ 2011-10-07 20:32 UTC (permalink / raw)
  To: Cord Seele; +Cc: Matthieu Moy, git, Junio C Hamano, Eric Wong, Cord Seele
In-Reply-To: <1317379945-9355-2-git-send-email-cowose@gmail.com>

Cord Seele <cowose@googlemail.com> writes:

> Use --path option when calling 'git config' thus allow for pathname
> expansion, e.g. a tilde.
> 
> Signed-off-by: Cord Seele <cowose@gmail.com>
> ---
>  perl/Git.pm |   32 ++++++++++++++++++++++++++++++++
>  1 files changed, 32 insertions(+), 0 deletions(-)

I think the following minimal test should be squashed in:

---
diff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh
index 3787186..7558f0c 100755
--- a/t/t9700-perl-git.sh
+++ b/t/t9700-perl-git.sh
@@ -43,7 +43,9 @@ test_expect_success \
      git config --add test.booltrue true &&
      git config --add test.boolfalse no &&
      git config --add test.boolother other &&
-     git config --add test.int 2k
+     git config --add test.int 2k &&
+     git config --add test.path "~/foo" &&
+     git config --add test.pathexpanded "$HOME/foo"
      '
 
 # The external test will outputs its own plan
diff --git a/t/t9700/test.pl b/t/t9700/test.pl
index 13ba96e..ce9340c 100755
--- a/t/t9700/test.pl
+++ b/t/t9700/test.pl
@@ -33,6 +33,8 @@ is($r->config_int("test.int"), 2048, "config_int: integer");
 is($r->config_int("test.nonexistent"), undef, "config_int: nonexistent");
 ok($r->config_bool("test.booltrue"), "config_bool: true");
 ok(!$r->config_bool("test.boolfalse"), "config_bool: false");
+is($r->config_path("test.path"), $r->config("test.pathexpanded"),
+   "config_path: ~/foo expansion");
 our $ansi_green = "\x1b[32m";
 is($r->get_color("color.test.slot1", "red"), $ansi_green, "get_color");
 # Cannot test $r->get_colorbool("color.foo")) because we do not

^ 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