@matheusbsbs / matheus-bagliano / commits / 6f1194246a

remote-curl: pass on atomic capability to remote side

When pushing more than one reference with the --atomic option, the server is supposed to perform a single atomic transaction to update the references, leaving them either all to succeed or all to fail. This works fine when pushing locally or over SSH, but when pushing over HTTP, we fail to pass the atomic capability to the remote side. In fact, we have not reported this capability to any remote helpers during the life of the feature. Now normally, things happen to work nevertheless, since we actually check for most types of failures, such as non-fast-forward updates, on the client side, and just abort the entire attempt. However, if the server side reports a problem, such as the inability to lock a ref, the transaction isn't atomic, because we haven't passed the appropriate capability over and the remote side has no way of knowing that we wanted atomic behavior. Fix this by passing the option from the transport code through to remote helpers, and from the HTTP remote helper down to send-pack. With this change, we can detect if the server side rejects the push and report back appropriately. Note the difference in the messages: the remote side reports "atomic transaction failed", while our own checking rejects pushes with the message "atomic push failed". Document the atomic option in the remote helper documentation, so other implementers can implement it if they like. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

brian m. carlson committed Oct 16, 2019 at 23:45 UTC 6f1194246a514ba093a11dde73aabe4c0da5526f

5 files changed +62 -3

Documentation/gitremote-helpers.txt

@@ -505,6 +505,11 @@ set by Git if the remote helper has the 'option' capability.

505 Indicate that only the objects wanted need to be fetched, not

506 their dependents.

507

508 +'option atomic' {'true'|'false'}::

509 + When pushing, request the remote server to update refs in a single atomic

510 + transaction. If successful, all refs will be updated, or none will. If the

511 + remote side does not support this capability, the push will fail.

512 +

513 SEE ALSO

514 --------

515 linkgit:git-remote[1]

remote-curl.c

+12 -1

@@ -40,7 +40,8 @@ struct options {

40 push_cert : 2,

41 deepen_relative : 1,

42 from_promisor : 1,

43 - no_dependents : 1;

43 + no_dependents : 1,

44 + atomic : 1;

45 };

46 static struct options options;

47 static struct string_list cas_options = STRING_LIST_INIT_DUP;

@@ -148,6 +149,14 @@ static int set_option(const char *name, const char *value)

149 else

150 return -1;

151 return 0;

152 + } else if (!strcmp(name, "atomic")) {

153 + if (!strcmp(value, "true"))

154 + options.atomic = 1;

155 + else if (!strcmp(value, "false"))

156 + options.atomic = 0;

157 + else

158 + return -1;

159 + return 0;

160 } else if (!strcmp(name, "push-option")) {

161 if (*value != '"')

162 string_list_append(&options.push_options, value);

@@ -1196,6 +1205,8 @@ static int push_git(struct discovery *heads, int nr_spec, char **specs)

1205 argv_array_push(&args, "--signed=yes");

1206 else if (options.push_cert == SEND_PACK_PUSH_CERT_IF_ASKED)

1207 argv_array_push(&args, "--signed=if-asked");

1208 + if (options.atomic)

1209 + argv_array_push(&args, "--atomic");

1210 if (options.verbosity == 0)

1211 argv_array_push(&args, "--quiet");

1212 else if (options.verbosity > 1)

t/t5541-http-push-smart.sh

+38 -2

@@ -184,11 +184,12 @@ test_expect_success 'push --atomic also prevents branch creation, reports collat

184 test_config -C "$d" http.receivepack true &&

185 up="$HTTPD_URL"/smart/atomic-branches.git &&

186

187 - # Tell "$up" about two branches for now

187 + # Tell "$up" about three branches for now

188 test_commit atomic1 &&

189 test_commit atomic2 &&

190 git branch collateral &&

191 - git push "$up" master collateral &&

191 + git branch other &&

192 + git push "$up" master collateral other &&

193

194 # collateral is a valid push, but should be failed by atomic push

195 git checkout collateral &&

@@ -226,6 +227,41 @@ test_expect_success 'push --atomic also prevents branch creation, reports collat

227 grep "^ ! .*rejected.* collateral -> collateral .*atomic push failed" output

228 '

229

230 +test_expect_success 'push --atomic fails on server-side errors' '

231 + # Use previously set up repository

232 + d=$HTTPD_DOCUMENT_ROOT_PATH/atomic-branches.git &&

233 + test_config -C "$d" http.receivepack true &&

234 + up="$HTTPD_URL"/smart/atomic-branches.git &&

235 +

236 + # break ref updates for other on the remote site

237 + mkdir "$d/refs/heads/other.lock" &&

238 +

239 + # add the new commit to other

240 + git branch -f other collateral &&

241 +

242 + # --atomic should cause entire push to be rejected

243 + test_must_fail git push --atomic "$up" atomic other 2>output &&

244 +

245 + # the new branch should not have been created upstream

246 + test_must_fail git -C "$d" show-ref --verify refs/heads/atomic &&

247 +

248 + # upstream should still reflect atomic2, the last thing we pushed

249 + # successfully

250 + git rev-parse atomic2 >expected &&

251 + # ...to other.

252 + git -C "$d" rev-parse refs/heads/other >actual &&

253 + test_cmp expected actual &&

254 +

255 + # the new branch should not have been created upstream

256 + test_must_fail git -C "$d" show-ref --verify refs/heads/atomic &&

257 +

258 + # the failed refs should be indicated to the user

259 + grep "^ ! .*rejected.* other -> other .*atomic transaction failed" output &&

260 +

261 + # the collateral failure refs should be indicated to the user

262 + grep "^ ! .*rejected.* atomic -> atomic .*atomic transaction failed" output

263 +'

264 +

265 test_expect_success 'push --all can push to empty repo' '

266 d=$HTTPD_DOCUMENT_ROOT_PATH/empty-all.git &&

267 git init --bare "$d" &&

transport-helper.c

@@ -840,6 +840,10 @@ static void set_common_push_options(struct transport *transport,

840 die(_("helper %s does not support --signed=if-asked"), name);

841 }

842

843 + if (flags & TRANSPORT_PUSH_ATOMIC)

844 + if (set_helper_option(transport, TRANS_OPT_ATOMIC, "true") != 0)

845 + die(_("helper %s does not support --atomic"), name);

846 +

847 if (flags & TRANSPORT_PUSH_OPTIONS) {

848 struct string_list_item *item;

849 for_each_string_list_item(item, transport->push_options)

transport.h

@@ -208,6 +208,9 @@ void transport_check_allowed(const char *type);

208 /* Filter objects for partial clone and fetch */

209 #define TRANS_OPT_LIST_OBJECTS_FILTER "filter"

210

211 +/* Request atomic (all-or-nothing) updates when pushing */

212 +#define TRANS_OPT_ATOMIC "atomic"

213 +

214 /**

215 * Returns 0 if the option was used, non-zero otherwise. Prints a

216 * message to stderr if the option is not used.