Git development
 help / color / mirror / Atom feed
* [PATCH] merge-one-file: fix "expr: non-numeric argument"
From: Jay Soffian @ 2011-10-06 18:25 UTC (permalink / raw)
  To: git; +Cc: Jay Soffian, Junio C Hamano

When invoking expr to compare two numbers, don't quote the
variables which are the output of 'wc -c'. On OS X, this output
includes spaces, which expr balks at:

  $ sz0=`wc -c </etc/passwd`
  $ sz1=`wc -c </etc/passwd`
  $ echo "'$sz0'"
  '    3667'

  $ expr "$sz0" \< "$sz1" \* 2
  expr: non-numeric argument

  $ expr $sz0 \< $sz1 \* 2
  1

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
Noticed this while working on the CHERRY_PICK_HEAD issue.

 git-merge-one-file.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh
index 7aeb96952f..f612cb847a 100755
--- a/git-merge-one-file.sh
+++ b/git-merge-one-file.sh
@@ -117,7 +117,7 @@ case "${1:-.}${2:-.}${3:-.}" in
 
 		# If we do not have enough common material, it is not
 		# worth trying two-file merge using common subsections.
-		expr "$sz0" \< "$sz1" \* 2 >/dev/null || : >$orig
+		expr $sz0 \< $sz1 \* 2 >/dev/null || : >$orig
 		;;
 	*)
 		echo "Auto-merging $4"
-- 
1.7.7.144.gbfcf9.dirty

^ permalink raw reply related

* Re: [PATCH] commit: teach --gpg-sign option
From: Michael J Gruber @ 2011-10-06 18:44 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Jonathan Nieder, Shawn Pearce, Junio C Hamano, git
In-Reply-To: <vpqfwj6xceo.fsf@bauges.imag.fr>

Matthieu Moy venit, vidit, dixit 06.10.2011 19:22:
> Jonathan Nieder <jrnieder@gmail.com> writes:
> 
>> I probably missed some earlier discussion (so please forgive me this),
> 
> (same here)
> 
>> What happens if my old key is compromised and I want to throw away the
>> signatures and replace them with signatures using my new key?
> 
> With the patch we're discussing, signatures are part of history, hence
> can't be modified after the fact without rewritting them.
> 
> *But*, by design, unless sha1 itself is compromized (in which case Git
> would need to change to another hash function, that would be no fun),
> signing the tip of every branch is sufficient to sign the whole history.
> 
> So, your old signatures would remain there, and your new signature, for
> new commits, would be added on top.
> 
>> How does this relate to the "push certificate" use case, which seemed
>> to be mostly about authenticating published branch tips with
>> signatures that are not necessarily important in the long term?
> 
> I'm wondering how this feature would fit in a typical flow, indeed.
> Usually, I hack for a while, and when I'm happy enough, I push. But I
> don't take the decision of what to push at commit time, so if the idea
> is to sign only a few commits (i.e. the ones you push), then you should
> decide this at commit time ("hmm, I should commit --gpg-sign this time
> because I'm going to push this one").
> 
> If the idea is to sign every commit, then there should be a config
> option so that we don't have to type it every time.
> 

Same concerns here. You can always

git commit --amend --gpg-sign

the commit at the tip, of course, and can even set things up to have
push do this or remind you thereof.

I really liked the signatures as notes (because I feel those signatures
are attachments after the fact, not part of the commit) but can see how
distributing and merging them is non-trivial, and similarly checking
them at a point in time when they are not in a notes tree yet.

Michael

^ permalink raw reply

* Re: [PATCH/RFC jn/ident-from-etc-mailname] ident: do not retrieve default ident when unnecessary
From: Jonathan Nieder @ 2011-10-06 18:48 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johannes Sixt, git, Matt Kraai, Gerrit Pape, Ian Jackson,
	Linus Torvalds
In-Reply-To: <7vy5wyyod1.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:

>> Avoid a getpwuid() call (which contacts the network if the password
>> database is not local), read of /etc/mailname, gethostname() call, and
>> reverse DNS lookup if the user has already chosen a name and email
>> through configuration, the environment, or the command line.
>
> Oh boy that is a hard to parse paragraph that took me three reads.

Here's a possible replacement:

 Avoid looking up the current user's password database entry (which
 might be on another machine) and the current machine's domain name for
 outgoing mail (which can involve a reverse DNS lookup) when the
 environment, configuration, or command line specifies a name and email
 that would override them.

Or it might make sense to drop that paragraph altogether, since the
subject line already says as much.

Thanks for looking it over.

Sleepily,
Jonathan

^ permalink raw reply

* [PATCH 1/3] completion: unite --reuse-message and --reedit-message handling
From: Teemu Matilainen @ 2011-10-06 18:40 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Shawn O. Pearce, Teemu Matilainen

Signed-off-by: Teemu Matilainen <teemu.matilainen@iki.fi>
---
 contrib/completion/git-completion.bash |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 8648a36..656297d 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1259,12 +1259,8 @@ _git_commit ()
 			" "" "${cur##--cleanup=}"
 		return
 		;;
-	--reuse-message=*)
-		__gitcomp "$(__git_refs)" "" "${cur##--reuse-message=}"
-		return
-		;;
-	--reedit-message=*)
-		__gitcomp "$(__git_refs)" "" "${cur##--reedit-message=}"
+	--reuse-message=*|--reedit-message=*)
+		__gitcomp "$(__git_refs)" "" "${cur#*=}"
 		return
 		;;
 	--untracked-files=*)
-- 
1.7.7.138.g7f41b6

^ permalink raw reply related

* [PATCH 2/3] completion: commit --fixup and --squash
From: Teemu Matilainen @ 2011-10-06 18:40 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Shawn O. Pearce, Teemu Matilainen
In-Reply-To: <1317926431-609-1-git-send-email-teemu.matilainen@iki.fi>

Signed-off-by: Teemu Matilainen <teemu.matilainen@iki.fi>
---
 contrib/completion/git-completion.bash |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 656297d..847e6e9 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1259,7 +1259,8 @@ _git_commit ()
 			" "" "${cur##--cleanup=}"
 		return
 		;;
-	--reuse-message=*|--reedit-message=*)
+	--reuse-message=*|--reedit-message=*|\
+	--fixup=*|--squash=*)
 		__gitcomp "$(__git_refs)" "" "${cur#*=}"
 		return
 		;;
@@ -1274,7 +1275,7 @@ _git_commit ()
 			--dry-run --reuse-message= --reedit-message=
 			--reset-author --file= --message= --template=
 			--cleanup= --untracked-files --untracked-files=
-			--verbose --quiet
+			--verbose --quiet --fixup= --squash=
 			"
 		return
 	esac
-- 
1.7.7.138.g7f41b6

^ permalink raw reply related

* [PATCH 3/3] completion: push --set-upstream
From: Teemu Matilainen @ 2011-10-06 18:40 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Shawn O. Pearce, Teemu Matilainen
In-Reply-To: <1317926431-609-1-git-send-email-teemu.matilainen@iki.fi>

Signed-off-by: Teemu Matilainen <teemu.matilainen@iki.fi>
---
 contrib/completion/git-completion.bash |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 847e6e9..344119b 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1730,7 +1730,7 @@ _git_push ()
 	--*)
 		__gitcomp "
 			--all --mirror --tags --dry-run --force --verbose
-			--receive-pack= --repo=
+			--receive-pack= --repo= --set-upstream
 		"
 		return
 		;;
-- 
1.7.7.138.g7f41b6

^ permalink raw reply related

* Re: Prompt for merge message?
From: in-gitvger @ 2011-10-06 18:51 UTC (permalink / raw)
  To: git; +Cc: Todd A. Jacobs
In-Reply-To: <20111006182534.GA13628@vfa-6h>


In message <20111006182534.GA13628@vfa-6h>, Jacob Helwig writes:

    On Thu, 06 Oct 2011 10:49:02 -0700, Todd A. Jacobs wrote:

    > I often find myself using "--no-ff -m foo" for merging short-lived
    > branches, because the merge commit usually needs to say something
    > about having finished a feature rather than referring to a branch that
    > will be deleted shortly anyway. However, it's a little annoying to
    > have to always write the commit message on the command-line,
    > especially in cases where a more expository multi-line message would
    > be useful.

    "git merge --no-ff --no-commit branch_foo && git commit" ?

While not ideal, you can use a multi-line message on the command line.
I do it all of the time.  Popping up an editor like in the separated
workflow is more user friendly.

----------------------------------------------------------------------
git merge --no-ff -m "My feature is very complex

It requires multiple lines to explain.

Or perhaps I am too verbose." branch_foo
----------------------------------------------------------------------

					-Seth Robertson

^ permalink raw reply

* Re: [PATCHv3 2/5] Learn to handle gitfiles in enter_repo
From: Phil Hord @ 2011-10-06 19:16 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy
  Cc: git@vger.kernel.org, Phil Hord, Junio C Hamano, Erik Faye-Lund
In-Reply-To: <CACsJy8BiZ7Ey95BOf4p-zwysyYwEY6WectRaj-GqnFvgDNTtZw@mail.gmail.com>

Nguyen Thai Ngoc Duy <pclouds@gmail.com> says,
> On Thu, Oct 6, 2011 at 12:31 AM, Phil Hord <hordp@cisco.com> wrote:
>> -               if (!suffix[i] || chdir(used_path))
>> +               if (!suffix[i])
>> +                       return NULL;
>> +               gitfile = read_gitfile(used_path) ;
>> +               if (gitfile)
>> +                       strcpy(used_path, gitfile);
>> +               if (chdir(used_path))
>>                        return NULL;
>>                path = validated_path;
>>        }
> 
> This is room for improvement, the patch is fine as it is now. We could
> improve error reporting here. If .git file points to nowhere, we get
> "not a repository-kind of message. Except daemon.c, enter_repo()
> callers always die() if enter_repo() returns NULL. We could move the
> die() part (with improved error message) into enter_repo().
> 
> We could update enter_repo(const char *, int) to enter_repo(const char
> *, int, int gently). If gently is 1, we never die() nor report
> anything (ie. what we're doing now). daemon.c will need this, the rest
> of callers will be happy with gently = 0.

I like that.  It wasn't clear to me what the 'gently' moniker meant
before, but now I understand it.  It could easily apply to this function
and the new is_gitfile() to help reduce code duplication.

In a different patch, though.

Phil

^ permalink raw reply

* Git tech talk speaker wanted
From: Bill Lear @ 2011-10-06 19:57 UTC (permalink / raw)
  To: git

My company is looking for a person who has in-depth experience with
git to come give a tech talk in early November (San Jose area).  Is
there anyone who knows of someone who might be able to do this?
Please reply to me, as I am not subscribed to the list.

Thanks.


Bill

^ permalink raw reply

* Re: Prompt for merge message?
From: Stephen Bash @ 2011-10-06 20:15 UTC (permalink / raw)
  To: in-gitvger; +Cc: Todd A. Jacobs, git
In-Reply-To: <201110061851.p96Ipfui016139@no.baka.org>

----- Original Message -----
> From: in-gitvger@baka.org
> Sent: Thursday, October 6, 2011 2:51:41 PM
> Subject: Re: Prompt for merge message?
>
> In message <20111006182534.GA13628@vfa-6h>, Jacob Helwig writes:
> 
> On Thu, 06 Oct 2011 10:49:02 -0700, Todd A. Jacobs wrote:
> 
> > I often find myself using "--no-ff -m foo" for merging short-lived
> > branches, because the merge commit usually needs to say something
> > about having finished a feature rather than referring to a branch
> > that
> > will be deleted shortly anyway. However, it's a little annoying to
> > have to always write the commit message on the command-line,
> > especially in cases where a more expository multi-line message would
> > be useful.
> 
> "git merge --no-ff --no-commit branch_foo && git commit" ?
> 
> While not ideal, you can use a multi-line message on the command line.
> I do it all of the time. Popping up an editor like in the separated
> workflow is more user friendly.
> 
> ----------------------------------------------------------------------
> git merge --no-ff -m "My feature is very complex
> 
> It requires multiple lines to explain.
> 
> Or perhaps I am too verbose." branch_foo
> ----------------------------------------------------------------------

And not so helpful if you want to use the --log option to pre-populate the merge message (which I often run into).

Thanks,
Stephen

^ permalink raw reply

* Re: Prompt for merge message?
From: Andreas Krey @ 2011-10-06 20:19 UTC (permalink / raw)
  To: Jacob Helwig; +Cc: Todd A. Jacobs, git
In-Reply-To: <20111006182534.GA13628@vfa-6h>

On Thu, 06 Oct 2011 11:25:34 +0000, Jacob Helwig wrote:
> "git merge --no-ff --no-commit branch_foo && git commit" ?

"git merge --no-ff branch_foo && git commit --amend"

Andreas

^ permalink raw reply

* [PATCH 2/3] fetch: honor the user-provided refspecs when pruning refs
From: Carlos Martín Nieto @ 2011-10-06 20:19 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano, mathstuf
In-Reply-To: <1317932385-28604-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 tag 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.

Teach get_stale_heads about user-provided refspecs and use them if
they're available.

Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
---
 builtin/fetch.c  |    6 ++--
 builtin/remote.c |    2 +-
 remote.c         |   74 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 remote.h         |    3 +-
 4 files changed, 73 insertions(+), 12 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 30b485e..b937d71 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 transport *transport, struct refspec *refs, int n, 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(transport->remote, ref_map, refs, n);
 	const char *dangling_msg = dry_run
 		? _("   (%s will become dangling)\n")
 		: _("   (%s has become dangling)\n");
@@ -700,7 +700,7 @@ static int do_fetch(struct transport *transport,
 		return 1;
 	}
 	if (prune)
-		prune_refs(transport, ref_map);
+		prune_refs(transport, refs, ref_count, 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 b25dfb4..91a2148 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -349,7 +349,7 @@ 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(states->remote, fetch_map, NULL, 0);
 	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 7840d2f..72a26d3 100644
--- a/remote.c
+++ b/remote.c
@@ -1684,26 +1684,84 @@ struct stale_heads_info {
 	struct remote *remote;
 	struct string_list *ref_names;
 	struct ref **stale_refs_tail;
+	struct refspec *refs;
+	int ref_count;
 };
 
+/*
+ * Find a refspec to a remote's
+ * Returns 0 on success, -1 if it couldn't find a the refspec
+ */
+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 '*' 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);
-		}
+
+	/*
+	 * If the user speicified refspecs on the command line, we
+	 * should only use those to check. Otherwise, look in the
+	 * remote's configuration for the branch.
+	 */
+	if (info->ref_count)
+		ret = find_in_refs(info->refs, info->ref_count, &refspec);
+	else
+		ret = remote_find_tracking(info->remote, &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 remote *remote, 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;
@@ -1711,6 +1769,8 @@ struct ref *get_stale_heads(struct remote *remote, struct ref *fetch_map)
 	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..2f753a0 100644
--- a/remote.h
+++ b/remote.h
@@ -164,6 +164,7 @@ 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 remote *remote, struct ref *fetch_map,
+			    struct refspec *refs, int ref_count);
 
 #endif
-- 
1.7.5.2.354.g349bf

^ permalink raw reply related

* [PATCH 0/3] Be more careful when prunning
From: Carlos Martín Nieto @ 2011-10-06 20:19 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 teaches get_stale_heads to use the user-provided
refspecs instead of the ones in the config. 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 third patch,
which pretends that a "refs/tags/*:refs/tags/*" refspec was given on
the command-line.

Cheers,
   cmn

Carlos Martín Nieto (3):
      fetch: free all the additional refspecs
      fetch: honor the user-provided refspecs when pruning refs
      fetch: treat --tags like refs/tags/*:refs/tags/* when pruning

 builtin/fetch.c  |   19 ++++++++++---
 builtin/remote.c |    2 +-
 remote.c         |   74 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 remote.h         |    3 +-
 4 files changed, 84 insertions(+), 14 deletions(-)

-- 
1.7.5.2.354.g349bf

^ permalink raw reply

* [PATCH 3/3] fetch: treat --tags like refs/tags/*:refs/tags/* when pruning
From: Carlos Martín Nieto @ 2011-10-06 20:19 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano, mathstuf
In-Reply-To: <1317932385-28604-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 |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index b937d71..94b2bd3 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -699,8 +699,17 @@ static int do_fetch(struct transport *transport,
 		free_refs(ref_map);
 		return 1;
 	}
-	if (prune)
+	if (prune) {
+		/* If --tags was specified, we need to tell prune_refs
+		 * that we're filtering the refs from the remote */
+		if (tags == TAGS_SET) {
+			const char * tags_refspec = "refs/tags/*:refs/tags/*";
+			refs = xrealloc(refs, (ref_count + 1) * sizeof(struct refspec));
+			refs[ref_count] = *parse_fetch_refspec(1, &tags_refspec);
+			ref_count++;
+		}
 		prune_refs(transport, refs, ref_count, ref_map);
+	}
 	free_refs(ref_map);
 
 	/* if neither --no-tags nor --tags was specified, do automated tag
-- 
1.7.5.2.354.g349bf

^ permalink raw reply related

* [PATCH 1/3] fetch: free all the additional refspecs
From: Carlos Martín Nieto @ 2011-10-06 20:19 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano, mathstuf
In-Reply-To: <1317932385-28604-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

* Pull --rebase looses merge information
From: Duane Murphy @ 2011-10-06 19:21 UTC (permalink / raw)
  To: git

Here is the synopsis:

$ git checkout master
$ git pull
... time passes ... 
$ git checkout topic # remote topic
$ git checkout master
$ git merge topic
$ git push
    non-fast-forward updates were rejected
$ git pull 
    merge by rebase; implied by config
$ git push

The result of this process is that the file changes are pushed but the reference back to the topic branch has been lost. This makes it appear as though the topic branch has not been merged properly.

The trigger appears to be the pull (with rebase) that occurs after the merge and before the push. This of course is caused by the repo being out of sync from the point when the merge was started. That window of time can be shortened but it can never be zero.

The problem is fixed by merging again, but it's difficult to notice that this problem has actually occurred.

It has been noted that the problem is that we are using rebase on pull. We set our configurations to always rebase on a pull from a remote branch. This makes sense as it prevents artificial merges and unusual modifications to the history with respect to the shared repository. That is, if I'm working on merging changes into a remote branch and some one beats me, my merge should be after his, not before.

I would hope that a rebase operates mostly like a merge (understanding that rebase is different). I would not expect the merge information (ie the source branch of the merge) to be lost just because of a rebase.

Is there a bug here? Is there some way to avoid this situation without sacrificing the benefits of pull --rebase?

Thanks,

 ...Duane

^ permalink raw reply

* Re: [PATCH] merge-one-file: fix "expr: non-numeric argument"
From: Junio C Hamano @ 2011-10-06 20:25 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git
In-Reply-To: <1317925555-65237-1-git-send-email-jaysoffian@gmail.com>

Thanks.

^ permalink raw reply

* Re: Pull --rebase looses merge information
From: in-gitvger @ 2011-10-06 20:31 UTC (permalink / raw)
  To: Duane Murphy; +Cc: git
In-Reply-To: <DECF417E-50BB-4963-965C-BEF1B5C95DAC@mac.com>


In message <DECF417E-50BB-4963-965C-BEF1B5C95DAC@mac.com>, Duane Murphy writes:

    $ git merge topic
    $ git pull 
        merge by rebase; implied by config
    $ git push

    The result of this process is that the file changes are pushed but
    the reference back to the topic branch has been lost. This makes
    it appear as though the topic branch has not been merged properly.

[...]

    Is there a bug here? Is there some way to avoid this situation
    without sacrificing the benefits of pull --rebase?

Yes, but it currently is annoying.

Instead of `git pull --rebase` you need to run
`git fetch && git rebase -p @{u}`

It would be very nice if the -p argument to rebase could be
automatically included in the `git pull --rebase`.

I personally believe all pull should be --rebase, all merges should be
--no-ff, and all rebases should be -p.  At least by default.  But that
is just me.

					-Seth Robertson

^ permalink raw reply

* Re: [PATCH 0/3] Be more careful when prunning
From: Ben Boeckel @ 2011-10-06 20:51 UTC (permalink / raw)
  To: Carlos Martín Nieto; +Cc: git, Jeff King, Junio C Hamano
In-Reply-To: <1317932385-28604-1-git-send-email-cmn@elego.de>

On Thu, Oct 06, 2011 at 22:19:42 +0200, Carlos Martín Nieto wrote:
> 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 teaches get_stale_heads to use the user-provided
> refspecs instead of the ones in the config. 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 third patch,
> which pretends that a "refs/tags/*:refs/tags/*" refspec was given on
> the command-line.

I applied the patches to current master (7f41b6b) and got a segfault
with:

    git fetch -p -t origin master

It does not happen with master.

Backtrace:

(gdb) bt
#0  0x00007ffff7395d18 in __strchr_sse42 () from /lib64/libc.so.6
#1  0x00000000004b2d39 in find_in_refs (query=0x7fffffffdb90, ref_count=2, refs=<optimized out>) at remote.c:1709
#2  get_stale_heads_cb (refname=0x7a8f31 "refs/heads/a/branch/name", sha1=0x7a8f09 "\367\343\375C٩\223u\305OG\233)z\347X\370\333\325", <incomplete sequence \335>, flags=0, cb_data=0x7fffffffdc50) at remote.c:1740
#3  0x00000000004adf19 in do_for_each_ref (submodule=<optimized out>, base=0x4ea1c2 "", fn=0x4b2ca0 <get_stale_heads_cb>, trim=0, flags=0, cb_data=0x7fffffffdc50) at refs.c:684
#4  0x00000000004b4249 in get_stale_heads (remote=<optimized out>, fetch_map=<optimized out>, refs=<optimized out>, ref_count=<optimized out>) at remote.c:1777
#5  0x0000000000426cfb in prune_refs (ref_map=<optimized out>, n=<optimized out>, refs=<optimized out>, transport=0x78e040) at builtin/fetch.c:511
#6  do_fetch (ref_count=<optimized out>, refs=<optimized out>, transport=0x78e040) at builtin/fetch.c:711
#7  fetch_one (remote=<optimized out>, argc=<optimized out>, argv=<optimized out>) at builtin/fetch.c:894
#8  0x0000000000427550 in cmd_fetch (argc=2, argv=0x7fffffffe070, prefix=0x0) at builtin/fetch.c:955
#9  0x0000000000405084 in run_builtin (argv=0x7fffffffe070, argc=5, p=0x731b08) at git.c:308
#10 handle_internal_command (argc=5, argv=0x7fffffffe070) at git.c:466
#11 0x000000000040448b in run_argv (argv=0x7fffffffdf10, argcp=0x7fffffffdf1c) at git.c:512
#12 main (argc=5, argv=0x7fffffffe070) at git.c:585

--Ben

^ permalink raw reply

* Re: [PATCH v3 0/5] reroll bc/attr-ignore-case
From: Junio C Hamano @ 2011-10-06 20:57 UTC (permalink / raw)
  To: Brandon Casey; +Cc: git, peff, j.sixt
In-Reply-To: <U4wiHVyDLLG1PhI-8iY3YvK8PNPcrE_H1LfZdeQFnOrSJ5O-Hev7KzBOnrAY-vjiT2yzgWpfSAPFG2-cG0LrUPezOcTUNC6trUWFODOXIkXyJzvz-maBv6HrPKDCs98TTT4OGmgo0j8@cipher.nrlssc.navy.mil>

Thanks.

^ permalink raw reply

* Re: [PATCH 0/3] Be more careful when prunning
From: Carlos Martín Nieto @ 2011-10-06 20:58 UTC (permalink / raw)
  To: mathstuf; +Cc: git, Jeff King, Junio C Hamano
In-Reply-To: <20111006205103.GA1271@erythro.kitwarein.com>

[-- Attachment #1: Type: text/plain, Size: 3136 bytes --]

On Thu, 2011-10-06 at 16:51 -0400, Ben Boeckel wrote:
> On Thu, Oct 06, 2011 at 22:19:42 +0200, Carlos Martín Nieto wrote:
> > 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 teaches get_stale_heads to use the user-provided
> > refspecs instead of the ones in the config. 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 third patch,
> > which pretends that a "refs/tags/*:refs/tags/*" refspec was given on
> > the command-line.
> 
> I applied the patches to current master (7f41b6b) and got a segfault
> with:
> 
>     git fetch -p -t origin master
> 
> It does not happen with master.

I thought I'd got rid of those problems. Thanks for noticing. I'll
investigate.

> 
> Backtrace:
> 
> (gdb) bt
> #0  0x00007ffff7395d18 in __strchr_sse42 () from /lib64/libc.so.6
> #1  0x00000000004b2d39 in find_in_refs (query=0x7fffffffdb90, ref_count=2, refs=<optimized out>) at remote.c:1709
> #2  get_stale_heads_cb (refname=0x7a8f31 "refs/heads/a/branch/name", sha1=0x7a8f09 "\367\343\375C٩\223u\305OG\233)z\347X\370\333\325", <incomplete sequence \335>, flags=0, cb_data=0x7fffffffdc50) at remote.c:1740
> #3  0x00000000004adf19 in do_for_each_ref (submodule=<optimized out>, base=0x4ea1c2 "", fn=0x4b2ca0 <get_stale_heads_cb>, trim=0, flags=0, cb_data=0x7fffffffdc50) at refs.c:684
> #4  0x00000000004b4249 in get_stale_heads (remote=<optimized out>, fetch_map=<optimized out>, refs=<optimized out>, ref_count=<optimized out>) at remote.c:1777
> #5  0x0000000000426cfb in prune_refs (ref_map=<optimized out>, n=<optimized out>, refs=<optimized out>, transport=0x78e040) at builtin/fetch.c:511
> #6  do_fetch (ref_count=<optimized out>, refs=<optimized out>, transport=0x78e040) at builtin/fetch.c:711
> #7  fetch_one (remote=<optimized out>, argc=<optimized out>, argv=<optimized out>) at builtin/fetch.c:894
> #8  0x0000000000427550 in cmd_fetch (argc=2, argv=0x7fffffffe070, prefix=0x0) at builtin/fetch.c:955
> #9  0x0000000000405084 in run_builtin (argv=0x7fffffffe070, argc=5, p=0x731b08) at git.c:308
> #10 handle_internal_command (argc=5, argv=0x7fffffffe070) at git.c:466
> #11 0x000000000040448b in run_argv (argv=0x7fffffffdf10, argcp=0x7fffffffdf1c) at git.c:512
> #12 main (argc=5, argv=0x7fffffffe070) at git.c:585
> 
> --Ben
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply

* [PATCHv2 0/3] Be more careful when prunning
From: Carlos Martín Nieto @ 2011-10-06 21:21 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano, mathstuf
In-Reply-To: <20111006205103.GA1271@erythro.kitwarein.com>

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 teaches get_stale_heads to use the user-provided
refspecs instead of the ones in the config. 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 third patch,
which pretends that a "refs/tags/*:refs/tags/*" refspec was given on
the command-line.

Version 1 assumed that a refspec would have its dst filled
automatically. This is not the case and was fixed in the second patch.

Cheers,
   cmn

Carlos Martín Nieto (3):
      fetch: free all the additional refspecs
      fetch: honor the user-provided refspecs when pruning refs
      fetch: treat --tags like refs/tags/*:refs/tags/* when pruning

 builtin/fetch.c  |   19 ++++++++++---
 builtin/remote.c |    2 +-
 remote.c         |   74 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 remote.h         |    3 +-
 4 files changed, 84 insertions(+), 14 deletions(-)

-- 
1.7.5.2.354.g349bf

^ permalink raw reply

* [PATCH 3/3] fetch: treat --tags like refs/tags/*:refs/tags/* when pruning
From: Carlos Martín Nieto @ 2011-10-06 21:21 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano, mathstuf
In-Reply-To: <1317936107-1230-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 |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index b937d71..94b2bd3 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -699,8 +699,17 @@ static int do_fetch(struct transport *transport,
 		free_refs(ref_map);
 		return 1;
 	}
-	if (prune)
+	if (prune) {
+		/* If --tags was specified, we need to tell prune_refs
+		 * that we're filtering the refs from the remote */
+		if (tags == TAGS_SET) {
+			const char * tags_refspec = "refs/tags/*:refs/tags/*";
+			refs = xrealloc(refs, (ref_count + 1) * sizeof(struct refspec));
+			refs[ref_count] = *parse_fetch_refspec(1, &tags_refspec);
+			ref_count++;
+		}
 		prune_refs(transport, refs, ref_count, ref_map);
+	}
 	free_refs(ref_map);
 
 	/* if neither --no-tags nor --tags was specified, do automated tag
-- 
1.7.5.2.354.g349bf

^ permalink raw reply related

* [PATCH 1/3] fetch: free all the additional refspecs
From: Carlos Martín Nieto @ 2011-10-06 21:21 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano, mathstuf
In-Reply-To: <1317936107-1230-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

* [PATCH 2/3] fetch: honor the user-provided refspecs when pruning refs
From: Carlos Martín Nieto @ 2011-10-06 21:21 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano, mathstuf
In-Reply-To: <1317936107-1230-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 tag 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.

Teach get_stale_heads about user-provided refspecs and use them if
they're available.

Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
---
 builtin/fetch.c  |    6 ++--
 builtin/remote.c |    2 +-
 remote.c         |   78 +++++++++++++++++++++++++++++++++++++++++++++++++-----
 remote.h         |    3 +-
 4 files changed, 77 insertions(+), 12 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 30b485e..b937d71 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 transport *transport, struct refspec *refs, int n, 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(transport->remote, ref_map, refs, n);
 	const char *dangling_msg = dry_run
 		? _("   (%s will become dangling)\n")
 		: _("   (%s has become dangling)\n");
@@ -700,7 +700,7 @@ static int do_fetch(struct transport *transport,
 		return 1;
 	}
 	if (prune)
-		prune_refs(transport, ref_map);
+		prune_refs(transport, refs, ref_count, 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 b25dfb4..91a2148 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -349,7 +349,7 @@ 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(states->remote, fetch_map, NULL, 0);
 	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 7840d2f..28f7917 100644
--- a/remote.c
+++ b/remote.c
@@ -1684,26 +1684,88 @@ struct stale_heads_info {
 	struct remote *remote;
 	struct string_list *ref_names;
 	struct ref **stale_refs_tail;
+	struct refspec *refs;
+	int ref_count;
 };
 
+/*
+ * Find a refspec to a remote's
+ * Returns 0 on success, -1 if it couldn't find a the refspec
+ */
+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);
-		}
+
+	/*
+	 * If the user speicified refspecs on the command line, we
+	 * should only use those to check. Otherwise, look in the
+	 * remote's configuration for the branch.
+	 */
+	if (info->ref_count)
+		ret = find_in_refs(info->refs, info->ref_count, &refspec);
+	else
+		ret = remote_find_tracking(info->remote, &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 remote *remote, 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;
@@ -1711,6 +1773,8 @@ struct ref *get_stale_heads(struct remote *remote, struct ref *fetch_map)
 	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..2f753a0 100644
--- a/remote.h
+++ b/remote.h
@@ -164,6 +164,7 @@ 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 remote *remote, struct ref *fetch_map,
+			    struct refspec *refs, int ref_count);
 
 #endif
-- 
1.7.5.2.354.g349bf

^ 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