[PATCH 1/8] receive-pack.c: add protocol support to negotiate atomic-push

[Ronnie Sahlberg]

This adds support to the protocol between send-pack and receive-pack to
* allow receive-pack to inform the client that it has atomic push capability
* allow send-pack to request atomic push back.

There is currently no setting in send-pack to actually request that atomic
pushes are to be used yet. This only adds protocol capability not ability
for the user to activate it.

Change-Id: I9a12940fb5c7443a1ddf9e45f6ea33b547c7ecfd
Signed-off-by: Ronnie Sahlberg <***@google.com>
---
Documentation/technical/protocol-capabilities.txt | 12 ++++++++++--
builtin/receive-pack.c | 6 +++++-
send-pack.c | 6 ++++++
3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt
index 0c92dee..26bc5b1 100644
--- a/Documentation/technical/protocol-capabilities.txt
+++ b/Documentation/technical/protocol-capabilities.txt
@@ -18,8 +18,9 @@ was sent. Server MUST NOT ignore capabilities that client requested
and server advertised. As a consequence of these rules, server MUST
NOT advertise capabilities it does not understand.

-The 'report-status', 'delete-refs', 'quiet', and 'push-cert' capabilities
-are sent and recognized by the receive-pack (push to server) process.
+The 'atomic-push', 'report-status', 'delete-refs', 'quiet', and 'push-cert'
+capabilities are sent and recognized by the receive-pack (push to server)
+process.

The 'ofs-delta' and 'side-band-64k' capabilities are sent and recognized
by both upload-pack and receive-pack protocols. The 'agent' capability
@@ -244,6 +245,13 @@ respond with the 'quiet' capability to suppress server-side progress
reporting if the local progress reporting is also being suppressed
(e.g., via `push -q`, or if stderr does not go to a tty).

+atomic-push
+-----------
+
+If the server sends the 'atomic-push' capability, it means it is
+capable of accepting atomic pushes. If the pushing client requests this
+capability, the server will update the refs in one single atomic transaction.
+
allow-tip-sha1-in-want
----------------------

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 7ddd756..b8ffd9e 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -40,6 +40,7 @@ static int transfer_unpack_limit = -1;
static int unpack_limit = 100;
static int report_status;
static int use_sideband;
+static int use_atomic_push;
static int quiet;
static int prefer_ofs_delta = 1;
static int auto_update_server_info;
@@ -171,7 +172,8 @@ static void show_ref(const char *path, const unsigned char *sha1)
struct strbuf cap = STRBUF_INIT;

strbuf_addstr(&cap,
- "report-status delete-refs side-band-64k quiet");
+ "report-status delete-refs side-band-64k quiet "
+ "atomic-push");
if (prefer_ofs_delta)
strbuf_addstr(&cap, " ofs-delta");
if (push_cert_nonce)
@@ -1173,6 +1175,8 @@ static struct command *read_head_info(struct sha1_array *shallow)
use_sideband = LARGE_PACKET_MAX;
if (parse_feature_request(feature_list, "quiet"))
quiet = 1;
+ if (parse_feature_request(feature_list, "atomic-push"))
+ use_atomic_push = 1;
}

if (!strcmp(line, "push-cert")) {
diff --git a/send-pack.c b/send-pack.c
index 949cb61..3520fe5 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -294,6 +294,8 @@ int send_pack(struct send_pack_args *args,
int use_sideband = 0;
int quiet_supported = 0;
int agent_supported = 0;
+ int atomic_push_supported = 0;
+ int atomic_push = 0;
unsigned cmds_sent = 0;
int ret;
struct async demux;
@@ -314,6 +316,8 @@ int send_pack(struct send_pack_args *args,
agent_supported = 1;
if (server_supports("no-thin"))
args->use_thin_pack = 0;
+ if (server_supports("atomic-push"))
+ atomic_push_supported = 1;
if (args->push_cert) {
int len;

@@ -337,6 +341,8 @@ int send_pack(struct send_pack_args *args,
strbuf_addstr(&cap_buf, " quiet");
if (agent_supported)
strbuf_addf(&cap_buf, " agent=%s", git_user_agent_sanitized());
+ if (atomic_push)
+ strbuf_addstr(&cap_buf, " atomic-push");

/*
* NEEDSWORK: why does delete-refs have to be so specific to

[Ronnie Sahlberg]

Add receive.preferatomicpush setting to receive-pack.c. This triggers
a new capability "prefer-atomic-push" to be sent back to the send-pack
client, requesting the client, if it supports it, to request
an atomic push.

This is an alternative way to trigger an atomic push instead of having
to rely of the user using the --atomic-push command line argument.
For backward compatibility, this is only a hint to the client that is should
request atomic pushes if it can. Old clients that do not support atomic pushes
will just ignore this capability and use a normal push.

The reason we need to signal this capability back to the client is due
to that send_pack() has push failure modes where it will detect that
certain refs can not be pushed and fail them early. Those refs would not
even be sent by the client unless atomic-pushes are activated.
This means that IF we activate this feature from the server side we must
tell the client to not fail refs early and use an atomic push. We can not
enforce this on the server side only.

Change-Id: I6677cd565f48a09bb552fe3f4c00bbb6d343c224
Signed-off-by: Ronnie Sahlberg <***@google.com>
---
Documentation/config.txt | 4 +++
Documentation/technical/protocol-capabilities.txt | 13 ++++++---
builtin/receive-pack.c | 8 ++++++
send-pack.c | 2 ++
t/t5543-atomic-push.sh | 32 +++++++++++++++++++++++
5 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 400dcad..78c427e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2135,6 +2135,10 @@ receive.shallowupdate::
If set to true, .git/shallow can be updated when new refs
require new shallow roots. Otherwise those refs are rejected.

+receive.preferatomicpush::
+ This option is used in receive-pack to tell the client to try
+ to use an atomic push, if the client supports it.
+
remote.pushdefault::
The remote to push to by default. Overrides
`branch.<name>.remote` for all branches, and is overridden by
diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt
index 26bc5b1..78c5469 100644
--- a/Documentation/technical/protocol-capabilities.txt
+++ b/Documentation/technical/protocol-capabilities.txt
@@ -18,9 +18,9 @@ was sent. Server MUST NOT ignore capabilities that client requested
and server advertised. As a consequence of these rules, server MUST
NOT advertise capabilities it does not understand.

-The 'atomic-push', 'report-status', 'delete-refs', 'quiet', and 'push-cert'
-capabilities are sent and recognized by the receive-pack (push to server)
-process.
+The 'atomic-push', 'report-status', 'delete-refs', 'prefer-atomic-push',
+'quiet', and 'push-cert' capabilities are sent and recognized by the
+receive-pack (push to server) process.

The 'ofs-delta' and 'side-band-64k' capabilities are sent and recognized
by both upload-pack and receive-pack protocols. The 'agent' capability
@@ -252,6 +252,13 @@ If the server sends the 'atomic-push' capability, it means it is
capable of accepting atomic pushes. If the pushing client requests this
capability, the server will update the refs in one single atomic transaction.

+prefer-atomic-push
+------------------
+
+If the receive-pack server advertises the 'prefer-atomic-push' capability,
+it means that the client should use an atomic push, if the client supports it,
+even if the user did not request it explicitly.
+
allow-tip-sha1-in-want
----------------------

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 6991d22..697f102 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -52,6 +52,7 @@ static const char *head_name;
static void *head_name_to_free;
static int sent_capabilities;
static int shallow_update;
+static int prefer_atomic_push;
static const char *alt_shallow_file;
static struct strbuf push_cert = STRBUF_INIT;
static unsigned char push_cert_sha1[20];
@@ -160,6 +161,11 @@ static int receive_pack_config(const char *var, const char *value, void *cb)
return 0;
}

+ if (strcmp(var, "receive.preferatomicpush") == 0) {
+ prefer_atomic_push = git_config_bool(var, value);
+ return 0;
+ }
+
return git_default_config(var, value, cb);
}

@@ -178,6 +184,8 @@ static void show_ref(const char *path, const unsigned char *sha1)
"atomic-push");
if (prefer_ofs_delta)
strbuf_addstr(&cap, " ofs-delta");
+ if (prefer_atomic_push)
+ strbuf_addstr(&cap, " prefer-atomic-push");
if (push_cert_nonce)
strbuf_addf(&cap, " push-cert=%s", push_cert_nonce);
strbuf_addf(&cap, " agent=%s", git_user_agent_sanitized());
diff --git a/send-pack.c b/send-pack.c
index 5208305..c5e0539 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -325,6 +325,8 @@ int send_pack(struct send_pack_args *args,
args->use_thin_pack = 0;
if (server_supports("atomic-push"))
atomic_push_supported = 1;
+ if (server_supports("prefer-atomic-push"))
+ args->use_atomic_push = 1;
if (args->push_cert) {
int len;

diff --git a/t/t5543-atomic-push.sh b/t/t5543-atomic-push.sh
index 4903227..3feb22e 100755
--- a/t/t5543-atomic-push.sh
+++ b/t/t5543-atomic-push.sh
@@ -98,4 +98,36 @@ test_expect_success 'atomic push fails if one branch fails' '
test "$master_second" != "$mirror_second"
'

+test_expect_success 'atomic push fails (receive.preferatomicpush)' '
+ mk_repo_pair &&
+ (
+ cd master &&
+ echo one >foo && git add foo && git commit -m one &&
+ git branch second &&
+ git checkout second &&
+ echo two >foo && git add foo && git commit -m two &&
+ echo three >foo && git add foo && git commit -m three &&
+ echo four >foo && git add foo && git commit -m four &&
+ git push --mirror up
+ git reset --hard HEAD~2 &&
+ git checkout master
+ echo five >foo && git add foo && git commit -m five
+ ) &&
+ (
+ cd mirror &&
+ git config --add receive.preferatomicpush true
+ ) &&
+ (
+ cd master &&
+ ! git push --atomic-push --all up
+ ) &&
+ master_master=$(cd master && git show-ref -s --verify refs/heads/master) &&
+ mirror_master=$(cd mirror && git show-ref -s --verify refs/heads/master) &&
+ test "$master_master" != "$mirror_master" &&
+
+ master_second=$(cd master && git show-ref -s --verify refs/heads/second) &&
+ mirror_second=$(cd mirror && git show-ref -s --verify refs/heads/second) &&
+ test "$master_second" != "$mirror_second"
+'
+
test_done

[Ronnie Sahlberg]

This adds support to send-pack to to negotiate and use atomic pushes
iff the server supports it. Atomic pushes are activated by a new command
line flag --atomic-push.

In order to do this we also need to change the semantics for send_pack()
slightly. The existing send_pack() function actually don't sent all the
refs back to the server when multiple refs are involved, for example
when using --all. Several of the failure modes for pushes can already be
detected locally in the send_pack client based on the information from the
initial server side list of all the refs as generated by receive-pack.
Any such refs that we thus know would fail to push are thus pruned from
the list of refs we send to the server to update.

For atomic pushes, we have to deal thus with both failures that are detected
locally as well as failures that are reported back from the server. In order
to do so we treat all local failures as push failures too.

We introduce a new status code REF_STATUS_ATOMIC_PUSH_FAILED so we can
flag all refs that we would normally have tried to push to the server
but we did not due to local failures. This is to improve the error message
back to the end user to flag that "these refs failed to update since the
atomic push operation failed."

Change-Id: Ifbcdc10c032a51d317ae7a6eacc03cf32e660bbe
Signed-off-by: Ronnie Sahlberg <***@google.com>
---
Documentation/git-send-pack.txt | 7 ++++++-
builtin/send-pack.c | 6 +++++-
remote.h | 3 ++-
send-pack.c | 39 ++++++++++++++++++++++++++++++++++-----
send-pack.h | 1 +
transport.c | 4 ++++
6 files changed, 52 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt
index 2a0de42..8f64feb 100644
--- a/Documentation/git-send-pack.txt
+++ b/Documentation/git-send-pack.txt
@@ -9,7 +9,7 @@ git-send-pack - Push objects over Git protocol to another repository
SYNOPSIS
--------
[verse]
-'git send-pack' [--all] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]
+'git send-pack' [--all] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [--atomic-push] [<host>:]<directory> [<ref>...]

DESCRIPTION
-----------
@@ -62,6 +62,11 @@ be in a separate packet, and the list must end with a flush packet.
Send a "thin" pack, which records objects in deltified form based
on objects not included in the pack to reduce network traffic.

+--atomic-push::
+ With atomic-push all refs are updated in one single atomic transaction.
+ This means that if any of the refs fails then the entire push will
+ fail without changing any refs.
+
<host>::
A remote host to house the repository. When this
part is specified, 'git-receive-pack' is invoked via
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index b564a77..93cb17c 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -13,7 +13,7 @@
#include "sha1-array.h"

static const char send_pack_usage[] =
-"git send-pack [--all | --mirror] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]\n"
+"git send-pack [--all | --mirror] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [--atomic-push] [<host>:]<directory> [<ref>...]\n"
" --all and explicit <ref> specification are mutually exclusive.";

static struct send_pack_args args;
@@ -170,6 +170,10 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
args.use_thin_pack = 1;
continue;
}
+ if (!strcmp(arg, "--atomic-push")) {
+ args.use_atomic_push = 1;
+ continue;
+ }
if (!strcmp(arg, "--stateless-rpc")) {
args.stateless_rpc = 1;
continue;
diff --git a/remote.h b/remote.h
index 8b62efd..f346524 100644
--- a/remote.h
+++ b/remote.h
@@ -115,7 +115,8 @@ struct ref {
REF_STATUS_REJECT_SHALLOW,
REF_STATUS_UPTODATE,
REF_STATUS_REMOTE_REJECT,
- REF_STATUS_EXPECTING_REPORT
+ REF_STATUS_EXPECTING_REPORT,
+ REF_STATUS_ATOMIC_PUSH_FAILED
} status;
char *remote_status;
struct ref *peer_ref; /* when renaming */
diff --git a/send-pack.c b/send-pack.c
index 3520fe5..5208305 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -190,7 +190,7 @@ static void advertise_shallow_grafts_buf(struct strbuf *sb)
for_each_commit_graft(advertise_shallow_grafts_cb, sb);
}

-static int ref_update_to_be_sent(const struct ref *ref, const struct send_pack_args *args)
+static int ref_update_to_be_sent(const struct ref *ref, const struct send_pack_args *args, int *atomic_push_failed)
{
if (!ref->peer_ref && !args->send_mirror)
return 0;
@@ -203,6 +203,13 @@ static int ref_update_to_be_sent(const struct ref *ref, const struct send_pack_a
case REF_STATUS_REJECT_NEEDS_FORCE:
case REF_STATUS_REJECT_STALE:
case REF_STATUS_REJECT_NODELETE:
+ if (atomic_push_failed && args->use_atomic_push) {
+ fprintf(stderr, "Atomic push is not possible "
+ "for ref %s. Status:%d\n", ref->name,
+ ref->status);
+ *atomic_push_failed = 1;
+ }
+ /* fallthrough */
case REF_STATUS_UPTODATE:
return 0;
default:
@@ -250,7 +257,7 @@ static int generate_push_cert(struct strbuf *req_buf,
strbuf_addstr(&cert, "\n");

for (ref = remote_refs; ref; ref = ref->next) {
- if (!ref_update_to_be_sent(ref, args))
+ if (!ref_update_to_be_sent(ref, args, NULL))
continue;
update_seen = 1;
strbuf_addf(&cert, "%s %s %s\n",
@@ -297,7 +304,7 @@ int send_pack(struct send_pack_args *args,
int atomic_push_supported = 0;
int atomic_push = 0;
unsigned cmds_sent = 0;
- int ret;
+ int ret, atomic_push_failed = 0;
struct async demux;
const char *push_cert_nonce = NULL;

@@ -332,6 +339,11 @@ int send_pack(struct send_pack_args *args,
"Perhaps you should specify a branch such as 'master'.\n");
return 0;
}
+ if (args->use_atomic_push && !atomic_push_supported) {
+ fprintf(stderr, "Server does not support atomic-push.");
+ return -1;
+ }
+ atomic_push = atomic_push_supported && args->use_atomic_push;

if (status_report)
strbuf_addstr(&cap_buf, " report-status");
@@ -365,7 +377,7 @@ int send_pack(struct send_pack_args *args,
* the pack data.
*/
for (ref = remote_refs; ref; ref = ref->next) {
- if (!ref_update_to_be_sent(ref, args))
+ if (!ref_update_to_be_sent(ref, args, &atomic_push_failed))
continue;

if (!ref->deletion)
@@ -377,6 +389,23 @@ int send_pack(struct send_pack_args *args,
ref->status = REF_STATUS_EXPECTING_REPORT;
}

+ if (atomic_push_failed) {
+ for (ref = remote_refs; ref; ref = ref->next) {
+ if (!ref->peer_ref && !args->send_mirror)
+ continue;
+
+ switch (ref->status) {
+ case REF_STATUS_EXPECTING_REPORT:
+ ref->status = REF_STATUS_ATOMIC_PUSH_FAILED;
+ continue;
+ default:
+ ; /* do nothing */
+ }
+ }
+ fprintf(stderr, "Atomic push failed.");
+ return -1;
+ }
+
/*
* Finally, tell the other end!
*/
@@ -386,7 +415,7 @@ int send_pack(struct send_pack_args *args,
if (args->dry_run || args->push_cert)
continue;

- if (!ref_update_to_be_sent(ref, args))
+ if (!ref_update_to_be_sent(ref, args, NULL))
continue;

old_hex = sha1_to_hex(ref->old_sha1);
diff --git a/send-pack.h b/send-pack.h
index 5635457..7486e65 100644
--- a/send-pack.h
+++ b/send-pack.h
@@ -11,6 +11,7 @@ struct send_pack_args {
force_update:1,
use_thin_pack:1,
use_ofs_delta:1,
+ use_atomic_push:1,
dry_run:1,
push_cert:1,
stateless_rpc:1;
diff --git a/transport.c b/transport.c
index 51570d6..d3e9e27 100644
--- a/transport.c
+++ b/transport.c
@@ -731,6 +731,10 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
ref->deletion ? NULL : ref->peer_ref,
"remote failed to report status", porcelain);
break;
+ case REF_STATUS_ATOMIC_PUSH_FAILED:
+ print_ref_status('!', "[rejected]", ref, ref->peer_ref,
+ "atomic-push-failed", porcelain);
+ break;
case REF_STATUS_OK:
print_ok_ref_status(ref, porcelain);
break;

[Ronnie Sahlberg]

Change-Id: I3a6491515b78b564d1cc0892826a4bc77f9bffb0
Signed-off-by: Ronnie Sahlberg <***@google.com>
---
t/t5543-atomic-push.sh | 101 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 101 insertions(+)
create mode 100755 t/t5543-atomic-push.sh

diff --git a/t/t5543-atomic-push.sh b/t/t5543-atomic-push.sh
new file mode 100755
index 0000000..4903227
--- /dev/null
+++ b/t/t5543-atomic-push.sh
@@ -0,0 +1,101 @@
+#!/bin/sh
+
+test_description='pushing to a mirror repository'
+
+. ./test-lib.sh
+
+D=`pwd`
+
+invert () {
+ if "$@"; then
+ return 1
+ else
+ return 0
+ fi
+}
+
+mk_repo_pair () {
+ rm -rf master mirror &&
+ mkdir mirror &&
+ (
+ cd mirror &&
+ git init &&
+ git config receive.denyCurrentBranch warn
+ ) &&
+ mkdir master &&
+ (
+ cd master &&
+ git init &&
+ git remote add $1 up ../mirror
+ )
+}
+
+
+test_expect_success 'atomic push works for a single branch' '
+
+ mk_repo_pair &&
+ (
+ cd master &&
+ echo one >foo && git add foo && git commit -m one &&
+ git push --mirror up
+ echo two >foo && git add foo && git commit -m two &&
+ git push --atomic-push --mirror up
+ ) &&
+ master_master=$(cd master && git show-ref -s --verify refs/heads/master) &&
+ mirror_master=$(cd mirror && git show-ref -s --verify refs/heads/master) &&
+ test "$master_master" = "$mirror_master"
+
+'
+
+test_expect_success 'atomic push works for two branches' '
+
+ mk_repo_pair &&
+ (
+ cd master &&
+ echo one >foo && git add foo && git commit -m one &&
+ git branch second &&
+ git push --mirror up
+ echo two >foo && git add foo && git commit -m two &&
+ git checkout second &&
+ echo three >foo && git add foo && git commit -m three &&
+ git checkout master &&
+ git push --atomic-push --mirror up
+ ) &&
+ master_master=$(cd master && git show-ref -s --verify refs/heads/master) &&
+ mirror_master=$(cd mirror && git show-ref -s --verify refs/heads/master) &&
+ test "$master_master" = "$mirror_master"
+
+ master_second=$(cd master && git show-ref -s --verify refs/heads/second) &&
+ mirror_second=$(cd mirror && git show-ref -s --verify refs/heads/second) &&
+ test "$master_second" = "$mirror_second"
+'
+
+# set up two branches where master can be pushed but second can not
+# (non-fast-forward). Since second can not be pushed the whole operation
+# will fail and leave master untouched.
+test_expect_success 'atomic push fails if one branch fails' '
+ mk_repo_pair &&
+ (
+ cd master &&
+ echo one >foo && git add foo && git commit -m one &&
+ git branch second &&
+ git checkout second &&
+ echo two >foo && git add foo && git commit -m two &&
+ echo three >foo && git add foo && git commit -m three &&
+ echo four >foo && git add foo && git commit -m four &&
+ git push --mirror up
+ git reset --hard HEAD~2 &&
+ git checkout master
+ echo five >foo && git add foo && git commit -m five &&
+ ! git push --atomic-push --all up
+ ) &&
+ master_master=$(cd master && git show-ref -s --verify refs/heads/master) &&
+ mirror_master=$(cd mirror && git show-ref -s --verify refs/heads/master) &&
+ test "$master_master" != "$mirror_master" &&
+
+ master_second=$(cd master && git show-ref -s --verify refs/heads/second) &&
+ mirror_second=$(cd mirror && git show-ref -s --verify refs/heads/second) &&
+ test "$master_second" != "$mirror_second"
+'
+
+test_done

[Ronnie Sahlberg]

Change-Id: I812e7600fb648df429df8a2c84745de4f5875626
Signed-off-by: Ronnie Sahlberg <***@google.com>
---
builtin/branch.c | 7 +++++--
builtin/checkout.c | 13 ++++++++++---
builtin/clone.c | 15 +++++++++++----
builtin/init-db.c | 8 ++++++--
builtin/notes.c | 7 ++++---
builtin/remote.c | 26 ++++++++++++++++++--------
builtin/symbolic-ref.c | 6 +++++-
cache.h | 1 -
refs.c | 30 ++++++++++++++++++------------
refs.h | 1 +
10 files changed, 78 insertions(+), 36 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 04f57d4..ab6d9f4 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -698,6 +698,7 @@ static void rename_branch(const char *oldname, const char *newname, int force)
{
struct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = STRBUF_INIT;
struct strbuf oldsection = STRBUF_INIT, newsection = STRBUF_INIT;
+ struct strbuf err = STRBUF_INIT;
int recovery = 0;
int clobber_head_ok;

@@ -734,8 +735,10 @@ static void rename_branch(const char *oldname, const char *newname, int force)
warning(_("Renamed a misnamed branch '%s' away"), oldref.buf + 11);

/* no need to pass logmsg here as HEAD didn't really move */
- if (!strcmp(oldname, head) && create_symref("HEAD", newref.buf, NULL))
- die(_("Branch renamed to %s, but HEAD is not updated!"), newname);
+ if (!strcmp(oldname, head) &&
+ create_symref("HEAD", newref.buf, NULL, &err))
+ die(_("Branch renamed to %s, but HEAD is not updated!. %s"),
+ newname, err.buf);

strbuf_addf(&oldsection, "branch.%s", oldref.buf + 11);
strbuf_release(&oldref);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index d9cb9c3..1efe353 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -634,7 +634,10 @@ static void update_refs_for_switch(const struct checkout_opts *opts,
describe_detached_head(_("HEAD is now at"), new->commit);
}
} else if (new->path) { /* Switch branches. */
- create_symref("HEAD", new->path, msg.buf);
+ if (create_symref("HEAD", new->path, msg.buf, &err)) {
+ error("%s", err.buf);
+ strbuf_release(&err);
+ }
if (!opts->quiet) {
if (old->path && !strcmp(new->path, old->path)) {
if (opts->new_branch_force)
@@ -1020,12 +1023,16 @@ static int parse_branchname_arg(int argc, const char **argv,
static int switch_unborn_to_new_branch(const struct checkout_opts *opts)
{
int status;
- struct strbuf branch_ref = STRBUF_INIT;
+ struct strbuf branch_ref = STRBUF_INIT, err = STRBUF_INIT;

if (!opts->new_branch)
die(_("You are on a branch yet to be born"));
strbuf_addf(&branch_ref, "refs/heads/%s", opts->new_branch);
- status = create_symref("HEAD", branch_ref.buf, "checkout -b");
+ status = create_symref("HEAD", branch_ref.buf, "checkout -b", &err);
+ if (status) {
+ error("%s", err.buf);
+ strbuf_release(&err);
+ }
strbuf_release(&branch_ref);
if (!opts->quiet)
fprintf(stderr, _("Switched to a new branch '%s'\n"),
diff --git a/builtin/clone.c b/builtin/clone.c
index e0a671d..1c92e84 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -565,6 +565,7 @@ static void update_remote_refs(const struct ref *refs,
int check_connectivity)
{
const struct ref *rm = mapped_refs;
+ struct strbuf err = STRBUF_INIT;

if (check_connectivity) {
if (transport->progress)
@@ -586,9 +587,12 @@ static void update_remote_refs(const struct ref *refs,
struct strbuf head_ref = STRBUF_INIT;
strbuf_addstr(&head_ref, branch_top);
strbuf_addstr(&head_ref, "HEAD");
- create_symref(head_ref.buf,
- remote_head_points_at->peer_ref->name,
- msg);
+ if (create_symref(head_ref.buf,
+ remote_head_points_at->peer_ref->name,
+ msg, &err)) {
+ error("%s", err.buf);
+ strbuf_release(&err);
+ }
}
}

@@ -599,7 +603,10 @@ static void update_head(const struct ref *our, const struct ref *remote,
const char *head;
if (our && skip_prefix(our->name, "refs/heads/", &head)) {
/* Local default branch link */
- create_symref("HEAD", our->name, NULL);
+ if (create_symref("HEAD", our->name, NULL, &err)) {
+ error("%s", err.buf);
+ strbuf_release(&err);
+ }
if (!option_bare) {
update_ref(msg, "HEAD", our->old_sha1, NULL, 0, &err);
install_branch_config(0, head, option_origin, our->name);
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 587a505..d6cdee8 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -7,6 +7,7 @@
#include "builtin.h"
#include "exec_cmd.h"
#include "parse-options.h"
+#include "refs.h"

#ifndef DEFAULT_GIT_TEMPLATE_DIR
#define DEFAULT_GIT_TEMPLATE_DIR "/usr/share/git-core/templates"
@@ -187,6 +188,7 @@ static int create_default_files(const char *template_path)
char junk[2];
int reinit;
int filemode;
+ struct strbuf err = STRBUF_INIT;

if (len > sizeof(path)-50)
die(_("insane git directory %s"), git_dir);
@@ -236,8 +238,10 @@ static int create_default_files(const char *template_path)
strcpy(path + len, "HEAD");
reinit = (!access(path, R_OK)
|| readlink(path, junk, sizeof(junk)-1) != -1);
- if (!reinit) {
- if (create_symref("HEAD", "refs/heads/master", NULL) < 0)
+ if (!reinit &&
+ create_symref("HEAD", "refs/heads/master", NULL, &err)) {
+ error("%s", err.buf);
+ strbuf_release(&err);
exit(1);
}

diff --git a/builtin/notes.c b/builtin/notes.c
index b9fec39..f6d4696 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -821,9 +821,10 @@ static int merge(int argc, const char **argv, const char *prefix)
0, &err))
die("%s", err.buf);
/* Store ref-to-be-updated into .git/NOTES_MERGE_REF */
- if (create_symref("NOTES_MERGE_REF", default_notes_ref(), NULL))
- die("Failed to store link to current notes ref (%s)",
- default_notes_ref());
+ if (create_symref("NOTES_MERGE_REF", default_notes_ref(),
+ NULL, &err))
+ die("Failed to store link to current notes ref (%s). "
+ "%s", default_notes_ref(), err.buf);
printf("Automatic notes merge failed. Fix conflicts in %s and "
"commit the result with 'git notes merge --commit', or "
"abort the merge with 'git notes merge --abort'.\n",
diff --git a/builtin/remote.c b/builtin/remote.c
index 42702d7..d9632df 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -147,6 +147,7 @@ static int add(int argc, const char **argv)
const char *master = NULL;
struct remote *remote;
struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT;
+ struct strbuf err = STRBUF_INIT;
const char *name, *url;
int i;

@@ -230,8 +231,12 @@ static int add(int argc, const char **argv)
strbuf_reset(&buf2);
strbuf_addf(&buf2, "refs/remotes/%s/%s", name, master);

- if (create_symref(buf.buf, buf2.buf, "remote add"))
- return error(_("Could not setup master '%s'"), master);
+ if (create_symref(buf.buf, buf2.buf, "remote add", &err)) {
+ error(_("Could not setup master '%s'. %s"),
+ master, err.buf);
+ strbuf_release(&err);
+ return -1;
+ }
}

strbuf_release(&buf);
@@ -617,8 +622,8 @@ static int mv(int argc, const char **argv)
OPT_END()
};
struct remote *oldremote, *newremote;
- struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT, buf3 = STRBUF_INIT,
- old_remote_context = STRBUF_INIT;
+ struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT, buf3 = STRBUF_INIT;
+ struct strbuf old_remote_context = STRBUF_INIT, err = STRBUF_INIT;
struct string_list remote_branches = STRING_LIST_INIT_NODUP;
struct rename_info rename;
int i, refspec_updated = 0;
@@ -742,8 +747,8 @@ static int mv(int argc, const char **argv)
strbuf_reset(&buf3);
strbuf_addf(&buf3, "remote: renamed %s to %s",
item->string, buf.buf);
- if (create_symref(buf.buf, buf2.buf, buf3.buf))
- die(_("creating '%s' failed"), buf.buf);
+ if (create_symref(buf.buf, buf2.buf, buf3.buf, &err))
+ die(_("creating '%s' failed. %s"), buf.buf, err.buf);
}
return 0;
}
@@ -1260,6 +1265,7 @@ static int set_head(int argc, const char **argv)
{
int i, opt_a = 0, opt_d = 0, result = 0;
struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT;
+ struct strbuf err = STRBUF_INIT;
char *head_name = NULL;

struct option options[] = {
@@ -1302,8 +1308,12 @@ static int set_head(int argc, const char **argv)
/* make sure it's valid */
if (!ref_exists(buf2.buf))
result |= error(_("Not a valid ref: %s"), buf2.buf);
- else if (create_symref(buf.buf, buf2.buf, "remote set-head"))
- result |= error(_("Could not setup %s"), buf.buf);
+ else if (create_symref(buf.buf, buf2.buf, "remote set-head",
+ &err)) {
+ error(_("Could not setup %s. %s"), buf.buf, err.buf);
+ strbuf_release(&err);
+ result = -1;
+ }
if (opt_a)
printf("%s/HEAD set to %s\n", argv[0], head_name);
free(head_name);
diff --git a/builtin/symbolic-ref.c b/builtin/symbolic-ref.c
index 29fb3f1..f9ca959 100644
--- a/builtin/symbolic-ref.c
+++ b/builtin/symbolic-ref.c
@@ -35,6 +35,7 @@ int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)
{
int quiet = 0, delete = 0, shorten = 0, ret = 0;
const char *msg = NULL;
+ struct strbuf err = STRBUF_INIT;
struct option options[] = {
OPT__QUIET(&quiet,
N_("suppress error message for non-symbolic (detached) refs")),
@@ -67,7 +68,10 @@ int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)
if (!strcmp(argv[0], "HEAD") &&
!starts_with(argv[1], "refs/"))
die("Refusing to point HEAD outside of refs/");
- create_symref(argv[0], argv[1], msg);
+ if (create_symref(argv[0], argv[1], msg, &err)) {
+ error("%s", err.buf);
+ strbuf_release(&err);
+ }
break;
default:
usage_with_options(git_symbolic_ref_usage, options);
diff --git a/cache.h b/cache.h
index 61e61af..3443da7 100644
--- a/cache.h
+++ b/cache.h
@@ -1026,7 +1026,6 @@ extern int get_sha1_mb(const char *str, unsigned char *sha1);
*/
extern int refname_match(const char *abbrev_name, const char *full_name);

-extern int create_symref(const char *ref, const char *refs_heads_master, const char *logmsg);
extern int validate_headref(const char *ref);

extern int base_name_compare(const char *name1, int len1, int mode1, const char *name2, int len2, int mode2);
diff --git a/refs.c b/refs.c
index 4d30623..5400868 100644
--- a/refs.c
+++ b/refs.c
@@ -3098,20 +3098,22 @@ static int write_ref_sha1(struct ref_lock *lock,
}

int create_symref(const char *ref_target, const char *refs_heads_master,
- const char *logmsg)
+ const char *logmsg, struct strbuf *err)
{
const char *lockpath;
char ref[1000];
int fd, len, written;
char *git_HEAD = git_pathdup("%s", ref_target);
unsigned char old_sha1[20], new_sha1[20];
- struct strbuf err = STRBUF_INIT;

if (logmsg && read_ref(ref_target, old_sha1))
hashclr(old_sha1);

- if (safe_create_leading_directories(git_HEAD) < 0)
- return error("unable to create directory for %s", git_HEAD);
+ if (safe_create_leading_directories(git_HEAD) < 0) {
+ strbuf_addf(err, "unable to create directory for %s.",
+ git_HEAD);
+ return -1;
+ }

#ifndef NO_SYMLINK_HEAD
if (prefer_symlink_refs) {
@@ -3124,26 +3126,29 @@ int create_symref(const char *ref_target, const char *refs_heads_master,

len = snprintf(ref, sizeof(ref), "ref: %s\n", refs_heads_master);
if (sizeof(ref) <= len) {
- error("refname too long: %s", refs_heads_master);
+ strbuf_addf(err, "refname too long: %s", refs_heads_master);
goto error_free_return;
}
lockpath = mkpath("%s.lock", git_HEAD);
fd = open(lockpath, O_CREAT | O_EXCL | O_WRONLY, 0666);
if (fd < 0) {
- error("Unable to open %s for writing", lockpath);
+ strbuf_addf(err, "Unable to open %s for writing. %s", lockpath,
+ strerror(errno));
goto error_free_return;
}
written = write_in_full(fd, ref, len);
if (close(fd) != 0 || written != len) {
- error("Unable to write to %s", lockpath);
+ strbuf_addf(err, "Unable to write to %s. %s", lockpath,
+ strerror(errno));
goto error_unlink_return;
}
if (rename(lockpath, git_HEAD) < 0) {
- error("Unable to create %s", git_HEAD);
+ strbuf_addf(err, "Unable to create %s. %s", git_HEAD,
+ strerror(errno));
goto error_unlink_return;
}
if (adjust_shared_perm(git_HEAD)) {
- error("Unable to fix permissions on %s", lockpath);
+ strbuf_addf(err, "Unable to fix permissions on %s", lockpath);
error_unlink_return:
unlink_or_warn(lockpath);
error_free_return:
@@ -3155,10 +3160,11 @@ int create_symref(const char *ref_target, const char *refs_heads_master,
done:
#endif
if (logmsg && !read_ref(refs_heads_master, new_sha1) &&
- log_ref_write(ref_target, old_sha1, new_sha1, logmsg, &err))
- error("%s", err.buf);
+ log_ref_write(ref_target, old_sha1, new_sha1, logmsg, err)) {
+ error("%s", err->buf);
+ strbuf_release(err);
+ }

- strbuf_release(&err);
free(git_HEAD);
return 0;
}
diff --git a/refs.h b/refs.h
index 1851fff..8148ab6 100644
--- a/refs.h
+++ b/refs.h
@@ -116,6 +116,7 @@ static inline const char *has_glob_specials(const char *pattern)
/* can be used to learn about broken ref and symref */
extern int for_each_rawref(each_ref_fn, void *);

+extern int create_symref(const char *ref, const char *refs_heads_master, const char *logmsg, struct strbuf *err);
extern void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname);
extern void warn_dangling_symrefs(FILE *fp, const char *msg_fmt, const struct string_list *refnames);

[Ronnie Sahlberg]

Add a command line argument to the git push command to request atomic
pushes.

Change-Id: I9f8d06970b2fdd1cf7d933e0cce1288752034af1
Signed-off-by: Ronnie Sahlberg <***@google.com>
---
Documentation/git-push.txt | 7 ++++++-
builtin/push.c | 2 ++
transport.c | 1 +
transport.h | 1 +
4 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 21b3f29..04de8d8 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -9,7 +9,7 @@ git-push - Update remote refs along with associated objects
SYNOPSIS
--------
[verse]
-'git push' [--all | --mirror | --tags] [--follow-tags] [-n | --dry-run] [--receive-pack=<git-receive-pack>]
+'git push' [--all | --mirror | --tags] [--follow-tags] [--atomic-push] [-n | --dry-run] [--receive-pack=<git-receive-pack>]
[--repo=<repository>] [-f | --force] [--prune] [-v | --verbose]
[-u | --set-upstream] [--signed]
[--force-with-lease[=<refname>[:<expect>]]]
@@ -136,6 +136,11 @@ already exists on the remote side.
logged. See linkgit:git-receive-pack[1] for the details
on the receiving end.

+--atomic-push::
+ Try using atomic push. If atomic push is negotiated with the server
+ then any push covering multiple refs will be atomic. Either all
+ refs are updated, or on error, no refs are updated.
+
--receive-pack=<git-receive-pack>::
--exec=<git-receive-pack>::
Path to the 'git-receive-pack' program on the remote
diff --git a/builtin/push.c b/builtin/push.c
index ae56f73..0b9f21a 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -507,6 +507,8 @@ int cmd_push(int argc, const char **argv, const char *prefix)
OPT_BIT(0, "follow-tags", &flags, N_("push missing but relevant tags"),
TRANSPORT_PUSH_FOLLOW_TAGS),
OPT_BIT(0, "signed", &flags, N_("GPG sign the push"), TRANSPORT_PUSH_CERT),
+ OPT_BIT(0, "atomic-push", &flags, N_("use atomic push, if available"),
+ TRANSPORT_ATOMIC_PUSH),
OPT_END()
};

diff --git a/transport.c b/transport.c
index d3e9e27..e2a16b2 100644
--- a/transport.c
+++ b/transport.c
@@ -832,6 +832,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
args.dry_run = !!(flags & TRANSPORT_PUSH_DRY_RUN);
args.porcelain = !!(flags & TRANSPORT_PUSH_PORCELAIN);
args.push_cert = !!(flags & TRANSPORT_PUSH_CERT);
+ args.use_atomic_push = !!(flags & TRANSPORT_ATOMIC_PUSH);
args.url = transport->url;

ret = send_pack(&args, data->fd, data->conn, remote_refs,
diff --git a/transport.h b/transport.h
index 3e0091e..25fa1da 100644
--- a/transport.h
+++ b/transport.h
@@ -125,6 +125,7 @@ struct transport {
#define TRANSPORT_PUSH_NO_HOOK 512
#define TRANSPORT_PUSH_FOLLOW_TAGS 1024
#define TRANSPORT_PUSH_CERT 2048
+#define TRANSPORT_ATOMIC_PUSH 4096

#define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)
#define TRANSPORT_SUMMARY(x) (int)(TRANSPORT_SUMMARY_WIDTH + strlen(x) - gettext_width(x)), (x)

[Ronnie Sahlberg]

Add err argument to create_reflog that can explain the reason for a
failure. This then eliminates the need to manage errno through this
function since we can just add strerror(errno) to the err string when
meaningful. No callers relied on errno from this function for anything
else than the error message.

log_ref_write is a private function that calls create_reflog. Update
this function to also take an err argument and pass it back to the caller.
This again eliminates the need to manage errno in this function.

Update the private function write_sha1_update_reflog to also take an
err argument.

Change-Id: I8f796a8c0c5f5d3f26e3e59fbc6421c894a4e814
Signed-off-by: Ronnie Sahlberg <***@google.com>
---
builtin/checkout.c | 8 +++---
refs.c | 71 +++++++++++++++++++++++++++---------------------------
refs.h | 4 +--
3 files changed, 42 insertions(+), 41 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 60a68f7..d9cb9c3 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -590,10 +590,12 @@ static void update_refs_for_switch(const struct checkout_opts *opts,
if (opts->new_orphan_branch) {
if (opts->new_branch_log && !log_all_ref_updates) {
char *ref_name = mkpath("refs/heads/%s", opts->new_orphan_branch);
+ struct strbuf err = STRBUF_INIT;

- if (create_reflog(ref_name)) {
- fprintf(stderr, _("Can not do reflog for '%s'\n"),
- opts->new_orphan_branch);
+ if (create_reflog(ref_name, &err)) {
+ fprintf(stderr, _("Can not do reflog for '%s'. %s\n"),
+ opts->new_orphan_branch, err.buf);
+ strbuf_release(&err);
return;
}
}
diff --git a/refs.c b/refs.c
index a5e1eff..4d30623 100644
--- a/refs.c
+++ b/refs.c
@@ -2889,8 +2889,7 @@ static int copy_msg(char *buf, const char *msg)
return cp - buf;
}

-/* This function must set a meaningful errno on failure */
-int create_reflog(const char *refname)
+int create_reflog(const char *refname, struct strbuf *err)
{
int logfd, oflags = O_APPEND | O_WRONLY;
char logfile[PATH_MAX];
@@ -2901,9 +2900,8 @@ int create_reflog(const char *refname)
starts_with(refname, "refs/notes/") ||
!strcmp(refname, "HEAD")) {
if (safe_create_leading_directories(logfile) < 0) {
- int save_errno = errno;
- error("unable to create directory for %s", logfile);
- errno = save_errno;
+ strbuf_addf(err, "unable to create directory for %s. "
+ "%s", logfile, strerror(errno));
return -1;
}
oflags |= O_CREAT;
@@ -2916,20 +2914,16 @@ int create_reflog(const char *refname)

if ((oflags & O_CREAT) && errno == EISDIR) {
if (remove_empty_directories(logfile)) {
- int save_errno = errno;
- error("There are still logs under '%s'",
- logfile);
- errno = save_errno;
+ strbuf_addf(err, "There are still logs under "
+ "'%s'", logfile);
return -1;
}
logfd = open(logfile, oflags, 0666);
}

if (logfd < 0) {
- int save_errno = errno;
- error("Unable to append to %s: %s", logfile,
- strerror(errno));
- errno = save_errno;
+ strbuf_addf(err, "Unable to append to %s: %s",
+ logfile, strerror(errno));
return -1;
}
}
@@ -2966,7 +2960,8 @@ static int log_ref_write_fd(int fd, const unsigned char *old_sha1,
}

static int log_ref_write(const char *refname, const unsigned char *old_sha1,
- const unsigned char *new_sha1, const char *msg)
+ const unsigned char *new_sha1, const char *msg,
+ struct strbuf *err)
{
int logfd, result = 0, oflags = O_APPEND | O_WRONLY;
char log_file[PATH_MAX];
@@ -2975,7 +2970,7 @@ static int log_ref_write(const char *refname, const unsigned char *old_sha1,
log_all_ref_updates = !is_bare_repository();

if (log_all_ref_updates && !reflog_exists(refname))
- result = create_reflog(refname);
+ result = create_reflog(refname, err);

if (result)
return result;
@@ -2988,16 +2983,14 @@ static int log_ref_write(const char *refname, const unsigned char *old_sha1,
result = log_ref_write_fd(logfd, old_sha1, new_sha1,
git_committer_info(0), msg);
if (result) {
- int save_errno = errno;
close(logfd);
- error("Unable to append to %s", log_file);
- errno = save_errno;
+ strbuf_addf(err, "Unable to append to %s. %s", log_file,
+ strerror(errno));
return -1;
}
if (close(logfd)) {
- int save_errno = errno;
- error("Unable to append to %s", log_file);
- errno = save_errno;
+ strbuf_addf(err, "Unable to append to %s. %s", log_file,
+ strerror(errno));
return -1;
}
return 0;
@@ -3009,12 +3002,12 @@ int is_branch(const char *refname)
}

static int write_sha1_update_reflog(struct ref_lock *lock,
- const unsigned char *sha1, const char *logmsg)
+ const unsigned char *sha1,
+ const char *logmsg, struct strbuf *err)
{
- if (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg) < 0 ||
+ if (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg, err) < 0 ||
(strcmp(lock->ref_name, lock->orig_ref_name) &&
- log_ref_write(lock->orig_ref_name, lock->old_sha1, sha1, logmsg) < 0)) {
- unlock_ref(lock);
+ log_ref_write(lock->orig_ref_name, lock->old_sha1, sha1, logmsg, err) < 0)) {
return -1;
}
if (strcmp(lock->orig_ref_name, "HEAD") != 0) {
@@ -3036,8 +3029,11 @@ static int write_sha1_update_reflog(struct ref_lock *lock,
head_ref = resolve_ref_unsafe("HEAD", RESOLVE_REF_READING,
head_sha1, &head_flag);
if (head_ref && (head_flag & REF_ISSYMREF) &&
- !strcmp(head_ref, lock->ref_name))
- log_ref_write("HEAD", lock->old_sha1, sha1, logmsg);
+ !strcmp(head_ref, lock->ref_name) &&
+ log_ref_write("HEAD", lock->old_sha1, sha1, logmsg, err)) {
+ error("%s", err->buf);
+ strbuf_release(err);
+ }
}
return 0;
}
@@ -3051,6 +3047,7 @@ static int write_ref_sha1(struct ref_lock *lock,
{
static char term = '\n';
struct object *o;
+ struct strbuf err = STRBUF_INIT;

if (!lock) {
errno = EINVAL;
@@ -3085,8 +3082,10 @@ static int write_ref_sha1(struct ref_lock *lock,
return -1;
}
clear_loose_ref_cache(&ref_cache);
- if (write_sha1_update_reflog(lock, sha1, logmsg)) {
+ if (write_sha1_update_reflog(lock, sha1, logmsg, &err)) {
unlock_ref(lock);
+ error("%s", err.buf);
+ strbuf_release(&err);
return -1;
}
if (commit_ref(lock)) {
@@ -3106,6 +3105,7 @@ int create_symref(const char *ref_target, const char *refs_heads_master,
int fd, len, written;
char *git_HEAD = git_pathdup("%s", ref_target);
unsigned char old_sha1[20], new_sha1[20];
+ struct strbuf err = STRBUF_INIT;

if (logmsg && read_ref(ref_target, old_sha1))
hashclr(old_sha1);
@@ -3154,9 +3154,11 @@ int create_symref(const char *ref_target, const char *refs_heads_master,
#ifndef NO_SYMLINK_HEAD
done:
#endif
- if (logmsg && !read_ref(refs_heads_master, new_sha1))
- log_ref_write(ref_target, old_sha1, new_sha1, logmsg);
-
+ if (logmsg && !read_ref(refs_heads_master, new_sha1) &&
+ log_ref_write(ref_target, old_sha1, new_sha1, logmsg, &err))
+ error("%s", err.buf);
+
+ strbuf_release(&err);
free(git_HEAD);
return 0;
}
@@ -3904,10 +3906,7 @@ int transaction_commit(struct transaction *transaction,
goto cleanup;
}
if (write_sha1_update_reflog(update->lock, update->new_sha1,
- update->msg)) {
- if (err)
- strbuf_addf(err, "Failed to update log '%s'.",
- update->refname);
+ update->msg, err)) {
ret = -1;
goto cleanup;
}
@@ -3944,7 +3943,7 @@ int transaction_commit(struct transaction *transaction,
continue;
}
if (log_all_ref_updates && !reflog_exists(update->refname) &&
- create_reflog(update->refname)) {
+ create_reflog(update->refname, err)) {
ret = -1;
if (err)
strbuf_addf(err, "Failed to setup reflog for "
diff --git a/refs.h b/refs.h
index be16c08..1851fff 100644
--- a/refs.h
+++ b/refs.h
@@ -170,8 +170,8 @@ extern int read_ref_at(const char *refname, unsigned int flags,
/** Check if a particular reflog exists */
extern int reflog_exists(const char *refname);

-/** Create reflog. Set errno to something meaningful on failure. */
-extern int create_reflog(const char *refname);
+/** Create reflog. Fill in err on failure. */
+extern int create_reflog(const char *refname, struct strbuf *err);

/** Delete a reflog */
extern int delete_reflog(const char *refname);

[Ronnie Sahlberg]

Update receive-pack to use an atomic transaction iff the client negotiated
that it wanted atomic-push.
This leaves the default behavior to be the old non-atomic one ref at a
time update. This is to cause as little disruption as possible to existing
clients. It is unknown if there are client scripts that depend on the old
non-atomic behavior so we make it opt-in for now.

Later patch in this series also adds a configuration variable where you can
override the atomic push behavior on the receiving repo and force it
to use atomic updates always.

If it turns out over time that there are no client scripts that depend on the
old behavior we can change git to default to use atomic pushes and instead
offer an opt-out argument for people that do not want atomic pushes.

Change-Id: Ice9739aa2676f76d2e7fab2d54f37047b2eb277e
Signed-off-by: Ronnie Sahlberg <***@google.com>
---
builtin/receive-pack.c | 73 +++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 58 insertions(+), 15 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index b8ffd9e..6991d22 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -67,6 +67,8 @@ static const char *NONCE_SLOP = "SLOP";
static const char *nonce_status;
static long nonce_stamp_slop;
static unsigned long nonce_stamp_slop_limit;
+struct strbuf err = STRBUF_INIT;
+struct transaction *transaction;

static enum deny_action parse_deny_action(const char *var, const char *value)
{
@@ -827,33 +829,55 @@ static const char *update(struct command *cmd, struct shallow_info *si)
cmd->did_not_exist = 1;
}
}
- if (delete_ref(namespaced_name, old_sha1, 0)) {
- rp_error("failed to delete %s", name);
- return "failed to delete";
+ if (!use_atomic_push) {
+ if (delete_ref(namespaced_name, old_sha1, 0)) {
+ rp_error("failed to delete %s", name);
+ return "failed to delete";
+ }
+ } else {
+ if (transaction_delete_ref(transaction,
+ namespaced_name,
+ old_sha1,
+ 0, old_sha1 != NULL,
+ "push", &err)) {
+ rp_error("%s", err.buf);
+ strbuf_release(&err);
+ return "failed to delete";
+ }
}
return NULL; /* good */
}
else {
- struct strbuf err = STRBUF_INIT;
- struct transaction *transaction;
-
if (shallow_update && si->shallow_ref[cmd->index] &&
update_shallow_ref(cmd, si))
return "shallow error";

- transaction = transaction_begin(&err);
- if (!transaction ||
- transaction_update_ref(transaction, namespaced_name,
- new_sha1, old_sha1, 0, 1, "push",
- &err) ||
- transaction_commit(transaction, &err)) {
- transaction_free(transaction);
+ if (!use_atomic_push) {
+ transaction = transaction_begin(&err);
+ if (!transaction) {
+ rp_error("%s", err.buf);
+ strbuf_release(&err);
+ return "failed to start transaction";
+ }
+ }
+ if (transaction_update_ref(transaction,
+ namespaced_name,
+ new_sha1, old_sha1,
+ 0, 1, "push",
+ &err)) {
rp_error("%s", err.buf);
strbuf_release(&err);
return "failed to update ref";
}
-
- transaction_free(transaction);
+ if (!use_atomic_push) {
+ if (transaction_commit(transaction, &err)) {
+ transaction_free(transaction);
+ rp_error("%s", err.buf);
+ strbuf_release(&err);
+ return "failed to update ref";
+ }
+ transaction_free(transaction);
+ }
strbuf_release(&err);
return NULL; /* good */
}
@@ -1053,6 +1077,16 @@ static void execute_commands(struct command *commands,
return;
}

+ if (use_atomic_push) {
+ transaction = transaction_begin(&err);
+ if (!transaction) {
+ error("%s", err.buf);
+ strbuf_release(&err);
+ for (cmd = commands; cmd; cmd = cmd->next)
+ cmd->error_string = "transaction error";
+ return;
+ }
+ }
data.cmds = commands;
data.si = si;
if (check_everything_connected(iterate_receive_command_list, 0, &data))
@@ -1090,6 +1124,14 @@ static void execute_commands(struct command *commands,
}
}

+ if (use_atomic_push) {
+ if (transaction_commit(transaction, &err)) {
+ rp_error("%s", err.buf);
+ for (cmd = commands; cmd; cmd = cmd->next)
+ cmd->error_string = err.buf;
+ }
+ transaction_free(transaction);
+ }
if (shallow_update && !checked_connectivity)
error("BUG: run 'git fsck' for safety.\n"
"If there are errors, try to remove "
@@ -1537,5 +1579,6 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
sha1_array_clear(&shallow);
sha1_array_clear(&ref);
free((void *)push_cert_nonce);
+ strbuf_release(&err);
return 0;
}