{"thread":{"id":"22876","subject":"[PATCH v2 0/7] http: minor improvements","startedAt":"2010-03-02T10:49:24Z","lastAt":"2010-03-04T20:38:11Z","messageCount":9,"participants":["Tay Ray Chuan","Clemens Buchacher"],"isPatch":true,"patchVersion":2,"patchTotal":7},"messages":[{"id":"136004","messageId":"1267526971-5552-1-git-send-email-rctay89@gmail.com","threadId":"22876","inReplyTo":null,"subject":"[PATCH v2 0/7] http: minor improvements","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-03-02T10:49:24Z","receivedAt":"2010-03-02T10:49:24Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"This patch series was generated on top of 'next'. It contains some\ngeneral changes to http usage.\n\nSome highlights:\n  - add non-ff test cases to \"dumb\" http push test\n  - separate init and cleanup of http from http-walker\n\nChanges from v1 (based on comments from Clemens):\n  - patch 3: don't say \"remove condition\", since we're actually enabling\n    it.\n  - patch 5: in keeping with separating http and http-walker, removed\n    http_cleanup() from http-walker->cleanup().\n  - patch 5: move http_cleanup() call after http-walker->cleanup(). This\n    isn't really needed, but do this just in case http-walker->cleanup()\n    does any http operations.\n\nContents:\n\n[PATCH v2 1/7] t5541-http-push: check that ref is unchanged for non-ff test\n[PATCH v2 2/7] t554[01]-http-push: refactor, add non-ff tests\n[PATCH v2 3/7] http-push: remove \"|| 1\" to enable verbose check\n[PATCH v2 4/7] http-walker: cleanup more thoroughly\n[PATCH v2 5/7] http: init and cleanup separately from http-walker\n[PATCH v2 6/7] remote-curl: use http_fetch_ref() instead of walker wrapper\n[PATCH v2 7/7] remote-curl: init walker only when needed\n\n http-fetch.c         |    5 ++++-\n http-push.c          |    2 +-\n http-walker.c        |   21 +++++++++++++++++----\n remote-curl.c        |   21 +++++++++------------\n t/lib-httpd.sh       |   29 +++++++++++++++++++++++++++++\n t/t5540-http-push.sh |    3 +++\n t/t5541-http-push.sh |   22 ++--------------------\n walker.h             |    2 +-\n 8 files changed, 66 insertions(+), 39 deletions(-)\n\n--\nCheers,\nRay Chuan\n"},{"id":"136008","messageId":"1267526971-5552-2-git-send-email-rctay89@gmail.com","threadId":"22876","inReplyTo":"1267526971-5552-1-git-send-email-rctay89@gmail.com","subject":"[PATCH v2 1/7] t5541-http-push: check that ref is unchanged for non-ff test","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-03-02T10:49:25Z","receivedAt":"2010-03-02T10:49:25Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n t/t5541-http-push.sh |    6 ++++--\n 1 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh\nindex 53f54a2..ff947f3 100755\n--- a/t/t5541-http-push.sh\n+++ b/t/t5541-http-push.sh\n@@ -89,15 +89,17 @@ test_expect_success 'used receive-pack service' '\n '\n \n test_expect_success 'non-fast-forward push fails' '\n+\tcd \"$HTTPD_DOCUMENT_ROOT_PATH\"/test_repo.git &&\n+\tHEAD=$(git rev-parse --verify HEAD) &&\n+\n \tcd \"$ROOT_PATH\"/test_repo_clone &&\n \tgit checkout master &&\n \techo \"changed\" > path2 &&\n \tgit commit -a -m path2 --amend &&\n \n-\tHEAD=$(git rev-parse --verify HEAD) &&\n \t!(git push -v origin >output 2>&1) &&\n \t(cd \"$HTTPD_DOCUMENT_ROOT_PATH\"/test_repo.git &&\n-\t test $HEAD != $(git rev-parse --verify HEAD))\n+\t test $HEAD = $(git rev-parse --verify HEAD))\n '\n \n test_expect_success 'non-fast-forward push show ref status' '\n-- \n1.7.0.26.gbfa16\n"},{"id":"136007","messageId":"1267526971-5552-3-git-send-email-rctay89@gmail.com","threadId":"22876","inReplyTo":"1267526971-5552-1-git-send-email-rctay89@gmail.com","subject":"[PATCH v2 2/7] t554[01]-http-push: refactor, add non-ff tests","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-03-02T10:49:26Z","receivedAt":"2010-03-02T10:49:26Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Move non-fast forward tests to lib-httpd.sh so that we don't have to\nduplicate the tests in both t5540 and t5541.\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n t/lib-httpd.sh       |   29 +++++++++++++++++++++++++++++\n t/t5540-http-push.sh |    3 +++\n t/t5541-http-push.sh |   24 ++----------------------\n 3 files changed, 34 insertions(+), 22 deletions(-)\n\ndiff --git a/t/lib-httpd.sh b/t/lib-httpd.sh\nindex 28aff88..da4b8d5 100644\n--- a/t/lib-httpd.sh\n+++ b/t/lib-httpd.sh\n@@ -131,3 +131,32 @@ stop_httpd() {\n \t\"$LIB_HTTPD_PATH\" -d \"$HTTPD_ROOT_PATH\" \\\n \t\t-f \"$TEST_PATH/apache.conf\" $HTTPD_PARA -k stop\n }\n+\n+test_http_push_nonff() {\n+\tREMOTE_REPO=$1\n+\tLOCAL_REPO=$2\n+\tBRANCH=$3\n+\n+\ttest_expect_success 'non-fast-forward push fails' '\n+\t\tcd \"$REMOTE_REPO\" &&\n+\t\tHEAD=$(git rev-parse --verify HEAD) &&\n+\n+\t\tcd \"$LOCAL_REPO\" &&\n+\t\tgit checkout $BRANCH &&\n+\t\techo \"changed\" > path2 &&\n+\t\tgit commit -a -m path2 --amend &&\n+\n+\t\t!(git push -v origin >output 2>&1) &&\n+\t\t(cd \"$REMOTE_REPO\" &&\n+\t\t test $HEAD = $(git rev-parse --verify HEAD))\n+\t'\n+\n+\ttest_expect_success 'non-fast-forward push show ref status' '\n+\t\tgrep \"^ ! \\[rejected\\][ ]*$BRANCH -> $BRANCH (non-fast-forward)$\" output\n+\t'\n+\n+\ttest_expect_success 'non-fast-forward push shows help message' '\n+\t\tgrep \"To prevent you from losing history, non-fast-forward updates were rejected\" \\\n+\t\t\toutput\n+\t'\n+}\ndiff --git a/t/t5540-http-push.sh b/t/t5540-http-push.sh\nindex bb18f8b..37fe875 100755\n--- a/t/t5540-http-push.sh\n+++ b/t/t5540-http-push.sh\n@@ -137,6 +137,9 @@ test_expect_success 'PUT and MOVE sends object to URLs with SHA-1 hash suffix' '\n \n '\n \n+test_http_push_nonff \"$HTTPD_DOCUMENT_ROOT_PATH\"/test_repo.git \\\n+\t\"$ROOT_PATH\"/test_repo_clone master\n+\n stop_httpd\n \n test_done\ndiff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh\nindex ff947f3..795dc2b 100755\n--- a/t/t5541-http-push.sh\n+++ b/t/t5541-http-push.sh\n@@ -88,28 +88,8 @@ test_expect_success 'used receive-pack service' '\n \ttest_cmp exp act\n '\n \n-test_expect_success 'non-fast-forward push fails' '\n-\tcd \"$HTTPD_DOCUMENT_ROOT_PATH\"/test_repo.git &&\n-\tHEAD=$(git rev-parse --verify HEAD) &&\n-\n-\tcd \"$ROOT_PATH\"/test_repo_clone &&\n-\tgit checkout master &&\n-\techo \"changed\" > path2 &&\n-\tgit commit -a -m path2 --amend &&\n-\n-\t!(git push -v origin >output 2>&1) &&\n-\t(cd \"$HTTPD_DOCUMENT_ROOT_PATH\"/test_repo.git &&\n-\t test $HEAD = $(git rev-parse --verify HEAD))\n-'\n-\n-test_expect_success 'non-fast-forward push show ref status' '\n-\tgrep \"^ ! \\[rejected\\][ ]*master -> master (non-fast-forward)$\" output\n-'\n-\n-test_expect_success 'non-fast-forward push shows help message' '\n-\tgrep \"To prevent you from losing history, non-fast-forward updates were rejected\" \\\n-\t\toutput\n-'\n+test_http_push_nonff \"$HTTPD_DOCUMENT_ROOT_PATH\"/test_repo.git \\\n+\t\"$ROOT_PATH\"/test_repo_clone master\n \n test_expect_success 'push fails for non-fast-forward refs unmatched by remote helper' '\n \t# create a dissimilarly-named remote ref so that git is unable to match the\n-- \n1.7.0.26.gbfa16\n"},{"id":"136010","messageId":"1267526971-5552-4-git-send-email-rctay89@gmail.com","threadId":"22876","inReplyTo":"1267526971-5552-1-git-send-email-rctay89@gmail.com","subject":"[PATCH v2 3/7] http-push: remove \"|| 1\" to enable verbose check","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-03-02T10:49:27Z","receivedAt":"2010-03-02T10:49:27Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n http-push.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex 432b20f..415b1ab 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -1965,7 +1965,7 @@ int main(int argc, char **argv)\n \t\t}\n \n \t\tif (!hashcmp(ref->old_sha1, ref->peer_ref->new_sha1)) {\n-\t\t\tif (push_verbosely || 1)\n+\t\t\tif (push_verbosely)\n \t\t\t\tfprintf(stderr, \"'%s': up-to-date\\n\", ref->name);\n \t\t\tif (helper_status)\n \t\t\t\tprintf(\"ok %s up to date\\n\", ref->name);\n-- \n1.7.0.26.gbfa16\n"},{"id":"136005","messageId":"1267526971-5552-5-git-send-email-rctay89@gmail.com","threadId":"22876","inReplyTo":"1267526971-5552-1-git-send-email-rctay89@gmail.com","subject":"[PATCH v2 4/7] http-walker: cleanup more thoroughly","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-03-02T10:49:28Z","receivedAt":"2010-03-02T10:49:28Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n http-walker.c |   17 +++++++++++++++++\n 1 files changed, 17 insertions(+), 0 deletions(-)\n\ndiff --git a/http-walker.c b/http-walker.c\nindex 700bc13..508e355 100644\n--- a/http-walker.c\n+++ b/http-walker.c\n@@ -543,6 +543,23 @@ static int fetch_ref(struct walker *walker, struct ref *ref)\n \n static void cleanup(struct walker *walker)\n {\n+\tstruct walker_data *data = walker->data;\n+\tstruct alt_base *alt, *alt_next;\n+\n+\tif (data) {\n+\t\talt = data->alt;\n+\t\twhile (alt) {\n+\t\t\talt_next = alt->next;\n+\n+\t\t\tfree(alt->base);\n+\t\t\tfree(alt);\n+\n+\t\t\talt = alt_next;\n+\t\t}\n+\t\tfree(data);\n+\t\twalker->data = NULL;\n+\t}\n+\n \thttp_cleanup();\n }\n \n-- \n1.7.0.26.gbfa16\n"},{"id":"136011","messageId":"1267526971-5552-6-git-send-email-rctay89@gmail.com","threadId":"22876","inReplyTo":"1267526971-5552-1-git-send-email-rctay89@gmail.com","subject":"[PATCH v2 5/7] http: init and cleanup separately from http-walker","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-03-02T10:49:29Z","receivedAt":"2010-03-02T10:49:29Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Previously, all our http operations were done with http-walker. With the\nnew remote-curl helper, we find ourselves using http methods outside of\nhttp-walker - for example, fetching info/refs.\n\nAccomodate this by separating http_init() and http_cleanup() invocations\nfrom http-walker.\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n http-fetch.c  |    5 ++++-\n http-walker.c |    6 +-----\n remote-curl.c |    7 ++++++-\n walker.h      |    2 +-\n 4 files changed, 12 insertions(+), 8 deletions(-)\n\ndiff --git a/http-fetch.c b/http-fetch.c\nindex ffd0ad7..762c750 100644\n--- a/http-fetch.c\n+++ b/http-fetch.c\n@@ -1,5 +1,6 @@\n #include \"cache.h\"\n #include \"exec_cmd.h\"\n+#include \"http.h\"\n #include \"walker.h\"\n \n static const char http_fetch_usage[] = \"git http-fetch \"\n@@ -69,7 +70,8 @@ int main(int argc, const char **argv)\n \t\turl = rewritten_url;\n \t}\n \n-\twalker = get_http_walker(url, NULL);\n+\thttp_init(NULL);\n+\twalker = get_http_walker(url);\n \twalker->get_tree = get_tree;\n \twalker->get_history = get_history;\n \twalker->get_all = get_all;\n@@ -89,6 +91,7 @@ int main(int argc, const char **argv)\n \t}\n \n \twalker_free(walker);\n+\thttp_cleanup();\n \n \tfree(rewritten_url);\n \ndiff --git a/http-walker.c b/http-walker.c\nindex 508e355..ef99ae6 100644\n--- a/http-walker.c\n+++ b/http-walker.c\n@@ -559,18 +559,14 @@ static void cleanup(struct walker *walker)\n \t\tfree(data);\n \t\twalker->data = NULL;\n \t}\n-\n-\thttp_cleanup();\n }\n \n-struct walker *get_http_walker(const char *url, struct remote *remote)\n+struct walker *get_http_walker(const char *url)\n {\n \tchar *s;\n \tstruct walker_data *data = xmalloc(sizeof(struct walker_data));\n \tstruct walker *walker = xmalloc(sizeof(struct walker));\n \n-\thttp_init(remote);\n-\n \tdata->alt = xmalloc(sizeof(*data->alt));\n \tdata->alt->base = xmalloc(strlen(url) + 1);\n \tstrcpy(data->alt->base, url);\ndiff --git a/remote-curl.c b/remote-curl.c\nindex d388120..1e13fb5 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -25,7 +25,7 @@ static struct options options;\n static void init_walker(void)\n {\n \tif (!walker)\n-\t\twalker = get_http_walker(url, remote);\n+\t\twalker = get_http_walker(url);\n }\n \n static int set_option(const char *name, const char *value)\n@@ -811,6 +811,8 @@ int main(int argc, const char **argv)\n \t\turl = remote->url[0];\n \t}\n \n+\thttp_init(remote);\n+\n \tdo {\n \t\tif (strbuf_getline(&buf, stdin, '\\n') == EOF)\n \t\t\tbreak;\n@@ -856,5 +858,8 @@ int main(int argc, const char **argv)\n \t\t}\n \t\tstrbuf_reset(&buf);\n \t} while (1);\n+\n+\thttp_cleanup();\n+\n \treturn 0;\n }\ndiff --git a/walker.h b/walker.h\nindex 8a149e1..95e5765 100644\n--- a/walker.h\n+++ b/walker.h\n@@ -34,6 +34,6 @@ int walker_fetch(struct walker *impl, int targets, char **target,\n \n void walker_free(struct walker *walker);\n \n-struct walker *get_http_walker(const char *url, struct remote *remote);\n+struct walker *get_http_walker(const char *url);\n \n #endif /* WALKER_H */\n-- \n1.7.0.26.gbfa16\n"},{"id":"136006","messageId":"1267526971-5552-7-git-send-email-rctay89@gmail.com","threadId":"22876","inReplyTo":"1267526971-5552-1-git-send-email-rctay89@gmail.com","subject":"[PATCH v2 6/7] remote-curl: use http_fetch_ref() instead of walker wrapper","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-03-02T10:49:30Z","receivedAt":"2010-03-02T10:49:30Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"The http-walker implementation of walker->fetch_ref() doesn't do\nanything special compared to http_fetch_ref() anyway.\n\nRemove init_walker() invocation before fetching the ref, since we aren't\nusing the walker wrapper and don't need a walker instance anymore.\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n remote-curl.c |    3 +--\n 1 files changed, 1 insertions(+), 2 deletions(-)\n\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 1e13fb5..5ace99e 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -250,9 +250,8 @@ static struct ref *parse_info_refs(struct discovery *heads)\n \t\ti++;\n \t}\n \n-\tinit_walker();\n \tref = alloc_ref(\"HEAD\");\n-\tif (!walker->fetch_ref(walker, ref) &&\n+\tif (!http_fetch_ref(url, ref) &&\n \t    !resolve_remote_symref(ref, refs)) {\n \t\tref->next = refs;\n \t\trefs = ref;\n-- \n1.7.0.26.gbfa16\n"},{"id":"136009","messageId":"1267526971-5552-8-git-send-email-rctay89@gmail.com","threadId":"22876","inReplyTo":"1267526971-5552-1-git-send-email-rctay89@gmail.com","subject":"[PATCH v2 7/7] remote-curl: init walker only when needed","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-03-02T10:49:31Z","receivedAt":"2010-03-02T10:49:31Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Invoke get_http_walker() only when fetching with the dumb protocol.\nAdditionally, add an invocation to walker_free() after we're done using\nthe walker.\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n remote-curl.c |   13 +++----------\n 1 files changed, 3 insertions(+), 10 deletions(-)\n\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 5ace99e..b76bfcb 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -10,7 +10,6 @@\n \n static struct remote *remote;\n static const char *url;\n-static struct walker *walker;\n \n struct options {\n \tint verbosity;\n@@ -22,12 +21,6 @@ struct options {\n };\n static struct options options;\n \n-static void init_walker(void)\n-{\n-\tif (!walker)\n-\t\twalker = get_http_walker(url);\n-}\n-\n static int set_option(const char *name, const char *value)\n {\n \tif (!strcmp(name, \"verbosity\")) {\n@@ -119,7 +112,6 @@ static struct discovery* discover_refs(const char *service)\n \t}\n \trefs_url = strbuf_detach(&buffer, NULL);\n \n-\tinit_walker();\n \thttp_ret = http_get_strbuf(refs_url, &buffer, HTTP_NO_CACHE);\n \n \t/* try again with \"plain\" url (no ? or & appended) */\n@@ -501,7 +493,6 @@ static int rpc_service(struct rpc_state *rpc, struct discovery *heads)\n \tstruct child_process client;\n \tint err = 0;\n \n-\tinit_walker();\n \tmemset(&client, 0, sizeof(client));\n \tclient.in = -1;\n \tclient.out = -1;\n@@ -553,6 +544,7 @@ static int rpc_service(struct rpc_state *rpc, struct discovery *heads)\n \n static int fetch_dumb(int nr_heads, struct ref **to_fetch)\n {\n+\tstruct walker *walker;\n \tchar **targets = xmalloc(nr_heads * sizeof(char*));\n \tint ret, i;\n \n@@ -561,13 +553,14 @@ static int fetch_dumb(int nr_heads, struct ref **to_fetch)\n \tfor (i = 0; i < nr_heads; i++)\n \t\ttargets[i] = xstrdup(sha1_to_hex(to_fetch[i]->old_sha1));\n \n-\tinit_walker();\n+\twalker = get_http_walker(url);\n \twalker->get_all = 1;\n \twalker->get_tree = 1;\n \twalker->get_history = 1;\n \twalker->get_verbosely = options.verbosity >= 3;\n \twalker->get_recover = 0;\n \tret = walker_fetch(walker, nr_heads, targets, NULL, NULL);\n+\twalker_free(walker);\n \n \tfor (i = 0; i < nr_heads; i++)\n \t\tfree(targets[i]);\n-- \n1.7.0.26.gbfa16\n"},{"id":"136165","messageId":"20100304203811.GB5626@darc.dnsalias.org","threadId":"22876","inReplyTo":"1267526971-5552-1-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH v2 0/7] http: minor improvements","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2010-03-04T20:38:11Z","receivedAt":"2010-03-04T20:38:11Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Tue, Mar 02, 2010 at 06:49:24PM +0800, Tay Ray Chuan wrote:\n\n> Changes from v1 (based on comments from Clemens):\n>   - patch 3: don't say \"remove condition\", since we're actually enabling\n>     it.\n>   - patch 5: in keeping with separating http and http-walker, removed\n>     http_cleanup() from http-walker->cleanup().\n>   - patch 5: move http_cleanup() call after http-walker->cleanup(). This\n>     isn't really needed, but do this just in case http-walker->cleanup()\n>     does any http operations.\n\nThanks.\n\n> [PATCH v2 1/7] t5541-http-push: check that ref is unchanged for non-ff test\n> [PATCH v2 2/7] t554[01]-http-push: refactor, add non-ff tests\n> [PATCH v2 3/7] http-push: remove \"|| 1\" to enable verbose check\n> [PATCH v2 4/7] http-walker: cleanup more thoroughly\n> [PATCH v2 5/7] http: init and cleanup separately from http-walker\n> [PATCH v2 6/7] remote-curl: use http_fetch_ref() instead of walker wrapper\n> [PATCH v2 7/7] remote-curl: init walker only when needed\n\nAcked-by: Clemens Buchacher <drizzd@aon.at>\n"}]}