threads / patch / 35320

v6, 10 partstransport-helper: add support to delete branches

Subject: [PATCH v6 10/10] transport-helper: add support to delete branches

## tl;dr

32 messages between Nov 11, 2013 and Apr 24, 2014. Diffs are folded; open one to read it.

replies: 31people: 3as markdown or json

Felipe Contreras· Nov 11, 2013, 22:54 UTC · lore
For remote-helpers that use 'export' to push.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/t5801-remote-helpers.sh |  8 ++++++++
 transport-helper.c        | 11 ++++++-----
 2 files changed, 14 insertions(+), 5 deletions(-)
Show changes to 2 files +14 −5

t/t5801-remote-helpers.sh, transport-helper.c

diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 454337e..c667965 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -94,6 +94,14 @@ test_expect_success 'push new branch with old:new refspec' '
 	compare_refs local HEAD server refs/heads/new-refspec
 '
 
+test_expect_success 'push delete branch' '
+	(cd local &&
+	 git push origin :new-name
+	) &&
+	test_must_fail git --git-dir="server/.git" \
+	 rev-parse --verify refs/heads/new-name
+'
+
 test_expect_success 'forced push' '
 	(cd local &&
 	git checkout -b force-test &&
diff --git a/transport-helper.c b/transport-helper.c
index 7411125..2257588 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -876,9 +876,6 @@ static int push_refs_with_export(struct transport *transport,
 		char *private;
 		unsigned char sha1[20];
 
-		if (ref->deletion)
-			die("remote-helpers do not support ref deletion");
-
 		private = apply_refspecs(data->refspecs, data->refspec_nr, ref->name);
 		if (private && !get_sha1(private, sha1)) {
 			strbuf_addf(&buf, "^%s", private);
@@ -890,12 +887,16 @@ static int push_refs_with_export(struct transport *transport,
 		if (ref->peer_ref) {
 			if (strcmp(ref->name, ref->peer_ref->name)) {
 				struct strbuf buf = STRBUF_INIT;
-				strbuf_addf(&buf, "%s:%s", ref->peer_ref->name, ref->name);
+				if (!ref->deletion)
+					strbuf_addf(&buf, "%s:%s", ref->peer_ref->name, ref->name);
+				else
+					strbuf_addf(&buf, ":%s", ref->name);
 				string_list_append(&revlist_args, "--refspec");
 				string_list_append(&revlist_args, buf.buf);
 				strbuf_release(&buf);
 			}
-			string_list_append(&revlist_args, ref->peer_ref->name);
+			if (!ref->deletion)
+				string_list_append(&revlist_args, ref->peer_ref->name);
 		}
 	}
 
-- 
1.8.4.2+fc1
Felipe Contreras· Nov 11, 2013, 22:54 UTC · re: Felipe Contreras · lore

[PATCH v6 00/10] transport-helper: updates

Hi,

Here are the patches that allow transport helpers to be completely transparent; renaming branches, deleting them, custom refspecs, --force, --dry-run, reporting forced update, everything works.

Small changes since v5:
Show changes to 2 files +5 −1

builtin/fast-export.c, git-remote-testgit.sh

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 8ed41b4..4b76222 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -736,9 +736,10 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 		usage_with_options (fast_export_usage, options);
 
 	if (refspecs_list.nr) {
-		const char *refspecs_str[refspecs_list.nr];
+		const char **refspecs_str;
 		int i;
 
+		refspecs_str = xmalloc(sizeof(*refspecs_str) * refspecs_list.nr);
 		for (i = 0; i < refspecs_list.nr; i++)
 			refspecs_str[i] = refspecs_list.items[i].string;
 
@@ -746,6 +747,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 		refspecs = parse_fetch_refspec(refspecs_nr, refspecs_str);
 
 		string_list_clear(&refspecs_list, 1);
+		free(refspecs_str);
 	}
 
 	if (use_done_feature)
diff --git a/git-remote-testgit.sh b/git-remote-testgit.sh
index 716aa4c..1c006a0 100755
--- a/git-remote-testgit.sh
+++ b/git-remote-testgit.sh
@@ -15,6 +15,8 @@ test -z "$refspec" && prefix="refs"
 
 export GIT_DIR="$url/.git"
 
+force=
+
 mkdir -p "$dir"
 
 if test -z "$GIT_REMOTE_TESTGIT_NO_MARKS"

Felipe Contreras (10):
  transport-helper: fix extra lines
  transport-helper: don't update refs in dry-run
  transport-helper: add 'force' to 'export' helpers
  transport-helper: check for 'forced update' message
  fast-export: improve argument parsing
  fast-export: add new --refspec option
  transport-helper: add support for old:new refspec
  fast-import: add support to delete refs
  fast-export: add support to delete refs
  transport-helper: add support to delete branches

 Documentation/git-fast-export.txt   |  4 +++
 Documentation/git-fast-import.txt   |  3 +++
 Documentation/gitremote-helpers.txt |  4 +++
 builtin/fast-export.c               | 49 ++++++++++++++++++++++++++++++++++++-
 fast-import.c                       | 13 +++++++---
 git-remote-testgit.sh               | 18 ++++++++++++++
 t/t5801-remote-helpers.sh           | 23 ++++++++++++++++-
 t/t9300-fast-import.sh              | 18 ++++++++++++++
 t/t9350-fast-export.sh              | 18 ++++++++++++++
 transport-helper.c                  | 47 +++++++++++++++++++++++------------
 10 files changed, 177 insertions(+), 20 deletions(-)
-- 
1.8.4.2+fc1
Junio C Hamano· Nov 11, 2013, 23:33 UTC · re: Felipe Contreras · lore

Re: [PATCH v6 00/10] transport-helper: updates

Felipe Contreras <felipe.contreras@gmail.com> writes:
> Here are the patches that allow transport helpers to be completely transparent;
> renaming branches, deleting them, custom refspecs, --force, --dry-run,
> reporting forced update, everything works.

How are you sending your patches? The Message-ID's suggest that git-send-email is being used, but when the patches are ordered by the sender datestamp, they seem to come out in a random order, unlike the patches other people send with git-send-email.

git-send-email gives a timestamp to the first message it sends out and then gives consecutive timestamps, one second apart, to subsequent messages, in order to make this "sorting by sender timestamp on Date: field" work. I am wondering if there is something you are doing differently, and/or if there is something that needs to be fixed in the version of git-send-email you are using.

Thanks.
Felipe Contreras· Nov 11, 2013, 23:44 UTC · re: Junio C Hamano · lore

Re: [PATCH v6 00/10] transport-helper: updates

On Mon, Nov 11, 2013 at 5:33 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 7 quoted lines
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> Here are the patches that allow transport helpers to be completely transparent;
>> renaming branches, deleting them, custom refspecs, --force, --dry-run,
>> reporting forced update, everything works.
>
> How are you sending your patches?
% git-send-email --no-annotate list-of-patches
However, I just noticed that the list-of-patches is in the wrong order.
-- 
Felipe Contreras
Junio C Hamano· Nov 11, 2013, 23:37 UTC · re: Felipe Contreras · lore

Re: [PATCH v6 00/10] transport-helper: updates

Felipe Contreras <felipe.contreras@gmail.com> writes:
Show 39 quoted lines
> Small changes since v5:
>
> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> index 8ed41b4..4b76222 100644
> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -736,9 +736,10 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
>  		usage_with_options (fast_export_usage, options);
>  
>  	if (refspecs_list.nr) {
> -		const char *refspecs_str[refspecs_list.nr];
> +		const char **refspecs_str;
>  		int i;
>  
> +		refspecs_str = xmalloc(sizeof(*refspecs_str) * refspecs_list.nr);
>  		for (i = 0; i < refspecs_list.nr; i++)
>  			refspecs_str[i] = refspecs_list.items[i].string;
>  
> @@ -746,6 +747,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
>  		refspecs = parse_fetch_refspec(refspecs_nr, refspecs_str);
>  
>  		string_list_clear(&refspecs_list, 1);
> +		free(refspecs_str);
>  	}
>  
>  	if (use_done_feature)
> diff --git a/git-remote-testgit.sh b/git-remote-testgit.sh
> index 716aa4c..1c006a0 100755
> --- a/git-remote-testgit.sh
> +++ b/git-remote-testgit.sh
> @@ -15,6 +15,8 @@ test -z "$refspec" && prefix="refs"
>  
>  export GIT_DIR="$url/.git"
>  
> +force=
> +
>  mkdir -p "$dir"
>  
>  if test -z "$GIT_REMOTE_TESTGIT_NO_MARKS"

Looking good, modulo a few minor nits I noticed while comparing with the one that has been queued in 'pu', which I commented separately.

Will re-queue.  Thanks.
Richard Hansen· Nov 12, 2013, 06:21 UTC · re: Felipe Contreras · lore

Re: [PATCH v6 00/10] transport-helper: updates

On 2013-11-11 17:54, Felipe Contreras wrote:
Show 45 quoted lines
> Hi,
> 
> Here are the patches that allow transport helpers to be completely transparent;
> renaming branches, deleting them, custom refspecs, --force, --dry-run,
> reporting forced update, everything works.
> 
> Small changes since v5:
> 
> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> index 8ed41b4..4b76222 100644
> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -736,9 +736,10 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
>  		usage_with_options (fast_export_usage, options);
>  
>  	if (refspecs_list.nr) {
> -		const char *refspecs_str[refspecs_list.nr];
> +		const char **refspecs_str;
>  		int i;
>  
> +		refspecs_str = xmalloc(sizeof(*refspecs_str) * refspecs_list.nr);
>  		for (i = 0; i < refspecs_list.nr; i++)
>  			refspecs_str[i] = refspecs_list.items[i].string;
>  
> @@ -746,6 +747,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
>  		refspecs = parse_fetch_refspec(refspecs_nr, refspecs_str);
>  
>  		string_list_clear(&refspecs_list, 1);
> +		free(refspecs_str);
>  	}
>  
>  	if (use_done_feature)
> diff --git a/git-remote-testgit.sh b/git-remote-testgit.sh
> index 716aa4c..1c006a0 100755
> --- a/git-remote-testgit.sh
> +++ b/git-remote-testgit.sh
> @@ -15,6 +15,8 @@ test -z "$refspec" && prefix="refs"
>  
>  export GIT_DIR="$url/.git"
>  
> +force=
> +
>  mkdir -p "$dir"
>  
>  if test -z "$GIT_REMOTE_TESTGIT_NO_MARKS"
What about changing those two test-hg.sh tests to test_expect_success?
  http://article.gmane.org/gmane.comp.version-control.git/237606

Should those changes be squashed into the "transport-helper: don't update refs in dry-run" and "transport-helper: add 'force' to 'export' helpers" commits? Or are those commits not really the appropriate place?

Thanks, Richard

Felipe Contreras· Nov 12, 2013, 20:55 UTC · re: Richard Hansen · lore

Re: [PATCH v6 00/10] transport-helper: updates

Richard Hansen wrote:
Show 50 quoted lines
> On 2013-11-11 17:54, Felipe Contreras wrote:
> > Hi,
> > 
> > Here are the patches that allow transport helpers to be completely transparent;
> > renaming branches, deleting them, custom refspecs, --force, --dry-run,
> > reporting forced update, everything works.
> > 
> > Small changes since v5:
> > 
> > diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> > index 8ed41b4..4b76222 100644
> > --- a/builtin/fast-export.c
> > +++ b/builtin/fast-export.c
> > @@ -736,9 +736,10 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
> >  		usage_with_options (fast_export_usage, options);
> >  
> >  	if (refspecs_list.nr) {
> > -		const char *refspecs_str[refspecs_list.nr];
> > +		const char **refspecs_str;
> >  		int i;
> >  
> > +		refspecs_str = xmalloc(sizeof(*refspecs_str) * refspecs_list.nr);
> >  		for (i = 0; i < refspecs_list.nr; i++)
> >  			refspecs_str[i] = refspecs_list.items[i].string;
> >  
> > @@ -746,6 +747,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
> >  		refspecs = parse_fetch_refspec(refspecs_nr, refspecs_str);
> >  
> >  		string_list_clear(&refspecs_list, 1);
> > +		free(refspecs_str);
> >  	}
> >  
> >  	if (use_done_feature)
> > diff --git a/git-remote-testgit.sh b/git-remote-testgit.sh
> > index 716aa4c..1c006a0 100755
> > --- a/git-remote-testgit.sh
> > +++ b/git-remote-testgit.sh
> > @@ -15,6 +15,8 @@ test -z "$refspec" && prefix="refs"
> >  
> >  export GIT_DIR="$url/.git"
> >  
> > +force=
> > +
> >  mkdir -p "$dir"
> >  
> >  if test -z "$GIT_REMOTE_TESTGIT_NO_MARKS"
> 
> What about changing those two test-hg.sh tests to test_expect_success?
> 
>   http://article.gmane.org/gmane.comp.version-control.git/237606
I can do that.
> Should those changes be squashed into the "transport-helper: don't
> update refs in dry-run" and "transport-helper: add 'force' to 'export'
> helpers" commits?  Or are those commits not really the appropriate place?

I think it should be a separate commit, since it's not part of the core, but contrib area.

-- 
Felipe Contreras
Richard Hansen· Nov 12, 2013, 07:03 UTC · re: Felipe Contreras · lore

[PATCH v2] remote-bzr: support the new 'force' option

Signed-off-by: Richard Hansen <rhansen@bbn.com>
---
This is a reroll of:
  http://article.gmane.org/gmane.comp.version-control.git/237607
based on feedback from Felipe:
  http://article.gmane.org/gmane.comp.version-control.git/237615
This patch is an optional extension to Felipe's "transport-helper:
updates" patch series:
  http://thread.gmane.org/gmane.comp.version-control.git/237663
and it requires those changes to work.
 contrib/remote-helpers/git-remote-bzr | 32 +++++++++++++++++++++++++++++++-
 contrib/remote-helpers/test-bzr.sh    | 22 +++++++++++++++++++++-
 2 files changed, 52 insertions(+), 2 deletions(-)
Show changes to 2 files +52 −2

contrib/remote-helpers/git-remote-bzr, contrib/remote-helpers/test-bzr.sh

diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
index 7e34532..2f481e9 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -42,6 +42,7 @@ import json
 import re
 import StringIO
 import atexit, shutil, hashlib, urlparse, subprocess
+import types
 
 NAME_RE = re.compile('^([^<>]+)')
 AUTHOR_RE = re.compile('^([^<>]+?)? ?[<>]([^<>]*)(?:$|>)')
@@ -684,7 +685,8 @@ def do_export(parser):
                 peer = bzrlib.branch.Branch.open(peers[name],
                                                  possible_transports=transports)
                 try:
-                    peer.bzrdir.push_branch(branch, revision_id=revid)
+                    peer.bzrdir.push_branch(branch, revision_id=revid,
+                                            overwrite=force)
                 except bzrlib.errors.DivergedBranches:
                     print "error %s non-fast forward" % ref
                     continue
@@ -718,8 +720,32 @@ def do_capabilities(parser):
         print "*import-marks %s" % path
     print "*export-marks %s" % path
 
+    print "option"
     print
 
+class InvalidOptionValue(Exception):
+    pass
+
+def get_bool_option(val):
+    if val == 'true':
+        return True
+    elif val == 'false':
+        return False
+    else:
+        raise InvalidOptionValue()
+
+def do_option(parser):
+    global force
+    (opt, val) = parser[1:3]
+    try:
+        if opt == 'force':
+            force = get_bool_option(val)
+            print 'ok'
+        else:
+            print 'unsupported'
+    except InvalidOptionValue:
+        print "error '%s' is not a valid value for option '%s'" % (val, opt)
+
 def ref_is_valid(name):
     return not True in [c in name for c in '~^: \\']
 
@@ -882,6 +908,7 @@ def main(args):
     global is_tmp
     global branches, peers
     global transports
+    global force
 
     alias = args[1]
     url = args[2]
@@ -895,6 +922,7 @@ def main(args):
     branches = {}
     peers = {}
     transports = []
+    force = False
 
     if alias[5:] == url:
         is_tmp = True
@@ -930,6 +958,8 @@ def main(args):
             do_import(parser)
         elif parser.check('export'):
             do_export(parser)
+        elif parser.check('option'):
+            do_option(parser)
         else:
             die('unhandled command: %s' % line)
         sys.stdout.flush()
diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh
index 1e53ff9..4f379c2 100755
--- a/contrib/remote-helpers/test-bzr.sh
+++ b/contrib/remote-helpers/test-bzr.sh
@@ -66,13 +66,33 @@ test_expect_success 'pushing' '
 	test_cmp expected actual
 '
 
+test_expect_success 'forced pushing' '
+	(
+	cd gitrepo &&
+	echo three-new >content &&
+	git commit -a --amend -m three-new &&
+	git push -f
+	) &&
+
+	(
+	cd bzrrepo &&
+	# the forced update overwrites the bzr branch but not the bzr
+	# working directory (it tries to merge instead)
+	bzr revert
+	) &&
+
+	echo three-new >expected &&
+	cat bzrrepo/content >actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'roundtrip' '
 	(
 	cd gitrepo &&
 	git pull &&
 	git log --format="%s" -1 origin/master >actual
 	) &&
-	echo three >expected &&
+	echo three-new >expected &&
 	test_cmp expected actual &&
 
 	(cd gitrepo && git push && git pull) &&
-- 
1.8.5.rc1.208.g8ff7964
Felipe Contreras· Nov 12, 2013, 21:01 UTC · re: Richard Hansen · lore

RE: [PATCH v2] remote-bzr: support the new 'force' option

Richard Hansen wrote:
Show 26 quoted lines
> Signed-off-by: Richard Hansen <rhansen@bbn.com>
> ---
> 
> This is a reroll of:
>   http://article.gmane.org/gmane.comp.version-control.git/237607
> based on feedback from Felipe:
>   http://article.gmane.org/gmane.comp.version-control.git/237615
> 
> This patch is an optional extension to Felipe's "transport-helper:
> updates" patch series:
>   http://thread.gmane.org/gmane.comp.version-control.git/237663
> and it requires those changes to work.
> 
>  contrib/remote-helpers/git-remote-bzr | 32 +++++++++++++++++++++++++++++++-
>  contrib/remote-helpers/test-bzr.sh    | 22 +++++++++++++++++++++-
>  2 files changed, 52 insertions(+), 2 deletions(-)
> 
> diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
> index 7e34532..2f481e9 100755
> --- a/contrib/remote-helpers/git-remote-bzr
> +++ b/contrib/remote-helpers/git-remote-bzr
> @@ -42,6 +42,7 @@ import json
>  import re
>  import StringIO
>  import atexit, shutil, hashlib, urlparse, subprocess
> +import types
No need for this any more.
Show 33 quoted lines
>  NAME_RE = re.compile('^([^<>]+)')
>  AUTHOR_RE = re.compile('^([^<>]+?)? ?[<>]([^<>]*)(?:$|>)')
> @@ -684,7 +685,8 @@ def do_export(parser):
>                  peer = bzrlib.branch.Branch.open(peers[name],
>                                                   possible_transports=transports)
>                  try:
> -                    peer.bzrdir.push_branch(branch, revision_id=revid)
> +                    peer.bzrdir.push_branch(branch, revision_id=revid,
> +                                            overwrite=force)
>                  except bzrlib.errors.DivergedBranches:
>                      print "error %s non-fast forward" % ref
>                      continue
> @@ -718,8 +720,32 @@ def do_capabilities(parser):
>          print "*import-marks %s" % path
>      print "*export-marks %s" % path
>  
> +    print "option"
>      print
>  
> +class InvalidOptionValue(Exception):
> +    pass
> +
> +def get_bool_option(val):
> +    if val == 'true':
> +        return True
> +    elif val == 'false':
> +        return False
> +    else:
> +        raise InvalidOptionValue()
> +
> +def do_option(parser):
> +    global force
> +    (opt, val) = parser[1:3]
I prefer:
  opt, val = parser[1:3]
But not a big deal.
Otherwise the patch looks OK to me.
-- 
Felipe Contreras
Felipe Contreras· Nov 11, 2013, 22:54 UTC · re: Felipe Contreras · lore

[PATCH v6 01/10] transport-helper: fix extra lines

Commit 9c51558 (transport-helper: trivial code shuffle) moved these lines above, but 99d9ec0 (Merge branch 'fc/transport-helper-no-refspec') had a wrong merge conflict and readded them.

Reported-by: Richard Hansen <rhansen@bbn.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 transport-helper.c | 3 ---
 1 file changed, 3 deletions(-)
Show changes to transport-helper.c +0 −3
diff --git a/transport-helper.c b/transport-helper.c
index 673b7c2..b66c7fd 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -875,9 +875,6 @@ static int push_refs_with_export(struct transport *transport,
 		}
 		free(private);
 
-		if (ref->deletion)
-			die("remote-helpers do not support ref deletion");
-
 		if (ref->peer_ref) {
 			if (strcmp(ref->peer_ref->name, ref->name))
 				die("remote-helpers do not support old:new syntax");
-- 
1.8.4.2+fc1
Felipe Contreras· Nov 11, 2013, 22:55 UTC · re: Felipe Contreras · lore

[PATCH v6 08/10] fast-import: add support to delete refs

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-fast-import.txt |  3 +++
 fast-import.c                     | 13 ++++++++++---
 t/t9300-fast-import.sh            | 18 ++++++++++++++++++
 3 files changed, 31 insertions(+), 3 deletions(-)
Show changes to 3 files +31 −3

Documentation/git-fast-import.txt, fast-import.c, t/t9300-fast-import.sh

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 73f9806..2ffae42 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -483,6 +483,9 @@ Marks must be declared (via `mark`) before they can be used.
 * Any valid Git SHA-1 expression that resolves to a commit.  See
   ``SPECIFYING REVISIONS'' in linkgit:gitrevisions[7] for details.
 
+* The special null SHA-1 (40 zeros) specifies that the branch is to be
+  removed.
+
 The special case of restarting an incremental import from the
 current branch value should be written as:
 ----
diff --git a/fast-import.c b/fast-import.c
index f4d9969..fdce0b7 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -248,6 +248,7 @@ struct branch {
 	uintmax_t last_commit;
 	uintmax_t num_notes;
 	unsigned active : 1;
+	unsigned delete : 1;
 	unsigned pack_id : PACK_ID_BITS;
 	unsigned char sha1[20];
 };
@@ -1690,10 +1691,13 @@ static int update_branch(struct branch *b)
 	struct ref_lock *lock;
 	unsigned char old_sha1[20];
 
-	if (is_null_sha1(b->sha1))
-		return 0;
 	if (read_ref(b->name, old_sha1))
 		hashclr(old_sha1);
+	if (is_null_sha1(b->sha1)) {
+		if (b->delete)
+			delete_ref(b->name, old_sha1, 0);
+		return 0;
+	}
 	lock = lock_any_ref_for_update(b->name, old_sha1, 0, NULL);
 	if (!lock)
 		return error("Unable to lock %s", b->name);
@@ -2620,8 +2624,11 @@ static int parse_from(struct branch *b)
 			free(buf);
 		} else
 			parse_from_existing(b);
-	} else if (!get_sha1(from, b->sha1))
+	} else if (!get_sha1(from, b->sha1)) {
 		parse_from_existing(b);
+		if (is_null_sha1(b->sha1))
+			b->delete = 1;
+	}
 	else
 		die("Invalid ref name or SHA1 expression: %s", from);
 
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 27263df..5fc9ef2 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -2999,4 +2999,22 @@ test_expect_success 'T: ls root tree' '
 	test_cmp expect actual
 '
 
+test_expect_success 'T: delete branch' '
+	git branch to-delete &&
+	git fast-import <<-EOF &&
+	reset refs/heads/to-delete
+	from 0000000000000000000000000000000000000000
+	EOF
+	test_must_fail git rev-parse --verify refs/heads/to-delete
+'
+
+test_expect_success 'T: empty reset doesnt delete branch' '
+	git branch not-to-delete &&
+	git fast-import <<-EOF &&
+	reset refs/heads/not-to-delete
+	EOF
+	git show-ref &&
+	git rev-parse --verify refs/heads/not-to-delete
+'
+
 test_done
-- 
1.8.4.2+fc1
Felipe Contreras· Nov 11, 2013, 22:55 UTC · re: Felipe Contreras · lore

[PATCH v6 05/10] fast-export: improve argument parsing

We don't want to pass arguments specific to fast-export to setup_revisions.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/fast-export.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Show changes to builtin/fast-export.c +2 −1
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 78250ea..eea5b8c 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -701,8 +701,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 	revs.topo_order = 1;
 	revs.show_source = 1;
 	revs.rewrite_parents = 1;
+	argc = parse_options(argc, argv, prefix, options, fast_export_usage,
+			PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN);
 	argc = setup_revisions(argc, argv, &revs, NULL);
-	argc = parse_options(argc, argv, prefix, options, fast_export_usage, 0);
 	if (argc > 1)
 		usage_with_options (fast_export_usage, options);
 
-- 
1.8.4.2+fc1
Felipe Contreras· Nov 11, 2013, 22:55 UTC · re: Felipe Contreras · lore

[PATCH v6 06/10] fast-export: add new --refspec option

So that we can convert the exported ref names.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-fast-export.txt |  4 ++++
 builtin/fast-export.c             | 32 ++++++++++++++++++++++++++++++++
 t/t9350-fast-export.sh            |  7 +++++++
 3 files changed, 43 insertions(+)
Show changes to 3 files +43 −0

Documentation/git-fast-export.txt, builtin/fast-export.c, t/t9350-fast-export.sh

diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt
index 85f1f30..221506b 100644
--- a/Documentation/git-fast-export.txt
+++ b/Documentation/git-fast-export.txt
@@ -105,6 +105,10 @@ marks the same across runs.
 	in the commit (as opposed to just listing the files which are
 	different from the commit's first parent).
 
+--refspec::
+	Apply the specified refspec to each ref exported. Multiple of them can
+	be specified.
+
 [<git-rev-list-args>...]::
 	A list of arguments, acceptable to 'git rev-parse' and
 	'git rev-list', that specifies the specific objects and references
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index eea5b8c..cf745ec 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -17,6 +17,7 @@
 #include "utf8.h"
 #include "parse-options.h"
 #include "quote.h"
+#include "remote.h"
 
 static const char *fast_export_usage[] = {
 	N_("git fast-export [rev-list-opts]"),
@@ -31,6 +32,8 @@ static int use_done_feature;
 static int no_data;
 static int full_tree;
 static struct string_list extra_refs = STRING_LIST_INIT_NODUP;
+static struct refspec *refspecs;
+static int refspecs_nr;
 
 static int parse_opt_signed_tag_mode(const struct option *opt,
 				     const char *arg, int unset)
@@ -525,6 +528,15 @@ static void get_tags_and_duplicates(struct rev_cmdline_info *info)
 		if (dwim_ref(e->name, strlen(e->name), sha1, &full_name) != 1)
 			continue;
 
+		if (refspecs) {
+			char *private;
+			private = apply_refspecs(refspecs, refspecs_nr, full_name);
+			if (private) {
+				free(full_name);
+				full_name = private;
+			}
+		}
+
 		commit = get_commit(e, full_name);
 		if (!commit) {
 			warning("%s: Unexpected object of type %s, skipping.",
@@ -668,6 +680,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 	struct commit *commit;
 	char *export_filename = NULL, *import_filename = NULL;
 	uint32_t lastimportid;
+	struct string_list refspecs_list = STRING_LIST_INIT_NODUP;
 	struct option options[] = {
 		OPT_INTEGER(0, "progress", &progress,
 			    N_("show progress after <n> objects")),
@@ -688,6 +701,8 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 		OPT_BOOL(0, "use-done-feature", &use_done_feature,
 			     N_("Use the done feature to terminate the stream")),
 		OPT_BOOL(0, "no-data", &no_data, N_("Skip output of blob data")),
+		OPT_STRING_LIST(0, "refspec", &refspecs_list, N_("refspec"),
+			     N_("Apply refspec to exported refs")),
 		OPT_END()
 	};
 
@@ -707,6 +722,21 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 	if (argc > 1)
 		usage_with_options (fast_export_usage, options);
 
+	if (refspecs_list.nr) {
+		const char **refspecs_str;
+		int i;
+
+		refspecs_str = xmalloc(sizeof(*refspecs_str) * refspecs_list.nr);
+		for (i = 0; i < refspecs_list.nr; i++)
+			refspecs_str[i] = refspecs_list.items[i].string;
+
+		refspecs_nr = refspecs_list.nr;
+		refspecs = parse_fetch_refspec(refspecs_nr, refspecs_str);
+
+		string_list_clear(&refspecs_list, 1);
+		free(refspecs_str);
+	}
+
 	if (use_done_feature)
 		printf("feature done\n");
 
@@ -741,5 +771,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 	if (use_done_feature)
 		printf("done\n");
 
+	free_refspec(refspecs_nr, refspecs);
+
 	return 0;
 }
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 2312dec..3d475af 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -504,4 +504,11 @@ test_expect_success 'refs are updated even if no commits need to be exported' '
 	test_cmp expected actual
 '
 
+test_expect_success 'use refspec' '
+	git fast-export --refspec refs/heads/master:refs/heads/foobar master | \
+		grep "^commit " | sort | uniq > actual &&
+	echo "commit refs/heads/foobar" > expected &&
+	test_cmp expected actual
+'
+
 test_done
-- 
1.8.4.2+fc1
Junio C Hamano· Nov 11, 2013, 23:25 UTC · re: Felipe Contreras · lore

Re: [PATCH v6 06/10] fast-export: add new --refspec option

Felipe Contreras <felipe.contreras@gmail.com> writes:
> So that we can convert the exported ref names.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---

I thought that the discussion agreed this option should not be called --refspec but something like --refmap?

Felipe Contreras· Nov 11, 2013, 23:50 UTC · re: Junio C Hamano · lore

Re: [PATCH v6 06/10] fast-export: add new --refspec option

On Mon, Nov 11, 2013 at 5:25 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 9 quoted lines
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> So that we can convert the exported ref names.
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>
> I thought that the discussion agreed this option should not be
> called --refspec but something like --refmap?

I don't know what you agreed to, but I didn't agree to anything. What you pass to this option is a refspec, so it makes sense to name the option --refspec.

-- 
Felipe Contreras
Richard Hansen· Nov 12, 2013, 07:39 UTC · re: Felipe Contreras · lore

Re: [PATCH v6 06/10] fast-export: add new --refspec option

On 2013-11-11 18:50, Felipe Contreras wrote:
Show 12 quoted lines
> On Mon, Nov 11, 2013 at 5:25 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>> So that we can convert the exported ref names.
>>>
>>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>>> ---
>>
>> I thought that the discussion agreed this option should not be
>> called --refspec but something like --refmap?
> 
> I don't know what you agreed to,
http://article.gmane.org/gmane.comp.version-control.git/237473
> but I didn't agree to anything.
Based on your silence I too thought that you had agreed.
> What you pass to this option is a refspec, so it makes sense to name
> the option --refspec.

As discussed in that thread, it's not really the same thing as a refspec used in push or fetch. In those commands, the refspec specifies two separable things: what to transfer, and how to translate refs names between the remote and local repositories. IIUC, the fast-export --refspec argument only specifies how to translate ref names, not what gets transferred.

If my understanding is correct, then I agree with Junio and Peff that --refmap is a better name.

-Richard
Junio C Hamano· Nov 12, 2013, 17:08 UTC · re: Richard Hansen · lore

Re: [PATCH v6 06/10] fast-export: add new --refspec option

Richard Hansen <rhansen@bbn.com> writes:
Show 6 quoted lines
>>> I thought that the discussion agreed this option should not be
>>> called --refspec but something like --refmap?
>> 
>> I don't know what you agreed to,
>
> http://article.gmane.org/gmane.comp.version-control.git/237473
Yup, that was what I had in mind.
>> but I didn't agree to anything.
>
> Based on your silence I too thought that you had agreed.

Careful. Silence does not mean agreement, at least around here. It may be just the person was busy and hasn't got around to it, was not paying attention and missed the discussion, or was not as interested in the topic as his/her other activities.

That, especially the last possibility among the three example reasons above, was why I said "the discussion agreed", not "you agreed".

Show 12 quoted lines
>> What you pass to this option is a refspec, so it makes sense to name
>> the option --refspec.
>
> As discussed in that thread, it's not really the same thing as a refspec
> used in push or fetch.  In those commands, the refspec specifies two
> separable things:  what to transfer, and how to translate refs names
> between the remote and local repositories.  IIUC, the fast-export
> --refspec argument only specifies how to translate ref names, not what
> gets transferred.
>
> If my understanding is correct, then I agree with Junio and Peff that
> --refmap is a better name.

I know from one of the tests that the option Felipe added is implemented in such a way that allows:

    git fast-export --option refs/heads/master:refs/heads/next master

to rename the destination, but I didn't check, when I wrote the message to envision how a similar mechanism could be used to enhance push/fetch in the future, if it can be actually used as a mapping

    git fast-export --option refs/heads/*:refs/remotes/mine/* master

Being able to do so was the only reason why I agree with the patch in question (not my toy patch, but [6/10] that started this thread) that it is a good idea in the longer term, as the other approach I suggested to teach revision command line parser to optionally take refspecs as if they specify LHS of the colon as object name with rev_cmdline annotations would not work well for such a purpose.

Thanks.
Felipe Contreras· Nov 12, 2013, 21:02 UTC · re: Richard Hansen · lore

Re: [PATCH v6 06/10] fast-export: add new --refspec option

Richard Hansen wrote:
Show 19 quoted lines
> On 2013-11-11 18:50, Felipe Contreras wrote:
> > On Mon, Nov 11, 2013 at 5:25 PM, Junio C Hamano <gitster@pobox.com> wrote:
> >> Felipe Contreras <felipe.contreras@gmail.com> writes:
> >>
> >>> So that we can convert the exported ref names.
> >>>
> >>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> >>> ---
> >>
> >> I thought that the discussion agreed this option should not be
> >> called --refspec but something like --refmap?
> > 
> > I don't know what you agreed to,
> 
> http://article.gmane.org/gmane.comp.version-control.git/237473
> 
> > but I didn't agree to anything.
> 
> Based on your silence I too thought that you had agreed.

Given that my opinion is regarded as inferior by those in the discussion, I don't see why I should share it, specially since when I do, it's considered toxic if I disagree.

Show 9 quoted lines
> > What you pass to this option is a refspec, so it makes sense to name
> > the option --refspec.
> 
> As discussed in that thread, it's not really the same thing as a refspec
> used in push or fetch.  In those commands, the refspec specifies two
> separable things:  what to transfer, and how to translate refs names
> between the remote and local repositories.  IIUC, the fast-export
> --refspec argument only specifies how to translate ref names, not what
> gets transferred.
Does it?
 % git config remote.origin.fetch '+refs/heads/*:refs/remotes-test/origin/*'
 % git fetch origin master
 From /home/felipec/dev/git
  * branch            master     -> FETCH_HEAD
  * [new branch]      master     -> refs/remotes-test/origin/master

In this case remote.origin.fetch is determining how to translate ref names, not what gets transferred, *exactly* the same as we are doing with --refspec. And as far as I know, remote.origin.fetch is a refspec.

-- 
Felipe Contreras
Junio C Hamano· Nov 12, 2013, 21:46 UTC · re: Felipe Contreras · lore

Re: [PATCH v6 06/10] fast-export: add new --refspec option

Felipe Contreras <felipe.contreras@gmail.com> writes:
Show 11 quoted lines
> Does it?
>
>  % git config remote.origin.fetch '+refs/heads/*:refs/remotes-test/origin/*'
>  % git fetch origin master
>  From /home/felipec/dev/git
>   * branch            master     -> FETCH_HEAD
>   * [new branch]      master     -> refs/remotes-test/origin/master
>
> In this case remote.origin.fetch is determining how to translate ref names, not
> what gets transferred, *exactly* the same as we are doing with --refspec. And
> as far as I know, remote.origin.fetch is a refspec.

If you had 'next' and 'pu' branches at the remote, do they get fetched with that command line?

Felipe Contreras· Nov 12, 2013, 23:20 UTC · re: Junio C Hamano · lore

Re: [PATCH v6 06/10] fast-export: add new --refspec option

Junio C Hamano wrote:
Show 16 quoted lines
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> > Does it?
> >
> >  % git config remote.origin.fetch '+refs/heads/*:refs/remotes-test/origin/*'
> >  % git fetch origin master
> >  From /home/felipec/dev/git
> >   * branch            master     -> FETCH_HEAD
> >   * [new branch]      master     -> refs/remotes-test/origin/master
> >
> > In this case remote.origin.fetch is determining how to translate ref names, not
> > what gets transferred, *exactly* the same as we are doing with --refspec. And
> > as far as I know, remote.origin.fetch is a refspec.
> 
> If you had 'next' and 'pu' branches at the remote, do they get
> fetched with that command line?
No, why would they? You specified a single branch to fetch. Try it yourself.
 % git clone git://git.kernel.org/pub/scm/git/git.git
 % git config remote.origin.fetch '+refs/heads/*:refs/remotes-test/origin/*'
 % git fetch origin master
-- 
Felipe Contreras
Junio C Hamano· Nov 12, 2013, 23:54 UTC · re: Junio C Hamano · lore

Re: [PATCH v6 06/10] fast-export: add new --refspec option

Junio C Hamano <gitster@pobox.com> writes:
Show 16 quoted lines
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> Does it?
>>
>>  % git config remote.origin.fetch '+refs/heads/*:refs/remotes-test/origin/*'
>>  % git fetch origin master
>>  From /home/felipec/dev/git
>>   * branch            master     -> FETCH_HEAD
>>   * [new branch]      master     -> refs/remotes-test/origin/master
>>
>> In this case remote.origin.fetch is determining how to translate ref names, not
>> what gets transferred, *exactly* the same as we are doing with --refspec. And
>> as far as I know, remote.origin.fetch is a refspec.
>
> If you had 'next' and 'pu' branches at the remote, do they get
> fetched with that command line?
More interestingly, if you have
	[remote "mothership"]
		push = +refs/heads/*:refs/remotes/satellite/*
in your configuration and then said
	$ git push mothership master

then configured remote.mothership.push is not even used (I bring this up because "export" is more closely related to "push" than "fetch").

This (and why 'next' and 'pu' are not fetched in the "fetch" example) is because traditionally, refspecs that are explicitly given on the command line overrides configured ones (in other words, configured ones are used as a fallback default).

This is a bit of tangent, but since the recent discussion on the triangular workflows, I've been wondering if we may want to have a new way to configure things so that we can say "When I push to mothership any one of my local branches, I want it to go to the ref with the same name at the mothership under refs/remotes/satellite/ hierarchy (because I am emulating 'git fetch' that is run on the mothership to get updates from this satellite)", somewhat similar to what you added to "fast-export" via the option in question.

But we cannot achieve that mapping by changing the meaning of the configured refspecs remote.mothership.push without having to worry about breaking existing practices of people. When they say "git push mothership master", they do mean to push refs/heads/master to refs/heads/master, and it will break the expectation in their existing repositories if we change the semantics under them.

A possible way to achieve that mapping "When I push to mothership any one of my local branches,..." could be to introduce a _new_ configuration (so that existing repositories will not suddenly change their behaviour):

	[remote "mothership"]
		pushmap = +refs/heads/*:refs/remotes/satellite/*
And then we can allow this command line
	$ git push mothership master

to be affected by the mapping when expanding the short-hand refspecs given on the command line. Traditionally, anything without colon stood as a short-hand for "push to the same name", e.g. 'master' is for 'refs/heads/master:refs/heads/master' in this example, and 'v1.0' would be for 'refs/tags/v1.0:refs/tags/v1.0'.

But it does not have to stay that way. In order to move things forward in that direction, this new configuration has to be distinguishable from the traditional "refspec", as it embodies a different concept.

Felipe Contreras· Dec 7, 2013, 10:00 UTC · re: Junio C Hamano · lore

Re: [PATCH v6 06/10] fast-export: add new --refspec option

Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> > Felipe Contreras <felipe.contreras@gmail.com> writes:
> But it does not have to stay that way.  In order to move things
> forward in that direction, this new configuration has to be
> distinguishable from the traditional "refspec", as it embodies a
> different concept.
Is it a different concept?
 % git config remote.origin.fetch '+refs/heads/*:refs/remotes-test/origin/*'
 % git fetch origin master
What do you call this thing? ------^
-- 
Felipe Contreras
Junio C Hamano· Dec 9, 2013, 21:00 UTC · re: Felipe Contreras · lore

Re: [PATCH v6 06/10] fast-export: add new --refspec option

Felipe Contreras <felipe.contreras@gmail.com> writes:
Show 15 quoted lines
> Junio C Hamano wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>> > Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> But it does not have to stay that way.  In order to move things
>> forward in that direction, this new configuration has to be
>> distinguishable from the traditional "refspec", as it embodies a
>> different concept.
>
> Is it a different concept?
>
>  % git config remote.origin.fetch '+refs/heads/*:refs/remotes-test/origin/*'
>  % git fetch origin master
>
> What do you call this thing? ------^

The answer to that question is the "value of the 'remote.*.fetch' configuration variable".

The refspec mechanism and syntax used in "fetch" and "push" were invented and designed to express two things [*1*]:

 1) what is the set of objects that are necessary and sufficient to
    complete some histories need to be transferred; what are the
    tips of these histories that are being completed?
 2) after the object transfer, some refs on the side that receive
    the objects are optionally to be updated;
    2-a) which refs are they?
    2-b) what objects are they updated to point at?

A refspec consists of one or more elements, each of which has right and left hand side separated by a colon, i.e. RHS:LHS, and

 1) is determined by the RHS
 2-a) is determined by the LHS
 2-b) is determined by the correspondence between RHS-to-LHS.

A wildcarded "refs/heads/*:refs/remotes/origin/*" dynamically expands to what the side that sends objects have, e.g. if fetching from a repository with 'master' and 'next' branches, it becomes

	refs/heads/master:refs/remotes/origin/master
        refs/heads/next:refs/remotes/origin/next
So with
	$ refspec='refs/heads/master:refs/remotes/origin/master
	        refs/heads/next:refs/remotes/origin/next'
	$ git fetch origin $refspec

we transfer objects to allow us to have complete histories leading to 'master' and 'next' from the origin repository. And we update our refs/remotes/origin/{next,master} refs.

Traditionally, when there is _no_ refspec on the command line, the value of 'remote.*.fetch' configuration variable is used as the fallback default, and that usage is still true. When used in that way, the value of the variable _is taken as_ a refspec.

However, we can no longer say that the variable _is_ a refspec with the modern Git, and here is why.

"git fetch origin master" used to ignore the 'remote.*.fetch' configuration variable completely, but since f2690487 (which is a fairly recent invention), the variable participates in the "fetch" process in a different way [*2*]. With this in the config:

    [remote "origin"]
    	fetch = refs/heads/master:refs/remotes/origin/master
        fetch = refs/heads/next:refs/remotes/origin/next

the command with 'master' refspec on the command line transfers the objects required to complete the history only for the 'master', and not 'next':

	$ git fetch origin master

In this usage, 'master' on the command line is the only thing that determines what histories are completed (because it does not say 'next', the objects necessary to complete its history are not transferred unless they are needed to complete 'master'). The value of the 'remote.*.fetch' configuration does not participate in the determination of the history being transferred at all. It is not used as a refspec.

But unlike Git of the last year, we do map this 'master' using the 'remote.*.fetch' configuration variable, in order to decide 2) above. We find that the given remote ref, 'master', has an element that corresopnds to it in the 'remote.*.fetch' configuration, and that element tells us to update refs/remotes/origin/master to point at the object they call 'master', effectively turning the above command line into this form (using "refspec" that has only one element, refs/heads/master:refs/remotes/origin/master):

	$ git fetch origin refs/heads/master:refs/remotes/origin/master

There is no "refs/heads/next:refs/remotes/origin/next" here, because the 'fetch' configuration is not used as a refspec, but as something else.

My understanding of the added option parameter to "git fast-export" is that it is not about specifying the history being transferred, but is about mapping the name of the destination. For example, does object between 'master' and 'next' participate in the datastream produced with this?

	fast-export \
            --refspec=refs/heads/master:refs/remotes/origin/master \
            --refspec=refs/heads/next:refs/remotes/origin/next \
            master

If this parameter were a refspec, as we have discussed already in previous rounds [*3*], we should be able to give it on the command line, like any normal refspec, instead of repeating the same thing (i.e. up to what commit should the history be transported) as in:

	fast-export --refspec=refs/heads/master:refs/remotes/origin/master master
but just
	fast-export refs/heads/master:refs/remotes/origin/master
[Footnote]
 *1* Note that readers are hearing the authoritative definition
     given by the person who invented and designed it back in August
     2005.
 *2* And a recent $gmane/238832 moves "push" in the direction to be
     in line with "fetch" in this regard.
 *3* And I think I even outlined the code to do so.
Junio C Hamano· Dec 9, 2013, 21:11 UTC · re: Junio C Hamano · lore

Re: [PATCH v6 06/10] fast-export: add new --refspec option

Junio C Hamano <gitster@pobox.com> writes:
Show 6 quoted lines
> A refspec consists of one or more elements, each of which has
> right and left hand side separated by a colon, i.e. RHS:LHS, and
>
>  1) is determined by the RHS
>  2-a) is determined by the LHS
>  2-b) is determined by the correspondence between RHS-to-LHS.
Heh, I got my right and left the other way around ;-)
Felipe Contreras· Apr 24, 2014, 03:55 UTC · re: Junio C Hamano · lore

Re: [PATCH v6 06/10] fast-export: add new --refspec option

Hi,
Sorry it took too long to reply to this.
Junio C Hamano wrote:
Show 20 quoted lines
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> > Junio C Hamano wrote:
> >> Junio C Hamano <gitster@pobox.com> writes:
> >> > Felipe Contreras <felipe.contreras@gmail.com> writes:
> >
> >> But it does not have to stay that way.  In order to move things
> >> forward in that direction, this new configuration has to be
> >> distinguishable from the traditional "refspec", as it embodies a
> >> different concept.
> >
> > Is it a different concept?
> >
> >  % git config remote.origin.fetch '+refs/heads/*:refs/remotes-test/origin/*'
> >  % git fetch origin master
> >
> > What do you call this thing? ------^
> 
> The answer to that question is the "value of the 'remote.*.fetch'
> configuration variable".
You are avoiding the question: it's a refspec.
> There is no "refs/heads/next:refs/remotes/origin/next" here, because
> the 'fetch' configuration is not used as a refspec, but as something
> else.

Yet both remote.fetch and remote.push are a 'struct refspec', and the documentation says they are a "refspec".

Show 21 quoted lines
> My understanding of the added option parameter to "git fast-export"
> is that it is not about specifying the history being transferred,
> but is about mapping the name of the destination.  For example, does
> object between 'master' and 'next' participate in the datastream
> produced with this?
> 
> 	fast-export \
>             --refspec=refs/heads/master:refs/remotes/origin/master \
>             --refspec=refs/heads/next:refs/remotes/origin/next \
>             master
> 
> If this parameter were a refspec, as we have discussed already in
> previous rounds [*3*], we should be able to give it on the command line,
> like any normal refspec, instead of repeating the same thing
> (i.e. up to what commit should the history be transported) as in:
> 
> 	fast-export --refspec=refs/heads/master:refs/remotes/origin/master master
> 
> but just
> 
> 	fast-export refs/heads/master:refs/remotes/origin/master
Maybe in an ideal world, which we don't live in.

My guess is that trying to accomplish such a goal would result in an unbelievable mess of the code that I wouldn't even want to think about where to start to code this.

Moreover, `git grep refmap` returns me the following:
  t/t5516-fetch-push.sh:test_expect_success 'pushing a specific ref applies remote.$name.push as refmap' '
  t/t5516-fetch-push.sh:test_expect_success 'with no remote.$name.push, it is not used as refmap' '

I'd say with --refspec is the simplest and most sensible way this can be implemented.

-- 
Felipe Contreras
Felipe Contreras· Nov 11, 2013, 22:55 UTC · re: Felipe Contreras · lore

[PATCH v6 03/10] transport-helper: add 'force' to 'export' helpers

Otherwise they cannot know when to force the push or not (other than hacks).

Tests-by: Richard Hansen <rhansen@bbn.com>
Documentation-by: Richard Hansen <rhansen@bbn.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/gitremote-helpers.txt |  4 ++++
 git-remote-testgit.sh               | 18 ++++++++++++++++++
 t/t5801-remote-helpers.sh           | 13 +++++++++++++
 transport-helper.c                  |  5 +++++
 4 files changed, 40 insertions(+)
Show changes to 4 files +40 −0

Documentation/gitremote-helpers.txt, git-remote-testgit.sh, t/t5801-remote-helpers.sh, transport-helper.c

diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt
index f1f4ca9..e75699c 100644
--- a/Documentation/gitremote-helpers.txt
+++ b/Documentation/gitremote-helpers.txt
@@ -437,6 +437,10 @@ set by Git if the remote helper has the 'option' capability.
 'option check-connectivity' \{'true'|'false'\}::
 	Request the helper to check connectivity of a clone.
 
+'option force' \{'true'|'false'\}::
+	Request the helper to perform a force update.  Defaults to
+	'false'.
+
 SEE ALSO
 --------
 linkgit:git-remote[1]
diff --git a/git-remote-testgit.sh b/git-remote-testgit.sh
index 6d2f282..1c006a0 100755
--- a/git-remote-testgit.sh
+++ b/git-remote-testgit.sh
@@ -15,6 +15,8 @@ test -z "$refspec" && prefix="refs"
 
 export GIT_DIR="$url/.git"
 
+force=
+
 mkdir -p "$dir"
 
 if test -z "$GIT_REMOTE_TESTGIT_NO_MARKS"
@@ -39,6 +41,7 @@ do
 		fi
 		test -n "$GIT_REMOTE_TESTGIT_SIGNED_TAGS" && echo "signed-tags"
 		test -n "$GIT_REMOTE_TESTGIT_NO_PRIVATE_UPDATE" && echo "no-private-update"
+		echo 'option'
 		echo
 		;;
 	list)
@@ -93,6 +96,7 @@ do
 		before=$(git for-each-ref --format=' %(refname) %(objectname) ')
 
 		git fast-import \
+			${force:+--force} \
 			${testgitmarks:+"--import-marks=$testgitmarks"} \
 			${testgitmarks:+"--export-marks=$testgitmarks"} \
 			--quiet
@@ -115,6 +119,20 @@ do
 
 		echo
 		;;
+	option\ *)
+		read cmd opt val <<-EOF
+		$line
+		EOF
+		case $opt in
+		force)
+			test $val = "true" && force="true" || force=
+			echo "ok"
+			;;
+		*)
+			echo "unsupported"
+			;;
+		esac
+		;;
 	'')
 		exit
 		;;
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 613f69a..c33cc25 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -94,6 +94,19 @@ test_expect_failure 'push new branch with old:new refspec' '
 	compare_refs local HEAD server refs/heads/new-refspec
 '
 
+test_expect_success 'forced push' '
+	(cd local &&
+	git checkout -b force-test &&
+	echo content >> file &&
+	git commit -a -m eight &&
+	git push origin force-test &&
+	echo content >> file &&
+	git commit -a --amend -m eight-modified &&
+	git push --force origin force-test
+	) &&
+	compare_refs local refs/heads/force-test server refs/heads/force-test
+'
+
 test_expect_success 'cloning without refspec' '
 	GIT_REMOTE_TESTGIT_REFSPEC="" \
 	git clone "testgit::${PWD}/server" local2 2>error &&
diff --git a/transport-helper.c b/transport-helper.c
index 9558a0d..bead9b9 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -855,6 +855,11 @@ static int push_refs_with_export(struct transport *transport,
 			die("helper %s does not support dry-run", data->name);
 	}
 
+	if (flags & TRANSPORT_PUSH_FORCE) {
+		if (set_helper_option(transport, "force", "true") != 0)
+			die("helper %s does not support 'force'", data->name);
+	}
+
 	helper = get_helper(transport);
 
 	write_constant(helper->in, "export\n");
-- 
1.8.4.2+fc1
Junio C Hamano· Nov 11, 2013, 23:28 UTC · re: Felipe Contreras · lore

Re: [PATCH v6 03/10] transport-helper: add 'force' to 'export' helpers

Felipe Contreras <felipe.contreras@gmail.com> writes:
Show 7 quoted lines
> Otherwise they cannot know when to force the push or not (other than
> hacks).
>
> Tests-by: Richard Hansen <rhansen@bbn.com>
> Documentation-by: Richard Hansen <rhansen@bbn.com>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
Didn't we agree that this should be warn, not die?
Show 8 quoted lines
> +	if (flags & TRANSPORT_PUSH_FORCE) {
> +		if (set_helper_option(transport, "force", "true") != 0)
> +			die("helper %s does not support 'force'", data->name);
> +	}
> +
>  	helper = get_helper(transport);
>  
>  	write_constant(helper->in, "export\n");
Felipe Contreras· Nov 11, 2013, 23:47 UTC · re: Junio C Hamano · lore

Re: [PATCH v6 03/10] transport-helper: add 'force' to 'export' helpers

On Mon, Nov 11, 2013 at 5:28 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 11 quoted lines
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> Otherwise they cannot know when to force the push or not (other than
>> hacks).
>>
>> Tests-by: Richard Hansen <rhansen@bbn.com>
>> Documentation-by: Richard Hansen <rhansen@bbn.com>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>
> Didn't we agree that this should be warn, not die?

Yes, and I assumed you would do it without a reroll, because no reroll was needed. I've updated my side as well now, for the next reroll.

-- 
Felipe Contreras
Felipe Contreras· Nov 11, 2013, 22:55 UTC · re: Felipe Contreras · lore

[PATCH v6 09/10] fast-export: add support to delete refs

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/fast-export.c  | 14 ++++++++++++++
 t/t9350-fast-export.sh | 11 +++++++++++
 2 files changed, 25 insertions(+)
Show changes to 2 files +25 −0

builtin/fast-export.c, t/t9350-fast-export.sh

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index cf745ec..4b76222 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -673,6 +673,19 @@ static void import_marks(char *input_file)
 	fclose(f);
 }
 
+static void handle_deletes(void)
+{
+	int i;
+	for (i = 0; i < refspecs_nr; i++) {
+		struct refspec *refspec = &refspecs[i];
+		if (*refspec->src)
+			continue;
+
+		printf("reset %s\nfrom %s\n\n",
+				refspec->dst, sha1_to_hex(null_sha1));
+	}
+}
+
 int cmd_fast_export(int argc, const char **argv, const char *prefix)
 {
 	struct rev_info revs;
@@ -764,6 +777,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 	}
 
 	handle_tags_and_duplicates();
+	handle_deletes();
 
 	if (export_filename && lastimportid != last_idnum)
 		export_marks(export_filename);
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 3d475af..66c8b0a 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -511,4 +511,15 @@ test_expect_success 'use refspec' '
 	test_cmp expected actual
 '
 
+test_expect_success 'delete refspec' '
+	git branch to-delete &&
+	git fast-export --refspec :refs/heads/to-delete to-delete ^to-delete > actual &&
+	cat > expected <<-EOF &&
+	reset refs/heads/to-delete
+	from 0000000000000000000000000000000000000000
+
+	EOF
+	test_cmp expected actual
+'
+
 test_done
-- 
1.8.4.2+fc1
Felipe Contreras· Nov 11, 2013, 22:55 UTC · re: Felipe Contreras · lore

[PATCH v6 07/10] transport-helper: add support for old:new refspec

By using fast-export's new --refspec option.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/t5801-remote-helpers.sh |  2 +-
 transport-helper.c        | 13 ++++++++++---
 2 files changed, 11 insertions(+), 4 deletions(-)
Show changes to 2 files +11 −4

t/t5801-remote-helpers.sh, transport-helper.c

diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index c33cc25..454337e 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -87,7 +87,7 @@ test_expect_success 'push new branch by name' '
 	compare_refs local HEAD server refs/heads/new-name
 '
 
-test_expect_failure 'push new branch with old:new refspec' '
+test_expect_success 'push new branch with old:new refspec' '
 	(cd local &&
 	 git push origin new-name:new-refspec
 	) &&
diff --git a/transport-helper.c b/transport-helper.c
index 9a5814d..7411125 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -850,7 +850,7 @@ static int push_refs_with_export(struct transport *transport,
 	struct ref *ref;
 	struct child_process *helper, exporter;
 	struct helper_data *data = transport->data;
-	struct string_list revlist_args = STRING_LIST_INIT_NODUP;
+	struct string_list revlist_args = STRING_LIST_INIT_DUP;
 	struct strbuf buf = STRBUF_INIT;
 
 	if (!data->refspecs)
@@ -888,8 +888,13 @@ static int push_refs_with_export(struct transport *transport,
 		free(private);
 
 		if (ref->peer_ref) {
-			if (strcmp(ref->peer_ref->name, ref->name))
-				die("remote-helpers do not support old:new syntax");
+			if (strcmp(ref->name, ref->peer_ref->name)) {
+				struct strbuf buf = STRBUF_INIT;
+				strbuf_addf(&buf, "%s:%s", ref->peer_ref->name, ref->name);
+				string_list_append(&revlist_args, "--refspec");
+				string_list_append(&revlist_args, buf.buf);
+				strbuf_release(&buf);
+			}
 			string_list_append(&revlist_args, ref->peer_ref->name);
 		}
 	}
@@ -897,6 +902,8 @@ static int push_refs_with_export(struct transport *transport,
 	if (get_exporter(transport, &exporter, &revlist_args))
 		die("Couldn't run fast-export");
 
+	string_list_clear(&revlist_args, 1);
+
 	if (finish_command(&exporter))
 		die("Error while running fast-export");
 	push_update_refs_status(data, remote_refs, flags);
-- 
1.8.4.2+fc1
Felipe Contreras· Nov 11, 2013, 22:55 UTC · re: Felipe Contreras · lore

[PATCH v6 04/10] transport-helper: check for 'forced update' message

So the remote-helpers can tell us when a forced push was needed.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 transport-helper.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
Show changes to transport-helper.c +7 −1
diff --git a/transport-helper.c b/transport-helper.c
index bead9b9..9a5814d 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -644,7 +644,7 @@ static int push_update_ref_status(struct strbuf *buf,
 				   struct ref *remote_refs)
 {
 	char *refname, *msg;
-	int status;
+	int status, forced = 0;
 
 	if (!prefixcmp(buf->buf, "ok ")) {
 		status = REF_STATUS_OK;
@@ -702,6 +702,11 @@ static int push_update_ref_status(struct strbuf *buf,
 			free(msg);
 			msg = NULL;
 		}
+		else if (!strcmp(msg, "forced update")) {
+			forced = 1;
+			free(msg);
+			msg = NULL;
+		}
 	}
 
 	if (*ref)
@@ -723,6 +728,7 @@ static int push_update_ref_status(struct strbuf *buf,
 	}
 
 	(*ref)->status = status;
+	(*ref)->forced_update = forced;
 	(*ref)->remote_status = msg;
 	return !(status == REF_STATUS_OK);
 }
-- 
1.8.4.2+fc1
Felipe Contreras· Nov 11, 2013, 22:55 UTC · re: Felipe Contreras · lore

[PATCH v6 02/10] transport-helper: don't update refs in dry-run

The remote helper namespace should not be updated.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 transport-helper.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
Show changes to transport-helper.c +5 −4
diff --git a/transport-helper.c b/transport-helper.c
index b66c7fd..9558a0d 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -728,7 +728,8 @@ static int push_update_ref_status(struct strbuf *buf,
 }
 
 static void push_update_refs_status(struct helper_data *data,
-				    struct ref *remote_refs)
+				    struct ref *remote_refs,
+				    int flags)
 {
 	struct strbuf buf = STRBUF_INIT;
 	struct ref *ref = remote_refs;
@@ -742,7 +743,7 @@ static void push_update_refs_status(struct helper_data *data,
 		if (push_update_ref_status(&buf, &ref, remote_refs))
 			continue;
 
-		if (!data->refspecs || data->no_private_update)
+		if (flags & TRANSPORT_PUSH_DRY_RUN || !data->refspecs || data->no_private_update)
 			continue;
 
 		/* propagate back the update to the remote namespace */
@@ -833,7 +834,7 @@ static int push_refs_with_push(struct transport *transport,
 	sendline(data, &buf);
 	strbuf_release(&buf);
 
-	push_update_refs_status(data, remote_refs);
+	push_update_refs_status(data, remote_refs, flags);
 	return 0;
 }
 
@@ -887,7 +888,7 @@ static int push_refs_with_export(struct transport *transport,
 
 	if (finish_command(&exporter))
 		die("Error while running fast-export");
-	push_update_refs_status(data, remote_refs);
+	push_update_refs_status(data, remote_refs, flags);
 	return 0;
 }
 
-- 
1.8.4.2+fc1

← back to recent threads