* [PATCH 0/7] http: minor improvements
@ 2010-02-21 3:08 Tay Ray Chuan
2010-02-21 3:08 ` [PATCH 1/7] t5541-http-push: check that ref is unchanged for non-ff test Tay Ray Chuan
` (6 more replies)
0 siblings, 7 replies; 13+ messages in thread
From: Tay Ray Chuan @ 2010-02-21 3:08 UTC (permalink / raw)
To: Git Mailing List; +Cc: Clemens Buchacher, Mike Hommey, Shawn O. Pearce
This patch series was generated on top of 'next'. It contains some
general changes to http users.
Some highlights:
- add non-ff test cases to "dumb" http push test
- separate init and cleanup of http from http-walker
[PATCH 1/7] t5541-http-push: check that ref is unchanged for non-ff test
[PATCH 2/7] t554[01]-http-push: refactor, add non-ff tests
[PATCH 3/7] http-push: remove useless condition
[PATCH 4/7] http-walker: cleanup more thoroughly
[PATCH 5/7] http: init and cleanup separately from http-walker
[PATCH 6/7] remote-curl: use http_fetch_ref() instead of walker wrapper
[PATCH 7/7] remote-curl: init walker only when needed
http-fetch.c | 4 +++-
http-push.c | 2 +-
http-walker.c | 21 ++++++++++++++++++---
remote-curl.c | 21 +++++++++------------
t/lib-httpd.sh | 29 +++++++++++++++++++++++++++++
t/t5540-http-push.sh | 3 +++
t/t5541-http-push.sh | 22 ++--------------------
walker.h | 2 +-
8 files changed, 66 insertions(+), 38 deletions(-)
--
Cheers,
Ray Chuan
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/7] t5541-http-push: check that ref is unchanged for non-ff test
2010-02-21 3:08 [PATCH 0/7] http: minor improvements Tay Ray Chuan
@ 2010-02-21 3:08 ` Tay Ray Chuan
2010-02-21 3:08 ` [PATCH 2/7] t554[01]-http-push: refactor, add non-ff tests Tay Ray Chuan
` (5 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Tay Ray Chuan @ 2010-02-21 3:08 UTC (permalink / raw)
To: Git Mailing List; +Cc: Clemens Buchacher, Mike Hommey, Shawn O. Pearce
Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
t/t5541-http-push.sh | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh
index 53f54a2..ff947f3 100755
--- a/t/t5541-http-push.sh
+++ b/t/t5541-http-push.sh
@@ -89,15 +89,17 @@ test_expect_success 'used receive-pack service' '
'
test_expect_success 'non-fast-forward push fails' '
+ cd "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo.git &&
+ HEAD=$(git rev-parse --verify HEAD) &&
+
cd "$ROOT_PATH"/test_repo_clone &&
git checkout master &&
echo "changed" > path2 &&
git commit -a -m path2 --amend &&
- HEAD=$(git rev-parse --verify HEAD) &&
!(git push -v origin >output 2>&1) &&
(cd "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo.git &&
- test $HEAD != $(git rev-parse --verify HEAD))
+ test $HEAD = $(git rev-parse --verify HEAD))
'
test_expect_success 'non-fast-forward push show ref status' '
--
1.7.0.26.gbfa16
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/7] t554[01]-http-push: refactor, add non-ff tests
2010-02-21 3:08 [PATCH 0/7] http: minor improvements Tay Ray Chuan
2010-02-21 3:08 ` [PATCH 1/7] t5541-http-push: check that ref is unchanged for non-ff test Tay Ray Chuan
@ 2010-02-21 3:08 ` Tay Ray Chuan
2010-02-21 3:08 ` [PATCH 3/7] http-push: remove useless condition Tay Ray Chuan
` (4 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Tay Ray Chuan @ 2010-02-21 3:08 UTC (permalink / raw)
To: Git Mailing List; +Cc: Clemens Buchacher, Mike Hommey, Shawn O. Pearce
Move non-fast forward tests to lib-httpd.sh so that we don't have to
duplicate the tests in both t5540 and t5541.
Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
t/lib-httpd.sh | 29 +++++++++++++++++++++++++++++
t/t5540-http-push.sh | 3 +++
t/t5541-http-push.sh | 24 ++----------------------
3 files changed, 34 insertions(+), 22 deletions(-)
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 28aff88..da4b8d5 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -131,3 +131,32 @@ stop_httpd() {
"$LIB_HTTPD_PATH" -d "$HTTPD_ROOT_PATH" \
-f "$TEST_PATH/apache.conf" $HTTPD_PARA -k stop
}
+
+test_http_push_nonff() {
+ REMOTE_REPO=$1
+ LOCAL_REPO=$2
+ BRANCH=$3
+
+ test_expect_success 'non-fast-forward push fails' '
+ cd "$REMOTE_REPO" &&
+ HEAD=$(git rev-parse --verify HEAD) &&
+
+ cd "$LOCAL_REPO" &&
+ git checkout $BRANCH &&
+ echo "changed" > path2 &&
+ git commit -a -m path2 --amend &&
+
+ !(git push -v origin >output 2>&1) &&
+ (cd "$REMOTE_REPO" &&
+ test $HEAD = $(git rev-parse --verify HEAD))
+ '
+
+ test_expect_success 'non-fast-forward push show ref status' '
+ grep "^ ! \[rejected\][ ]*$BRANCH -> $BRANCH (non-fast-forward)$" output
+ '
+
+ test_expect_success 'non-fast-forward push shows help message' '
+ grep "To prevent you from losing history, non-fast-forward updates were rejected" \
+ output
+ '
+}
diff --git a/t/t5540-http-push.sh b/t/t5540-http-push.sh
index bb18f8b..37fe875 100755
--- a/t/t5540-http-push.sh
+++ b/t/t5540-http-push.sh
@@ -137,6 +137,9 @@ test_expect_success 'PUT and MOVE sends object to URLs with SHA-1 hash suffix' '
'
+test_http_push_nonff "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo.git \
+ "$ROOT_PATH"/test_repo_clone master
+
stop_httpd
test_done
diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh
index ff947f3..795dc2b 100755
--- a/t/t5541-http-push.sh
+++ b/t/t5541-http-push.sh
@@ -88,28 +88,8 @@ test_expect_success 'used receive-pack service' '
test_cmp exp act
'
-test_expect_success 'non-fast-forward push fails' '
- cd "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo.git &&
- HEAD=$(git rev-parse --verify HEAD) &&
-
- cd "$ROOT_PATH"/test_repo_clone &&
- git checkout master &&
- echo "changed" > path2 &&
- git commit -a -m path2 --amend &&
-
- !(git push -v origin >output 2>&1) &&
- (cd "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo.git &&
- test $HEAD = $(git rev-parse --verify HEAD))
-'
-
-test_expect_success 'non-fast-forward push show ref status' '
- grep "^ ! \[rejected\][ ]*master -> master (non-fast-forward)$" output
-'
-
-test_expect_success 'non-fast-forward push shows help message' '
- grep "To prevent you from losing history, non-fast-forward updates were rejected" \
- output
-'
+test_http_push_nonff "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo.git \
+ "$ROOT_PATH"/test_repo_clone master
test_expect_success 'push fails for non-fast-forward refs unmatched by remote helper' '
# create a dissimilarly-named remote ref so that git is unable to match the
--
1.7.0.26.gbfa16
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/7] http-push: remove useless condition
2010-02-21 3:08 [PATCH 0/7] http: minor improvements Tay Ray Chuan
2010-02-21 3:08 ` [PATCH 1/7] t5541-http-push: check that ref is unchanged for non-ff test Tay Ray Chuan
2010-02-21 3:08 ` [PATCH 2/7] t554[01]-http-push: refactor, add non-ff tests Tay Ray Chuan
@ 2010-02-21 3:08 ` Tay Ray Chuan
2010-02-21 10:40 ` Clemens Buchacher
2010-02-21 3:08 ` [PATCH 4/7] http-walker: cleanup more thoroughly Tay Ray Chuan
` (3 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Tay Ray Chuan @ 2010-02-21 3:08 UTC (permalink / raw)
To: Git Mailing List; +Cc: Clemens Buchacher, Mike Hommey, Shawn O. Pearce
Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
http-push.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/http-push.c b/http-push.c
index 432b20f..415b1ab 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1965,7 +1965,7 @@ int main(int argc, char **argv)
}
if (!hashcmp(ref->old_sha1, ref->peer_ref->new_sha1)) {
- if (push_verbosely || 1)
+ if (push_verbosely)
fprintf(stderr, "'%s': up-to-date\n", ref->name);
if (helper_status)
printf("ok %s up to date\n", ref->name);
--
1.7.0.26.gbfa16
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/7] http-walker: cleanup more thoroughly
2010-02-21 3:08 [PATCH 0/7] http: minor improvements Tay Ray Chuan
` (2 preceding siblings ...)
2010-02-21 3:08 ` [PATCH 3/7] http-push: remove useless condition Tay Ray Chuan
@ 2010-02-21 3:08 ` Tay Ray Chuan
2010-02-21 3:08 ` [PATCH 5/7] http: init and cleanup separately from http-walker Tay Ray Chuan
` (2 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Tay Ray Chuan @ 2010-02-21 3:08 UTC (permalink / raw)
To: Git Mailing List; +Cc: Clemens Buchacher, Mike Hommey, Shawn O. Pearce
Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
http-walker.c | 17 +++++++++++++++++
1 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/http-walker.c b/http-walker.c
index 700bc13..508e355 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -543,6 +543,23 @@ static int fetch_ref(struct walker *walker, struct ref *ref)
static void cleanup(struct walker *walker)
{
+ struct walker_data *data = walker->data;
+ struct alt_base *alt, *alt_next;
+
+ if (data) {
+ alt = data->alt;
+ while (alt) {
+ alt_next = alt->next;
+
+ free(alt->base);
+ free(alt);
+
+ alt = alt_next;
+ }
+ free(data);
+ walker->data = NULL;
+ }
+
http_cleanup();
}
--
1.7.0.26.gbfa16
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 5/7] http: init and cleanup separately from http-walker
2010-02-21 3:08 [PATCH 0/7] http: minor improvements Tay Ray Chuan
` (3 preceding siblings ...)
2010-02-21 3:08 ` [PATCH 4/7] http-walker: cleanup more thoroughly Tay Ray Chuan
@ 2010-02-21 3:08 ` Tay Ray Chuan
2010-02-21 10:38 ` Clemens Buchacher
2010-02-21 3:08 ` [PATCH 6/7] remote-curl: use http_fetch_ref() instead of walker wrapper Tay Ray Chuan
2010-02-21 3:08 ` [PATCH 7/7] remote-curl: init walker only when needed Tay Ray Chuan
6 siblings, 1 reply; 13+ messages in thread
From: Tay Ray Chuan @ 2010-02-21 3:08 UTC (permalink / raw)
To: Git Mailing List; +Cc: Clemens Buchacher, Mike Hommey, Shawn O. Pearce
Previously, all our http operations were done with http-walker. With the
new remote-curl helper, we find ourselves using http methods outside of
http-walker - for example, fetching info/refs.
Accomodate this by separating http_init() and http_cleanup() invocations
from http-walker.
Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
http-fetch.c | 5 ++++-
http-walker.c | 4 +---
remote-curl.c | 7 ++++++-
walker.h | 2 +-
4 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/http-fetch.c b/http-fetch.c
index ffd0ad7..e05b23a 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -1,5 +1,6 @@
#include "cache.h"
#include "exec_cmd.h"
+#include "http.h"
#include "walker.h"
static const char http_fetch_usage[] = "git http-fetch "
@@ -69,7 +70,8 @@ int main(int argc, const char **argv)
url = rewritten_url;
}
- walker = get_http_walker(url, NULL);
+ http_init(NULL);
+ walker = get_http_walker(url);
walker->get_tree = get_tree;
walker->get_history = get_history;
walker->get_all = get_all;
@@ -88,6 +90,7 @@ int main(int argc, const char **argv)
"status code. Suggest running 'git fsck'.\n");
}
+ http_cleanup();
walker_free(walker);
free(rewritten_url);
diff --git a/http-walker.c b/http-walker.c
index 508e355..bc39451 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -563,14 +563,12 @@ static void cleanup(struct walker *walker)
http_cleanup();
}
-struct walker *get_http_walker(const char *url, struct remote *remote)
+struct walker *get_http_walker(const char *url)
{
char *s;
struct walker_data *data = xmalloc(sizeof(struct walker_data));
struct walker *walker = xmalloc(sizeof(struct walker));
- http_init(remote);
-
data->alt = xmalloc(sizeof(*data->alt));
data->alt->base = xmalloc(strlen(url) + 1);
strcpy(data->alt->base, url);
diff --git a/remote-curl.c b/remote-curl.c
index d388120..1e13fb5 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -25,7 +25,7 @@ static struct options options;
static void init_walker(void)
{
if (!walker)
- walker = get_http_walker(url, remote);
+ walker = get_http_walker(url);
}
static int set_option(const char *name, const char *value)
@@ -811,6 +811,8 @@ int main(int argc, const char **argv)
url = remote->url[0];
}
+ http_init(remote);
+
do {
if (strbuf_getline(&buf, stdin, '\n') == EOF)
break;
@@ -856,5 +858,8 @@ int main(int argc, const char **argv)
}
strbuf_reset(&buf);
} while (1);
+
+ http_cleanup();
+
return 0;
}
diff --git a/walker.h b/walker.h
index 8a149e1..95e5765 100644
--- a/walker.h
+++ b/walker.h
@@ -34,6 +34,6 @@ int walker_fetch(struct walker *impl, int targets, char **target,
void walker_free(struct walker *walker);
-struct walker *get_http_walker(const char *url, struct remote *remote);
+struct walker *get_http_walker(const char *url);
#endif /* WALKER_H */
--
1.7.0.26.gbfa16
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 6/7] remote-curl: use http_fetch_ref() instead of walker wrapper
2010-02-21 3:08 [PATCH 0/7] http: minor improvements Tay Ray Chuan
` (4 preceding siblings ...)
2010-02-21 3:08 ` [PATCH 5/7] http: init and cleanup separately from http-walker Tay Ray Chuan
@ 2010-02-21 3:08 ` Tay Ray Chuan
2010-02-21 3:08 ` [PATCH 7/7] remote-curl: init walker only when needed Tay Ray Chuan
6 siblings, 0 replies; 13+ messages in thread
From: Tay Ray Chuan @ 2010-02-21 3:08 UTC (permalink / raw)
To: Git Mailing List; +Cc: Clemens Buchacher, Mike Hommey, Shawn O. Pearce
The http-walker implementation of walker->fetch_ref() doesn't do
anything special compared to http_fetch_ref() anyway.
Remove init_walker() invocation before fetching the ref, since we aren't
using the walker wrapper and don't need a walker instance anymore.
Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
remote-curl.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/remote-curl.c b/remote-curl.c
index 1e13fb5..5ace99e 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -250,9 +250,8 @@ static struct ref *parse_info_refs(struct discovery *heads)
i++;
}
- init_walker();
ref = alloc_ref("HEAD");
- if (!walker->fetch_ref(walker, ref) &&
+ if (!http_fetch_ref(url, ref) &&
!resolve_remote_symref(ref, refs)) {
ref->next = refs;
refs = ref;
--
1.7.0.26.gbfa16
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 7/7] remote-curl: init walker only when needed
2010-02-21 3:08 [PATCH 0/7] http: minor improvements Tay Ray Chuan
` (5 preceding siblings ...)
2010-02-21 3:08 ` [PATCH 6/7] remote-curl: use http_fetch_ref() instead of walker wrapper Tay Ray Chuan
@ 2010-02-21 3:08 ` Tay Ray Chuan
6 siblings, 0 replies; 13+ messages in thread
From: Tay Ray Chuan @ 2010-02-21 3:08 UTC (permalink / raw)
To: Git Mailing List; +Cc: Clemens Buchacher, Mike Hommey, Shawn O. Pearce
Invoke get_http_walker() only when fetching with the dumb protocol.
Additionally, add an invocation to walker_free() after we're done using
the walker.
Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
remote-curl.c | 13 +++----------
1 files changed, 3 insertions(+), 10 deletions(-)
diff --git a/remote-curl.c b/remote-curl.c
index 5ace99e..b76bfcb 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -10,7 +10,6 @@
static struct remote *remote;
static const char *url;
-static struct walker *walker;
struct options {
int verbosity;
@@ -22,12 +21,6 @@ struct options {
};
static struct options options;
-static void init_walker(void)
-{
- if (!walker)
- walker = get_http_walker(url);
-}
-
static int set_option(const char *name, const char *value)
{
if (!strcmp(name, "verbosity")) {
@@ -119,7 +112,6 @@ static struct discovery* discover_refs(const char *service)
}
refs_url = strbuf_detach(&buffer, NULL);
- init_walker();
http_ret = http_get_strbuf(refs_url, &buffer, HTTP_NO_CACHE);
/* try again with "plain" url (no ? or & appended) */
@@ -501,7 +493,6 @@ static int rpc_service(struct rpc_state *rpc, struct discovery *heads)
struct child_process client;
int err = 0;
- init_walker();
memset(&client, 0, sizeof(client));
client.in = -1;
client.out = -1;
@@ -553,6 +544,7 @@ static int rpc_service(struct rpc_state *rpc, struct discovery *heads)
static int fetch_dumb(int nr_heads, struct ref **to_fetch)
{
+ struct walker *walker;
char **targets = xmalloc(nr_heads * sizeof(char*));
int ret, i;
@@ -561,13 +553,14 @@ static int fetch_dumb(int nr_heads, struct ref **to_fetch)
for (i = 0; i < nr_heads; i++)
targets[i] = xstrdup(sha1_to_hex(to_fetch[i]->old_sha1));
- init_walker();
+ walker = get_http_walker(url);
walker->get_all = 1;
walker->get_tree = 1;
walker->get_history = 1;
walker->get_verbosely = options.verbosity >= 3;
walker->get_recover = 0;
ret = walker_fetch(walker, nr_heads, targets, NULL, NULL);
+ walker_free(walker);
for (i = 0; i < nr_heads; i++)
free(targets[i]);
--
1.7.0.26.gbfa16
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 5/7] http: init and cleanup separately from http-walker
2010-02-21 3:08 ` [PATCH 5/7] http: init and cleanup separately from http-walker Tay Ray Chuan
@ 2010-02-21 10:38 ` Clemens Buchacher
2010-02-21 10:57 ` Clemens Buchacher
0 siblings, 1 reply; 13+ messages in thread
From: Clemens Buchacher @ 2010-02-21 10:38 UTC (permalink / raw)
To: Tay Ray Chuan; +Cc: Git Mailing List, Mike Hommey, Shawn O. Pearce
On Sun, Feb 21, 2010 at 11:08:26AM +0800, Tay Ray Chuan wrote:
> diff --git a/http-fetch.c b/http-fetch.c
[...]
> @@ -69,7 +70,8 @@ int main(int argc, const char **argv)
> url = rewritten_url;
> }
>
> - walker = get_http_walker(url, NULL);
> + http_init(NULL);
> + walker = get_http_walker(url);
> walker->get_tree = get_tree;
> walker->get_history = get_history;
> walker->get_all = get_all;
You changed the order of get_http_walker and http_init. But
add_fill_function(walker, (int (*)(void *)) fill_active_slot);
already deals with curl functionality. So even though I think it technically
doesn't break, I would prefer if this dependency were still expressed in the
code.
> @@ -88,6 +90,7 @@ int main(int argc, const char **argv)
> "status code. Suggest running 'git fsck'.\n");
> }
>
> + http_cleanup();
> walker_free(walker);
>
> free(rewritten_url);
Same as above.
Otherwise the series looks good to me.
Liked-by: Clemens Buchacher <drizzd@aon.at>
Clemens
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/7] http-push: remove useless condition
2010-02-21 3:08 ` [PATCH 3/7] http-push: remove useless condition Tay Ray Chuan
@ 2010-02-21 10:40 ` Clemens Buchacher
2010-02-21 15:23 ` Tay Ray Chuan
0 siblings, 1 reply; 13+ messages in thread
From: Clemens Buchacher @ 2010-02-21 10:40 UTC (permalink / raw)
To: Tay Ray Chuan; +Cc: Git Mailing List, Mike Hommey, Shawn O. Pearce
On Sun, Feb 21, 2010 at 11:08:24AM +0800, Tay Ray Chuan wrote:
> - if (push_verbosely || 1)
> + if (push_verbosely)
> fprintf(stderr, "'%s': up-to-date\n", ref->name);
Just a minor nit-pick. If you end up doing a resend, maybe you could change
the commit message to this one, since you're actually adding a condition
that was previously disabled, not removing it.
Clemens
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 5/7] http: init and cleanup separately from http-walker
2010-02-21 10:38 ` Clemens Buchacher
@ 2010-02-21 10:57 ` Clemens Buchacher
2010-02-21 15:23 ` Tay Ray Chuan
0 siblings, 1 reply; 13+ messages in thread
From: Clemens Buchacher @ 2010-02-21 10:57 UTC (permalink / raw)
To: Tay Ray Chuan; +Cc: Git Mailing List, Mike Hommey, Shawn O. Pearce
On Sun, Feb 21, 2010 at 11:38:20AM +0100, Clemens Buchacher wrote:
> On Sun, Feb 21, 2010 at 11:08:26AM +0800, Tay Ray Chuan wrote:
> > diff --git a/http-fetch.c b/http-fetch.c
> [...]
> > @@ -69,7 +70,8 @@ int main(int argc, const char **argv)
> > url = rewritten_url;
> > }
> >
> > - walker = get_http_walker(url, NULL);
> > + http_init(NULL);
> > + walker = get_http_walker(url);
> > walker->get_tree = get_tree;
> > walker->get_history = get_history;
> > walker->get_all = get_all;
>
> You changed the order of get_http_walker and http_init. But
Umh, actually you didn't. Sorry about that.
>
> add_fill_function(walker, (int (*)(void *)) fill_active_slot);
>
> already deals with curl functionality. So even though I think it technically
> doesn't break, I would prefer if this dependency were still expressed in the
> code.
>
> > @@ -88,6 +90,7 @@ int main(int argc, const char **argv)
> > "status code. Suggest running 'git fsck'.\n");
> > }
> >
> > + http_cleanup();
> > walker_free(walker);
> >
> > free(rewritten_url);
>
> Same as above.
But I think this is still valid.
Clemens
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/7] http-push: remove useless condition
2010-02-21 10:40 ` Clemens Buchacher
@ 2010-02-21 15:23 ` Tay Ray Chuan
0 siblings, 0 replies; 13+ messages in thread
From: Tay Ray Chuan @ 2010-02-21 15:23 UTC (permalink / raw)
To: Clemens Buchacher; +Cc: Git Mailing List, Mike Hommey, Shawn O. Pearce
Hi,
On Sun, Feb 21, 2010 at 6:40 PM, Clemens Buchacher <drizzd@aon.at> wrote:
> On Sun, Feb 21, 2010 at 11:08:24AM +0800, Tay Ray Chuan wrote:
>> - if (push_verbosely || 1)
>> + if (push_verbosely)
>> fprintf(stderr, "'%s': up-to-date\n", ref->name);
>
> Just a minor nit-pick. If you end up doing a resend, maybe you could change
> the commit message to this one, since you're actually adding a condition
> that was previously disabled, not removing it.
yes, that makes sense..
--
Cheers,
Ray Chuan
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 5/7] http: init and cleanup separately from http-walker
2010-02-21 10:57 ` Clemens Buchacher
@ 2010-02-21 15:23 ` Tay Ray Chuan
0 siblings, 0 replies; 13+ messages in thread
From: Tay Ray Chuan @ 2010-02-21 15:23 UTC (permalink / raw)
To: Clemens Buchacher; +Cc: Git Mailing List, Mike Hommey, Shawn O. Pearce
Hi,
On Sun, Feb 21, 2010 at 6:57 PM, Clemens Buchacher <drizzd@aon.at> wrote:
> On Sun, Feb 21, 2010 at 11:38:20AM +0100, Clemens Buchacher wrote:
>> On Sun, Feb 21, 2010 at 11:08:26AM +0800, Tay Ray Chuan wrote:
>> > diff --git a/http-fetch.c b/http-fetch.c
>> [...]
>>
>> add_fill_function(walker, (int (*)(void *)) fill_active_slot);
>>
>> already deals with curl functionality. So even though I think it technically
>> doesn't break, I would prefer if this dependency were still expressed in the
>> code.
>>
>> > @@ -88,6 +90,7 @@ int main(int argc, const char **argv)
>> > "status code. Suggest running 'git fsck'.\n");
>> > }
>> >
>> > + http_cleanup();
>> > walker_free(walker);
>> >
>> > free(rewritten_url);
>>
>> Same as above.
>
> But I think this is still valid.
regarding the order of invocation - I could put http_cleanup() after
walker_free().
Perhaps I can express this dependency with a comment in that area in
http-fetch.c and/or http-walker::cleanup()?
Don't invoke http_cleanup() yet, we might still need to perform http
operations in http-walker.
(Separately, on re-reading the code, I noticed that
http-walker::cleanup() calls http_cleanup() again, so I dropped that
invocation from http-walker.)
--
Cheers,
Ray Chuan
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2010-02-21 15:23 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-02-21 3:08 [PATCH 0/7] http: minor improvements Tay Ray Chuan
2010-02-21 3:08 ` [PATCH 1/7] t5541-http-push: check that ref is unchanged for non-ff test Tay Ray Chuan
2010-02-21 3:08 ` [PATCH 2/7] t554[01]-http-push: refactor, add non-ff tests Tay Ray Chuan
2010-02-21 3:08 ` [PATCH 3/7] http-push: remove useless condition Tay Ray Chuan
2010-02-21 10:40 ` Clemens Buchacher
2010-02-21 15:23 ` Tay Ray Chuan
2010-02-21 3:08 ` [PATCH 4/7] http-walker: cleanup more thoroughly Tay Ray Chuan
2010-02-21 3:08 ` [PATCH 5/7] http: init and cleanup separately from http-walker Tay Ray Chuan
2010-02-21 10:38 ` Clemens Buchacher
2010-02-21 10:57 ` Clemens Buchacher
2010-02-21 15:23 ` Tay Ray Chuan
2010-02-21 3:08 ` [PATCH 6/7] remote-curl: use http_fetch_ref() instead of walker wrapper Tay Ray Chuan
2010-02-21 3:08 ` [PATCH 7/7] remote-curl: init walker only when needed Tay Ray Chuan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).