[PATCH v3 0/6] "git push --cas/--lockref" renamed to "--force-with-lease"

[PATCH v3 0/6] "git push --cas/--lockref" renamed to "--force-with-lease"

[Junio C Hamano]

This is mostly unchanged since the previous round, except that

 * The option is spelled "--force-with-lease=<ref>:<expect>".
   Nobody liked "cas" as it was too technical, many disliked
   "lockref" because "lock" sounded as if push by others were
   excluded by it while in fact this is to fail us.

   The final name implies that it is related to the "--force" that
   breaks the "must fast-forward" safety, but "with-lease" part
   conveys that there is some reservation with the forcing.  The
   observation you make before you start rebasing (or you ensure
   that everything is expendable and decide to delete) is like
   taking a lease on the ref, and as long as the lease is not
   broken by others, you can push a non-fast-forward history to
   replace what is at the remote.

 * The logic to choose default when the option is not given with an
   explicit expected value is still "the remote-tracking branch for
   the ref being updated", but the documentation warns users against
   relying on that semantics, as it was shown to be fragile during
   the discussion, and hopefully we will come up with a better and
   more robust one to replace it.

The first two preparatory patches are the same since v2.  For the
remainder, other than changes necessary to rename the option, the
documentation part of [PATCH 3/6] has been updated to mark forms
without explicit expect value as experimental.

Junio C Hamano (6):
  cache.h: move remote/connect API out of it
  builtin/push.c: use OPT_BOOL, not OPT_BOOLEAN
  remote.c: add command line option parser for "--force-with-lease"
  push --force-with-lease: implement logic to populate old_sha1_expect[]
  push --force-with-lease: tie it all together
  t5533: test "push --force-with-lease"

 Documentation/git-push.txt |  77 +++++++++++++++---
 builtin/fetch-pack.c       |   2 +
 builtin/push.c             |  19 ++++-
 builtin/receive-pack.c     |   1 +
 builtin/send-pack.c        |  26 +++++++
 cache.h                    |  62 ---------------
 connect.c                  |   1 +
 connect.h                  |  13 ++++
 fetch-pack.c               |   1 +
 fetch-pack.h               |   1 +
 refs.c                     |   8 --
 remote.c                   | 175 +++++++++++++++++++++++++++++++++++++----
 remote.h                   |  83 ++++++++++++++++++++
 send-pack.c                |   2 +
 t/t5533-push-cas.sh        | 189 +++++++++++++++++++++++++++++++++++++++++++++
 transport-helper.c         |   6 ++
 transport.c                |  13 ++++
 transport.h                |   5 ++
 upload-pack.c              |   1 +
 19 files changed, 588 insertions(+), 97 deletions(-)
 create mode 100644 connect.h
 create mode 100755 t/t5533-push-cas.sh

-- 
1.8.3.4-980-g8decd39

[PATCH v3 1/6] cache.h: move remote/connect API out of it

[Junio C Hamano]

The definition of "struct ref" in "cache.h", a header file so
central to the system, always confused me.  This structure is not
about the local ref used by sha1-name API to name local objects.

It is what refspecs are expanded into, after finding out what refs
the other side has, to define what refs are updated after object
transfer succeeds to what values.  It belongs to "remote.h" together
with "struct refspec".

While we are at it, also move the types and functions related to the
Git transport connection to a new header file connect.h

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/fetch-pack.c   |  2 ++
 builtin/receive-pack.c |  1 +
 builtin/send-pack.c    |  1 +
 cache.h                | 62 --------------------------------------------------
 connect.c              |  1 +
 connect.h              | 13 +++++++++++
 fetch-pack.c           |  1 +
 fetch-pack.h           |  1 +
 refs.c                 |  8 -------
 remote.c               |  8 +++++++
 remote.h               | 54 +++++++++++++++++++++++++++++++++++++++++++
 send-pack.c            |  1 +
 transport.c            |  2 ++
 transport.h            |  1 +
 upload-pack.c          |  1 +
 15 files changed, 87 insertions(+), 70 deletions(-)
 create mode 100644 connect.h

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index aba4465..c6888c6 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -1,6 +1,8 @@
 #include "builtin.h"
 #include "pkt-line.h"
 #include "fetch-pack.h"
+#include "remote.h"
+#include "connect.h"
 
 static const char fetch_pack_usage[] =
 "git fetch-pack [--all] [--stdin] [--quiet|-q] [--keep|-k] [--thin] "
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index e3eb5fc..7434d9b 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -8,6 +8,7 @@
 #include "commit.h"
 #include "object.h"
 #include "remote.h"
+#include "connect.h"
 #include "transport.h"
 #include "string-list.h"
 #include "sha1-array.h"
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 152c4ea..e86d3b5 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -5,6 +5,7 @@
 #include "sideband.h"
 #include "run-command.h"
 #include "remote.h"
+#include "connect.h"
 #include "send-pack.h"
 #include "quote.h"
 #include "transport.h"
diff --git a/cache.h b/cache.h
index dd0fb33..cb2891d 100644
--- a/cache.h
+++ b/cache.h
@@ -1035,68 +1035,6 @@ struct pack_entry {
 	struct packed_git *p;
 };
 
-struct ref {
-	struct ref *next;
-	unsigned char old_sha1[20];
-	unsigned char new_sha1[20];
-	char *symref;
-	unsigned int
-		force:1,
-		forced_update:1,
-		deletion:1,
-		matched:1;
-
-	/*
-	 * Order is important here, as we write to FETCH_HEAD
-	 * in numeric order. And the default NOT_FOR_MERGE
-	 * should be 0, so that xcalloc'd structures get it
-	 * by default.
-	 */
-	enum {
-		FETCH_HEAD_MERGE = -1,
-		FETCH_HEAD_NOT_FOR_MERGE = 0,
-		FETCH_HEAD_IGNORE = 1
-	} fetch_head_status;
-
-	enum {
-		REF_STATUS_NONE = 0,
-		REF_STATUS_OK,
-		REF_STATUS_REJECT_NONFASTFORWARD,
-		REF_STATUS_REJECT_ALREADY_EXISTS,
-		REF_STATUS_REJECT_NODELETE,
-		REF_STATUS_REJECT_FETCH_FIRST,
-		REF_STATUS_REJECT_NEEDS_FORCE,
-		REF_STATUS_UPTODATE,
-		REF_STATUS_REMOTE_REJECT,
-		REF_STATUS_EXPECTING_REPORT
-	} status;
-	char *remote_status;
-	struct ref *peer_ref; /* when renaming */
-	char name[FLEX_ARRAY]; /* more */
-};
-
-#define REF_NORMAL	(1u << 0)
-#define REF_HEADS	(1u << 1)
-#define REF_TAGS	(1u << 2)
-
-extern struct ref *find_ref_by_name(const struct ref *list, const char *name);
-
-#define CONNECT_VERBOSE       (1u << 0)
-extern struct child_process *git_connect(int fd[2], const char *url, const char *prog, int flags);
-extern int finish_connect(struct child_process *conn);
-extern int git_connection_is_socket(struct child_process *conn);
-struct extra_have_objects {
-	int nr, alloc;
-	unsigned char (*array)[20];
-};
-extern struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
-				     struct ref **list, unsigned int flags,
-				     struct extra_have_objects *);
-extern int server_supports(const char *feature);
-extern int parse_feature_request(const char *features, const char *feature);
-extern const char *server_feature_value(const char *feature, int *len_ret);
-extern const char *parse_feature_value(const char *feature_list, const char *feature, int *len_ret);
-
 extern struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path);
 
 /* A hook for count-objects to report invalid files in pack directory */
diff --git a/connect.c b/connect.c
index a0783d4..a80ebd3 100644
--- a/connect.c
+++ b/connect.c
@@ -5,6 +5,7 @@
 #include "refs.h"
 #include "run-command.h"
 #include "remote.h"
+#include "connect.h"
 #include "url.h"
 
 static char *server_capabilities;
diff --git a/connect.h b/connect.h
new file mode 100644
index 0000000..9dff25c
--- /dev/null
+++ b/connect.h
@@ -0,0 +1,13 @@
+#ifndef CONNECT_H
+#define CONNECT_H
+
+#define CONNECT_VERBOSE       (1u << 0)
+extern struct child_process *git_connect(int fd[2], const char *url, const char *prog, int flags);
+extern int finish_connect(struct child_process *conn);
+extern int git_connection_is_socket(struct child_process *conn);
+extern int server_supports(const char *feature);
+extern int parse_feature_request(const char *features, const char *feature);
+extern const char *server_feature_value(const char *feature, int *len_ret);
+extern const char *parse_feature_value(const char *feature_list, const char *feature, int *len_ret);
+
+#endif
diff --git a/fetch-pack.c b/fetch-pack.c
index abe5ffb..c2bab42 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -9,6 +9,7 @@
 #include "fetch-pack.h"
 #include "remote.h"
 #include "run-command.h"
+#include "connect.h"
 #include "transport.h"
 #include "version.h"
 
diff --git a/fetch-pack.h b/fetch-pack.h
index 40f08ba..461cbf3 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -2,6 +2,7 @@
 #define FETCH_PACK_H
 
 #include "string-list.h"
+#include "run-command.h"
 
 struct fetch_pack_args {
 	const char *uploadpack;
diff --git a/refs.c b/refs.c
index 4302206..330060c 100644
--- a/refs.c
+++ b/refs.c
@@ -3193,14 +3193,6 @@ int update_ref(const char *action, const char *refname,
 	return 0;
 }
 
-struct ref *find_ref_by_name(const struct ref *list, const char *name)
-{
-	for ( ; list; list = list->next)
-		if (!strcmp(list->name, name))
-			return (struct ref *)list;
-	return NULL;
-}
-
 /*
  * generate a format suitable for scanf from a ref_rev_parse_rules
  * rule, that is replace the "%.*s" spec with a "%s" spec
diff --git a/remote.c b/remote.c
index 6f57830..b1ff7a2 100644
--- a/remote.c
+++ b/remote.c
@@ -1302,6 +1302,14 @@ static void add_missing_tags(struct ref *src, struct ref **dst, struct ref ***ds
 	free(sent_tips.tip);
 }
 
+struct ref *find_ref_by_name(const struct ref *list, const char *name)
+{
+	for ( ; list; list = list->next)
+		if (!strcmp(list->name, name))
+			return (struct ref *)list;
+	return NULL;
+}
+
 /*
  * Given the set of refs the local repository has, the set of refs the
  * remote repository has, and the refspec used for push, determine
diff --git a/remote.h b/remote.h
index cf56724..a850059 100644
--- a/remote.h
+++ b/remote.h
@@ -71,6 +71,52 @@ struct refspec {
 
 extern const struct refspec *tag_refspec;
 
+struct ref {
+	struct ref *next;
+	unsigned char old_sha1[20];
+	unsigned char new_sha1[20];
+	char *symref;
+	unsigned int
+		force:1,
+		forced_update:1,
+		deletion:1,
+		matched:1;
+
+	/*
+	 * Order is important here, as we write to FETCH_HEAD
+	 * in numeric order. And the default NOT_FOR_MERGE
+	 * should be 0, so that xcalloc'd structures get it
+	 * by default.
+	 */
+	enum {
+		FETCH_HEAD_MERGE = -1,
+		FETCH_HEAD_NOT_FOR_MERGE = 0,
+		FETCH_HEAD_IGNORE = 1
+	} fetch_head_status;
+
+	enum {
+		REF_STATUS_NONE = 0,
+		REF_STATUS_OK,
+		REF_STATUS_REJECT_NONFASTFORWARD,
+		REF_STATUS_REJECT_ALREADY_EXISTS,
+		REF_STATUS_REJECT_NODELETE,
+		REF_STATUS_REJECT_FETCH_FIRST,
+		REF_STATUS_REJECT_NEEDS_FORCE,
+		REF_STATUS_UPTODATE,
+		REF_STATUS_REMOTE_REJECT,
+		REF_STATUS_EXPECTING_REPORT
+	} status;
+	char *remote_status;
+	struct ref *peer_ref; /* when renaming */
+	char name[FLEX_ARRAY]; /* more */
+};
+
+#define REF_NORMAL	(1u << 0)
+#define REF_HEADS	(1u << 1)
+#define REF_TAGS	(1u << 2)
+
+extern struct ref *find_ref_by_name(const struct ref *list, const char *name);
+
 struct ref *alloc_ref(const char *name);
 struct ref *copy_ref(const struct ref *ref);
 struct ref *copy_ref_list(const struct ref *ref);
@@ -84,6 +130,14 @@ int check_ref_type(const struct ref *ref, int flags);
  */
 void free_refs(struct ref *ref);
 
+struct extra_have_objects {
+	int nr, alloc;
+	unsigned char (*array)[20];
+};
+extern struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
+				     struct ref **list, unsigned int flags,
+				     struct extra_have_objects *);
+
 int resolve_remote_symref(struct ref *ref, struct ref *list);
 int ref_newer(const unsigned char *new_sha1, const unsigned char *old_sha1);
 
diff --git a/send-pack.c b/send-pack.c
index 7d172ef..9a9908c 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -5,6 +5,7 @@
 #include "sideband.h"
 #include "run-command.h"
 #include "remote.h"
+#include "connect.h"
 #include "send-pack.h"
 #include "quote.h"
 #include "transport.h"
diff --git a/transport.c b/transport.c
index e15db98..b84dbf0 100644
--- a/transport.c
+++ b/transport.c
@@ -3,6 +3,8 @@
 #include "run-command.h"
 #include "pkt-line.h"
 #include "fetch-pack.h"
+#include "remote.h"
+#include "connect.h"
 #include "send-pack.h"
 #include "walker.h"
 #include "bundle.h"
diff --git a/transport.h b/transport.h
index ea70ea7..b551f99 100644
--- a/transport.h
+++ b/transport.h
@@ -2,6 +2,7 @@
 #define TRANSPORT_H
 
 #include "cache.h"
+#include "run-command.h"
 #include "remote.h"
 
 struct git_transport_options {
diff --git a/upload-pack.c b/upload-pack.c
index 127e59a..b03492e 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -10,6 +10,7 @@
 #include "revision.h"
 #include "list-objects.h"
 #include "run-command.h"
+#include "connect.h"
 #include "sigchain.h"
 #include "version.h"
 #include "string-list.h"
-- 
1.8.3.4-980-g8decd39

[PATCH v3 2/6] builtin/push.c: use OPT_BOOL, not OPT_BOOLEAN

[Junio C Hamano]

The command line parser of "git push" for "--tags", "--delete", and
"--thin" options still used outdated OPT_BOOLEAN.  Because these
options do not give escalating levels when given multiple times,
they should use OPT_BOOL.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/push.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index 2d84d10..342d792 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -427,15 +427,15 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		OPT_BIT( 0 , "all", &flags, N_("push all refs"), TRANSPORT_PUSH_ALL),
 		OPT_BIT( 0 , "mirror", &flags, N_("mirror all refs"),
 			    (TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE)),
-		OPT_BOOLEAN( 0, "delete", &deleterefs, N_("delete refs")),
-		OPT_BOOLEAN( 0 , "tags", &tags, N_("push tags (can't be used with --all or --mirror)")),
+		OPT_BOOL( 0, "delete", &deleterefs, N_("delete refs")),
+		OPT_BOOL( 0 , "tags", &tags, N_("push tags (can't be used with --all or --mirror)")),
 		OPT_BIT('n' , "dry-run", &flags, N_("dry run"), TRANSPORT_PUSH_DRY_RUN),
 		OPT_BIT( 0,  "porcelain", &flags, N_("machine-readable output"), TRANSPORT_PUSH_PORCELAIN),
 		OPT_BIT('f', "force", &flags, N_("force updates"), TRANSPORT_PUSH_FORCE),
 		{ OPTION_CALLBACK, 0, "recurse-submodules", &flags, N_("check"),
 			N_("control recursive pushing of submodules"),
 			PARSE_OPT_OPTARG, option_parse_recurse_submodules },
-		OPT_BOOLEAN( 0 , "thin", &thin, N_("use thin pack")),
+		OPT_BOOL( 0 , "thin", &thin, N_("use thin pack")),
 		OPT_STRING( 0 , "receive-pack", &receivepack, "receive-pack", N_("receive pack program")),
 		OPT_STRING( 0 , "exec", &receivepack, "receive-pack", N_("receive pack program")),
 		OPT_BIT('u', "set-upstream", &flags, N_("set upstream for git pull/status"),
-- 
1.8.3.4-980-g8decd39

[PATCH v3 3/6] remote.c: add command line option parser for "--force-with-lease"

[Junio C Hamano]

Update "git push" and "git send-pack" to parse this commnd line
option.

The intended sematics is:

 * "--force-with-lease" alone, without specifying the details, will
   protect _all_ remote refs that are going to be updated by
   requiring their current value to be the same as some reasonable
   default, unless otherwise specified;

 * "--force-with-lease=refname", without specifying the expected
   value, will protect that refname, if it is going to be updated,
   by requiring its current value to be the same as some reasonable
   default.

 * "--force-with-lease=refname:value" will protect that refname, if
   it is going to be updated, by requiring its current value to be
   the same as the specified value; and

 * "--no-force-with-lease" will cancel all the previous --force-with-lease on the
   command line.

For now, "some reasonable default" is tentatively defined as "the
value of the remote-tracking branch we have for the ref of the
remote being updated", and it is an error if we do not have such a
remote-tracking branch.  But this is known to be fragile, its use is
not yet recommended, and hopefully we will find more reasonable
default as we gain experience with this feature.  The manual marks
the feature as experimental unless the expected value is specified
explicitly for this reason.

Because the command line options are parsed _before_ we know which
remote we are pushing to, there needs further processing to the
parsed data after we instantiate the transport object to:

 * expand "refname" given by the user to a full refname to be
   matched with the list of "struct ref" used in match_push_refs()
   and set_ref_status_for_push(); and

 * learning the actual local ref that is the remote-tracking branch
   for the specified remote ref.

Further, some processing need to be deferred until we find the set
of remote refs and match_push_refs() returns in order to find the
ones that need to be checked after explicit ones have been processed
for "--force-with-lease" (no specific details).

These post-processing will be the topic of the next patch.

This option was originally called "cas" (for "compare and swap"),
the name which nobody liked because it was too technical.  The
second attempt called it "lockref" (because it is conceptually like
pushing after taking a lock) but the word "lock" was hated because
it implied that it may reject push by others, which is not the way
this option works.  This round calls it "force-with-lease".  You
assume you took the lease on the ref when you fetched to decide what
the rebased history should be, and you can push back only if the
lease has not been broken.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-push.txt | 77 +++++++++++++++++++++++++++++++++++++++-------
 builtin/push.c             |  6 ++++
 builtin/send-pack.c        | 17 ++++++++++
 remote.c                   | 57 ++++++++++++++++++++++++++++++++++
 remote.h                   | 22 +++++++++++++
 5 files changed, 168 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index f7dfe48..e2992f1 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -11,6 +11,7 @@ SYNOPSIS
 [verse]
 'git push' [--all | --mirror | --tags] [--follow-tags] [-n | --dry-run] [--receive-pack=<git-receive-pack>]
 	   [--repo=<repository>] [-f | --force] [--prune] [-v | --verbose] [-u | --set-upstream]
+	   [--force-with-lease[=<refname>[:<expect>]]]
 	   [--no-verify] [<repository> [<refspec>...]]
 
 DESCRIPTION
@@ -130,21 +131,75 @@ already exists on the remote side.
 	repository over ssh, and you do not have the program in
 	a directory on the default $PATH.
 
+--[no-]force-with-lease::
+--force-with-lease=<refname>::
+--force-with-lease=<refname>:<expect>::
+	Usually, "git push" refuses to update a remote ref that is
+	not an ancestor of the local ref used to overwrite it.
++
+This option bypasses the check, but instead requires that the
+current value of the ref to be the expected value.  "git push"
+fails otherwise.
++
+Imagine that you have to rebase what you have already published.
+You will have to bypass the "must fast-forward" rule in order to
+replace the history you originally published with the rebased history.
+If somebody else built on top of your original history while you are
+rebasing, the tip of the branch at the remote may advance with her
+commit, and blindly pushing with `--force` will lose her work.
++
+This option allows you to say that you expect the history you are
+updating is what you rebased and want to replace. If the remote ref
+still points at the commit you specified, you can be sure that no
+other people did anything to the ref (it is like taking a "lease" on
+the ref without explicitly locking it, and you update the ref while
+making sure that your earlier "lease" is still valid).
++
+`--force-with-lease` alone, without specifying the details, will protect
+all remote refs that are going to be updated by requiring their
+current value to be the same as the remote-tracking branch we have
+for them, unless specified with a `--force-with-lease=<refname>:<expect>`
+option that explicitly states what the expected value is.
++
+`--force-with-lease=<refname>`, without specifying the expected value, will
+protect the named ref (alone), if it is going to be updated, by
+requiring its current value to be the same as the remote-tracking
+branch we have for it.
++
+`--force-with-lease=<refname>:<expect>` will protect the named ref (alone),
+if it is going to be updated, by requiring its current value to be
+the same as the specified value <expect> (which is allowed to be
+different from the remote-tracking branch we have for the refname,
+or we do not even have to have such a remote-tracking branch when
+this form is used).
++
+Note that all forms other than `--force-with-lease=<refname>:<expect>`
+that specifies the expected current value of the ref explicitly are
+still experimental and their semantics may change as we gain experience
+with this feature.
++
+"--no-force-with-lease" will cancel all the previous --force-with-lease on the
+command line.
+
 -f::
 --force::
 	Usually, the command refuses to update a remote ref that is
 	not an ancestor of the local ref used to overwrite it.
-	This flag disables the check.  This can cause the
-	remote repository to lose commits; use it with care.
-	Note that `--force` applies to all the refs that are pushed,
-	hence using it with `push.default` set to `matching` or with
-	multiple push destinations configured with `remote.*.push`
-	may overwrite refs other than the current branch (including
-	local refs that are strictly behind their remote counterpart).
-	To force a push to only one branch, use a `+` in front of the
-	refspec to push	(e.g `git push origin +master` to force a push
-	to the `master`	branch). See the `<refspec>...` section above
-	for details.
+	Also, when `--force-with-lease` option is used, the command refuses
+	to update a remote ref whose current value does not match
+	what is expected.
++
+This flag disables these checks, and can cause the remote repository
+to lose commits; use it with care.
++
+Note that `--force` applies to all the refs that are pushed, hence
+using it with `push.default` set to `matching` or with multiple push
+destinations configured with `remote.*.push` may overwrite refs
+other than the current branch (including local refs that are
+strictly behind their remote counterpart).  To force a push to only
+one branch, use a `+` in front of the refspec to push (e.g `git push
+origin +master` to force a push to the `master` branch). See the
+`<refspec>...` section above for details.
 
 --repo=<repository>::
 	This option is only relevant if no <repository> argument is
diff --git a/builtin/push.c b/builtin/push.c
index 342d792..31a5ba0 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -21,6 +21,8 @@ static const char *receivepack;
 static int verbosity;
 static int progress = -1;
 
+static struct push_cas_option cas;
+
 static const char **refspec;
 static int refspec_nr;
 static int refspec_alloc;
@@ -432,6 +434,10 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		OPT_BIT('n' , "dry-run", &flags, N_("dry run"), TRANSPORT_PUSH_DRY_RUN),
 		OPT_BIT( 0,  "porcelain", &flags, N_("machine-readable output"), TRANSPORT_PUSH_PORCELAIN),
 		OPT_BIT('f', "force", &flags, N_("force updates"), TRANSPORT_PUSH_FORCE),
+		{ OPTION_CALLBACK,
+		  0, CAS_OPT_NAME, &cas, N_("refname>:<expect"),
+		  N_("require old value of ref to be at this value"),
+		  PARSE_OPT_OPTARG, parseopt_push_cas_option },
 		{ OPTION_CALLBACK, 0, "recurse-submodules", &flags, N_("check"),
 			N_("control recursive pushing of submodules"),
 			PARSE_OPT_OPTARG, option_parse_recurse_submodules },
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index e86d3b5..a23b26d 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -103,6 +103,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
 	int flags;
 	unsigned int reject_reasons;
 	int progress = -1;
+	struct push_cas_option cas = {0};
 
 	argv++;
 	for (i = 1; i < argc; i++, argv++) {
@@ -165,6 +166,22 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
 				helper_status = 1;
 				continue;
 			}
+			if (!strcmp(arg, "--" CAS_OPT_NAME)) {
+				if (parse_push_cas_option(&cas, NULL, 0) < 0)
+					exit(1);
+				continue;
+			}
+			if (!strcmp(arg, "--no-" CAS_OPT_NAME)) {
+				if (parse_push_cas_option(&cas, NULL, 1) < 0)
+					exit(1);
+				continue;
+			}
+			if (!prefixcmp(arg, "--" CAS_OPT_NAME "=")) {
+				if (parse_push_cas_option(&cas,
+							  strchr(arg, '=') + 1, 1) < 0)
+					exit(1);
+				continue;
+			}
 			usage(send_pack_usage);
 		}
 		if (!dest) {
diff --git a/remote.c b/remote.c
index b1ff7a2..0d38353 100644
--- a/remote.c
+++ b/remote.c
@@ -1921,3 +1921,60 @@ struct ref *get_stale_heads(struct refspec *refs, int ref_count, struct ref *fet
 	string_list_clear(&ref_names, 0);
 	return stale_refs;
 }
+
+/*
+ * Compare-and-swap
+ */
+void clear_cas_option(struct push_cas_option *cas)
+{
+	int i;
+
+	for (i = 0; i < cas->nr; i++)
+		free(cas->entry[i].refname);
+	free(cas->entry);
+	memset(cas, 0, sizeof(*cas));
+}
+
+static struct push_cas *add_cas_entry(struct push_cas_option *cas,
+				      const char *refname,
+				      size_t refnamelen)
+{
+	struct push_cas *entry;
+	ALLOC_GROW(cas->entry, cas->nr + 1, cas->alloc);
+	entry = &cas->entry[cas->nr++];
+	memset(entry, 0, sizeof(*entry));
+	entry->refname = xmemdupz(refname, refnamelen);
+	return entry;
+}
+
+int parse_push_cas_option(struct push_cas_option *cas, const char *arg, int unset)
+{
+	const char *colon;
+	struct push_cas *entry;
+
+	if (unset) {
+		/* "--no-<option>" */
+		clear_cas_option(cas);
+		return 0;
+	}
+
+	if (!arg) {
+		/* just "--<option>" */
+		cas->use_tracking_for_rest = 1;
+		return 0;
+	}
+
+	/* "--<option>=refname" or "--<option>=refname:value" */
+	colon = strchrnul(arg, ':');
+	entry = add_cas_entry(cas, arg, colon - arg);
+	if (!*colon)
+		entry->use_tracking = 1;
+	else if (get_sha1(colon + 1, entry->expect))
+		return error("cannot parse expected object name '%s'", colon + 1);
+	return 0;
+}
+
+int parseopt_push_cas_option(const struct option *opt, const char *arg, int unset)
+{
+	return parse_push_cas_option(opt->value, arg, unset);
+}
diff --git a/remote.h b/remote.h
index a850059..843c3ce 100644
--- a/remote.h
+++ b/remote.h
@@ -1,6 +1,8 @@
 #ifndef REMOTE_H
 #define REMOTE_H
 
+#include "parse-options.h"
+
 enum {
 	REMOTE_CONFIG,
 	REMOTE_REMOTES,
@@ -226,4 +228,24 @@ struct ref *guess_remote_head(const struct ref *head,
 /* Return refs which no longer exist on remote */
 struct ref *get_stale_heads(struct refspec *refs, int ref_count, struct ref *fetch_map);
 
+/*
+ * Compare-and-swap
+ */
+#define CAS_OPT_NAME "force-with-lease"
+
+struct push_cas_option {
+	unsigned use_tracking_for_rest:1;
+	struct push_cas {
+		unsigned char expect[20];
+		unsigned use_tracking:1;
+		char *refname;
+	} *entry;
+	int nr;
+	int alloc;
+};
+
+extern int parseopt_push_cas_option(const struct option *, const char *arg, int unset);
+extern int parse_push_cas_option(struct push_cas_option *, const char *arg, int unset);
+extern void clear_cas_option(struct push_cas_option *);
+
 #endif
-- 
1.8.3.4-980-g8decd39

[PATCH v3 4/6] push --force-with-lease: implement logic to populate old_sha1_expect[]

[Junio C Hamano]

This plugs the push_cas_option data collected by the command line
option parser to the transport system with a new function
apply_push_cas(), which is called after match_push_refs() has
already been called.

At this point, we know which remote we are talking to, and what
remote refs we are going to update, so we can fill in the details
that may have been missing from the command line, such as

 (1) what abbreviated refname the user gave us matches the actual
     refname at the remote; and

 (2) which remote-tracking branch in our local repository to read
     the value of the object to expect at the remote.

to populate the old_sha1_expect[] field of each of the remote ref.
As stated in the documentation, the use of remote-tracking branch
as the default is a tentative one, and we may come up with a better
logic as we gain experience.

Still nobody uses this information, which is the topic of the next
patch.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/push.c      |  7 ++++++
 builtin/send-pack.c |  3 +++
 remote.c            | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 remote.h            |  6 ++++++
 transport.c         |  6 ++++++
 transport.h         |  4 ++++
 6 files changed, 87 insertions(+)

diff --git a/builtin/push.c b/builtin/push.c
index 31a5ba0..2fd0a70 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -299,6 +299,13 @@ static int push_with_options(struct transport *transport, int flags)
 	if (thin)
 		transport_set_option(transport, TRANS_OPT_THIN, "yes");
 
+	if (!is_empty_cas(&cas)) {
+		if (!transport->smart_options)
+			die("underlying transport does not support --%s option",
+			    CAS_OPT_NAME);
+		transport->smart_options->cas = &cas;
+	}
+
 	if (verbosity > 0)
 		fprintf(stderr, _("Pushing to %s\n"), transport->url);
 	err = transport_push(transport, refspec_nr, refspec, flags,
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index a23b26d..6027ead 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -242,6 +242,9 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
 	if (match_push_refs(local_refs, &remote_refs, nr_refspecs, refspecs, flags))
 		return -1;
 
+	if (!is_empty_cas(&cas))
+		apply_push_cas(&cas, remote, remote_refs);
+
 	set_ref_status_for_push(remote_refs, args.send_mirror,
 		args.force_update);
 
diff --git a/remote.c b/remote.c
index 0d38353..52e3a12 100644
--- a/remote.c
+++ b/remote.c
@@ -1978,3 +1978,64 @@ int parseopt_push_cas_option(const struct option *opt, const char *arg, int unse
 {
 	return parse_push_cas_option(opt->value, arg, unset);
 }
+
+int is_empty_cas(const struct push_cas_option *cas)
+{
+	return !cas->use_tracking_for_rest && !cas->nr;
+}
+
+/*
+ * Look at remote.fetch refspec and see if we have a remote
+ * tracking branch for the refname there.  Fill its current
+ * value in sha1[].
+ * If we cannot do so, return negative to signal an error.
+ */
+static int remote_tracking(struct remote *remote, const char *refname,
+			   unsigned char sha1[20])
+{
+	char *dst;
+
+	dst = apply_refspecs(remote->fetch, remote->fetch_refspec_nr, refname);
+	if (!dst)
+		return -1; /* no tracking ref for refname at remote */
+	if (read_ref(dst, sha1))
+		return -1; /* we know what the tracking ref is but we cannot read it */
+	return 0;
+}
+
+static void apply_cas(struct push_cas_option *cas,
+		      struct remote *remote,
+		      struct ref *ref)
+{
+	int i;
+
+	/* Find an explicit --<option>=<name>[:<value>] entry */
+	for (i = 0; i < cas->nr; i++) {
+		struct push_cas *entry = &cas->entry[i];
+		if (!refname_match(entry->refname, ref->name, ref_rev_parse_rules))
+			continue;
+		ref->expect_old_sha1 = 1;
+		if (!entry->use_tracking)
+			hashcpy(ref->old_sha1_expect, cas->entry[i].expect);
+		else if (remote_tracking(remote, ref->name, ref->old_sha1_expect))
+			ref->expect_old_no_trackback = 1;
+		return;
+	}
+
+	/* Are we using "--<option>" to cover all? */
+	if (!cas->use_tracking_for_rest)
+		return;
+
+	ref->expect_old_sha1 = 1;
+	if (remote_tracking(remote, ref->name, ref->old_sha1_expect))
+		ref->expect_old_no_trackback = 1;
+}
+
+void apply_push_cas(struct push_cas_option *cas,
+		    struct remote *remote,
+		    struct ref *remote_refs)
+{
+	struct ref *ref;
+	for (ref = remote_refs; ref; ref = ref->next)
+		apply_cas(cas, remote, ref);
+}
diff --git a/remote.h b/remote.h
index 843c3ce..ca3c8c8 100644
--- a/remote.h
+++ b/remote.h
@@ -77,10 +77,13 @@ struct ref {
 	struct ref *next;
 	unsigned char old_sha1[20];
 	unsigned char new_sha1[20];
+	unsigned char old_sha1_expect[20]; /* used by expect-old */
 	char *symref;
 	unsigned int
 		force:1,
 		forced_update:1,
+		expect_old_sha1:1,
+		expect_old_no_trackback:1,
 		deletion:1,
 		matched:1;
 
@@ -248,4 +251,7 @@ extern int parseopt_push_cas_option(const struct option *, const char *arg, int
 extern int parse_push_cas_option(struct push_cas_option *, const char *arg, int unset);
 extern void clear_cas_option(struct push_cas_option *);
 
+extern int is_empty_cas(const struct push_cas_option *);
+void apply_push_cas(struct push_cas_option *, struct remote *, struct ref *);
+
 #endif
diff --git a/transport.c b/transport.c
index b84dbf0..5dd92b7 100644
--- a/transport.c
+++ b/transport.c
@@ -1142,6 +1142,12 @@ int transport_push(struct transport *transport,
 			return -1;
 		}
 
+		if (transport->smart_options &&
+		    transport->smart_options->cas &&
+		    !is_empty_cas(transport->smart_options->cas))
+			apply_push_cas(transport->smart_options->cas,
+				       transport->remote, remote_refs);
+
 		set_ref_status_for_push(remote_refs,
 			flags & TRANSPORT_PUSH_MIRROR,
 			flags & TRANSPORT_PUSH_FORCE);
diff --git a/transport.h b/transport.h
index b551f99..10f7556 100644
--- a/transport.h
+++ b/transport.h
@@ -14,6 +14,7 @@ struct git_transport_options {
 	int depth;
 	const char *uploadpack;
 	const char *receivepack;
+	struct push_cas_option *cas;
 };
 
 struct transport {
@@ -127,6 +128,9 @@ struct transport *transport_get(struct remote *, const char *);
 /* Transfer the data as a thin pack if not null */
 #define TRANS_OPT_THIN "thin"
 
+/* Check the current value of the remote ref */
+#define TRANS_OPT_CAS "cas"
+
 /* Keep the pack that was transferred if not null */
 #define TRANS_OPT_KEEP "keep"
 
-- 
1.8.3.4-980-g8decd39

[PATCH v3 5/6] push --force-with-lease: tie it all together

[Junio C Hamano]

This teaches the deepest part of the callchain for "git push" (and
"git send-pack") to enforce "the old value of the ref must be this,
otherwise fail this push" (aka "compare-and-swap" / "--lockref").

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/send-pack.c |  5 +++++
 remote.c            | 49 ++++++++++++++++++++++++++++++++++++-------------
 remote.h            |  1 +
 send-pack.c         |  1 +
 transport-helper.c  |  6 ++++++
 transport.c         |  5 +++++
 6 files changed, 54 insertions(+), 13 deletions(-)

diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 6027ead..41dc512 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -55,6 +55,11 @@ static void print_helper_status(struct ref *ref)
 			msg = "needs force";
 			break;
 
+		case REF_STATUS_REJECT_STALE:
+			res = "error";
+			msg = "stale info";
+			break;
+
 		case REF_STATUS_REJECT_ALREADY_EXISTS:
 			res = "error";
 			msg = "already exists";
diff --git a/remote.c b/remote.c
index 52e3a12..922822c 100644
--- a/remote.c
+++ b/remote.c
@@ -1396,12 +1396,13 @@ int match_push_refs(struct ref *src, struct ref **dst,
 }
 
 void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
-	int force_update)
+			     int force_update)
 {
 	struct ref *ref;
 
 	for (ref = remote_refs; ref; ref = ref->next) {
 		int force_ref_update = ref->force || force_update;
+		int reject_reason = 0;
 
 		if (ref->peer_ref)
 			hashcpy(ref->new_sha1, ref->peer_ref->new_sha1);
@@ -1416,6 +1417,26 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
 		}
 
 		/*
+		 * Bypass the usual "must fast-forward" check but
+		 * replace it with a weaker "the old value must be
+		 * this value we observed".  If the remote ref has
+		 * moved and is now different from what we expect,
+		 * reject any push.
+		 *
+		 * It also is an error if the user told us to check
+		 * with the remote-tracking branch to find the value
+		 * to expect, but we did not have such a tracking
+		 * branch.
+		 */
+		if (ref->expect_old_sha1) {
+			if (ref->expect_old_no_trackback ||
+			    hashcmp(ref->old_sha1, ref->old_sha1_expect))
+				reject_reason = REF_STATUS_REJECT_STALE;
+		}
+
+		/*
+		 * The usual "must fast-forward" rules.
+		 *
 		 * Decide whether an individual refspec A:B can be
 		 * pushed.  The push will succeed if any of the
 		 * following are true:
@@ -1433,24 +1454,26 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
 		 *     passing the --force argument
 		 */
 
-		if (!ref->deletion && !is_null_sha1(ref->old_sha1)) {
-			int why = 0; /* why would this push require --force? */
-
+		else if (!ref->deletion && !is_null_sha1(ref->old_sha1)) {
 			if (!prefixcmp(ref->name, "refs/tags/"))
-				why = REF_STATUS_REJECT_ALREADY_EXISTS;
+				reject_reason = REF_STATUS_REJECT_ALREADY_EXISTS;
 			else if (!has_sha1_file(ref->old_sha1))
-				why = REF_STATUS_REJECT_FETCH_FIRST;
+				reject_reason = REF_STATUS_REJECT_FETCH_FIRST;
 			else if (!lookup_commit_reference_gently(ref->old_sha1, 1) ||
 				 !lookup_commit_reference_gently(ref->new_sha1, 1))
-				why = REF_STATUS_REJECT_NEEDS_FORCE;
+				reject_reason = REF_STATUS_REJECT_NEEDS_FORCE;
 			else if (!ref_newer(ref->new_sha1, ref->old_sha1))
-				why = REF_STATUS_REJECT_NONFASTFORWARD;
-
-			if (!force_ref_update)
-				ref->status = why;
-			else if (why)
-				ref->forced_update = 1;
+				reject_reason = REF_STATUS_REJECT_NONFASTFORWARD;
 		}
+
+		/*
+		 * "--force" will defeat any rejection implemented
+		 * by the rules above.
+		 */
+		if (!force_ref_update)
+			ref->status = reject_reason;
+		else if (reject_reason)
+			ref->forced_update = 1;
 	}
 }
 
diff --git a/remote.h b/remote.h
index ca3c8c8..6c42554 100644
--- a/remote.h
+++ b/remote.h
@@ -107,6 +107,7 @@ struct ref {
 		REF_STATUS_REJECT_NODELETE,
 		REF_STATUS_REJECT_FETCH_FIRST,
 		REF_STATUS_REJECT_NEEDS_FORCE,
+		REF_STATUS_REJECT_STALE,
 		REF_STATUS_UPTODATE,
 		REF_STATUS_REMOTE_REJECT,
 		REF_STATUS_EXPECTING_REPORT
diff --git a/send-pack.c b/send-pack.c
index 9a9908c..b228d65 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -227,6 +227,7 @@ int send_pack(struct send_pack_args *args,
 		case REF_STATUS_REJECT_ALREADY_EXISTS:
 		case REF_STATUS_REJECT_FETCH_FIRST:
 		case REF_STATUS_REJECT_NEEDS_FORCE:
+		case REF_STATUS_REJECT_STALE:
 		case REF_STATUS_UPTODATE:
 			continue;
 		default:
diff --git a/transport-helper.c b/transport-helper.c
index db9bd18..95d22f8 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -683,6 +683,11 @@ static int push_update_ref_status(struct strbuf *buf,
 			free(msg);
 			msg = NULL;
 		}
+		else if (!strcmp(msg, "stale info")) {
+			status = REF_STATUS_REJECT_STALE;
+			free(msg);
+			msg = NULL;
+		}
 	}
 
 	if (*ref)
@@ -756,6 +761,7 @@ static int push_refs_with_push(struct transport *transport,
 		/* Check for statuses set by set_ref_status_for_push() */
 		switch (ref->status) {
 		case REF_STATUS_REJECT_NONFASTFORWARD:
+		case REF_STATUS_REJECT_STALE:
 		case REF_STATUS_REJECT_ALREADY_EXISTS:
 		case REF_STATUS_UPTODATE:
 			continue;
diff --git a/transport.c b/transport.c
index 5dd92b7..b321d6a 100644
--- a/transport.c
+++ b/transport.c
@@ -709,6 +709,10 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
 		print_ref_status('!', "[rejected]", ref, ref->peer_ref,
 						 "needs force", porcelain);
 		break;
+	case REF_STATUS_REJECT_STALE:
+		print_ref_status('!', "[rejected]", ref, ref->peer_ref,
+						 "stale info", porcelain);
+		break;
 	case REF_STATUS_REMOTE_REJECT:
 		print_ref_status('!', "[remote rejected]", ref,
 						 ref->deletion ? NULL : ref->peer_ref,
@@ -1078,6 +1082,7 @@ static int run_pre_push_hook(struct transport *transport,
 	for (r = remote_refs; r; r = r->next) {
 		if (!r->peer_ref) continue;
 		if (r->status == REF_STATUS_REJECT_NONFASTFORWARD) continue;
+		if (r->status == REF_STATUS_REJECT_STALE) continue;
 		if (r->status == REF_STATUS_UPTODATE) continue;
 
 		strbuf_reset(&buf);
-- 
1.8.3.4-980-g8decd39

[PATCH v3 6/6] t5533: test "push --force-with-lease"

[Junio C Hamano]

Prepare two repositories, src and dst, the latter of which is a
clone of the former (with tracking branches), and push from the
latter into the former, with various --force-with-lease options.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5533-push-cas.sh | 189 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 189 insertions(+)
 create mode 100755 t/t5533-push-cas.sh

diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh
new file mode 100755
index 0000000..ba20d83
--- /dev/null
+++ b/t/t5533-push-cas.sh
@@ -0,0 +1,189 @@
+#!/bin/sh
+
+test_description='compare & swap push force/delete safety'
+
+. ./test-lib.sh
+
+setup_srcdst_basic () {
+	rm -fr src dst &&
+	git clone --no-local . src &&
+	git clone --no-local src dst &&
+	(
+		cd src && git checkout HEAD^0
+	)
+}
+
+test_expect_success setup '
+	: create template repository
+	test_commit A &&
+	test_commit B &&
+	test_commit C
+'
+
+test_expect_success 'push to update (protected)' '
+	setup_srcdst_basic &&
+	(
+		cd dst &&
+		test_commit D &&
+		test_must_fail git push --force-with-lease=master:master origin master
+	) &&
+	git ls-remote . refs/heads/master >expect &&
+	git ls-remote src refs/heads/master >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'push to update (protected, forced)' '
+	setup_srcdst_basic &&
+	(
+		cd dst &&
+		test_commit D &&
+		git push --force --force-with-lease=master:master origin master
+	) &&
+	git ls-remote dst refs/heads/master >expect &&
+	git ls-remote src refs/heads/master >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'push to update (protected, tracking)' '
+	setup_srcdst_basic &&
+	(
+		cd src &&
+		git checkout master &&
+		test_commit D &&
+		git checkout HEAD^0
+	) &&
+	git ls-remote src refs/heads/master >expect &&
+	(
+		cd dst &&
+		test_commit E &&
+		git ls-remote . refs/remotes/origin/master >expect &&
+		test_must_fail git push --force-with-lease=master origin master &&
+		git ls-remote . refs/remotes/origin/master >actual &&
+		test_cmp expect actual
+	) &&
+	git ls-remote src refs/heads/master >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'push to update (protected, tracking, forced)' '
+	setup_srcdst_basic &&
+	(
+		cd src &&
+		git checkout master &&
+		test_commit D &&
+		git checkout HEAD^0
+	) &&
+	(
+		cd dst &&
+		test_commit E &&
+		git ls-remote . refs/remotes/origin/master >expect &&
+		git push --force --force-with-lease=master origin master
+	) &&
+	git ls-remote dst refs/heads/master >expect &&
+	git ls-remote src refs/heads/master >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'push to update (allowed)' '
+	setup_srcdst_basic &&
+	(
+		cd dst &&
+		test_commit D &&
+		git push --force-with-lease=master:master^ origin master
+	) &&
+	git ls-remote dst refs/heads/master >expect &&
+	git ls-remote src refs/heads/master >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'push to update (allowed, tracking)' '
+	setup_srcdst_basic &&
+	(
+		cd dst &&
+		test_commit D &&
+		git push --force-with-lease=master origin master
+	) &&
+	git ls-remote dst refs/heads/master >expect &&
+	git ls-remote src refs/heads/master >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'push to update (allowed even though no-ff)' '
+	setup_srcdst_basic &&
+	(
+		cd dst &&
+		git reset --hard HEAD^ &&
+		test_commit D &&
+		git push --force-with-lease=master origin master
+	) &&
+	git ls-remote dst refs/heads/master >expect &&
+	git ls-remote src refs/heads/master >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'push to delete (protected)' '
+	setup_srcdst_basic &&
+	git ls-remote src refs/heads/master >expect &&
+	(
+		cd dst &&
+		test_must_fail git push --force-with-lease=master:master^ origin :master
+	) &&
+	git ls-remote src refs/heads/master >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'push to delete (protected, forced)' '
+	setup_srcdst_basic &&
+	(
+		cd dst &&
+		git push --force --force-with-lease=master:master^ origin :master
+	) &&
+	>expect &&
+	git ls-remote src refs/heads/master >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'push to delete (allowed)' '
+	setup_srcdst_basic &&
+	(
+		cd dst &&
+		git push --force-with-lease=master origin :master
+	) &&
+	>expect &&
+	git ls-remote src refs/heads/master >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'cover everything with default force-with-lease (protected)' '
+	setup_srcdst_basic &&
+	(
+		cd src &&
+		git branch naster master^
+	)
+	git ls-remote src refs/heads/\* >expect &&
+	(
+		cd dst &&
+		test_must_fail git push --force-with-lease origin master master:naster
+	) &&
+	git ls-remote src refs/heads/\* >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'cover everything with default force-with-lease (allowed)' '
+	setup_srcdst_basic &&
+	(
+		cd src &&
+		git branch naster master^
+	)
+	(
+		cd dst &&
+		git fetch &&
+		git push --force-with-lease origin master master:naster
+	) &&
+	git ls-remote dst refs/heads/master |
+	sed -e "s/master/naster/" >expect &&
+	git ls-remote src refs/heads/naster >actual &&
+	test_cmp expect actual
+'
+
+test_done
-- 
1.8.3.4-980-g8decd39

Re: [PATCH v3 0/6]

[Jakub Narebski]

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

> 
> This is mostly unchanged since the previous round, except that
> 
>  * The option is spelled "--force-with-lease=<ref>:<expect>".
>    Nobody liked "cas" as it was too technical, many disliked
>    "lockref" because "lock" sounded as if push by others were
>    excluded by it while in fact this is to fail us.

Perhaps "--force-gently" ? :-)

-- 
Jakub Narębski

Re: [PATCH v3 0/6]

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> Junio C Hamano <gitster <at> pobox.com> writes:
>
>> 
>> This is mostly unchanged since the previous round, except that
>> 
>>  * The option is spelled "--force-with-lease=<ref>:<expect>".
>>    Nobody liked "cas" as it was too technical, many disliked
>>    "lockref" because "lock" sounded as if push by others were
>>    excluded by it while in fact this is to fail us.
>
> Perhaps "--force-gently" ? :-)

Hmph.  But we usually use "gently" to mean "do not give the end user
an error message--the caller handles the error itself".

While the option lets you break the usual "must fast-forward" rule,
it is more precise in that the remote ref must be pointing at not
just any ancestor of what you are pushing, but has to be at the
exact commit you specify.

E.g. if you have built one commit on top of the shared branch, and
try to push it with "push --cas=pu:HEAD^ HEAD:pu" (because you know
one commit before the tip is where you started from), your push will
be rejected if somebody else did an equivalent of "reset HEAD~3" on
the receiving end (perhaps because the top commits recorded some
material inappropriate for the project).  Your new commit is still a
decendant of that rewound tip, and usual "must fast-forward" rule
would accept the push, but with "push --cas=pu:HEAD^ HEAD:pu", you
will notice that somebody wanted to rewind the tip and pushing your
work that contains these dropped commit contradicts with that wish.

So I dunno.

Re: [PATCH v3 0/6]

[Philip Oakley]

From: "Junio C Hamano" <gitster@pobox.com>
Sent: Tuesday, July 23, 2013 7:26 PM
> Jakub Narebski <jnareb@gmail.com> writes:
>
>> Junio C Hamano <gitster <at> pobox.com> writes:
>>
>>>
>>> This is mostly unchanged since the previous round, except that
>>>
>>>  * The option is spelled "--force-with-lease=<ref>:<expect>".
>>>    Nobody liked "cas" as it was too technical, many disliked
>>>    "lockref" because "lock" sounded as if push by others were
>>>    excluded by it while in fact this is to fail us.
>>
>> Perhaps "--force-gently" ? :-)

Or "--force-carefully" to better indicate the safety / care that is 
being applied?

>
> Hmph.  But we usually use "gently" to mean "do not give the end user
> an error message--the caller handles the error itself".
>
> While the option lets you break the usual "must fast-forward" rule,
> it is more precise in that the remote ref must be pointing at not
> just any ancestor of what you are pushing, but has to be at the
> exact commit you specify.
>
> E.g. if you have built one commit on top of the shared branch, and
> try to push it with "push --cas=pu:HEAD^ HEAD:pu" (because you know
> one commit before the tip is where you started from), your push will
> be rejected if somebody else did an equivalent of "reset HEAD~3" on
> the receiving end (perhaps because the top commits recorded some
> material inappropriate for the project).  Your new commit is still a
> decendant of that rewound tip, and usual "must fast-forward" rule
> would accept the push, but with "push --cas=pu:HEAD^ HEAD:pu", you
> will notice that somebody wanted to rewind the tip and pushing your
> work that contains these dropped commit contradicts with that wish.
>
> So I dunno.
> --

Re: [PATCH v3 0/6]

[Eric Sunshine]

On Tue, Jul 23, 2013 at 5:26 PM, Philip Oakley <philipoakley@iee.org> wrote:
> From: "Junio C Hamano" <gitster@pobox.com>
> Sent: Tuesday, July 23, 2013 7:26 PM
>> Jakub Narebski <jnareb@gmail.com> writes:
>>> Junio C Hamano <gitster <at> pobox.com> writes:
>>>> This is mostly unchanged since the previous round, except that
>>>>
>>>>  * The option is spelled "--force-with-lease=<ref>:<expect>".
>>>>    Nobody liked "cas" as it was too technical, many disliked
>>>>    "lockref" because "lock" sounded as if push by others were
>>>>    excluded by it while in fact this is to fail us.
>>>
>>> Perhaps "--force-gently" ? :-)
>
> Or "--force-carefully" to better indicate the safety / care that is being
> applied?

[bike-shedding: on]

--force-if-safe

[bike-shedding: off]

[PATCH v3 7/6] t5540/5541: smart-http does not support "--force-with-lease"

[Junio C Hamano]

The push() method in remote-curl.c is not told and does not pass the
necessary information to underlying send-pack, so this extension
does not yet work.  Leave a note in the test suite.

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

 * This is primarily to give a target for other people to shoot at;
   patches to allow us to flip this expect_failure to expect_success
   are very much welcomed ;-)

 t/lib-httpd.sh | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 895b925..e2eca1f 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -167,6 +167,22 @@ test_http_push_nonff() {
 	test_expect_success 'non-fast-forward push shows help message' '
 		test_i18ngrep "Updates were rejected because" output
 	'
+
+	test_expect_failure 'force with lease aka cas' '
+		HEAD=$(	cd "$REMOTE_REPO" && git rev-parse --verify HEAD ) &&
+		test_when_finished '\''
+			(cd "$REMOTE_REPO" && git update-ref HEAD "$HEAD")
+		'\'' &&
+		(
+			cd "$LOCAL_REPO" &&
+			git push -v --force-with-lease=$BRANCH:$HEAD origin
+		) &&
+		git rev-parse --verify "$BRANCH" >expect &&
+		(
+			cd "$REMOTE_REPO" && git rev-parse --verify HEAD
+		) >actual &&
+		test_cmp expect actual
+	'
 }
 
 setup_askpass_helper() {
-- 
1.8.4-rc0-154-gc668397

[PATCH v3 8/6] send-pack: fix parsing of --force-with-lease option

[Junio C Hamano]

The last argument for parse_push_cas_option() is if it is "unset"
(i.e. --no-force-with-lease), and we are parsing the option with an
explicit value here, so it has to be 0.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/send-pack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 41dc512..4482f16 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -183,7 +183,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
 			}
 			if (!prefixcmp(arg, "--" CAS_OPT_NAME "=")) {
 				if (parse_push_cas_option(&cas,
-							  strchr(arg, '=') + 1, 1) < 0)
+							  strchr(arg, '=') + 1, 0) < 0)
 					exit(1);
 				continue;
 			}
-- 
1.8.4-rc1-129-g1f3472b

[PATCH v3 9/6] push: teach --force-with-lease to smart-http transport

[Junio C Hamano]

We have been passing enough information to enable the
compare-and-swap logic down to the transport layer, but the
transport helper was not passing it to smart-http transport.

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

 * I didn't bother with the dumb commit walker push for obvious
   reasons, but if somebody is inclined to add one, it shouldn't be
   too hard to add.

 remote-curl.c        | 16 +++++++++++++++-
 t/lib-httpd.sh       |  3 ++-
 t/t5541-http-push.sh |  2 +-
 transport-helper.c   | 24 ++++++++++++++++++++++--
 4 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index 60eda63..53c8a3d 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -6,6 +6,7 @@
 #include "exec_cmd.h"
 #include "run-command.h"
 #include "pkt-line.h"
+#include "string-list.h"
 #include "sideband.h"
 
 static struct remote *remote;
@@ -20,6 +21,7 @@ struct options {
 		thin : 1;
 };
 static struct options options;
+static struct string_list cas_options = STRING_LIST_INIT_DUP;
 
 static int set_option(const char *name, const char *value)
 {
@@ -66,6 +68,13 @@ static int set_option(const char *name, const char *value)
 			return -1;
 		return 0;
 	}
+	else if (!strcmp(name, "cas")) {
+		struct strbuf val = STRBUF_INIT;
+		strbuf_addf(&val, "--" CAS_OPT_NAME "=%s", value);
+		string_list_append(&cas_options, val.buf);
+		strbuf_release(&val);
+		return 0;
+	}
 	else {
 		return 1 /* unsupported */;
 	}
@@ -789,8 +798,9 @@ static int push_git(struct discovery *heads, int nr_spec, char **specs)
 	struct rpc_state rpc;
 	const char **argv;
 	int argc = 0, i, err;
+	struct string_list_item *cas_option;
 
-	argv = xmalloc((10 + nr_spec) * sizeof(char*));
+	argv = xmalloc((10 + nr_spec + cas_options.nr) * sizeof(char *));
 	argv[argc++] = "send-pack";
 	argv[argc++] = "--stateless-rpc";
 	argv[argc++] = "--helper-status";
@@ -803,6 +813,10 @@ static int push_git(struct discovery *heads, int nr_spec, char **specs)
 	else if (options.verbosity > 1)
 		argv[argc++] = "--verbose";
 	argv[argc++] = options.progress ? "--progress" : "--no-progress";
+
+	for_each_string_list_item(cas_option, &cas_options)
+		argv[argc++] = cas_option->string;
+
 	argv[argc++] = url;
 	for (i = 0; i < nr_spec; i++)
 		argv[argc++] = specs[i];
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index e2eca1f..dab405d 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -141,10 +141,11 @@ stop_httpd() {
 		-f "$TEST_PATH/apache.conf" $HTTPD_PARA -k stop
 }
 
-test_http_push_nonff() {
+test_http_push_nonff () {
 	REMOTE_REPO=$1
 	LOCAL_REPO=$2
 	BRANCH=$3
+	EXPECT_CAS_RESULT=${4-failure}
 
 	test_expect_success 'non-fast-forward push fails' '
 		cd "$REMOTE_REPO" &&
diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh
index beb00be..470ac54 100755
--- a/t/t5541-http-push.sh
+++ b/t/t5541-http-push.sh
@@ -153,7 +153,7 @@ test_expect_success 'used receive-pack service' '
 '
 
 test_http_push_nonff "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo.git \
-	"$ROOT_PATH"/test_repo_clone master
+	"$ROOT_PATH"/test_repo_clone master 		success
 
 test_expect_success 'push fails for non-fast-forward refs unmatched by remote helper' '
 	# create a dissimilarly-named remote ref so that git is unable to match the
diff --git a/transport-helper.c b/transport-helper.c
index 95d22f8..e3a60d7 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -742,13 +742,15 @@ static void push_update_refs_status(struct helper_data *data,
 }
 
 static int push_refs_with_push(struct transport *transport,
-		struct ref *remote_refs, int flags)
+			       struct ref *remote_refs, int flags)
 {
 	int force_all = flags & TRANSPORT_PUSH_FORCE;
 	int mirror = flags & TRANSPORT_PUSH_MIRROR;
 	struct helper_data *data = transport->data;
 	struct strbuf buf = STRBUF_INIT;
 	struct ref *ref;
+	struct string_list cas_options = STRING_LIST_INIT_DUP;
+	struct string_list_item *cas_option;
 
 	get_helper(transport);
 	if (!data->push)
@@ -784,11 +786,29 @@ static int push_refs_with_push(struct transport *transport,
 		strbuf_addch(&buf, ':');
 		strbuf_addstr(&buf, ref->name);
 		strbuf_addch(&buf, '\n');
+
+		/*
+		 * The "--force-with-lease" options without explicit
+		 * values to expect have already been expanded into
+		 * the ref->old_sha1_expect[] field; we can ignore
+		 * transport->smart_options->cas altogether and instead
+		 * can enumerate them from the refs.
+		 */
+		if (ref->expect_old_sha1) {
+			struct strbuf cas = STRBUF_INIT;
+			strbuf_addf(&cas, "%s:%s",
+				    ref->name, sha1_to_hex(ref->old_sha1_expect));
+			string_list_append(&cas_options, strbuf_detach(&cas, NULL));
+		}
 	}
-	if (buf.len == 0)
+	if (buf.len == 0) {
+		string_list_clear(&cas_options, 0);
 		return 0;
+	}
 
 	standard_options(transport);
+	for_each_string_list_item(cas_option, &cas_options)
+		set_helper_option(transport, "cas", cas_option->string);
 
 	if (flags & TRANSPORT_PUSH_DRY_RUN) {
 		if (set_helper_option(transport, "dry-run", "true") != 0)
-- 
1.8.4-rc1-129-g1f3472b