{"thread":{"id":"22748","subject":"[PATCH 3/7] http-push: remove useless condition","startedAt":"2010-02-21T03:08:21Z","lastAt":"2010-02-21T15:23:30Z","messageCount":13,"participants":["Tay Ray Chuan","Clemens Buchacher"],"isPatch":true,"patchVersion":1,"patchTotal":7},"messages":[{"id":"135211","messageId":"1266721708-1060-1-git-send-email-rctay89@gmail.com","threadId":"22748","inReplyTo":null,"subject":"[PATCH 0/7] http: minor improvements","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-02-21T03:08:21Z","receivedAt":"2010-02-21T03:08:21Z","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 users.\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\n[PATCH 1/7] t5541-http-push: check that ref is unchanged for non-ff test\n[PATCH 2/7] t554[01]-http-push: refactor, add non-ff tests\n[PATCH 3/7] http-push: remove useless condition\n[PATCH 4/7] http-walker: cleanup more thoroughly\n[PATCH 5/7] http: init and cleanup separately from http-walker\n[PATCH 6/7] remote-curl: use http_fetch_ref() instead of walker wrapper\n[PATCH 7/7] remote-curl: init walker only when needed\n\n http-fetch.c         |    4 +++-\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(+), 38 deletions(-)\n\n--\nCheers,\nRay Chuan\n"},{"id":"135216","messageId":"1266721708-1060-2-git-send-email-rctay89@gmail.com","threadId":"22748","inReplyTo":"1266721708-1060-1-git-send-email-rctay89@gmail.com","subject":"[PATCH 1/7] t5541-http-push: check that ref is unchanged for non-ff test","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-02-21T03:08:22Z","receivedAt":"2010-02-21T03:08:22Z","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":"135217","messageId":"1266721708-1060-3-git-send-email-rctay89@gmail.com","threadId":"22748","inReplyTo":"1266721708-1060-1-git-send-email-rctay89@gmail.com","subject":"[PATCH 2/7] t554[01]-http-push: refactor, add non-ff tests","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-02-21T03:08:23Z","receivedAt":"2010-02-21T03:08:23Z","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":"135210","messageId":"1266721708-1060-4-git-send-email-rctay89@gmail.com","threadId":"22748","inReplyTo":"1266721708-1060-1-git-send-email-rctay89@gmail.com","subject":"[PATCH 3/7] http-push: remove useless condition","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-02-21T03:08:24Z","receivedAt":"2010-02-21T03:08:24Z","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":"135212","messageId":"1266721708-1060-5-git-send-email-rctay89@gmail.com","threadId":"22748","inReplyTo":"1266721708-1060-1-git-send-email-rctay89@gmail.com","subject":"[PATCH 4/7] http-walker: cleanup more thoroughly","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-02-21T03:08:25Z","receivedAt":"2010-02-21T03:08: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 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":"135218","messageId":"1266721708-1060-6-git-send-email-rctay89@gmail.com","threadId":"22748","inReplyTo":"1266721708-1060-1-git-send-email-rctay89@gmail.com","subject":"[PATCH 5/7] http: init and cleanup separately from http-walker","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-02-21T03:08:26Z","receivedAt":"2010-02-21T03:08:26Z","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 |    4 +---\n remote-curl.c |    7 ++++++-\n walker.h      |    2 +-\n 4 files changed, 12 insertions(+), 6 deletions(-)\n\ndiff --git a/http-fetch.c b/http-fetch.c\nindex ffd0ad7..e05b23a 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@@ -88,6 +90,7 @@ int main(int argc, const char **argv)\n \"status code.  Suggest running 'git fsck'.\\n\");\n \t}\n \n+\thttp_cleanup();\n \twalker_free(walker);\n \n \tfree(rewritten_url);\ndiff --git a/http-walker.c b/http-walker.c\nindex 508e355..bc39451 100644\n--- a/http-walker.c\n+++ b/http-walker.c\n@@ -563,14 +563,12 @@ static void cleanup(struct walker *walker)\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":"135219","messageId":"1266721708-1060-7-git-send-email-rctay89@gmail.com","threadId":"22748","inReplyTo":"1266721708-1060-1-git-send-email-rctay89@gmail.com","subject":"[PATCH 6/7] remote-curl: use http_fetch_ref() instead of walker wrapper","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-02-21T03:08:27Z","receivedAt":"2010-02-21T03:08:27Z","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":"135213","messageId":"1266721708-1060-8-git-send-email-rctay89@gmail.com","threadId":"22748","inReplyTo":"1266721708-1060-1-git-send-email-rctay89@gmail.com","subject":"[PATCH 7/7] remote-curl: init walker only when needed","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-02-21T03:08:28Z","receivedAt":"2010-02-21T03:08:28Z","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":"135238","messageId":"20100221103820.GA5166@localhost","threadId":"22748","inReplyTo":"1266721708-1060-6-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH 5/7] http: init and cleanup separately from http-walker","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2010-02-21T10:38:20Z","receivedAt":"2010-02-21T10:38:20Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Sun, Feb 21, 2010 at 11:08:26AM +0800, Tay Ray Chuan wrote:\n> diff --git a/http-fetch.c b/http-fetch.c\n[...]\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\nYou changed the order of get_http_walker and http_init. But\n\n        add_fill_function(walker, (int (*)(void *)) fill_active_slot);\n\nalready deals with curl functionality. So even though I think it technically\ndoesn't break, I would prefer if this dependency were still expressed in the\ncode.\n\n> @@ -88,6 +90,7 @@ int main(int argc, const char **argv)\n>  \"status code.  Suggest running 'git fsck'.\\n\");\n>  \t}\n>  \n> +\thttp_cleanup();\n>  \twalker_free(walker);\n>  \n>  \tfree(rewritten_url);\n\nSame as above.\n\nOtherwise the series looks good to me.\n\nLiked-by: Clemens Buchacher <drizzd@aon.at>\n\nClemens\n"},{"id":"135237","messageId":"20100221104015.GB5166@localhost","threadId":"22748","inReplyTo":"1266721708-1060-4-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH 3/7] http-push: remove useless condition","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2010-02-21T10:40:15Z","receivedAt":"2010-02-21T10:40:15Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Sun, Feb 21, 2010 at 11:08:24AM +0800, Tay Ray Chuan wrote:\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\nJust a minor nit-pick. If you end up doing a resend, maybe you could change\nthe commit message to this one, since you're actually adding a condition\nthat was previously disabled, not removing it.\n\nClemens\n"},{"id":"135236","messageId":"20100221105707.GA22534@localhost","threadId":"22748","inReplyTo":"20100221103820.GA5166@localhost","subject":"Re: [PATCH 5/7] http: init and cleanup separately from http-walker","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2010-02-21T10:57:07Z","receivedAt":"2010-02-21T10:57:07Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Sun, Feb 21, 2010 at 11:38:20AM +0100, Clemens Buchacher wrote:\n> On Sun, Feb 21, 2010 at 11:08:26AM +0800, Tay Ray Chuan wrote:\n> > diff --git a/http-fetch.c b/http-fetch.c\n> [...]\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> \n> You changed the order of get_http_walker and http_init. But\n\nUmh, actually you didn't. Sorry about that.\n\n> \n>         add_fill_function(walker, (int (*)(void *)) fill_active_slot);\n> \n> already deals with curl functionality. So even though I think it technically\n> doesn't break, I would prefer if this dependency were still expressed in the\n> code.\n> \n> > @@ -88,6 +90,7 @@ int main(int argc, const char **argv)\n> >  \"status code.  Suggest running 'git fsck'.\\n\");\n> >  \t}\n> >  \n> > +\thttp_cleanup();\n> >  \twalker_free(walker);\n> >  \n> >  \tfree(rewritten_url);\n> \n> Same as above.\n\nBut I think this is still valid.\n\nClemens\n"},{"id":"299148","messageId":"be6fef0d1002210723t2cb55460n4dcc1d27ebef03fd@mail.gmail.com","threadId":"22748","inReplyTo":"20100221104015.GB5166@localhost","subject":"Re: [PATCH 3/7] http-push: remove useless condition","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-02-21T15:23:23Z","receivedAt":"2010-02-21T15:23:23Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Sun, Feb 21, 2010 at 6:40 PM, Clemens Buchacher <drizzd@aon.at> wrote:\n> On Sun, Feb 21, 2010 at 11:08:24AM +0800, Tay Ray Chuan wrote:\n>> -                     if (push_verbosely || 1)\n>> +                     if (push_verbosely)\n>>                               fprintf(stderr, \"'%s': up-to-date\\n\", ref->name);\n>\n> Just a minor nit-pick. If you end up doing a resend, maybe you could change\n> the commit message to this one, since you're actually adding a condition\n> that was previously disabled, not removing it.\n\nyes, that makes sense..\n\n-- \nCheers,\nRay Chuan\n"},{"id":"299149","messageId":"be6fef0d1002210723j742c0d8ex86b9d552f924408f@mail.gmail.com","threadId":"22748","inReplyTo":"20100221105707.GA22534@localhost","subject":"Re: [PATCH 5/7] http: init and cleanup separately from http-walker","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-02-21T15:23:30Z","receivedAt":"2010-02-21T15:23:30Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Sun, Feb 21, 2010 at 6:57 PM, Clemens Buchacher <drizzd@aon.at> wrote:\n> On Sun, Feb 21, 2010 at 11:38:20AM +0100, Clemens Buchacher wrote:\n>> On Sun, Feb 21, 2010 at 11:08:26AM +0800, Tay Ray Chuan wrote:\n>> > diff --git a/http-fetch.c b/http-fetch.c\n>> [...]\n>>\n>>         add_fill_function(walker, (int (*)(void *)) fill_active_slot);\n>>\n>> already deals with curl functionality. So even though I think it technically\n>> doesn't break, I would prefer if this dependency were still expressed in the\n>> code.\n>>\n>> > @@ -88,6 +90,7 @@ int main(int argc, const char **argv)\n>> >  \"status code.  Suggest running 'git fsck'.\\n\");\n>> >     }\n>> >\n>> > +   http_cleanup();\n>> >     walker_free(walker);\n>> >\n>> >     free(rewritten_url);\n>>\n>> Same as above.\n>\n> But I think this is still valid.\n\nregarding the order of invocation - I could put http_cleanup() after\nwalker_free().\n\nPerhaps I can express this dependency with a comment in that area in\nhttp-fetch.c and/or http-walker::cleanup()?\n\n  Don't invoke http_cleanup() yet, we might still need to perform http\noperations in http-walker.\n\n(Separately, on re-reading the code, I noticed that\nhttp-walker::cleanup() calls http_cleanup() again, so I dropped that\ninvocation from http-walker.)\n\n-- \nCheers,\nRay Chuan\n"}]}