Skip to content

Commit

Permalink
Merge branch 'jc/push-cas'
Browse files Browse the repository at this point in the history
Allow a safer "rewind of the remote tip" push than blind "--force",
by requiring that the overwritten remote ref to be unchanged since
the new history to replace it was prepared.

The machinery is more or less ready.  The "--force" option is again
the big red button to override any safety, thanks to J6t's sanity
(the original round allowed --lockref to defeat --force).

The logic to choose the default implemented here is fragile
(e.g. "git fetch" after seeing a failure will update the
remote-tracking branch and will make the next "push" pass,
defeating the safety pretty easily).  It is suitable only for the
simplest workflows, and it may hurt users more than it helps them.

* jc/push-cas:
  push: teach --force-with-lease to smart-http transport
  send-pack: fix parsing of --force-with-lease option
  t5540/5541: smart-http does not support "--force-with-lease"
  t5533: test "push --force-with-lease"
  push --force-with-lease: tie it all together
  push --force-with-lease: implement logic to populate old_sha1_expect[]
  remote.c: add command line option parser for "--force-with-lease"
  builtin/push.c: use OPT_BOOL, not OPT_BOOLEAN
  cache.h: move remote/connect API out of it
  • Loading branch information
gitster committed Sep 9, 2013
2 parents 711b276 + 05c1eb1 commit 2233ad4
Show file tree
Hide file tree
Showing 22 changed files with 638 additions and 98 deletions.
77 changes: 66 additions & 11 deletions Documentation/git-push.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions builtin/fetch-pack.c
Original file line number Diff line number Diff line change
@@ -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] "
Expand Down
13 changes: 13 additions & 0 deletions builtin/push.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -316,6 +318,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,
Expand Down Expand Up @@ -451,6 +460,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 },
Expand Down
1 change: 1 addition & 0 deletions builtin/receive-pack.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
26 changes: 26 additions & 0 deletions builtin/send-pack.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -54,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";
Expand Down Expand Up @@ -102,6 +108,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++) {
Expand Down Expand Up @@ -164,6 +171,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, 0) < 0)
exit(1);
continue;
}
usage(send_pack_usage);
}
if (!dest) {
Expand Down Expand Up @@ -224,6 +247,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);

Expand Down
62 changes: 0 additions & 62 deletions cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -1038,68 +1038,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 */
Expand Down
1 change: 1 addition & 0 deletions connect.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "refs.h"
#include "run-command.h"
#include "remote.h"
#include "connect.h"
#include "url.h"

static char *server_capabilities;
Expand Down
13 changes: 13 additions & 0 deletions connect.h
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions fetch-pack.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "fetch-pack.h"
#include "remote.h"
#include "run-command.h"
#include "connect.h"
#include "transport.h"
#include "version.h"
#include "prio-queue.h"
Expand Down
1 change: 1 addition & 0 deletions fetch-pack.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#define FETCH_PACK_H

#include "string-list.h"
#include "run-command.h"

struct fetch_pack_args {
const char *uploadpack;
Expand Down
8 changes: 0 additions & 8 deletions refs.c
Original file line number Diff line number Diff line change
Expand Up @@ -3196,14 +3196,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
Expand Down
12 changes: 12 additions & 0 deletions remote-curl.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "exec_cmd.h"
#include "run-command.h"
#include "pkt-line.h"
#include "string-list.h"
#include "sideband.h"
#include "argv-array.h"

Expand All @@ -22,6 +23,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)
{
Expand Down Expand Up @@ -77,6 +79,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 */;
}
Expand Down Expand Up @@ -802,6 +811,7 @@ static int push_git(struct discovery *heads, int nr_spec, char **specs)
struct rpc_state rpc;
int i, err;
struct argv_array args;
struct string_list_item *cas_option;

argv_array_init(&args);
argv_array_pushl(&args, "send-pack", "--stateless-rpc", "--helper-status",
Expand All @@ -816,6 +826,8 @@ static int push_git(struct discovery *heads, int nr_spec, char **specs)
else if (options.verbosity > 1)
argv_array_push(&args, "--verbose");
argv_array_push(&args, options.progress ? "--progress" : "--no-progress");
for_each_string_list_item(cas_option, &cas_options)
argv_array_push(&args, cas_option->string);
argv_array_push(&args, url);
for (i = 0; i < nr_spec; i++)
argv_array_push(&args, specs[i]);
Expand Down
Loading

0 comments on commit 2233ad4

Please sign in to comment.