Revert "Merge branch 'fc/transport-helper-sync-error-fix'"

This reverts commit d508e4a8e2391ae2596403b6478d01cf3d5f928f,
reversing changes made to e42552135a2a396f37053a89f44952ea907870b2.

The author of the original topic says he broke the upcoming 2.0
release with something that relates to "synchronization crash
regression" while refusing to give further specifics, so this would
unfortunately be the safest option for the upcoming release.

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

diff --git a/Documentation/RelNotes/2.0.0.txt b/Documentation/RelNotes/2.0.0.txt
index 6e628d47995c58c5d46cb3a737bb7f5a983e8ac6..97f7df04eccc1b2e156815f30ec65347eccf4c40 100644 (file)
--- a/Documentation/RelNotes/2.0.0.txt
+++ b/Documentation/RelNotes/2.0.0.txt
@@ -88,10 +88,6 @@ UI, Workflows & Features
  * "git grep" learned to behave in a way similar to native grep when
    "-h" (no header) and "-c" (count) options are given.
 
  * "git grep" learned to behave in a way similar to native grep when
    "-h" (no header) and "-c" (count) options are given.
 

- * "git push" via transport-helper interface (e.g. remote-hg) has
-   been updated to allow forced ref updates in a way similar to the
-   natively supported transports.
-

  * The "simple" mode is the default for "git push".
 
  * "git add -u" and "git add -A", when run without any pathspec, is a
  * The "simple" mode is the default for "git push".
 
  * "git add -u" and "git add -A", when run without any pathspec, is a

diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index a00a660763451d57b2a9537ec1b99fcfd0c9a831..25fd2e7f46cb559372125e08671445476cbf6022 100755 (executable)
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -212,30 +212,19 @@ test_expect_success 'push update refs failure' '
        echo "update fail" >>file &&
        git commit -a -m "update fail" &&
        git rev-parse --verify testgit/origin/heads/update >expect &&
        echo "update fail" >>file &&
        git commit -a -m "update fail" &&
        git rev-parse --verify testgit/origin/heads/update >expect &&

-       test_expect_code 1 env GIT_REMOTE_TESTGIT_FAILURE="non-fast forward" \
-               git push origin update &&
+       GIT_REMOTE_TESTGIT_PUSH_ERROR="non-fast forward" &&
+       export GIT_REMOTE_TESTGIT_PUSH_ERROR &&
+       test_expect_code 1 git push origin update &&

        git rev-parse --verify testgit/origin/heads/update >actual &&
        test_cmp expect actual
        )
 '
 
        git rev-parse --verify testgit/origin/heads/update >actual &&
        test_cmp expect actual
        )
 '
 

-clean_mark () {
-       cut -f 2 -d ' ' "$1" |
-       git cat-file --batch-check |
-       grep commit |
-       sort >$(basename "$1")
-}
-
-cmp_marks () {
-       test_when_finished "rm -rf git.marks testgit.marks" &&
-       clean_mark ".git/testgit/$1/git.marks" &&
-       clean_mark ".git/testgit/$1/testgit.marks" &&
-       test_cmp git.marks testgit.marks
-}
-

 test_expect_success 'proper failure checks for fetching' '
 test_expect_success 'proper failure checks for fetching' '

-       (cd local &&
-       test_must_fail env GIT_REMOTE_TESTGIT_FAILURE=1 git fetch 2>error &&
+       (GIT_REMOTE_TESTGIT_FAILURE=1 &&
+       export GIT_REMOTE_TESTGIT_FAILURE &&
+       cd local &&
+       test_must_fail git fetch 2> error &&

        cat error &&
        grep -q "Error while running fast-import" error
        )
        cat error &&
        grep -q "Error while running fast-import" error
        )


@@ -243,11 +232,7 @@ test_expect_success 'proper failure checks for fetching' '
 
 test_expect_success 'proper failure checks for pushing' '
        (cd local &&
 
 test_expect_success 'proper failure checks for pushing' '
        (cd local &&

-       git checkout -b crash master &&
-       echo crash >>file &&
-       git commit -a -m crash &&
-       test_must_fail env GIT_REMOTE_TESTGIT_FAILURE=1 git push --all &&
-       cmp_marks origin
+       test_must_fail env GIT_REMOTE_TESTGIT_FAILURE=1 git push --all

        )
 '
 
        )
 '
 

diff --git a/transport-helper.c b/transport-helper.c
index b468e4f88e730f88a1d231a574585ace56e7066a..86e1679c1e0e1e58ebe5297f4c5f48ca1a1f6418 100644 (file)
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -58,7 +58,7 @@ static int recvline_fh(FILE *helper, struct strbuf *buffer, const char *name)
        if (strbuf_getline(buffer, helper, '\n') == EOF) {
                if (debug)
                        fprintf(stderr, "Debug: Remote helper quit.\n");
        if (strbuf_getline(buffer, helper, '\n') == EOF) {
                if (debug)
                        fprintf(stderr, "Debug: Remote helper quit.\n");

-               return 1;
+               exit(128);

        }
 
        if (debug)
        }
 
        if (debug)


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

+static void xchgline(struct helper_data *helper, struct strbuf *buffer)
+{
+       sendline(helper, buffer);
+       recvline(helper, buffer);
+}
+

 static void write_constant(int fd, const char *str)
 {
        if (debug)
 static void write_constant(int fd, const char *str)
 {
        if (debug)


@@ -157,8 +163,7 @@ static struct child_process *get_helper(struct transport *transport)
        while (1) {
                const char *capname;
                int mandatory = 0;
        while (1) {
                const char *capname;
                int mandatory = 0;

-               if (recvline(data, &buf))
-                       exit(128);
+               recvline(data, &buf);

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


@@ -195,9 +200,15 @@ static struct child_process *get_helper(struct transport *transport)
                } else if (!strcmp(capname, "signed-tags")) {
                        data->signed_tags = 1;
                } else if (starts_with(capname, "export-marks ")) {
                } else if (!strcmp(capname, "signed-tags")) {
                        data->signed_tags = 1;
                } else if (starts_with(capname, "export-marks ")) {

-                       data->export_marks = xstrdup(capname + strlen("export-marks "));
+                       struct strbuf arg = STRBUF_INIT;
+                       strbuf_addstr(&arg, "--export-marks=");
+                       strbuf_addstr(&arg, capname + strlen("export-marks "));
+                       data->export_marks = strbuf_detach(&arg, NULL);

                } else if (starts_with(capname, "import-marks")) {
                } else if (starts_with(capname, "import-marks")) {

-                       data->import_marks = xstrdup(capname + strlen("import-marks "));
+                       struct strbuf arg = STRBUF_INIT;
+                       strbuf_addstr(&arg, "--import-marks=");
+                       strbuf_addstr(&arg, capname + strlen("import-marks "));
+                       data->import_marks = strbuf_detach(&arg, NULL);

                } else if (starts_with(capname, "no-private-update")) {
                        data->no_private_update = 1;
                } else if (mandatory) {
                } else if (starts_with(capname, "no-private-update")) {
                        data->no_private_update = 1;
                } else if (mandatory) {


@@ -296,9 +307,7 @@ static int set_helper_option(struct transport *transport,
                quote_c_style(value, &buf, NULL, 0);
        strbuf_addch(&buf, '\n');
 
                quote_c_style(value, &buf, NULL, 0);
        strbuf_addch(&buf, '\n');
 

-       sendline(data, &buf);
-       if (recvline(data, &buf))
-               exit(128);
+       xchgline(data, &buf);

 
        if (!strcmp(buf.buf, "ok"))
                ret = 0;
 
        if (!strcmp(buf.buf, "ok"))
                ret = 0;


@@ -370,8 +379,7 @@ static int fetch_with_fetch(struct transport *transport,
        sendline(data, &buf);
 
        while (1) {
        sendline(data, &buf);
 
        while (1) {

-               if (recvline(data, &buf))
-                       exit(128);
+               recvline(data, &buf);

 
                if (starts_with(buf.buf, "lock ")) {
                        const char *name = buf.buf + 5;
 
                if (starts_with(buf.buf, "lock ")) {
                        const char *name = buf.buf + 5;


@@ -422,8 +430,6 @@ static int get_exporter(struct transport *transport,
        struct helper_data *data = transport->data;
        struct child_process *helper = get_helper(transport);
        int argc = 0, i;
        struct helper_data *data = transport->data;
        struct child_process *helper = get_helper(transport);
        int argc = 0, i;

-       struct strbuf tmp = STRBUF_INIT;
-

        memset(fastexport, 0, sizeof(*fastexport));
 
        /* we need to duplicate helper->in because we want to use it after
        memset(fastexport, 0, sizeof(*fastexport));
 
        /* we need to duplicate helper->in because we want to use it after


@@ -434,14 +440,10 @@ static int get_exporter(struct transport *transport,
        fastexport->argv[argc++] = "--use-done-feature";
        fastexport->argv[argc++] = data->signed_tags ?
                "--signed-tags=verbatim" : "--signed-tags=warn-strip";
        fastexport->argv[argc++] = "--use-done-feature";
        fastexport->argv[argc++] = data->signed_tags ?
                "--signed-tags=verbatim" : "--signed-tags=warn-strip";

-       if (data->export_marks) {
-               strbuf_addf(&tmp, "--export-marks=%s.tmp", data->export_marks);
-               fastexport->argv[argc++] = strbuf_detach(&tmp, NULL);
-       }
-       if (data->import_marks) {
-               strbuf_addf(&tmp, "--import-marks=%s", data->import_marks);
-               fastexport->argv[argc++] = strbuf_detach(&tmp, NULL);
-       }
+       if (data->export_marks)
+               fastexport->argv[argc++] = data->export_marks;
+       if (data->import_marks)
+               fastexport->argv[argc++] = data->import_marks;

 
        for (i = 0; i < revlist_args->nr; i++)
                fastexport->argv[argc++] = revlist_args->items[i].string;
 
        for (i = 0; i < revlist_args->nr; i++)
                fastexport->argv[argc++] = revlist_args->items[i].string;


@@ -561,9 +563,7 @@ static int process_connect_service(struct transport *transport,
                goto exit;
 
        sendline(data, &cmdbuf);
                goto exit;
 
        sendline(data, &cmdbuf);

-       if (recvline_fh(input, &cmdbuf, name))
-               exit(128);
-
+       recvline_fh(input, &cmdbuf, name);

        if (!strcmp(cmdbuf.buf, "")) {
                data->no_disconnect_req = 1;
                if (debug)
        if (!strcmp(cmdbuf.buf, "")) {
                data->no_disconnect_req = 1;
                if (debug)


@@ -739,22 +739,16 @@ static int push_update_ref_status(struct strbuf *buf,
        return !(status == REF_STATUS_OK);
 }
 
        return !(status == REF_STATUS_OK);
 }
 

-static int push_update_refs_status(struct helper_data *data,
+static void push_update_refs_status(struct helper_data *data,

                                    struct ref *remote_refs,
                                    int flags)
 {
        struct strbuf buf = STRBUF_INIT;
        struct ref *ref = remote_refs;
                                    struct ref *remote_refs,
                                    int flags)
 {
        struct strbuf buf = STRBUF_INIT;
        struct ref *ref = remote_refs;

-       int ret = 0;
-

        for (;;) {
                char *private;
 
        for (;;) {
                char *private;
 

-               if (recvline(data, &buf)) {
-                       ret = 1;
-                       break;
-               }
-
+               recvline(data, &buf);

                if (!buf.len)
                        break;
 
                if (!buf.len)
                        break;
 


@@ -772,7 +766,6 @@ static int push_update_refs_status(struct helper_data *data,
                free(private);
        }
        strbuf_release(&buf);
                free(private);
        }
        strbuf_release(&buf);

-       return ret;

 }
 
 static int push_refs_with_push(struct transport *transport,
 }
 
 static int push_refs_with_push(struct transport *transport,


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

-       return push_update_refs_status(data, remote_refs, flags);
+       push_update_refs_status(data, remote_refs, flags);
+       return 0;

 }
 
 static int push_refs_with_export(struct transport *transport,
 }
 
 static int push_refs_with_export(struct transport *transport,


@@ -911,15 +905,7 @@ static int push_refs_with_export(struct transport *transport,
 
        if (finish_command(&exporter))
                die("Error while running fast-export");
 
        if (finish_command(&exporter))
                die("Error while running fast-export");

-       if (push_update_refs_status(data, remote_refs, flags))
-               return 1;
-
-       if (data->export_marks) {
-               strbuf_addf(&buf, "%s.tmp", data->export_marks);
-               rename(buf.buf, data->export_marks);
-               strbuf_release(&buf);
-       }
-
+       push_update_refs_status(data, remote_refs, flags);

        return 0;
 }
 
        return 0;
 }
 


@@ -988,8 +974,7 @@ static struct ref *get_refs_list(struct transport *transport, int for_push)
 
        while (1) {
                char *eov, *eon;
 
        while (1) {
                char *eov, *eon;

-               if (recvline(data, &buf))
-                       exit(128);
+               recvline(data, &buf);

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