Skip to content

Commit d508e4a

Browse files
gitstergitster
committed
Merge branch 'fc/transport-helper-sync-error-fix'
Make sure the marks are not written out when the transport helper did not finish happily, to avoid leaving a marks file that is out of sync with the reality. * fc/transport-helper-sync-error-fix: t5801 (remote-helpers): cleanup environment sets transport-helper: fix sync issue on crashes transport-helper: trivial cleanup transport-helper: propagate recvline() error pushing remote-helpers: make recvline return an error transport-helper: remove barely used xchgline()
2 parents e425521 + 3667a5b commit d508e4a

File tree

2 files changed

+67
-37
lines changed

2 files changed

+67
-37
lines changed

‎t/t5801-remote-helpers.sh‎

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -212,27 +212,42 @@ test_expect_success 'push update refs failure' '
212212
echo "update fail" >>file &&
213213
git commit -a -m "update fail" &&
214214
git rev-parse --verify testgit/origin/heads/update >expect &&
215-
GIT_REMOTE_TESTGIT_PUSH_ERROR="non-fast forward" &&
216-
export GIT_REMOTE_TESTGIT_PUSH_ERROR &&
217-
test_expect_code 1 git push origin update &&
215+
test_expect_code 1 env GIT_REMOTE_TESTGIT_FAILURE="non-fast forward" \
216+
git push origin update &&
218217
git rev-parse --verify testgit/origin/heads/update >actual &&
219218
test_cmp expect actual
220219
)
221220
'
222221

222+
clean_mark () {
223+
cut -f 2 -d ' ' "$1" |
224+
git cat-file --batch-check |
225+
grep commit |
226+
sort >$(basename "$1")
227+
}
228+
229+
cmp_marks () {
230+
test_when_finished "rm -rf git.marks testgit.marks" &&
231+
clean_mark ".git/testgit/$1/git.marks" &&
232+
clean_mark ".git/testgit/$1/testgit.marks" &&
233+
test_cmp git.marks testgit.marks
234+
}
235+
223236
test_expect_success 'proper failure checks for fetching' '
224-
(GIT_REMOTE_TESTGIT_FAILURE=1 &&
225-
export GIT_REMOTE_TESTGIT_FAILURE &&
226-
cd local &&
227-
test_must_fail git fetch 2> error &&
237+
(cd local &&
238+
test_must_fail env GIT_REMOTE_TESTGIT_FAILURE=1 git fetch 2>error &&
228239
cat error &&
229240
grep -q "Error while running fast-import" error
230241
)
231242
'
232243

233244
test_expect_success 'proper failure checks for pushing' '
234245
(cd local &&
235-
test_must_fail env GIT_REMOTE_TESTGIT_FAILURE=1 git push --all
246+
git checkout -b crash master &&
247+
echo crash >>file &&
248+
git commit -a -m crash &&
249+
test_must_fail env GIT_REMOTE_TESTGIT_FAILURE=1 git push --all &&
250+
cmp_marks origin
236251
)
237252
'
238253

‎transport-helper.c‎

Lines changed: 44 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ static int recvline_fh(FILE *helper, struct strbuf *buffer, const char *name)
5858
if (strbuf_getline(buffer, helper, '\n') == EOF) {
5959
if (debug)
6060
fprintf(stderr, "Debug: Remote helper quit.\n");
61-
exit(128);
61+
return 1;
6262
}
6363

6464
if (debug)
@@ -71,12 +71,6 @@ static int recvline(struct helper_data *helper, struct strbuf *buffer)
7171
return recvline_fh(helper->out, buffer, helper->name);
7272
}
7373

74-
static void xchgline(struct helper_data *helper, struct strbuf *buffer)
75-
{
76-
sendline(helper, buffer);
77-
recvline(helper, buffer);
78-
}
79-
8074
static void write_constant(int fd, const char *str)
8175
{
8276
if (debug)
@@ -163,7 +157,8 @@ static struct child_process *get_helper(struct transport *transport)
163157
while (1) {
164158
const char *capname;
165159
int mandatory = 0;
166-
recvline(data, &buf);
160+
if (recvline(data, &buf))
161+
exit(128);
167162

168163
if (!*buf.buf)
169164
break;
@@ -200,15 +195,9 @@ static struct child_process *get_helper(struct transport *transport)
200195
} else if (!strcmp(capname, "signed-tags")) {
201196
data->signed_tags = 1;
202197
} else if (starts_with(capname, "export-marks ")) {
203-
struct strbuf arg = STRBUF_INIT;
204-
strbuf_addstr(&arg, "--export-marks=");
205-
strbuf_addstr(&arg, capname + strlen("export-marks "));
206-
data->export_marks = strbuf_detach(&arg, NULL);
198+
data->export_marks = xstrdup(capname + strlen("export-marks "));
207199
} else if (starts_with(capname, "import-marks")) {
208-
struct strbuf arg = STRBUF_INIT;
209-
strbuf_addstr(&arg, "--import-marks=");
210-
strbuf_addstr(&arg, capname + strlen("import-marks "));
211-
data->import_marks = strbuf_detach(&arg, NULL);
200+
data->import_marks = xstrdup(capname + strlen("import-marks "));
212201
} else if (starts_with(capname, "no-private-update")) {
213202
data->no_private_update = 1;
214203
} else if (mandatory) {
@@ -307,7 +296,9 @@ static int set_helper_option(struct transport *transport,
307296
quote_c_style(value, &buf, NULL, 0);
308297
strbuf_addch(&buf, '\n');
309298

310-
xchgline(data, &buf);
299+
sendline(data, &buf);
300+
if (recvline(data, &buf))
301+
exit(128);
311302

312303
if (!strcmp(buf.buf, "ok"))
313304
ret = 0;
@@ -379,7 +370,8 @@ static int fetch_with_fetch(struct transport *transport,
379370
sendline(data, &buf);
380371

381372
while (1) {
382-
recvline(data, &buf);
373+
if (recvline(data, &buf))
374+
exit(128);
383375

384376
if (starts_with(buf.buf, "lock ")) {
385377
const char *name = buf.buf + 5;
@@ -430,6 +422,8 @@ static int get_exporter(struct transport *transport,
430422
struct helper_data *data = transport->data;
431423
struct child_process *helper = get_helper(transport);
432424
int argc = 0, i;
425+
struct strbuf tmp = STRBUF_INIT;
426+
433427
memset(fastexport, 0, sizeof(*fastexport));
434428

435429
/* we need to duplicate helper->in because we want to use it after
@@ -440,10 +434,14 @@ static int get_exporter(struct transport *transport,
440434
fastexport->argv[argc++] = "--use-done-feature";
441435
fastexport->argv[argc++] = data->signed_tags ?
442436
"--signed-tags=verbatim" : "--signed-tags=warn-strip";
443-
if (data->export_marks)
444-
fastexport->argv[argc++] = data->export_marks;
445-
if (data->import_marks)
446-
fastexport->argv[argc++] = data->import_marks;
437+
if (data->export_marks) {
438+
strbuf_addf(&tmp, "--export-marks=%s.tmp", data->export_marks);
439+
fastexport->argv[argc++] = strbuf_detach(&tmp, NULL);
440+
}
441+
if (data->import_marks) {
442+
strbuf_addf(&tmp, "--import-marks=%s", data->import_marks);
443+
fastexport->argv[argc++] = strbuf_detach(&tmp, NULL);
444+
}
447445

448446
for (i = 0; i < revlist_args->nr; i++)
449447
fastexport->argv[argc++] = revlist_args->items[i].string;
@@ -563,7 +561,9 @@ static int process_connect_service(struct transport *transport,
563561
goto exit;
564562

565563
sendline(data, &cmdbuf);
566-
recvline_fh(input, &cmdbuf, name);
564+
if (recvline_fh(input, &cmdbuf, name))
565+
exit(128);
566+
567567
if (!strcmp(cmdbuf.buf, "")) {
568568
data->no_disconnect_req = 1;
569569
if (debug)
@@ -739,16 +739,22 @@ static int push_update_ref_status(struct strbuf *buf,
739739
return !(status == REF_STATUS_OK);
740740
}
741741

742-
static void push_update_refs_status(struct helper_data *data,
742+
static int push_update_refs_status(struct helper_data *data,
743743
struct ref *remote_refs,
744744
int flags)
745745
{
746746
struct strbuf buf = STRBUF_INIT;
747747
struct ref *ref = remote_refs;
748+
int ret = 0;
749+
748750
for (;;) {
749751
char *private;
750752

751-
recvline(data, &buf);
753+
if (recvline(data, &buf)) {
754+
ret = 1;
755+
break;
756+
}
757+
752758
if (!buf.len)
753759
break;
754760

@@ -766,6 +772,7 @@ static void push_update_refs_status(struct helper_data *data,
766772
free(private);
767773
}
768774
strbuf_release(&buf);
775+
return ret;
769776
}
770777

771778
static int push_refs_with_push(struct transport *transport,
@@ -846,8 +853,7 @@ static int push_refs_with_push(struct transport *transport,
846853
sendline(data, &buf);
847854
strbuf_release(&buf);
848855

849-
push_update_refs_status(data, remote_refs, flags);
850-
return 0;
856+
return push_update_refs_status(data, remote_refs, flags);
851857
}
852858

853859
static int push_refs_with_export(struct transport *transport,
@@ -905,7 +911,15 @@ static int push_refs_with_export(struct transport *transport,
905911

906912
if (finish_command(&exporter))
907913
die("Error while running fast-export");
908-
push_update_refs_status(data, remote_refs, flags);
914+
if (push_update_refs_status(data, remote_refs, flags))
915+
return 1;
916+
917+
if (data->export_marks) {
918+
strbuf_addf(&buf, "%s.tmp", data->export_marks);
919+
rename(buf.buf, data->export_marks);
920+
strbuf_release(&buf);
921+
}
922+
909923
return 0;
910924
}
911925

@@ -974,7 +988,8 @@ static struct ref *get_refs_list(struct transport *transport, int for_push)
974988

975989
while (1) {
976990
char *eov, *eon;
977-
recvline(data, &buf);
991+
if (recvline(data, &buf))
992+
exit(128);
978993

979994
if (!*buf.buf)
980995
break;

0 commit comments

Comments
 (0)