From 8988688c479c6e511432187c7e7e746aefb23c08 Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Wed, 25 Sep 2013 20:41:56 -0700 Subject: [PATCH 01/37] Migrate redirect URL handling to common utility --- src/netops.c | 75 +++++++++++++++++++++++++ src/netops.h | 25 +++++++++ src/transports/http.c | 102 ++++++++++------------------------ tests-clar/network/urlparse.c | 42 ++++++++++++++ 4 files changed, 172 insertions(+), 72 deletions(-) diff --git a/src/netops.c b/src/netops.c index c1e74546f..bda064cfb 100644 --- a/src/netops.c +++ b/src/netops.c @@ -573,6 +573,81 @@ int gitno_select_in(gitno_buffer *buf, long int sec, long int usec) return select((int)buf->socket->socket + 1, &fds, NULL, NULL, &tv); } +static const char *prefix_http = "http://"; +static const char *prefix_https = "https://"; + +int gitno_connection_data_from_url( + gitno_connection_data *data, + const char *url, + const char *service_suffix, + const char *original_host, + bool original_use_ssl) +{ + int error = 0; + const char *default_port = NULL; + + /* service_suffix is optional */ + assert(data && url); + + if (!git__prefixcmp(url, prefix_http)) { + url = url + strlen(prefix_http); + default_port = "80"; + + if (data->use_ssl) { + giterr_set(GITERR_NET, "Redirect from HTTPS to HTTP is not allowed"); + return -1; + } + } + + if (!git__prefixcmp(url, prefix_https)) { + url += strlen(prefix_https); + default_port = "443"; + data->use_ssl = true; + } + + if (!default_port) { + giterr_set(GITERR_NET, "Unrecognized URL prefix"); + return -1; + } + + error = gitno_extract_url_parts( + &data->host, &data->port, &data->user, &data->pass, + url, default_port); + + if (!error) { + const char *path = strchr(url, '/'); + size_t pathlen = strlen(path); + size_t suffixlen = service_suffix ? strlen(service_suffix) : 0; + + if (suffixlen && + !memcmp(path + pathlen - suffixlen, service_suffix, suffixlen)) + data->path = git__strndup(path, pathlen - suffixlen); + else + data->path = git__strdup(path); + + /* Check for errors in the resulting data */ + if (original_use_ssl && !data->use_ssl) { + giterr_set(GITERR_NET, "Redirect from HTTPS to HTTP not allowed"); + error = -1; + } + if (original_host && url[0] != '/' && strcmp(original_host, data->host)) { + giterr_set(GITERR_NET, "Cross host redirect not allowed"); + error = -1; + } + } + + return error; +} + +void gitno_connection_data_free_ptrs(gitno_connection_data *d) +{ + git__free(d->host); d->host = NULL; + git__free(d->port); d->port = NULL; + git__free(d->path); d->path = NULL; + git__free(d->user); d->user = NULL; + git__free(d->pass); d->pass = NULL; +} + int gitno_extract_url_parts( char **host, char **port, diff --git a/src/netops.h b/src/netops.h index d352bf3b6..0c6e571d9 100644 --- a/src/netops.h +++ b/src/netops.h @@ -66,6 +66,31 @@ int gitno_send(gitno_socket *socket, const char *msg, size_t len, int flags); int gitno_close(gitno_socket *s); int gitno_select_in(gitno_buffer *buf, long int sec, long int usec); +typedef struct gitno_connection_data { + char *host; + char *port; + char *path; + char *user; + char *pass; + bool use_ssl; +} gitno_connection_data; + +/* + * This replaces all the pointers in `data` with freshly-allocated strings, + * that the caller is responsible for freeing. + * `gitno_connection_data_free_ptrs` is good for this. + */ + +int gitno_connection_data_from_url( + gitno_connection_data *data, + const char *url, + const char *service_suffix, + const char *original_host, + bool original_use_ssl); + +/* This frees all the pointers IN the struct, but not the struct itself. */ +void gitno_connection_data_free_ptrs(gitno_connection_data *data); + int gitno_extract_url_parts( char **host, char **port, diff --git a/src/transports/http.c b/src/transports/http.c index ab2f9a47f..8d28d5b47 100644 --- a/src/transports/http.c +++ b/src/transports/http.c @@ -67,8 +67,8 @@ typedef struct { git_cred *cred; git_cred *url_cred; http_authmechanism_t auth_mechanism; - unsigned connected : 1, - use_ssl : 1; + bool connected, + use_ssl; /* Parser structures */ http_parser parser; @@ -277,70 +277,6 @@ static void free_connection_data(http_subtransport *t) } } -static int set_connection_data_from_url( - http_subtransport *t, const char *url, const char *service_suffix) -{ - int error = 0; - const char *default_port = NULL; - char *original_host = NULL; - - if (!git__prefixcmp(url, prefix_http)) { - url = url + strlen(prefix_http); - default_port = "80"; - - if (t->use_ssl) { - giterr_set(GITERR_NET, "Redirect from HTTPS to HTTP not allowed"); - return -1; - } - } - - if (!git__prefixcmp(url, prefix_https)) { - url += strlen(prefix_https); - default_port = "443"; - t->use_ssl = 1; - } - - if (!default_port) { - giterr_set(GITERR_NET, "Unrecognized URL prefix"); - return -1; - } - - /* preserve original host name for checking */ - original_host = t->host; - t->host = NULL; - - free_connection_data(t); - - error = gitno_extract_url_parts( - &t->host, &t->port, &t->user_from_url, &t->pass_from_url, - url, default_port); - - if (!error) { - const char *path = strchr(url, '/'); - size_t pathlen = strlen(path); - size_t suffixlen = service_suffix ? strlen(service_suffix) : 0; - - if (suffixlen && - !memcmp(path + pathlen - suffixlen, service_suffix, suffixlen)) - t->path = git__strndup(path, pathlen - suffixlen); - else - t->path = git__strdup(path); - - /* Allow '/'-led urls, or a change of protocol */ - if (original_host != NULL) { - if (strcmp(original_host, t->host) && t->location[0] != '/') { - giterr_set(GITERR_NET, "Cross host redirect not allowed"); - error = -1; - } - - git__free(original_host); - } - } - - return error; -} - - static int on_headers_complete(http_parser *parser) { parser_context *ctx = (parser_context *) parser->data; @@ -384,18 +320,30 @@ static int on_headers_complete(http_parser *parser) /* Check for a redirect. * Right now we only permit a redirect to the same hostname. */ if ((parser->status_code == 301 || - parser->status_code == 302 || - (parser->status_code == 303 && get_verb == s->verb) || - parser->status_code == 307) && - t->location) { + parser->status_code == 302 || + (parser->status_code == 303 && get_verb == s->verb) || + parser->status_code == 307) && + t->location) { + gitno_connection_data connection_data = {0}; if (s->redirect_count >= 7) { giterr_set(GITERR_NET, "Too many redirects"); return t->parse_error = PARSE_ERROR_GENERIC; } - if (set_connection_data_from_url(t, t->location, s->service_url) < 0) + if (gitno_connection_data_from_url(&connection_data, t->location, + s->service_url, t->host, t->use_ssl) < 0) { + gitno_connection_data_free_ptrs(&connection_data); return t->parse_error = PARSE_ERROR_GENERIC; + } + + free_connection_data(t); + t->host = connection_data.host; + t->port = connection_data.port; + t->path = connection_data.path; + t->user_from_url = connection_data.user; + t->pass_from_url = connection_data.pass; + t->use_ssl = connection_data.use_ssl; /* Set the redirect URL on the stream. This is a transfer of * ownership of the memory. */ @@ -912,8 +860,18 @@ static int http_action( return -1; if (!t->host || !t->port || !t->path) { - if ((ret = set_connection_data_from_url(t, url, NULL)) < 0) + gitno_connection_data data = {0}; + if ((ret = gitno_connection_data_from_url(&data, + url, NULL, NULL, false)) < 0) { + gitno_connection_data_free_ptrs(&data); return ret; + } + t->host = data.host; + t->port = data.port; + t->path = data.path; + t->user_from_url = data.user; + t->pass_from_url = data.pass; + t->use_ssl = data.use_ssl; } if (http_connect(t) < 0) diff --git a/tests-clar/network/urlparse.c b/tests-clar/network/urlparse.c index 173e57d0f..66f714823 100644 --- a/tests-clar/network/urlparse.c +++ b/tests-clar/network/urlparse.c @@ -2,10 +2,12 @@ #include "netops.h" char *host, *port, *user, *pass; +gitno_connection_data conndata; void test_network_urlparse__initialize(void) { host = port = user = pass = NULL; + memset(&conndata, 0, sizeof(conndata)); } void test_network_urlparse__cleanup(void) @@ -15,6 +17,8 @@ void test_network_urlparse__cleanup(void) FREE_AND_NULL(port); FREE_AND_NULL(user); FREE_AND_NULL(pass); + + gitno_connection_data_free_ptrs(&conndata); } void test_network_urlparse__trivial(void) @@ -80,3 +84,41 @@ void test_network_urlparse__user_pass_port(void) cl_assert_equal_s(user, "user"); cl_assert_equal_s(pass, "pass"); } + +void test_network_urlparse__connection_data_http(void) +{ + cl_git_pass(gitno_connection_data_from_url(&conndata, + "http://example.com/foo/bar/baz", "bar/baz", NULL, false)); + cl_assert_equal_s(conndata.host, "example.com"); + cl_assert_equal_s(conndata.port, "80"); + cl_assert_equal_s(conndata.path, "/foo/"); + cl_assert_equal_p(conndata.user, NULL); + cl_assert_equal_p(conndata.pass, NULL); + cl_assert_equal_i(conndata.use_ssl, false); +} + +void test_network_urlparse__connection_data_ssl(void) +{ + cl_git_pass(gitno_connection_data_from_url(&conndata, + "https://example.com/foo/bar/baz", "bar/baz", NULL, false)); + cl_assert_equal_s(conndata.host, "example.com"); + cl_assert_equal_s(conndata.port, "443"); + cl_assert_equal_s(conndata.path, "/foo/"); + cl_assert_equal_p(conndata.user, NULL); + cl_assert_equal_p(conndata.pass, NULL); + cl_assert_equal_i(conndata.use_ssl, true); +} + +void test_network_urlparse__connection_data_cross_host_redirect(void) +{ + cl_git_fail_with(gitno_connection_data_from_url(&conndata, + "https://foo.com/bar/baz", NULL, "bar.com", true), + -1); +} + +void test_network_urlparse__connection_data_http_downgrade(void) +{ + cl_git_fail_with(gitno_connection_data_from_url(&conndata, + "http://foo.com/bar/baz", NULL, NULL, true), + -1); +} From f30d91ce48f444031b1c65749758008c6f204130 Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Thu, 26 Sep 2013 11:03:27 -0700 Subject: [PATCH 02/37] Refactor URL handling to use library call --- src/transports/winhttp.c | 97 ++++++++++++---------------------------- 1 file changed, 28 insertions(+), 69 deletions(-) diff --git a/src/transports/winhttp.c b/src/transports/winhttp.c index 377f2ef97..7ec936d69 100644 --- a/src/transports/winhttp.c +++ b/src/transports/winhttp.c @@ -420,70 +420,6 @@ static void free_connection_data(winhttp_subtransport *t) } } -static int set_connection_data_from_url( - winhttp_subtransport *t, const char *url, const char *service_suffix) -{ - int error = 0; - const char *default_port = NULL; - char *original_host = NULL; - const char *original_url = url; - - if (!git__prefixcmp(url, prefix_http)) { - url += strlen(prefix_http); - default_port = "80"; - - if (t->use_ssl) { - giterr_set(GITERR_NET, "Redirect from HTTPS to HTTP not allowed"); - return -1; - } - } - - if (!git__prefixcmp(url, prefix_https)) { - url += strlen(prefix_https); - default_port = "443"; - t->use_ssl = 1; - } - - if (!default_port) { - giterr_set(GITERR_NET, "Unrecognized URL prefix"); - return -1; - } - - /* preserve original host name for checking */ - original_host = t->host; - t->host = NULL; - - free_connection_data(t); - - error = gitno_extract_url_parts( - &t->host, &t->port, &t->user_from_url, &t->pass_from_url, - url, default_port); - - if (!error) { - const char *path = strchr(url, '/'); - size_t pathlen = strlen(path); - size_t suffixlen = service_suffix ? strlen(service_suffix) : 0; - - if (suffixlen && - !memcmp(path + pathlen - suffixlen, service_suffix, suffixlen)) - t->path = git__strndup(path, pathlen - suffixlen); - else - t->path = git__strdup(path); - - /* Allow '/'-led urls, or a change of protocol */ - if (original_host != NULL) { - if (strcmp(original_host, t->host) && original_url[0] != '/') { - giterr_set(GITERR_NET, "Cross host redirect not allowed"); - error = -1; - } - - git__free(original_host); - } - } - - return error; -} - static int winhttp_connect( winhttp_subtransport *t, const char *url) @@ -699,7 +635,18 @@ replay: if (!git__prefixcmp_icase(location8, prefix_https)) { /* Upgrade to secure connection; disconnect and start over */ - set_connection_data_from_url(t, location8, s->service_url); + gitno_connection_data data = { 0 }; + if (gitno_connection_data_from_url(&data, location8, s->service_url, t->host, t->use_ssl) < 0) { + gitno_connection_data_free_ptrs(&data); + return -1; + } + free_connection_data(t); + t->host = data.host; + t->port = data.port; + t->path = data.path; + t->user_from_url = data.user; + t->pass_from_url = data.pass; + t->use_ssl = data.use_ssl; winhttp_connect(t, location8); } @@ -1101,10 +1048,22 @@ static int winhttp_action( winhttp_stream *s; int ret = -1; - if (!t->connection && - (set_connection_data_from_url(t, url, NULL) < 0 || - winhttp_connect(t, url) < 0)) - return -1; + if (!t->connection) { + gitno_connection_data data = { 0 }; + if (gitno_connection_data_from_url(&data, url, NULL, NULL, false) < 0) { + gitno_connection_data_free_ptrs(&data); + return -1; + } + free_connection_data(t); + t->host = data.host; + t->port = data.port; + t->path = data.path; + t->user_from_url = data.user; + t->pass_from_url = data.pass; + t->use_ssl = data.use_ssl; + if (winhttp_connect(t, url) < 0) + return -1; + } if (winhttp_stream_alloc(t, &s) < 0) return -1; From 8a1e925dde2f7deaf640757b1c309fcc6be1b0e1 Mon Sep 17 00:00:00 2001 From: nulltoken Date: Thu, 26 Sep 2013 12:00:35 +0200 Subject: [PATCH 03/37] Fix warnings --- src/indexer.c | 2 +- tests-clar/clar_libgit2.c | 2 +- tests-clar/clar_libgit2.h | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/indexer.c b/src/indexer.c index ceb11f0b6..3b160df5d 100644 --- a/src/indexer.c +++ b/src/indexer.c @@ -426,7 +426,7 @@ int git_indexer_stream_add(git_indexer_stream *idx, const void *data, size_t siz if (git_filebuf_write(&idx->pack_file, data, size) < 0) return -1; - hash_partially(idx, data, size); + hash_partially(idx, data, (int)size); /* Make sure we set the new size of the pack */ if (idx->opened_pack) { diff --git a/tests-clar/clar_libgit2.c b/tests-clar/clar_libgit2.c index 555af38db..6063bf91c 100644 --- a/tests-clar/clar_libgit2.c +++ b/tests-clar/clar_libgit2.c @@ -440,7 +440,7 @@ void clar__assert_equal_file( int ignore_cr, const char *path, const char *file, - size_t line) + int line) { char buf[4000]; ssize_t bytes, total_bytes = 0; diff --git a/tests-clar/clar_libgit2.h b/tests-clar/clar_libgit2.h index 9d4d63e6c..b9ef5627e 100644 --- a/tests-clar/clar_libgit2.h +++ b/tests-clar/clar_libgit2.h @@ -44,7 +44,7 @@ GIT_INLINE(void) clar__assert_in_range( } #define cl_assert_equal_sz(sz1,sz2) do { \ - size_t __sz1 = (sz1), __sz2 = (sz2); \ + size_t __sz1 = (size_t)(sz1), __sz2 = (size_t)(sz2); \ clar__assert_equal(__FILE__,__LINE__,#sz1 " != " #sz2, 1, "%"PRIuZ, __sz1, __sz2); \ } while (0) @@ -52,10 +52,10 @@ GIT_INLINE(void) clar__assert_in_range( clar__assert_in_range((L),(V),(H),__FILE__,__LINE__,"Range check: " #V " in [" #L "," #H "]", 1) #define cl_assert_equal_file(DATA,SIZE,PATH) \ - clar__assert_equal_file(DATA,SIZE,0,PATH,__FILE__,__LINE__) + clar__assert_equal_file(DATA,SIZE,0,PATH,__FILE__,(int)__LINE__) #define cl_assert_equal_file_ignore_cr(DATA,SIZE,PATH) \ - clar__assert_equal_file(DATA,SIZE,1,PATH,__FILE__,__LINE__) + clar__assert_equal_file(DATA,SIZE,1,PATH,__FILE__,(int)__LINE__) void clar__assert_equal_file( const char *expected_data, @@ -63,7 +63,7 @@ void clar__assert_equal_file( int ignore_cr, const char *path, const char *file, - size_t line); + int line); /* * Some utility macros for building long strings From 83fbd36869e9126bd0fd2139af4d8f4c45865266 Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Thu, 26 Sep 2013 15:58:41 -0700 Subject: [PATCH 04/37] Deploy gitno_connection_data into transport --- src/transports/http.c | 89 ++++++++----------------------------------- 1 file changed, 16 insertions(+), 73 deletions(-) diff --git a/src/transports/http.c b/src/transports/http.c index 8d28d5b47..81157889e 100644 --- a/src/transports/http.c +++ b/src/transports/http.c @@ -12,8 +12,6 @@ #include "netops.h" #include "smart.h" -static const char *prefix_http = "http://"; -static const char *prefix_https = "https://"; static const char *upload_pack_service = "upload-pack"; static const char *upload_pack_ls_service_url = "/info/refs?service=git-upload-pack"; static const char *upload_pack_service_url = "/git-upload-pack"; @@ -59,16 +57,11 @@ typedef struct { git_smart_subtransport parent; transport_smart *owner; gitno_socket socket; - char *path; - char *host; - char *port; - char *user_from_url; - char *pass_from_url; + gitno_connection_data connection_data; git_cred *cred; git_cred *url_cred; http_authmechanism_t auth_mechanism; - bool connected, - use_ssl; + bool connected; /* Parser structures */ http_parser parser; @@ -125,12 +118,12 @@ static int gen_request( size_t content_length) { http_subtransport *t = OWNING_SUBTRANSPORT(s); - const char *path = t->path ? t->path : "/"; + const char *path = t->connection_data.path ? t->connection_data.path : "/"; git_buf_printf(buf, "%s %s%s HTTP/1.1\r\n", s->verb, path, s->service_url); git_buf_puts(buf, "User-Agent: git/1.0 (libgit2 " LIBGIT2_VERSION ")\r\n"); - git_buf_printf(buf, "Host: %s\r\n", t->host); + git_buf_printf(buf, "Host: %s\r\n", t->connection_data.host); if (s->chunked || content_length > 0) { git_buf_printf(buf, "Accept: application/x-git-%s-result\r\n", s->service); @@ -150,9 +143,9 @@ static int gen_request( return -1; /* Use url-parsed basic auth if username and password are both provided */ - if (!t->cred && t->user_from_url && t->pass_from_url) { - if (!t->url_cred && - git_cred_userpass_plaintext_new(&t->url_cred, t->user_from_url, t->pass_from_url) < 0) + if (!t->cred && t->connection_data.user && t->connection_data.pass) { + if (!t->url_cred && git_cred_userpass_plaintext_new(&t->url_cred, + t->connection_data.user, t->connection_data.pass) < 0) return -1; if (apply_basic_credential(buf, t->url_cred) < 0) return -1; } @@ -249,34 +242,6 @@ static int on_header_value(http_parser *parser, const char *str, size_t len) return 0; } -static void free_connection_data(http_subtransport *t) -{ - if (t->host) { - git__free(t->host); - t->host = NULL; - } - - if (t->port) { - git__free(t->port); - t->port = NULL; - } - - if (t->user_from_url) { - git__free(t->user_from_url); - t->user_from_url = NULL; - } - - if (t->pass_from_url) { - git__free(t->pass_from_url); - t->pass_from_url = NULL; - } - - if (t->path) { - git__free(t->path); - t->path = NULL; - } -} - static int on_headers_complete(http_parser *parser) { parser_context *ctx = (parser_context *) parser->data; @@ -305,7 +270,7 @@ static int on_headers_complete(http_parser *parser) if (t->owner->cred_acquire_cb(&t->cred, t->owner->url, - t->user_from_url, + t->connection_data.user, allowed_types, t->owner->cred_acquire_payload) < 0) return PARSE_ERROR_GENERIC; @@ -324,26 +289,15 @@ static int on_headers_complete(http_parser *parser) (parser->status_code == 303 && get_verb == s->verb) || parser->status_code == 307) && t->location) { - gitno_connection_data connection_data = {0}; if (s->redirect_count >= 7) { giterr_set(GITERR_NET, "Too many redirects"); return t->parse_error = PARSE_ERROR_GENERIC; } - if (gitno_connection_data_from_url(&connection_data, t->location, - s->service_url, t->host, t->use_ssl) < 0) { - gitno_connection_data_free_ptrs(&connection_data); + if (gitno_connection_data_from_url(&t->connection_data, t->location, + s->service_url, t->connection_data.host, t->connection_data.use_ssl) < 0) return t->parse_error = PARSE_ERROR_GENERIC; - } - - free_connection_data(t); - t->host = connection_data.host; - t->port = connection_data.port; - t->path = connection_data.path; - t->user_from_url = connection_data.user; - t->pass_from_url = connection_data.pass; - t->use_ssl = connection_data.use_ssl; /* Set the redirect URL on the stream. This is a transfer of * ownership of the memory. */ @@ -500,7 +454,7 @@ static int http_connect(http_subtransport *t) if (t->socket.socket) gitno_close(&t->socket); - if (t->use_ssl) { + if (t->connection_data.use_ssl) { int tflags; if (t->owner->parent.read_flags(&t->owner->parent, &tflags) < 0) @@ -512,7 +466,7 @@ static int http_connect(http_subtransport *t) flags |= GITNO_CONNECT_SSL_NO_CHECK_CERT; } - if (gitno_connect(&t->socket, t->host, t->port, flags) < 0) + if (gitno_connect(&t->socket, t->connection_data.host, t->connection_data.port, flags) < 0) return -1; t->connected = 1; @@ -859,20 +813,9 @@ static int http_action( if (!stream) return -1; - if (!t->host || !t->port || !t->path) { - gitno_connection_data data = {0}; - if ((ret = gitno_connection_data_from_url(&data, - url, NULL, NULL, false)) < 0) { - gitno_connection_data_free_ptrs(&data); - return ret; - } - t->host = data.host; - t->port = data.port; - t->path = data.path; - t->user_from_url = data.user; - t->pass_from_url = data.pass; - t->use_ssl = data.use_ssl; - } + if ((!t->connection_data.host || !t->connection_data.port || !t->connection_data.path) && + (ret = gitno_connection_data_from_url(&t->connection_data, url, NULL, NULL, false)) < 0) + return ret; if (http_connect(t) < 0) return -1; @@ -916,7 +859,7 @@ static int http_close(git_smart_subtransport *subtransport) t->url_cred = NULL; } - free_connection_data(t); + gitno_connection_data_free_ptrs(&t->connection_data); return 0; } From ea59f6597707107940d2a615b5fc4621d62149d3 Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Thu, 26 Sep 2013 16:20:30 -0700 Subject: [PATCH 05/37] Deploy gitno_connection_data into transport (winhttp) ...and have that call manage replaced memory in the output structure. --- src/netops.c | 22 ++++++--- src/netops.h | 4 +- src/transports/winhttp.c | 85 +++++++---------------------------- tests-clar/network/urlparse.c | 10 +++-- 4 files changed, 38 insertions(+), 83 deletions(-) diff --git a/src/netops.c b/src/netops.c index bda064cfb..aca46245d 100644 --- a/src/netops.c +++ b/src/netops.c @@ -579,23 +579,29 @@ static const char *prefix_https = "https://"; int gitno_connection_data_from_url( gitno_connection_data *data, const char *url, - const char *service_suffix, - const char *original_host, - bool original_use_ssl) + const char *service_suffix) { - int error = 0; + int error = -1; const char *default_port = NULL; + char *original_host = NULL; + bool original_use_ssl; /* service_suffix is optional */ assert(data && url); + /* Save these for comparison later */ + if (data->host) + original_host = git__strdup(data->host); + original_use_ssl = data->use_ssl; + gitno_connection_data_free_ptrs(data); + if (!git__prefixcmp(url, prefix_http)) { url = url + strlen(prefix_http); default_port = "80"; if (data->use_ssl) { giterr_set(GITERR_NET, "Redirect from HTTPS to HTTP is not allowed"); - return -1; + goto cleanup; } } @@ -607,7 +613,7 @@ int gitno_connection_data_from_url( if (!default_port) { giterr_set(GITERR_NET, "Unrecognized URL prefix"); - return -1; + goto cleanup; } error = gitno_extract_url_parts( @@ -620,7 +626,7 @@ int gitno_connection_data_from_url( size_t suffixlen = service_suffix ? strlen(service_suffix) : 0; if (suffixlen && - !memcmp(path + pathlen - suffixlen, service_suffix, suffixlen)) + !memcmp(path + pathlen - suffixlen, service_suffix, suffixlen)) data->path = git__strndup(path, pathlen - suffixlen); else data->path = git__strdup(path); @@ -636,6 +642,8 @@ int gitno_connection_data_from_url( } } +cleanup: + if (original_host) git__free(original_host); return error; } diff --git a/src/netops.h b/src/netops.h index 0c6e571d9..5c105d6e3 100644 --- a/src/netops.h +++ b/src/netops.h @@ -84,9 +84,7 @@ typedef struct gitno_connection_data { int gitno_connection_data_from_url( gitno_connection_data *data, const char *url, - const char *service_suffix, - const char *original_host, - bool original_use_ssl); + const char *service_suffix); /* This frees all the pointers IN the struct, but not the struct itself. */ void gitno_connection_data_free_ptrs(gitno_connection_data *data); diff --git a/src/transports/winhttp.c b/src/transports/winhttp.c index 7ec936d69..e2a4acf61 100644 --- a/src/transports/winhttp.c +++ b/src/transports/winhttp.c @@ -73,17 +73,12 @@ typedef struct { typedef struct { git_smart_subtransport parent; transport_smart *owner; - char *path; - char *host; - char *port; - char *user_from_url; - char *pass_from_url; + gitno_connection_data connection_data; git_cred *cred; git_cred *url_cred; int auth_mechanism; HINTERNET session; HINTERNET connection; - unsigned use_ssl : 1; } winhttp_subtransport; static int apply_basic_credential(HINTERNET request, git_cred *cred) @@ -155,7 +150,7 @@ static int winhttp_stream_connect(winhttp_stream *s) unsigned long disable_redirects = WINHTTP_DISABLE_REDIRECTS; /* Prepare URL */ - git_buf_printf(&buf, "%s%s", t->path, s->service_url); + git_buf_printf(&buf, "%s%s", t->connection_data.path, s->service_url); if (git_buf_oom(&buf)) return -1; @@ -188,7 +183,7 @@ static int winhttp_stream_connect(winhttp_stream *s) NULL, WINHTTP_NO_REFERER, types, - t->use_ssl ? WINHTTP_FLAG_SECURE : 0); + t->connection_data.use_ssl ? WINHTTP_FLAG_SECURE : 0); if (!s->request) { giterr_set(GITERR_OS, "Failed to open request"); @@ -196,7 +191,7 @@ static int winhttp_stream_connect(winhttp_stream *s) } /* Set proxy if necessary */ - if (git_remote__get_http_proxy(t->owner->owner, !!t->use_ssl, &proxy_url) < 0) + if (git_remote__get_http_proxy(t->owner->owner, !!t->connection_data.use_ssl, &proxy_url) < 0) goto on_error; if (proxy_url) { @@ -285,7 +280,7 @@ static int winhttp_stream_connect(winhttp_stream *s) } /* If requested, disable certificate validation */ - if (t->use_ssl) { + if (t->connection_data.use_ssl) { int flags; if (t->owner->parent.read_flags(&t->owner->parent, &flags) < 0) @@ -308,9 +303,9 @@ static int winhttp_stream_connect(winhttp_stream *s) /* If no other credentials have been applied and the URL has username and * password, use those */ - if (!t->cred && t->user_from_url && t->pass_from_url) { + if (!t->cred && t->connection_data.user && t->connection_data.pass) { if (!t->url_cred && - git_cred_userpass_plaintext_new(&t->url_cred, t->user_from_url, t->pass_from_url) < 0) + git_cred_userpass_plaintext_new(&t->url_cred, t->connection_data.user, t->connection_data.pass) < 0) goto on_error; if (apply_basic_credential(s->request, t->url_cred) < 0) goto on_error; @@ -392,34 +387,6 @@ static int write_chunk(HINTERNET request, const char *buffer, size_t len) return 0; } -static void free_connection_data(winhttp_subtransport *t) -{ - if (t->host) { - git__free(t->host); - t->host = NULL; - } - - if (t->port) { - git__free(t->port); - t->port = NULL; - } - - if (t->user_from_url) { - git__free(t->user_from_url); - t->user_from_url = NULL; - } - - if (t->pass_from_url) { - git__free(t->pass_from_url); - t->pass_from_url = NULL; - } - - if (t->path) { - git__free(t->path); - t->path = NULL; - } -} - static int winhttp_connect( winhttp_subtransport *t, const char *url) @@ -430,11 +397,11 @@ static int winhttp_connect( const char *default_port = "80"; /* Prepare port */ - if (git__strtol32(&port, t->port, NULL, 10) < 0) + if (git__strtol32(&port, t->connection_data.port, NULL, 10) < 0) return -1; /* Prepare host */ - git_win32_path_from_c(host, t->host); + git_win32_path_from_c(host, t->connection_data.host); /* Establish session */ t->session = WinHttpOpen( @@ -636,17 +603,8 @@ replay: if (!git__prefixcmp_icase(location8, prefix_https)) { /* Upgrade to secure connection; disconnect and start over */ gitno_connection_data data = { 0 }; - if (gitno_connection_data_from_url(&data, location8, s->service_url, t->host, t->use_ssl) < 0) { - gitno_connection_data_free_ptrs(&data); + if (gitno_connection_data_from_url(&data, location8, s->service_url) < 0) return -1; - } - free_connection_data(t); - t->host = data.host; - t->port = data.port; - t->path = data.path; - t->user_from_url = data.user; - t->pass_from_url = data.pass; - t->use_ssl = data.use_ssl; winhttp_connect(t, location8); } @@ -665,7 +623,8 @@ replay: if (allowed_types && (!t->cred || 0 == (t->cred->credtype & allowed_types))) { - if (t->owner->cred_acquire_cb(&t->cred, t->owner->url, t->user_from_url, allowed_types, t->owner->cred_acquire_payload) < 0) + if (t->owner->cred_acquire_cb(&t->cred, t->owner->url, t->connection_data.user, allowed_types, + t->owner->cred_acquire_payload) < 0) return -1; assert(t->cred); @@ -1048,22 +1007,10 @@ static int winhttp_action( winhttp_stream *s; int ret = -1; - if (!t->connection) { - gitno_connection_data data = { 0 }; - if (gitno_connection_data_from_url(&data, url, NULL, NULL, false) < 0) { - gitno_connection_data_free_ptrs(&data); + if (!t->connection) + if (gitno_connection_data_from_url(&t->connection_data, url, NULL) < 0 || + winhttp_connect(t, url) < 0) return -1; - } - free_connection_data(t); - t->host = data.host; - t->port = data.port; - t->path = data.path; - t->user_from_url = data.user; - t->pass_from_url = data.pass; - t->use_ssl = data.use_ssl; - if (winhttp_connect(t, url) < 0) - return -1; - } if (winhttp_stream_alloc(t, &s) < 0) return -1; @@ -1104,7 +1051,7 @@ static int winhttp_close(git_smart_subtransport *subtransport) winhttp_subtransport *t = (winhttp_subtransport *)subtransport; int ret = 0; - free_connection_data(t); + gitno_connection_data_free_ptrs(&t->connection_data); if (t->cred) { t->cred->free(t->cred); diff --git a/tests-clar/network/urlparse.c b/tests-clar/network/urlparse.c index 66f714823..679197d6c 100644 --- a/tests-clar/network/urlparse.c +++ b/tests-clar/network/urlparse.c @@ -88,7 +88,7 @@ void test_network_urlparse__user_pass_port(void) void test_network_urlparse__connection_data_http(void) { cl_git_pass(gitno_connection_data_from_url(&conndata, - "http://example.com/foo/bar/baz", "bar/baz", NULL, false)); + "http://example.com/foo/bar/baz", "bar/baz")); cl_assert_equal_s(conndata.host, "example.com"); cl_assert_equal_s(conndata.port, "80"); cl_assert_equal_s(conndata.path, "/foo/"); @@ -100,7 +100,7 @@ void test_network_urlparse__connection_data_http(void) void test_network_urlparse__connection_data_ssl(void) { cl_git_pass(gitno_connection_data_from_url(&conndata, - "https://example.com/foo/bar/baz", "bar/baz", NULL, false)); + "https://example.com/foo/bar/baz", "bar/baz")); cl_assert_equal_s(conndata.host, "example.com"); cl_assert_equal_s(conndata.port, "443"); cl_assert_equal_s(conndata.path, "/foo/"); @@ -111,14 +111,16 @@ void test_network_urlparse__connection_data_ssl(void) void test_network_urlparse__connection_data_cross_host_redirect(void) { + conndata.host = git__strdup("bar.com"); cl_git_fail_with(gitno_connection_data_from_url(&conndata, - "https://foo.com/bar/baz", NULL, "bar.com", true), + "https://foo.com/bar/baz", NULL), -1); } void test_network_urlparse__connection_data_http_downgrade(void) { + conndata.use_ssl = true; cl_git_fail_with(gitno_connection_data_from_url(&conndata, - "http://foo.com/bar/baz", NULL, NULL, true), + "http://foo.com/bar/baz", NULL), -1); } From 1b02baf40be4913a4f3044987eea85c72ce4b0a7 Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Thu, 26 Sep 2013 16:25:05 -0700 Subject: [PATCH 06/37] Adjust to new utility signature --- src/transports/http.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/transports/http.c b/src/transports/http.c index 81157889e..ace0d97d0 100644 --- a/src/transports/http.c +++ b/src/transports/http.c @@ -295,8 +295,7 @@ static int on_headers_complete(http_parser *parser) return t->parse_error = PARSE_ERROR_GENERIC; } - if (gitno_connection_data_from_url(&t->connection_data, t->location, - s->service_url, t->connection_data.host, t->connection_data.use_ssl) < 0) + if (gitno_connection_data_from_url(&t->connection_data, t->location, s->service_url) < 0) return t->parse_error = PARSE_ERROR_GENERIC; /* Set the redirect URL on the stream. This is a transfer of @@ -814,7 +813,7 @@ static int http_action( return -1; if ((!t->connection_data.host || !t->connection_data.port || !t->connection_data.path) && - (ret = gitno_connection_data_from_url(&t->connection_data, url, NULL, NULL, false)) < 0) + (ret = gitno_connection_data_from_url(&t->connection_data, url, NULL)) < 0) return ret; if (http_connect(t) < 0) From 0049d4d1d2dd4cb6d65ea81ecdc8611a41f0bace Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Thu, 26 Sep 2013 16:25:34 -0700 Subject: [PATCH 07/37] Make sure utility doesn't leak memory --- tests-clar/network/urlparse.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests-clar/network/urlparse.c b/tests-clar/network/urlparse.c index 679197d6c..8892781ad 100644 --- a/tests-clar/network/urlparse.c +++ b/tests-clar/network/urlparse.c @@ -124,3 +124,12 @@ void test_network_urlparse__connection_data_http_downgrade(void) "http://foo.com/bar/baz", NULL), -1); } + +/* Run this under valgrind */ +void test_network_urlparse__connection_data_cleanup(void) +{ + cl_git_pass(gitno_connection_data_from_url(&conndata, + "http://foo.com/bar/baz/biff", "baz/biff")); + cl_git_pass(gitno_connection_data_from_url(&conndata, + "https://foo.com/bar/baz/biff", "baz/biff")); +} From 256961e45d574ace62a7a7d13b697aa05e8a9466 Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Thu, 26 Sep 2013 16:36:05 -0700 Subject: [PATCH 08/37] WHOOPS --- src/transports/winhttp.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/transports/winhttp.c b/src/transports/winhttp.c index e2a4acf61..067d6fcc3 100644 --- a/src/transports/winhttp.c +++ b/src/transports/winhttp.c @@ -602,8 +602,7 @@ replay: if (!git__prefixcmp_icase(location8, prefix_https)) { /* Upgrade to secure connection; disconnect and start over */ - gitno_connection_data data = { 0 }; - if (gitno_connection_data_from_url(&data, location8, s->service_url) < 0) + if (gitno_connection_data_from_url(&t->connection_data, location8, s->service_url) < 0) return -1; winhttp_connect(t, location8); } From b59344bf83049a5639c32ab52efceea2eec9484b Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Thu, 26 Sep 2013 16:48:08 -0700 Subject: [PATCH 09/37] Tighten up url-connection utility --- src/netops.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/netops.c b/src/netops.c index aca46245d..3257e7749 100644 --- a/src/netops.c +++ b/src/netops.c @@ -584,15 +584,13 @@ int gitno_connection_data_from_url( int error = -1; const char *default_port = NULL; char *original_host = NULL; - bool original_use_ssl; /* service_suffix is optional */ assert(data && url); /* Save these for comparison later */ - if (data->host) - original_host = git__strdup(data->host); - original_use_ssl = data->use_ssl; + original_host = data->host; + data->host = NULL; gitno_connection_data_free_ptrs(data); if (!git__prefixcmp(url, prefix_http)) { @@ -632,10 +630,6 @@ int gitno_connection_data_from_url( data->path = git__strdup(path); /* Check for errors in the resulting data */ - if (original_use_ssl && !data->use_ssl) { - giterr_set(GITERR_NET, "Redirect from HTTPS to HTTP not allowed"); - error = -1; - } if (original_host && url[0] != '/' && strcmp(original_host, data->host)) { giterr_set(GITERR_NET, "Cross host redirect not allowed"); error = -1; From 7d6924541508b453c22fac2ac3bc76fadc0a58ba Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Fri, 27 Sep 2013 16:08:24 -0400 Subject: [PATCH 10/37] Add refdb.h to git2.h, reorder git2.h sanely --- include/git2.h | 95 +++++++++++++++++++++++--------------------------- 1 file changed, 44 insertions(+), 51 deletions(-) diff --git a/include/git2.h b/include/git2.h index 73c11ad83..93049aad9 100644 --- a/include/git2.h +++ b/include/git2.h @@ -8,57 +8,50 @@ #ifndef INCLUDE_git_git_h__ #define INCLUDE_git_git_h__ +#include "git2/attr.h" +#include "git2/blob.h" +#include "git2/branch.h" +#include "git2/buffer.h" +#include "git2/checkout.h" +#include "git2/clone.h" +#include "git2/commit.h" +#include "git2/common.h" +#include "git2/config.h" +#include "git2/diff.h" +#include "git2/errors.h" +#include "git2/filter.h" +#include "git2/graph.h" +#include "git2/ignore.h" +#include "git2/index.h" +#include "git2/indexer.h" +#include "git2/merge.h" +#include "git2/message.h" +#include "git2/net.h" +#include "git2/notes.h" +#include "git2/object.h" +#include "git2/odb.h" +#include "git2/oid.h" +#include "git2/pack.h" +#include "git2/pathspec.h" +#include "git2/push.h" +#include "git2/refdb.h" +#include "git2/reflog.h" +#include "git2/refs.h" +#include "git2/refspec.h" +#include "git2/remote.h" +#include "git2/repository.h" +#include "git2/reset.h" +#include "git2/revparse.h" +#include "git2/revwalk.h" +#include "git2/signature.h" +#include "git2/stash.h" +#include "git2/status.h" +#include "git2/submodule.h" +#include "git2/tag.h" +#include "git2/threads.h" +#include "git2/transport.h" +#include "git2/tree.h" +#include "git2/types.h" #include "git2/version.h" -#include "git2/common.h" -#include "git2/threads.h" -#include "git2/errors.h" - -#include "git2/types.h" - -#include "git2/oid.h" -#include "git2/signature.h" -#include "git2/odb.h" - -#include "git2/repository.h" -#include "git2/revwalk.h" -#include "git2/merge.h" -#include "git2/graph.h" -#include "git2/refs.h" -#include "git2/reflog.h" -#include "git2/revparse.h" - -#include "git2/object.h" -#include "git2/blob.h" -#include "git2/commit.h" -#include "git2/tag.h" -#include "git2/tree.h" -#include "git2/diff.h" - -#include "git2/index.h" -#include "git2/config.h" -#include "git2/transport.h" -#include "git2/remote.h" -#include "git2/clone.h" -#include "git2/checkout.h" -#include "git2/push.h" - -#include "git2/attr.h" -#include "git2/ignore.h" -#include "git2/branch.h" -#include "git2/refspec.h" -#include "git2/net.h" -#include "git2/status.h" -#include "git2/indexer.h" -#include "git2/submodule.h" -#include "git2/notes.h" -#include "git2/reset.h" -#include "git2/message.h" -#include "git2/pack.h" -#include "git2/stash.h" -#include "git2/pathspec.h" - -#include "git2/buffer.h" -#include "git2/filter.h" - #endif From 4fe0b0b34b3700d6da681498f0077a455d14812b Mon Sep 17 00:00:00 2001 From: Justin Spahr-Summers Date: Fri, 27 Sep 2013 17:07:06 -0700 Subject: [PATCH 11/37] Never consider submodules for stashing --- src/stash.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/stash.c b/src/stash.c index ab4a68575..7742eee19 100644 --- a/src/stash.c +++ b/src/stash.c @@ -316,6 +316,8 @@ static int build_workdir_tree( struct cb_data data = {0}; int error; + opts.flags = GIT_DIFF_IGNORE_SUBMODULES; + if ((error = git_commit_tree(&b_tree, b_commit)) < 0) goto cleanup; @@ -474,12 +476,14 @@ static int ensure_there_are_changes_to_stash( git_status_options opts = GIT_STATUS_OPTIONS_INIT; opts.show = GIT_STATUS_SHOW_INDEX_AND_WORKDIR; + opts.flags = GIT_STATUS_OPT_EXCLUDE_SUBMODULES; + if (include_untracked_files) - opts.flags = GIT_STATUS_OPT_INCLUDE_UNTRACKED | + opts.flags |= GIT_STATUS_OPT_INCLUDE_UNTRACKED | GIT_STATUS_OPT_RECURSE_UNTRACKED_DIRS; if (include_ignored_files) - opts.flags = GIT_STATUS_OPT_INCLUDE_IGNORED; + opts.flags |= GIT_STATUS_OPT_INCLUDE_IGNORED; error = git_status_foreach_ext(repo, &opts, is_dirty_cb, NULL); From 526d4c949c4b87c01e74a19ab47171ee08c9673a Mon Sep 17 00:00:00 2001 From: Justin Spahr-Summers Date: Fri, 27 Sep 2013 21:39:28 -0700 Subject: [PATCH 12/37] Test that submodules don't affect stashing --- tests-clar/stash/save.c | 68 ++++++++++++-------------------- tests-clar/stash/stash_helpers.c | 19 +++++++++ tests-clar/stash/stash_helpers.h | 5 +++ tests-clar/stash/submodules.c | 68 ++++++++++++++++++++++++++++++++ 4 files changed, 117 insertions(+), 43 deletions(-) create mode 100644 tests-clar/stash/submodules.c diff --git a/tests-clar/stash/save.c b/tests-clar/stash/save.c index 035b62279..3d92b26bd 100644 --- a/tests-clar/stash/save.c +++ b/tests-clar/stash/save.c @@ -113,33 +113,15 @@ $ git status --short cl_assert_equal_i(GIT_STATUS_WT_NEW, status); } -static void assert_status( - const char *path, - int status_flags) -{ - unsigned int status; - int error; - - error = git_status_file(&status, repo, path); - - if (status_flags < 0) { - cl_assert_equal_i(status_flags, error); - return; - } - - cl_assert_equal_i(0, error); - cl_assert_equal_i((unsigned int)status_flags, status); -} - void test_stash_save__can_keep_index(void) { cl_git_pass(git_stash_save(&stash_tip_oid, repo, signature, NULL, GIT_STASH_KEEP_INDEX)); - assert_status("what", GIT_STATUS_INDEX_MODIFIED); - assert_status("how", GIT_STATUS_INDEX_MODIFIED); - assert_status("who", GIT_STATUS_CURRENT); - assert_status("when", GIT_STATUS_WT_NEW); - assert_status("just.ignore", GIT_STATUS_IGNORED); + assert_status(repo, "what", GIT_STATUS_INDEX_MODIFIED); + assert_status(repo, "how", GIT_STATUS_INDEX_MODIFIED); + assert_status(repo, "who", GIT_STATUS_CURRENT); + assert_status(repo, "when", GIT_STATUS_WT_NEW); + assert_status(repo, "just.ignore", GIT_STATUS_IGNORED); } static void assert_commit_message_contains(const char *revision, const char *fragment) @@ -308,25 +290,25 @@ void test_stash_save__can_stage_normal_then_stage_untracked(void) * 100644 blob b6ed15e81e2593d7bb6265eb4a991d29dc3e628b when */ - assert_status("what", GIT_STATUS_WT_MODIFIED | GIT_STATUS_INDEX_MODIFIED); - assert_status("how", GIT_STATUS_INDEX_MODIFIED); - assert_status("who", GIT_STATUS_WT_MODIFIED); - assert_status("when", GIT_STATUS_WT_NEW); - assert_status("just.ignore", GIT_STATUS_IGNORED); + assert_status(repo, "what", GIT_STATUS_WT_MODIFIED | GIT_STATUS_INDEX_MODIFIED); + assert_status(repo, "how", GIT_STATUS_INDEX_MODIFIED); + assert_status(repo, "who", GIT_STATUS_WT_MODIFIED); + assert_status(repo, "when", GIT_STATUS_WT_NEW); + assert_status(repo, "just.ignore", GIT_STATUS_IGNORED); cl_git_pass(git_stash_save(&stash_tip_oid, repo, signature, NULL, GIT_STASH_DEFAULT)); - assert_status("what", GIT_STATUS_CURRENT); - assert_status("how", GIT_STATUS_CURRENT); - assert_status("who", GIT_STATUS_CURRENT); - assert_status("when", GIT_STATUS_WT_NEW); - assert_status("just.ignore", GIT_STATUS_IGNORED); + assert_status(repo, "what", GIT_STATUS_CURRENT); + assert_status(repo, "how", GIT_STATUS_CURRENT); + assert_status(repo, "who", GIT_STATUS_CURRENT); + assert_status(repo, "when", GIT_STATUS_WT_NEW); + assert_status(repo, "just.ignore", GIT_STATUS_IGNORED); cl_git_pass(git_stash_save(&stash_tip_oid, repo, signature, NULL, GIT_STASH_INCLUDE_UNTRACKED)); - assert_status("what", GIT_STATUS_CURRENT); - assert_status("how", GIT_STATUS_CURRENT); - assert_status("who", GIT_STATUS_CURRENT); - assert_status("when", GIT_ENOTFOUND); - assert_status("just.ignore", GIT_STATUS_IGNORED); + assert_status(repo, "what", GIT_STATUS_CURRENT); + assert_status(repo, "how", GIT_STATUS_CURRENT); + assert_status(repo, "who", GIT_STATUS_CURRENT); + assert_status(repo, "when", GIT_ENOTFOUND); + assert_status(repo, "just.ignore", GIT_STATUS_IGNORED); assert_blob_oid("stash@{1}^0:what", "bc99dc98b3eba0e9157e94769cd4d49cb49de449"); /* see you later */ @@ -360,11 +342,11 @@ void test_stash_save__including_untracked_without_any_untracked_file_creates_an_ { cl_git_pass(p_unlink("stash/when")); - assert_status("what", GIT_STATUS_WT_MODIFIED | GIT_STATUS_INDEX_MODIFIED); - assert_status("how", GIT_STATUS_INDEX_MODIFIED); - assert_status("who", GIT_STATUS_WT_MODIFIED); - assert_status("when", GIT_ENOTFOUND); - assert_status("just.ignore", GIT_STATUS_IGNORED); + assert_status(repo, "what", GIT_STATUS_WT_MODIFIED | GIT_STATUS_INDEX_MODIFIED); + assert_status(repo, "how", GIT_STATUS_INDEX_MODIFIED); + assert_status(repo, "who", GIT_STATUS_WT_MODIFIED); + assert_status(repo, "when", GIT_ENOTFOUND); + assert_status(repo, "just.ignore", GIT_STATUS_IGNORED); cl_git_pass(git_stash_save(&stash_tip_oid, repo, signature, NULL, GIT_STASH_INCLUDE_UNTRACKED)); diff --git a/tests-clar/stash/stash_helpers.c b/tests-clar/stash/stash_helpers.c index 06b63f177..8b7d685f8 100644 --- a/tests-clar/stash/stash_helpers.c +++ b/tests-clar/stash/stash_helpers.c @@ -35,3 +35,22 @@ void setup_stash(git_repository *repo, git_signature *signature) git_index_free(index); } + +void assert_status( + git_repository *repo, + const char *path, + int status_flags) +{ + unsigned int status; + int error; + + error = git_status_file(&status, repo, path); + + if (status_flags < 0) { + cl_assert_equal_i(status_flags, error); + return; + } + + cl_assert_equal_i(0, error); + cl_assert_equal_i((unsigned int)status_flags, status); +} diff --git a/tests-clar/stash/stash_helpers.h b/tests-clar/stash/stash_helpers.h index 7c3e13de3..66d758fe2 100644 --- a/tests-clar/stash/stash_helpers.h +++ b/tests-clar/stash/stash_helpers.h @@ -1,3 +1,8 @@ void setup_stash( git_repository *repo, git_signature *signature); + +void assert_status( + git_repository *repo, + const char *path, + int status_flags); diff --git a/tests-clar/stash/submodules.c b/tests-clar/stash/submodules.c new file mode 100644 index 000000000..60dbbad2e --- /dev/null +++ b/tests-clar/stash/submodules.c @@ -0,0 +1,68 @@ +#include "clar_libgit2.h" +#include "stash_helpers.h" +#include "../submodule/submodule_helpers.h" + +static git_repository *repo; +static git_signature *signature; +static git_oid stash_tip_oid; + +static git_index *smindex; +static git_submodule *sm; +static git_repository *smrepo; + +void test_stash_submodules__initialize(void) +{ + cl_git_pass(git_signature_new(&signature, "nulltoken", "emeric.fermas@gmail.com", 1323847743, 60)); /* Wed Dec 14 08:29:03 2011 +0100 */ + + repo = setup_fixture_submodules(); + + cl_git_pass(git_submodule_lookup(&sm, repo, "testrepo")); + cl_git_pass(git_submodule_open(&smrepo, sm)); + cl_git_pass(git_repository_index(&smindex, smrepo)); +} + +void test_stash_submodules__cleanup(void) +{ + git_signature_free(signature); + signature = NULL; +} + +void test_stash_submodules__does_not_stash_modified_submodules(void) +{ + assert_status(repo, "modified", GIT_STATUS_WT_MODIFIED); + + /* modify file in submodule */ + cl_git_rewritefile("submodules/testrepo/README", "heyheyhey"); + assert_status(repo, "testrepo", GIT_STATUS_WT_MODIFIED); + + /* add file to index in submodule */ + cl_git_pass(git_index_add_bypath(smindex, "README")); + + /* commit changed index of submodule */ + cl_repo_commit_from_index(NULL, smrepo, NULL, 1372350000, "Modify it"); + assert_status(repo, "testrepo", GIT_STATUS_WT_MODIFIED); + + cl_git_pass(git_stash_save(&stash_tip_oid, repo, signature, NULL, GIT_STASH_DEFAULT)); + + assert_status(repo, "testrepo", GIT_STATUS_WT_MODIFIED); + assert_status(repo, "modified", GIT_STATUS_CURRENT); +} + +void test_stash_submodules__stash_is_empty_with_modified_submodules(void) +{ + cl_git_pass(git_stash_save(&stash_tip_oid, repo, signature, NULL, GIT_STASH_DEFAULT)); + assert_status(repo, "modified", GIT_STATUS_CURRENT); + + /* modify file in submodule */ + cl_git_rewritefile("submodules/testrepo/README", "heyheyhey"); + assert_status(repo, "testrepo", GIT_STATUS_WT_MODIFIED); + + /* add file to index in submodule */ + cl_git_pass(git_index_add_bypath(smindex, "README")); + + /* commit changed index of submodule */ + cl_repo_commit_from_index(NULL, smrepo, NULL, 1372350000, "Modify it"); + assert_status(repo, "testrepo", GIT_STATUS_WT_MODIFIED); + + cl_git_fail_with(git_stash_save(&stash_tip_oid, repo, signature, NULL, GIT_STASH_DEFAULT), GIT_ENOTFOUND); +} From 27c8eb2a1a881be12e281b606730ff15a8382ab1 Mon Sep 17 00:00:00 2001 From: nulltoken Date: Mon, 30 Sep 2013 11:18:06 +0200 Subject: [PATCH 13/37] Tabify indentations --- tests-clar/commit/parse.c | 158 +++++++++++++++++++------------------- 1 file changed, 79 insertions(+), 79 deletions(-) diff --git a/tests-clar/commit/parse.c b/tests-clar/commit/parse.c index 415860a6e..e91815def 100644 --- a/tests-clar/commit/parse.c +++ b/tests-clar/commit/parse.c @@ -7,77 +7,77 @@ static git_repository *g_repo; void test_commit_parse__initialize(void) { - g_repo = cl_git_sandbox_init("testrepo"); + g_repo = cl_git_sandbox_init("testrepo"); } void test_commit_parse__cleanup(void) { - cl_git_sandbox_cleanup(); + cl_git_sandbox_cleanup(); } // Header parsing typedef struct { - const char *line; - const char *header; + const char *line; + const char *header; } parse_test_case; static parse_test_case passing_header_cases[] = { - { "parent 05452d6349abcd67aa396dfb28660d765d8b2a36\n", "parent " }, - { "tree 05452d6349abcd67aa396dfb28660d765d8b2a36\n", "tree " }, - { "random_heading 05452d6349abcd67aa396dfb28660d765d8b2a36\n", "random_heading " }, - { "stuck_heading05452d6349abcd67aa396dfb28660d765d8b2a36\n", "stuck_heading" }, - { "tree 5F4BEFFC0759261D015AA63A3A85613FF2F235DE\n", "tree " }, - { "tree 1A669B8AB81B5EB7D9DB69562D34952A38A9B504\n", "tree " }, - { "tree 5B20DCC6110FCC75D31C6CEDEBD7F43ECA65B503\n", "tree " }, - { "tree 173E7BF00EA5C33447E99E6C1255954A13026BE4\n", "tree " }, - { NULL, NULL } + { "parent 05452d6349abcd67aa396dfb28660d765d8b2a36\n", "parent " }, + { "tree 05452d6349abcd67aa396dfb28660d765d8b2a36\n", "tree " }, + { "random_heading 05452d6349abcd67aa396dfb28660d765d8b2a36\n", "random_heading " }, + { "stuck_heading05452d6349abcd67aa396dfb28660d765d8b2a36\n", "stuck_heading" }, + { "tree 5F4BEFFC0759261D015AA63A3A85613FF2F235DE\n", "tree " }, + { "tree 1A669B8AB81B5EB7D9DB69562D34952A38A9B504\n", "tree " }, + { "tree 5B20DCC6110FCC75D31C6CEDEBD7F43ECA65B503\n", "tree " }, + { "tree 173E7BF00EA5C33447E99E6C1255954A13026BE4\n", "tree " }, + { NULL, NULL } }; static parse_test_case failing_header_cases[] = { - { "parent 05452d6349abcd67aa396dfb28660d765d8b2a36", "parent " }, - { "05452d6349abcd67aa396dfb28660d765d8b2a36\n", "tree " }, - { "parent05452d6349abcd67aa396dfb28660d765d8b2a6a\n", "parent " }, - { "parent 05452d6349abcd67aa396dfb280d765d8b2a6\n", "parent " }, - { "tree 05452d6349abcd67aa396dfb28660d765d8b2a36\n", "tree " }, - { "parent 0545xd6349abcd67aa396dfb28660d765d8b2a36\n", "parent " }, - { "parent 0545xd6349abcd67aa396dfb28660d765d8b2a36FF\n", "parent " }, - { "", "tree " }, - { "", "" }, - { NULL, NULL } + { "parent 05452d6349abcd67aa396dfb28660d765d8b2a36", "parent " }, + { "05452d6349abcd67aa396dfb28660d765d8b2a36\n", "tree " }, + { "parent05452d6349abcd67aa396dfb28660d765d8b2a6a\n", "parent " }, + { "parent 05452d6349abcd67aa396dfb280d765d8b2a6\n", "parent " }, + { "tree 05452d6349abcd67aa396dfb28660d765d8b2a36\n", "tree " }, + { "parent 0545xd6349abcd67aa396dfb28660d765d8b2a36\n", "parent " }, + { "parent 0545xd6349abcd67aa396dfb28660d765d8b2a36FF\n", "parent " }, + { "", "tree " }, + { "", "" }, + { NULL, NULL } }; void test_commit_parse__header(void) { - git_oid oid; + git_oid oid; - parse_test_case *testcase; - for (testcase = passing_header_cases; testcase->line != NULL; testcase++) - { - const char *line = testcase->line; - const char *line_end = line + strlen(line); + parse_test_case *testcase; + for (testcase = passing_header_cases; testcase->line != NULL; testcase++) + { + const char *line = testcase->line; + const char *line_end = line + strlen(line); - cl_git_pass(git_oid__parse(&oid, &line, line_end, testcase->header)); - cl_assert(line == line_end); - } + cl_git_pass(git_oid__parse(&oid, &line, line_end, testcase->header)); + cl_assert(line == line_end); + } - for (testcase = failing_header_cases; testcase->line != NULL; testcase++) - { - const char *line = testcase->line; - const char *line_end = line + strlen(line); + for (testcase = failing_header_cases; testcase->line != NULL; testcase++) + { + const char *line = testcase->line; + const char *line_end = line + strlen(line); - cl_git_fail(git_oid__parse(&oid, &line, line_end, testcase->header)); - } + cl_git_fail(git_oid__parse(&oid, &line, line_end, testcase->header)); + } } // Signature parsing typedef struct { - const char *string; - const char *header; - const char *name; - const char *email; - git_time_t time; - int offset; + const char *string; + const char *header; + const char *name; + const char *email; + git_time_t time; + int offset; } passing_signature_test_case; passing_signature_test_case passing_signature_cases[] = { @@ -122,12 +122,12 @@ passing_signature_test_case passing_signature_cases[] = { {"author Vicent Marti 4294967296 \n", "author ", "Vicent Marti", "tanoku@gmail.com", 4294967296, 0}, {"author Vicent Marti 8589934592 \n", "author ", "Vicent Marti", "tanoku@gmail.com", 8589934592, 0}, - {NULL,NULL,NULL,NULL,0,0} + {NULL,NULL,NULL,NULL,0,0} }; typedef struct { - const char *string; - const char *header; + const char *string; + const char *header; } failing_signature_test_case; failing_signature_test_case failing_signature_cases[] = { @@ -143,31 +143,31 @@ failing_signature_test_case failing_signature_cases[] = { void test_commit_parse__signature(void) { - passing_signature_test_case *passcase; - failing_signature_test_case *failcase; + passing_signature_test_case *passcase; + failing_signature_test_case *failcase; - for (passcase = passing_signature_cases; passcase->string != NULL; passcase++) - { - const char *str = passcase->string; - size_t len = strlen(passcase->string); - struct git_signature person = {0}; + for (passcase = passing_signature_cases; passcase->string != NULL; passcase++) + { + const char *str = passcase->string; + size_t len = strlen(passcase->string); + struct git_signature person = {0}; - cl_git_pass(git_signature__parse(&person, &str, str + len, passcase->header, '\n')); - cl_assert_equal_s(passcase->name, person.name); - cl_assert_equal_s(passcase->email, person.email); - cl_assert_equal_i((int)passcase->time, (int)person.when.time); - cl_assert_equal_i(passcase->offset, person.when.offset); - git__free(person.name); git__free(person.email); - } + cl_git_pass(git_signature__parse(&person, &str, str + len, passcase->header, '\n')); + cl_assert_equal_s(passcase->name, person.name); + cl_assert_equal_s(passcase->email, person.email); + cl_assert_equal_i((int)passcase->time, (int)person.when.time); + cl_assert_equal_i(passcase->offset, person.when.offset); + git__free(person.name); git__free(person.email); + } - for (failcase = failing_signature_cases; failcase->string != NULL; failcase++) - { - const char *str = failcase->string; - size_t len = strlen(failcase->string); - git_signature person = {0}; - cl_git_fail(git_signature__parse(&person, &str, str + len, failcase->header, '\n')); - git__free(person.name); git__free(person.email); - } + for (failcase = failing_signature_cases; failcase->string != NULL; failcase++) + { + const char *str = failcase->string; + size_t len = strlen(failcase->string); + git_signature person = {0}; + cl_git_fail(git_signature__parse(&person, &str, str + len, failcase->header, '\n')); + git__free(person.name); git__free(person.email); + } } @@ -312,17 +312,17 @@ void test_commit_parse__entire_commit(void) // query the details on a parsed commit void test_commit_parse__details0(void) { - static const char *commit_ids[] = { - "a4a7dce85cf63874e984719f4fdd239f5145052f", /* 0 */ - "9fd738e8f7967c078dceed8190330fc8648ee56a", /* 1 */ - "4a202b346bb0fb0db7eff3cffeb3c70babbd2045", /* 2 */ - "c47800c7266a2be04c571c04d5a6614691ea99bd", /* 3 */ - "8496071c1b46c854b31185ea97743be6a8774479", /* 4 */ - "5b5b025afb0b4c913b4c338a42934a3863bf3644", /* 5 */ - "a65fedf39aefe402d3bb6e24df4d4f5fe4547750", /* 6 */ - }; + static const char *commit_ids[] = { + "a4a7dce85cf63874e984719f4fdd239f5145052f", /* 0 */ + "9fd738e8f7967c078dceed8190330fc8648ee56a", /* 1 */ + "4a202b346bb0fb0db7eff3cffeb3c70babbd2045", /* 2 */ + "c47800c7266a2be04c571c04d5a6614691ea99bd", /* 3 */ + "8496071c1b46c854b31185ea97743be6a8774479", /* 4 */ + "5b5b025afb0b4c913b4c338a42934a3863bf3644", /* 5 */ + "a65fedf39aefe402d3bb6e24df4d4f5fe4547750", /* 6 */ + }; const size_t commit_count = sizeof(commit_ids) / sizeof(const char *); - unsigned int i; + unsigned int i; for (i = 0; i < commit_count; ++i) { git_oid id; From d27a441ddefff8934e43a546b23f582437ba4399 Mon Sep 17 00:00:00 2001 From: nulltoken Date: Mon, 30 Sep 2013 11:30:28 +0200 Subject: [PATCH 14/37] commit: Trim message leading newlines Fix libgit2/libgit2sharp#522 --- src/commit.c | 2 +- tests-clar/commit/parse.c | 23 +++++++++++++++++++---- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/commit.c b/src/commit.c index 15a195fe5..ab475a8f8 100644 --- a/src/commit.c +++ b/src/commit.c @@ -240,7 +240,7 @@ int git_commit__parse(void *_commit, git_odb_object *odb_obj) buffer_end = buffer + git_odb_object_size(odb_obj); buffer += header_len; - if (*buffer == '\n') + while (buffer < buffer_end && *buffer == '\n') ++buffer; /* extract commit message */ diff --git a/tests-clar/commit/parse.c b/tests-clar/commit/parse.c index e91815def..c191b3421 100644 --- a/tests-clar/commit/parse.c +++ b/tests-clar/commit/parse.c @@ -349,7 +349,6 @@ void test_commit_parse__details0(void) { cl_assert_equal_s("Scott Chacon", committer->name); cl_assert_equal_s("schacon@gmail.com", committer->email); cl_assert(message != NULL); - cl_assert(strchr(message, '\n') != NULL); cl_assert(commit_time > 0); cl_assert(parents <= 2); for (p = 0;p < parents;p++) { @@ -382,9 +381,25 @@ committer Vicent Marti 1273848544 +0200\n\ \n\ This commit has a few LF at the start of the commit message"; const char *message = -"\n\ -\n\ -This commit has a few LF at the start of the commit message"; +"This commit has a few LF at the start of the commit message"; + + cl_git_pass(parse_commit(&commit, buffer)); + cl_assert_equal_s(message, git_commit_message(commit)); + git_commit__free(commit); +} + +void test_commit_parse__only_lf(void) +{ + git_commit *commit; + const char *buffer = +"tree 1810dff58d8a660512d4832e740f692884338ccd\n\ +parent e90810b8df3e80c413d903f631643c716887138d\n\ +author Vicent Marti 1273848544 +0200\n\ +committer Vicent Marti 1273848544 +0200\n\ +\n\ +\n\ +\n"; + const char *message = ""; cl_git_pass(parse_commit(&commit, buffer)); cl_assert_equal_s(message, git_commit_message(commit)); From b176ededb7d226ac85809b3ec594d185e7e3e866 Mon Sep 17 00:00:00 2001 From: Jameson Miller Date: Thu, 19 Sep 2013 14:52:57 -0400 Subject: [PATCH 15/37] Initial Implementation of progress reports during push This adds the basics of progress reporting during push. While progress for all aspects of a push operation are not reported with this change, it lays the foundation to add these later. Push progress reporting can be improved in the future - and consumers of the API should just get more accurate information at that point. The main areas where this is lacking are: 1) packbuilding progress: does not report progress during deltafication, as this involves coordinating progress from multiple threads. 2) network progress: reports progress as objects and bytes are going to be written to the subtransport (instead of as client gets confirmation that they have been received by the server) and leaves out some of the bytes that are transfered as part of the push protocol. Basically, this reports the pack bytes that are written to the subtransport. It does not report the bytes sent on the wire that are received by the server. This should be a good estimate of progress (and an improvement over no progress). --- CMakeLists.txt | 2 + include/git2/pack.h | 30 ++++++++++++ include/git2/push.h | 29 ++++++++++++ src/pack-objects.c | 30 ++++++++++++ src/pack-objects.h | 5 ++ src/push.c | 23 +++++++++ src/push.h | 5 ++ src/transports/smart_protocol.c | 51 +++++++++++++++++--- src/util.h | 61 ++++++++++++++++++++++++ tests-clar/online/push.c | 84 +++++++++++++++++++++------------ 10 files changed, 283 insertions(+), 37 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5c09b4178..df3936496 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -56,6 +56,8 @@ FUNCTION(TARGET_OS_LIBRARIES target) TARGET_LINK_LIBRARIES(${target} ws2_32) ELSEIF(CMAKE_SYSTEM_NAME MATCHES "(Solaris|SunOS)") TARGET_LINK_LIBRARIES(${target} socket nsl) + ELSEIF(CMAKE_SYSTEM_NAME MATCHES "Linux") + TARGET_LINK_LIBRARIES(${target} rt) ENDIF () IF(THREADSAFE) TARGET_LINK_LIBRARIES(${target} ${CMAKE_THREAD_LIBS_INIT}) diff --git a/include/git2/pack.h b/include/git2/pack.h index 976e39cb4..7ebdd5cf3 100644 --- a/include/git2/pack.h +++ b/include/git2/pack.h @@ -45,6 +45,14 @@ */ GIT_BEGIN_DECL +/** + * Stages that are reported by the packbuilder progress callback. + */ +typedef enum { + GIT_PACKBUILDER_ADDING_OBJECTS = 0, + GIT_PACKBUILDER_DELTAFICATION = 1, +} git_packbuilder_stage_t; + /** * Initialize a new packbuilder * @@ -149,6 +157,28 @@ GIT_EXTERN(uint32_t) git_packbuilder_object_count(git_packbuilder *pb); */ GIT_EXTERN(uint32_t) git_packbuilder_written(git_packbuilder *pb); +/** Packbuilder progress notification function */ +typedef void (*git_packbuilder_progress)( + int stage, + unsigned int current, + unsigned int total, + void *payload); + +/** + * Set the callbacks for a packbuilder + * + * @param pb The packbuilder object + * @param progress_cb Function to call with progress information during + * pack building. Be aware that this is called inline with pack building + * operations, so performance may be affected. + * @param progress_cb_payload Payload for progress callback. + * @return 0 or an error code + */ +GIT_EXTERN(int) git_packbuilder_set_callbacks( + git_packbuilder *pb, + git_packbuilder_progress progress_cb, + void *progress_cb_payload); + /** * Free the packbuilder and all associated data * diff --git a/include/git2/push.h b/include/git2/push.h index ed6253afb..ecfd862d4 100644 --- a/include/git2/push.h +++ b/include/git2/push.h @@ -8,6 +8,7 @@ #define INCLUDE_git_push_h__ #include "common.h" +#include "pack.h" /** * @file git2/push.h @@ -38,6 +39,13 @@ typedef struct { #define GIT_PUSH_OPTIONS_VERSION 1 #define GIT_PUSH_OPTIONS_INIT { GIT_PUSH_OPTIONS_VERSION } +/** Push network progress notification function */ +typedef void (*git_push_transfer_progress)( + unsigned int current, + unsigned int total, + size_t bytes, + void* payload); + /** * Create a new push object * @@ -60,6 +68,27 @@ GIT_EXTERN(int) git_push_set_options( git_push *push, const git_push_options *opts); +/** + * Set the callbacks for a push + * + * @param push The push object + * @param pack_progress_cb Function to call with progress information during + * pack building. Be aware that this is called inline with pack building + * operations, so performance may be affected. + * @param pack_progress_cb_payload Payload for the pack progress callback. + * @param transfer_progress_cb Function to call with progress information during + * the upload portion of a push. Be aware that this is called inline with + * pack building operations, so performance may be affected. + * @param transfer_progress_cb_payload Payload for the network progress callback. + * @return 0 or an error code + */ +GIT_EXTERN(int) git_push_set_callbacks( + git_push *push, + git_packbuilder_progress pack_progress_cb, + void *pack_progress_cb_payload, + git_push_transfer_progress transfer_progress_cb, + void *transfer_progress_cb_payload); + /** * Add a refspec to be pushed * diff --git a/src/pack-objects.c b/src/pack-objects.c index 7f427e3bd..2a2f36223 100644 --- a/src/pack-objects.c +++ b/src/pack-objects.c @@ -14,6 +14,7 @@ #include "pack.h" #include "thread-utils.h" #include "tree.h" +#include "util.h" #include "git2/pack.h" #include "git2/commit.h" @@ -57,6 +58,9 @@ struct pack_write_context { #define git_packbuilder__progress_lock(pb) GIT_PACKBUILDER__MUTEX_OP(pb, progress_mutex, lock) #define git_packbuilder__progress_unlock(pb) GIT_PACKBUILDER__MUTEX_OP(pb, progress_mutex, unlock) +/* The minimal interval between progress updates (in seconds). */ +#define MIN_PROGRESS_UPDATE_INTERVAL 0.5 + static unsigned name_hash(const char *name) { unsigned c, hash = 0; @@ -212,6 +216,14 @@ int git_packbuilder_insert(git_packbuilder *pb, const git_oid *oid, assert(ret != 0); kh_value(pb->object_ix, pos) = po; + if (pb->progress_cb) { + double current_time = git__timer(); + if ((current_time - pb->last_progress_report_time) >= MIN_PROGRESS_UPDATE_INTERVAL) { + pb->last_progress_report_time = current_time; + pb->progress_cb(GIT_PACKBUILDER_ADDING_OBJECTS, pb->nr_objects, 0, pb->progress_cb_payload); + } + } + pb->done = false; return 0; } @@ -1207,6 +1219,13 @@ static int prepare_pack(git_packbuilder *pb) if (pb->nr_objects == 0 || pb->done) return 0; /* nothing to do */ + /* + * Although we do not report progress during deltafication, we + * at least report that we are in the deltafication stage + */ + if (pb->progress_cb) + pb->progress_cb(GIT_PACKBUILDER_DELTAFICATION, 0, pb->nr_objects, pb->progress_cb_payload); + delta_list = git__malloc(pb->nr_objects * sizeof(*delta_list)); GITERR_CHECK_ALLOC(delta_list); @@ -1348,6 +1367,17 @@ uint32_t git_packbuilder_written(git_packbuilder *pb) return pb->nr_written; } +int git_packbuilder_set_callbacks(git_packbuilder *pb, git_packbuilder_progress progress_cb, void *progress_cb_payload) +{ + if (!pb) + return -1; + + pb->progress_cb = progress_cb; + pb->progress_cb_payload = progress_cb_payload; + + return 0; +} + void git_packbuilder_free(git_packbuilder *pb) { if (pb == NULL) diff --git a/src/pack-objects.h b/src/pack-objects.h index 8e7ba7f78..0c94a5a7a 100644 --- a/src/pack-objects.h +++ b/src/pack-objects.h @@ -16,6 +16,7 @@ #include "netops.h" #include "git2/oid.h" +#include "git2/pack.h" #define GIT_PACK_WINDOW 10 /* number of objects to possibly delta against */ #define GIT_PACK_DEPTH 50 /* max delta depth */ @@ -79,6 +80,10 @@ struct git_packbuilder { int nr_threads; /* nr of threads to use */ + git_packbuilder_progress progress_cb; + void *progress_cb_payload; + double last_progress_report_time; /* the time progress was last reported */ + bool done; }; diff --git a/src/push.c b/src/push.c index eaaa46248..698079253 100644 --- a/src/push.c +++ b/src/push.c @@ -70,6 +70,25 @@ int git_push_set_options(git_push *push, const git_push_options *opts) return 0; } +int git_push_set_callbacks( + git_push *push, + git_packbuilder_progress pack_progress_cb, + void *pack_progress_cb_payload, + git_push_transfer_progress transfer_progress_cb, + void *transfer_progress_cb_payload) +{ + if (!push) + return -1; + + push->pack_progress_cb = pack_progress_cb; + push->pack_progress_cb_payload = pack_progress_cb_payload; + + push->transfer_progress_cb = transfer_progress_cb; + push->transfer_progress_cb_payload = transfer_progress_cb_payload; + + return 0; +} + static void free_refspec(push_spec *spec) { if (spec == NULL) @@ -583,6 +602,10 @@ static int do_push(git_push *push) git_packbuilder_set_threads(push->pb, push->pb_parallelism); + if (push->pack_progress_cb) + if ((error = git_packbuilder_set_callbacks(push->pb, push->pack_progress_cb, push->pack_progress_cb_payload)) < 0) + goto on_error; + if ((error = calculate_work(push)) < 0 || (error = queue_objects(push)) < 0 || (error = transport->push(transport, push)) < 0) diff --git a/src/push.h b/src/push.h index e982b8385..6c8bf7229 100644 --- a/src/push.h +++ b/src/push.h @@ -39,6 +39,11 @@ struct git_push { /* options */ unsigned pb_parallelism; + + git_packbuilder_progress pack_progress_cb; + void *pack_progress_cb_payload; + git_push_transfer_progress transfer_progress_cb; + void *transfer_progress_cb_payload; }; /** diff --git a/src/transports/smart_protocol.c b/src/transports/smart_protocol.c index 0cd5e831d..156b69e1f 100644 --- a/src/transports/smart_protocol.c +++ b/src/transports/smart_protocol.c @@ -13,8 +13,11 @@ #include "push.h" #include "pack-objects.h" #include "remote.h" +#include "util.h" #define NETWORK_XFER_THRESHOLD (100*1024) +/* The minimal interval between progress updates (in seconds). */ +#define MIN_PROGRESS_UPDATE_INTERVAL 0.5 int git_smart__store_refs(transport_smart *t, int flushes) { @@ -801,22 +804,53 @@ static int update_refs_from_report( return 0; } +struct push_packbuilder_payload +{ + git_smart_subtransport_stream *stream; + git_packbuilder *pb; + git_push_transfer_progress cb; + void *cb_payload; + size_t last_bytes; + double last_progress_report_time; +}; + static int stream_thunk(void *buf, size_t size, void *data) { - git_smart_subtransport_stream *s = (git_smart_subtransport_stream *)data; + int error = 0; + struct push_packbuilder_payload *payload = data; - return s->write(s, (const char *)buf, size); + if ((error = payload->stream->write(payload->stream, (const char *)buf, size)) < 0) + return error; + + if (payload->cb) { + double current_time = git__timer(); + payload->last_bytes += size; + + if ((current_time - payload->last_progress_report_time) >= MIN_PROGRESS_UPDATE_INTERVAL) { + payload->last_progress_report_time = current_time; + payload->cb(payload->pb->nr_written, payload->pb->nr_objects, payload->last_bytes, payload->cb_payload); + } + } + + return error; } int git_smart__push(git_transport *transport, git_push *push) { transport_smart *t = (transport_smart *)transport; - git_smart_subtransport_stream *s; + struct push_packbuilder_payload packbuilder_payload = {0}; git_buf pktline = GIT_BUF_INIT; int error = -1, need_pack = 0; push_spec *spec; unsigned int i; + packbuilder_payload.pb = push->pb; + + if (push->transfer_progress_cb) { + packbuilder_payload.cb = push->transfer_progress_cb; + packbuilder_payload.cb_payload = push->transfer_progress_cb_payload; + } + #ifdef PUSH_DEBUG { git_remote_head *head; @@ -848,12 +882,12 @@ int git_smart__push(git_transport *transport, git_push *push) } } - if (git_smart__get_push_stream(t, &s) < 0 || + if (git_smart__get_push_stream(t, &packbuilder_payload.stream) < 0 || gen_pktline(&pktline, push) < 0 || - s->write(s, git_buf_cstr(&pktline), git_buf_len(&pktline)) < 0) + packbuilder_payload.stream->write(packbuilder_payload.stream, git_buf_cstr(&pktline), git_buf_len(&pktline)) < 0) goto on_error; - if (need_pack && git_packbuilder_foreach(push->pb, &stream_thunk, s) < 0) + if (need_pack && git_packbuilder_foreach(push->pb, &stream_thunk, &packbuilder_payload) < 0) goto on_error; /* If we sent nothing or the server doesn't support report-status, then @@ -863,6 +897,11 @@ int git_smart__push(git_transport *transport, git_push *push) else if (parse_report(&t->buffer, push) < 0) goto on_error; + /* If progress is being reported write the final report */ + if (push->transfer_progress_cb) { + push->transfer_progress_cb(push->pb->nr_written, push->pb->nr_objects, packbuilder_payload.last_bytes, push->transfer_progress_cb_payload); + } + if (push->status.length && update_refs_from_report(&t->refs, &push->specs, &push->status) < 0) goto on_error; diff --git a/src/util.h b/src/util.h index bd93b46b5..78e2f2e79 100644 --- a/src/util.h +++ b/src/util.h @@ -353,4 +353,65 @@ GIT_INLINE(void) git__memzero(void *data, size_t size) #endif } +#ifdef GIT_WIN32 + +GIT_INLINE(double) git__timer(void) +{ + /* We need the initial tick count to detect if the tick + * count has rolled over. */ + static DWORD initial_tick_count = 0; + + /* GetTickCount returns the number of milliseconds that have + * elapsed since the system was started. */ + DWORD count = GetTickCount(); + + if(initial_tick_count == 0) { + initial_tick_count = count; + } else if (count < initial_tick_count) { + /* The tick count has rolled over - adjust for it. */ + count = (0xFFFFFFFF - initial_tick_count) + count; + } + + return (double) count / (double) 1000; +} + +#elif __APPLE__ + +#include + +double git__timer(void) +{ + uint64_t time = mach_absolute_time(); + static double scaling_factor = 0; + + if (scaling_factor == 0) { + mach_timebase_info_data_t info; + (void)mach_timebase_info(&info); + scaling_factor = (double)info.numer / (double)info.denom; + } + + return (double)time * scaling_factor / 1.0E-9; +} + +#else + +#include + +GIT_INLINE(double) git__timer(void) +{ + struct timespec tp; + + if (clock_gettime(CLOCK_MONOTONIC, &tp) == 0) { + return (double) tp.tv_sec + (double) tp.tv_nsec / 1E-9; + } else { + /* Fall back to using gettimeofday */ + struct timeval tv; + struct timezone tz; + gettimeofday(&tv, &tz); + return (double)tv.tv_sec + (double)tv.tv_usec / 1E-6; + } +} + +#endif + #endif /* INCLUDE_util_h__ */ diff --git a/tests-clar/online/push.c b/tests-clar/online/push.c index 4c0a28c1c..6a4a9b281 100644 --- a/tests-clar/online/push.c +++ b/tests-clar/online/push.c @@ -348,6 +348,18 @@ void test_online_push__cleanup(void) cl_git_sandbox_cleanup(); } +static void push_pack_progress_cb(int stage, unsigned int current, unsigned int total, void* payload) +{ + int *was_called = (int *) payload; + *was_called = 1; +} + +static void push_transfer_progress_cb(unsigned int current, unsigned int total, size_t bytes, void* payload) +{ + int *was_called = (int *) payload; + *was_called = 1; +} + /** * Calls push and relists refs on remote to verify success. * @@ -356,15 +368,17 @@ void test_online_push__cleanup(void) * @param expected_refs expected remote refs after push * @param expected_refs_len length of expected_refs * @param expected_ret expected return value from git_push_finish() + * @param check_progress_cb Check that the push progress callbacks are called */ static void do_push(const char *refspecs[], size_t refspecs_len, push_status expected_statuses[], size_t expected_statuses_len, - expected_ref expected_refs[], size_t expected_refs_len, int expected_ret) + expected_ref expected_refs[], size_t expected_refs_len, int expected_ret, int check_progress_cb) { git_push *push; git_push_options opts = GIT_PUSH_OPTIONS_INIT; size_t i; int ret; + int pack_progress_called = 0, transfer_progress_called = 0; if (_remote) { /* Auto-detect the number of threads to use */ @@ -375,6 +389,9 @@ static void do_push(const char *refspecs[], size_t refspecs_len, cl_git_pass(git_push_new(&push, _remote)); cl_git_pass(git_push_set_options(push, &opts)); + if (check_progress_cb) + cl_git_pass(git_push_set_callbacks(push, push_pack_progress_cb, &pack_progress_called, push_transfer_progress_cb, &transfer_progress_called)); + for (i = 0; i < refspecs_len; i++) cl_git_pass(git_push_add_refspec(push, refspecs[i])); @@ -387,6 +404,11 @@ static void do_push(const char *refspecs[], size_t refspecs_len, cl_assert_equal_i(1, git_push_unpack_ok(push)); } + if (check_progress_cb) { + cl_assert_equal_i(1, pack_progress_called); + cl_assert_equal_i(1, transfer_progress_called); + } + do_verify_push_status(push, expected_statuses, expected_statuses_len); cl_assert_equal_i(expected_ret, ret); @@ -405,7 +427,7 @@ static void do_push(const char *refspecs[], size_t refspecs_len, /* Call push_finish() without ever calling git_push_add_refspec() */ void test_online_push__noop(void) { - do_push(NULL, 0, NULL, 0, NULL, 0, 0); + do_push(NULL, 0, NULL, 0, NULL, 0, 0, 0); } void test_online_push__b1(void) @@ -415,7 +437,7 @@ void test_online_push__b1(void) expected_ref exp_refs[] = { { "refs/heads/b1", &_oid_b1 } }; do_push(specs, ARRAY_SIZE(specs), exp_stats, ARRAY_SIZE(exp_stats), - exp_refs, ARRAY_SIZE(exp_refs), 0); + exp_refs, ARRAY_SIZE(exp_refs), 0, 1); } void test_online_push__b2(void) @@ -425,7 +447,7 @@ void test_online_push__b2(void) expected_ref exp_refs[] = { { "refs/heads/b2", &_oid_b2 } }; do_push(specs, ARRAY_SIZE(specs), exp_stats, ARRAY_SIZE(exp_stats), - exp_refs, ARRAY_SIZE(exp_refs), 0); + exp_refs, ARRAY_SIZE(exp_refs), 0, 1); } void test_online_push__b3(void) @@ -435,7 +457,7 @@ void test_online_push__b3(void) expected_ref exp_refs[] = { { "refs/heads/b3", &_oid_b3 } }; do_push(specs, ARRAY_SIZE(specs), exp_stats, ARRAY_SIZE(exp_stats), - exp_refs, ARRAY_SIZE(exp_refs), 0); + exp_refs, ARRAY_SIZE(exp_refs), 0, 1); } void test_online_push__b4(void) @@ -445,7 +467,7 @@ void test_online_push__b4(void) expected_ref exp_refs[] = { { "refs/heads/b4", &_oid_b4 } }; do_push(specs, ARRAY_SIZE(specs), exp_stats, ARRAY_SIZE(exp_stats), - exp_refs, ARRAY_SIZE(exp_refs), 0); + exp_refs, ARRAY_SIZE(exp_refs), 0, 1); } void test_online_push__b5(void) @@ -455,7 +477,7 @@ void test_online_push__b5(void) expected_ref exp_refs[] = { { "refs/heads/b5", &_oid_b5 } }; do_push(specs, ARRAY_SIZE(specs), exp_stats, ARRAY_SIZE(exp_stats), - exp_refs, ARRAY_SIZE(exp_refs), 0); + exp_refs, ARRAY_SIZE(exp_refs), 0, 1); } void test_online_push__multi(void) @@ -483,7 +505,7 @@ void test_online_push__multi(void) }; do_push(specs, ARRAY_SIZE(specs), exp_stats, ARRAY_SIZE(exp_stats), - exp_refs, ARRAY_SIZE(exp_refs), 0); + exp_refs, ARRAY_SIZE(exp_refs), 0, 1); } void test_online_push__implicit_tgt(void) @@ -501,10 +523,10 @@ void test_online_push__implicit_tgt(void) do_push(specs1, ARRAY_SIZE(specs1), exp_stats1, ARRAY_SIZE(exp_stats1), - exp_refs1, ARRAY_SIZE(exp_refs1), 0); + exp_refs1, ARRAY_SIZE(exp_refs1), 0, 1); do_push(specs2, ARRAY_SIZE(specs2), exp_stats2, ARRAY_SIZE(exp_stats2), - exp_refs2, ARRAY_SIZE(exp_refs2), 0); + exp_refs2, ARRAY_SIZE(exp_refs2), 0, 0); } void test_online_push__fast_fwd(void) @@ -526,19 +548,19 @@ void test_online_push__fast_fwd(void) do_push(specs_init, ARRAY_SIZE(specs_init), exp_stats_init, ARRAY_SIZE(exp_stats_init), - exp_refs_init, ARRAY_SIZE(exp_refs_init), 0); + exp_refs_init, ARRAY_SIZE(exp_refs_init), 0, 1); do_push(specs_ff, ARRAY_SIZE(specs_ff), exp_stats_ff, ARRAY_SIZE(exp_stats_ff), - exp_refs_ff, ARRAY_SIZE(exp_refs_ff), 0); + exp_refs_ff, ARRAY_SIZE(exp_refs_ff), 0, 0); do_push(specs_reset, ARRAY_SIZE(specs_reset), exp_stats_init, ARRAY_SIZE(exp_stats_init), - exp_refs_init, ARRAY_SIZE(exp_refs_init), 0); + exp_refs_init, ARRAY_SIZE(exp_refs_init), 0, 0); do_push(specs_ff_force, ARRAY_SIZE(specs_ff_force), exp_stats_ff, ARRAY_SIZE(exp_stats_ff), - exp_refs_ff, ARRAY_SIZE(exp_refs_ff), 0); + exp_refs_ff, ARRAY_SIZE(exp_refs_ff), 0, 0); } void test_online_push__tag_commit(void) @@ -548,7 +570,7 @@ void test_online_push__tag_commit(void) expected_ref exp_refs[] = { { "refs/tags/tag-commit", &_tag_commit } }; do_push(specs, ARRAY_SIZE(specs), exp_stats, ARRAY_SIZE(exp_stats), - exp_refs, ARRAY_SIZE(exp_refs), 0); + exp_refs, ARRAY_SIZE(exp_refs), 0, 1); } void test_online_push__tag_tree(void) @@ -558,7 +580,7 @@ void test_online_push__tag_tree(void) expected_ref exp_refs[] = { { "refs/tags/tag-tree", &_tag_tree } }; do_push(specs, ARRAY_SIZE(specs), exp_stats, ARRAY_SIZE(exp_stats), - exp_refs, ARRAY_SIZE(exp_refs), 0); + exp_refs, ARRAY_SIZE(exp_refs), 0, 1); } void test_online_push__tag_blob(void) @@ -568,7 +590,7 @@ void test_online_push__tag_blob(void) expected_ref exp_refs[] = { { "refs/tags/tag-blob", &_tag_blob } }; do_push(specs, ARRAY_SIZE(specs), exp_stats, ARRAY_SIZE(exp_stats), - exp_refs, ARRAY_SIZE(exp_refs), 0); + exp_refs, ARRAY_SIZE(exp_refs), 0, 1); } void test_online_push__tag_lightweight(void) @@ -578,7 +600,7 @@ void test_online_push__tag_lightweight(void) expected_ref exp_refs[] = { { "refs/tags/tag-lightweight", &_tag_lightweight } }; do_push(specs, ARRAY_SIZE(specs), exp_stats, ARRAY_SIZE(exp_stats), - exp_refs, ARRAY_SIZE(exp_refs), 0); + exp_refs, ARRAY_SIZE(exp_refs), 0, 1); } void test_online_push__tag_to_tag(void) @@ -588,7 +610,7 @@ void test_online_push__tag_to_tag(void) expected_ref exp_refs[] = { { "refs/tags/tag-tag", &_tag_tag } }; do_push(specs, ARRAY_SIZE(specs), exp_stats, ARRAY_SIZE(exp_stats), - exp_refs, ARRAY_SIZE(exp_refs), 0); + exp_refs, ARRAY_SIZE(exp_refs), 0, 0); } void test_online_push__force(void) @@ -605,16 +627,16 @@ void test_online_push__force(void) do_push(specs1, ARRAY_SIZE(specs1), exp_stats1, ARRAY_SIZE(exp_stats1), - exp_refs1, ARRAY_SIZE(exp_refs1), 0); + exp_refs1, ARRAY_SIZE(exp_refs1), 0, 1); do_push(specs2, ARRAY_SIZE(specs2), NULL, 0, - exp_refs1, ARRAY_SIZE(exp_refs1), GIT_ENONFASTFORWARD); + exp_refs1, ARRAY_SIZE(exp_refs1), GIT_ENONFASTFORWARD, 0); /* Non-fast-forward update with force should pass. */ do_push(specs2_force, ARRAY_SIZE(specs2_force), exp_stats2_force, ARRAY_SIZE(exp_stats2_force), - exp_refs2_force, ARRAY_SIZE(exp_refs2_force), 0); + exp_refs2_force, ARRAY_SIZE(exp_refs2_force), 0, 1); } void test_online_push__delete(void) @@ -645,7 +667,7 @@ void test_online_push__delete(void) do_push(specs1, ARRAY_SIZE(specs1), exp_stats1, ARRAY_SIZE(exp_stats1), - exp_refs1, ARRAY_SIZE(exp_refs1), 0); + exp_refs1, ARRAY_SIZE(exp_refs1), 0, 1); /* When deleting a non-existent branch, the git client sends zero for both * the old and new commit id. This should succeed on the server with the @@ -655,23 +677,23 @@ void test_online_push__delete(void) */ do_push(specs_del_fake, ARRAY_SIZE(specs_del_fake), exp_stats_fake, 1, - exp_refs1, ARRAY_SIZE(exp_refs1), 0); + exp_refs1, ARRAY_SIZE(exp_refs1), 0, 0); do_push(specs_del_fake_force, ARRAY_SIZE(specs_del_fake_force), exp_stats_fake, 1, - exp_refs1, ARRAY_SIZE(exp_refs1), 0); + exp_refs1, ARRAY_SIZE(exp_refs1), 0, 0); /* Delete one of the pushed branches. */ do_push(specs_delete, ARRAY_SIZE(specs_delete), exp_stats_delete, ARRAY_SIZE(exp_stats_delete), - exp_refs_delete, ARRAY_SIZE(exp_refs_delete), 0); + exp_refs_delete, ARRAY_SIZE(exp_refs_delete), 0, 0); /* Re-push branches and retry delete with force. */ do_push(specs1, ARRAY_SIZE(specs1), exp_stats1, ARRAY_SIZE(exp_stats1), - exp_refs1, ARRAY_SIZE(exp_refs1), 0); + exp_refs1, ARRAY_SIZE(exp_refs1), 0, 0); do_push(specs_delete_force, ARRAY_SIZE(specs_delete_force), exp_stats_delete, ARRAY_SIZE(exp_stats_delete), - exp_refs_delete, ARRAY_SIZE(exp_refs_delete), 0); + exp_refs_delete, ARRAY_SIZE(exp_refs_delete), 0, 0); } void test_online_push__bad_refspecs(void) @@ -703,11 +725,11 @@ void test_online_push__expressions(void) /* TODO: Find a more precise way of checking errors than a exit code of -1. */ do_push(specs_left_expr, ARRAY_SIZE(specs_left_expr), NULL, 0, - NULL, 0, -1); + NULL, 0, -1, 0); do_push(specs_right_expr, ARRAY_SIZE(specs_right_expr), exp_stats_right_expr, ARRAY_SIZE(exp_stats_right_expr), - NULL, 0, 0); + NULL, 0, 0, 1); } void test_online_push__notes(void) @@ -727,7 +749,7 @@ void test_online_push__notes(void) do_push(specs, ARRAY_SIZE(specs), exp_stats, ARRAY_SIZE(exp_stats), - exp_refs, ARRAY_SIZE(exp_refs), 0); + exp_refs, ARRAY_SIZE(exp_refs), 0, 1); git_signature_free(signature); } From ae5a935290c695d721f69b8836f891a639c0aff3 Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Mon, 30 Sep 2013 14:47:56 -0700 Subject: [PATCH 16/37] Ensure submodule repos and indices are freed ...before the helper's cleanup method tries to delete their files. --- tests-clar/stash/submodules.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/tests-clar/stash/submodules.c b/tests-clar/stash/submodules.c index 60dbbad2e..137c4408c 100644 --- a/tests-clar/stash/submodules.c +++ b/tests-clar/stash/submodules.c @@ -6,9 +6,7 @@ static git_repository *repo; static git_signature *signature; static git_oid stash_tip_oid; -static git_index *smindex; static git_submodule *sm; -static git_repository *smrepo; void test_stash_submodules__initialize(void) { @@ -17,8 +15,6 @@ void test_stash_submodules__initialize(void) repo = setup_fixture_submodules(); cl_git_pass(git_submodule_lookup(&sm, repo, "testrepo")); - cl_git_pass(git_submodule_open(&smrepo, sm)); - cl_git_pass(git_repository_index(&smindex, smrepo)); } void test_stash_submodules__cleanup(void) @@ -29,6 +25,9 @@ void test_stash_submodules__cleanup(void) void test_stash_submodules__does_not_stash_modified_submodules(void) { + static git_index *smindex; + static git_repository *smrepo; + assert_status(repo, "modified", GIT_STATUS_WT_MODIFIED); /* modify file in submodule */ @@ -36,6 +35,8 @@ void test_stash_submodules__does_not_stash_modified_submodules(void) assert_status(repo, "testrepo", GIT_STATUS_WT_MODIFIED); /* add file to index in submodule */ + cl_git_pass(git_submodule_open(&smrepo, sm)); + cl_git_pass(git_repository_index(&smindex, smrepo)); cl_git_pass(git_index_add_bypath(smindex, "README")); /* commit changed index of submodule */ @@ -46,10 +47,16 @@ void test_stash_submodules__does_not_stash_modified_submodules(void) assert_status(repo, "testrepo", GIT_STATUS_WT_MODIFIED); assert_status(repo, "modified", GIT_STATUS_CURRENT); + + git_index_free(smindex); + git_repository_free(smrepo); } void test_stash_submodules__stash_is_empty_with_modified_submodules(void) { + static git_index *smindex; + static git_repository *smrepo; + cl_git_pass(git_stash_save(&stash_tip_oid, repo, signature, NULL, GIT_STASH_DEFAULT)); assert_status(repo, "modified", GIT_STATUS_CURRENT); @@ -58,6 +65,8 @@ void test_stash_submodules__stash_is_empty_with_modified_submodules(void) assert_status(repo, "testrepo", GIT_STATUS_WT_MODIFIED); /* add file to index in submodule */ + cl_git_pass(git_submodule_open(&smrepo, sm)); + cl_git_pass(git_repository_index(&smindex, smrepo)); cl_git_pass(git_index_add_bypath(smindex, "README")); /* commit changed index of submodule */ @@ -65,4 +74,7 @@ void test_stash_submodules__stash_is_empty_with_modified_submodules(void) assert_status(repo, "testrepo", GIT_STATUS_WT_MODIFIED); cl_git_fail_with(git_stash_save(&stash_tip_oid, repo, signature, NULL, GIT_STASH_DEFAULT), GIT_ENOTFOUND); + + git_index_free(smindex); + git_repository_free(smrepo); } From 566dd8cec06e511490c6473d2440c39ff1b2851c Mon Sep 17 00:00:00 2001 From: Linquize Date: Mon, 30 Sep 2013 23:38:22 +0800 Subject: [PATCH 17/37] Config subsection name should allow to have ']' and '\\' should allow to escape any characters --- src/config_file.c | 41 +++++++++++++++-------------------------- 1 file changed, 15 insertions(+), 26 deletions(-) diff --git a/src/config_file.c b/src/config_file.c index 1a845d8ba..8fb43b990 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -712,7 +712,6 @@ static int parse_section_header_ext(struct reader *reader, const char *line, con int c, rpos; char *first_quote, *last_quote; git_buf buf = GIT_BUF_INIT; - int quote_marks; /* * base_name is what came before the space. We should be at the * first quotation mark, except for now, line isn't being kept in @@ -731,21 +730,15 @@ static int parse_section_header_ext(struct reader *reader, const char *line, con git_buf_printf(&buf, "%s.", base_name); rpos = 0; - quote_marks = 0; line = first_quote; - c = line[rpos++]; + c = line[++rpos]; /* * At the end of each iteration, whatever is stored in c will be * added to the string. In case of error, jump to out */ do { - if (quote_marks == 2) { - set_parse_error(reader, rpos, "Unexpected text after closing quotes"); - git_buf_free(&buf); - return -1; - } switch (c) { case 0: @@ -754,25 +747,13 @@ static int parse_section_header_ext(struct reader *reader, const char *line, con return -1; case '"': - ++quote_marks; - continue; + goto end_parse; case '\\': - c = line[rpos++]; + c = line[++rpos]; - switch (c) { - case '"': - if (&line[rpos-1] == last_quote) { - set_parse_error(reader, 0, "Missing closing quotation mark in section header"); - git_buf_free(&buf); - return -1; - } - - case '\\': - break; - - default: - set_parse_error(reader, rpos, "Unsupported escape sequence"); + if (c == 0) { + set_parse_error(reader, rpos, "Unexpected end-of-line in section header"); git_buf_free(&buf); return -1; } @@ -782,7 +763,15 @@ static int parse_section_header_ext(struct reader *reader, const char *line, con } git_buf_putc(&buf, (char)c); - } while ((c = line[rpos++]) != ']'); + c = line[++rpos]; + } while (line + rpos < last_quote); + +end_parse: + if (line[rpos] != '"' || line[rpos + 1] != ']') { + set_parse_error(reader, rpos, "Unexpected text after closing quotes"); + git_buf_free(&buf); + return -1; + } *section_name = git_buf_detach(&buf); return 0; @@ -800,7 +789,7 @@ static int parse_section_header(struct reader *reader, char **section_out) return -1; /* find the end of the variable's name */ - name_end = strchr(line, ']'); + name_end = strrchr(line, ']'); if (name_end == NULL) { git__free(line); set_parse_error(reader, 0, "Missing ']' in section header"); From d52a93fab330dbbe1271c10b527d333261638fe3 Mon Sep 17 00:00:00 2001 From: Linquize Date: Mon, 30 Sep 2013 23:58:58 +0800 Subject: [PATCH 18/37] Add test case to test ']' and '\\' characters in config subsection --- tests-clar/config/read.c | 7 +++++++ tests-clar/resources/config/config20 | 11 +++++++++++ 2 files changed, 18 insertions(+) create mode 100644 tests-clar/resources/config/config20 diff --git a/tests-clar/config/read.c b/tests-clar/config/read.c index 722a15a71..ab068eaa7 100644 --- a/tests-clar/config/read.c +++ b/tests-clar/config/read.c @@ -164,6 +164,13 @@ void test_config_read__empty_files(void) git_config_free(cfg); } +void test_config_read__symbol_headers(void) +{ + git_config *cfg; + cl_git_pass(git_config_open_ondisk(&cfg, cl_fixture("config/config20"))); + git_config_free(cfg); +} + void test_config_read__header_in_last_line(void) { git_config *cfg; diff --git a/tests-clar/resources/config/config20 b/tests-clar/resources/config/config20 new file mode 100644 index 000000000..8f0f12c4c --- /dev/null +++ b/tests-clar/resources/config/config20 @@ -0,0 +1,11 @@ +[valid "[subsection]"] + something = a +; we don't allow anything after closing " +[sec "[subsec]/child"] + parent = grand +[sec2 "[subsec2]/child2"] + type = dvcs +[sec3 "escape\"quote"] + vcs = git +[sec4 "escaping\\slash"] + lib = git2 From 8d741253849c5e377e50490818326adc56be9fce Mon Sep 17 00:00:00 2001 From: Linquize Date: Tue, 1 Oct 2013 09:46:56 +0800 Subject: [PATCH 19/37] Add negative test cases for config header with invalid characters --- tests-clar/config/read.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests-clar/config/read.c b/tests-clar/config/read.c index ab068eaa7..abc088d59 100644 --- a/tests-clar/config/read.c +++ b/tests-clar/config/read.c @@ -531,6 +531,28 @@ void test_config_read__corrupt_header(void) git_config_free(cfg); } +void test_config_read__corrupt_header2(void) +{ + git_config *cfg; + + cl_set_cleanup(&clean_test_config, NULL); + cl_git_mkfile("./testconfig", "[unclosed \"bracket\"\n lib = git2\n"); + cl_git_fail(git_config_open_ondisk(&cfg, "./testconfig")); + + git_config_free(cfg); +} + +void test_config_read__corrupt_header3(void) +{ + git_config *cfg; + + cl_set_cleanup(&clean_test_config, NULL); + cl_git_mkfile("./testconfig", "[unclosed \"slash\\\"]\n lib = git2\n"); + cl_git_fail(git_config_open_ondisk(&cfg, "./testconfig")); + + git_config_free(cfg); +} + void test_config_read__override_variable(void) { git_config *cfg; From 816d28e7bc9473482664d374786dac484c0e1156 Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Tue, 1 Oct 2013 12:56:47 -0700 Subject: [PATCH 20/37] Mark git__timer as inline on OSX --- src/util.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util.h b/src/util.h index 78e2f2e79..f9de909e9 100644 --- a/src/util.h +++ b/src/util.h @@ -379,7 +379,7 @@ GIT_INLINE(double) git__timer(void) #include -double git__timer(void) +GIT_INLINE(double) git__timer(void) { uint64_t time = mach_absolute_time(); static double scaling_factor = 0; From 8378695671cec5d45464cea65aa5199ecf242212 Mon Sep 17 00:00:00 2001 From: Philip Kelley Date: Fri, 4 Jan 2013 14:39:05 -0500 Subject: [PATCH 21/37] Add git_transport_register, git_transport_unregister --- include/git2/transport.h | 34 +++++++++++++++++++++ src/transport.c | 66 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+) diff --git a/include/git2/transport.h b/include/git2/transport.h index e61b10423..9901b15de 100644 --- a/include/git2/transport.h +++ b/include/git2/transport.h @@ -253,6 +253,40 @@ GIT_EXTERN(int) git_transport_new(git_transport **out, git_remote *owner, const /* Signature of a function which creates a transport */ typedef int (*git_transport_cb)(git_transport **out, git_remote *owner, void *param); +/** + * Add a custom transport definition, to be used in addition to the built-in + * set of transports that come with libgit2. + * + * The caller is responsible for synchronizing calls to git_transport_register + * and git_transport_unregister with other calls to the library that + * instantiate transports. + * + * @param prefix The scheme (ending in "://") to match, i.e. "git://" + * @param priority The priority of this transport relative to others with + * the same prefix. Built-in transports have a priority of 1. + * @param cb The callback used to create an instance of the transport + * @param param A fixed parameter to pass to cb at creation time + * @return 0 or an error code + */ +GIT_EXTERN(int) git_transport_register( + const char *prefix, + unsigned priority, + git_transport_cb cb, + void *param); + +/** + * + * Unregister a custom transport definition which was previously registered + * with git_transport_register. + * + * @param prefix From the previous call to git_transport_register + * @param priority From the previous call to git_transport_register + * @return 0 or an error code + */ +GIT_EXTERN(int) git_transport_unregister( + const char *prefix, + unsigned priority); + /* Transports which come with libgit2 (match git_transport_cb). The expected * value for "param" is listed in-line below. */ diff --git a/src/transport.c b/src/transport.c index 354789db1..ff926b1be 100644 --- a/src/transport.c +++ b/src/transport.c @@ -42,6 +42,8 @@ static transport_definition transports[] = { {NULL, 0, 0} }; +static git_vector additional_transports = GIT_VECTOR_INIT; + #define GIT_TRANSPORT_COUNT (sizeof(transports)/sizeof(transports[0])) - 1 static int transport_find_fn(const char *url, git_transport_cb *callback, void **param) @@ -61,6 +63,14 @@ static int transport_find_fn(const char *url, git_transport_cb *callback, void * definition = definition_iter; } + git_vector_foreach(&additional_transports, i, definition_iter) { + if (strncasecmp(url, definition_iter->prefix, strlen(definition_iter->prefix))) + continue; + + if (definition_iter->priority > priority) + definition = definition_iter; + } + #ifdef GIT_WIN32 /* On Windows, it might not be possible to discern between absolute local * and ssh paths - first check if this is a valid local path that points @@ -135,6 +145,62 @@ int git_transport_new(git_transport **out, git_remote *owner, const char *url) return 0; } +int git_transport_register( + const char *prefix, + unsigned priority, + git_transport_cb cb, + void *param) +{ + transport_definition *d; + + d = git__calloc(sizeof(transport_definition), 1); + GITERR_CHECK_ALLOC(d); + + d->prefix = git__strdup(prefix); + + if (!d->prefix) + goto on_error; + + d->priority = priority; + d->fn = cb; + d->param = param; + + if (git_vector_insert(&additional_transports, d) < 0) + goto on_error; + + return 0; + +on_error: + git__free(d->prefix); + git__free(d); + return -1; +} + +int git_transport_unregister( + const char *prefix, + unsigned priority) +{ + transport_definition *d; + unsigned i; + + git_vector_foreach(&additional_transports, i, d) { + if (d->priority == priority && !strcasecmp(d->prefix, prefix)) { + if (git_vector_remove(&additional_transports, i) < 0) + return -1; + + git__free(d->prefix); + git__free(d); + + if (!additional_transports.length) + git_vector_free(&additional_transports); + + return 0; + } + } + + return GIT_ENOTFOUND; +} + /* from remote.h */ int git_remote_valid_url(const char *url) { From d31402a3fc4aa1b7d48ba43fd3bb072e7d69a527 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Mon, 16 Sep 2013 04:20:05 +0200 Subject: [PATCH 22/37] remote: put the _download() callback with the others The text progress and update_tips callbacks are already part of the struct, which was meant to unify the callback setup, but the download one was left out. --- examples/network/clone.c | 6 ++++-- examples/network/fetch.c | 2 +- include/git2/clone.h | 2 -- include/git2/remote.h | 11 ++--------- src/clone.c | 3 +-- src/fetch.c | 8 +++----- src/fetch.h | 5 +---- src/remote.c | 7 ++----- tests-clar/network/fetchlocal.c | 14 ++++++++++++-- tests-clar/network/remote/local.c | 10 +++++----- tests-clar/online/clone.c | 20 +++++++++++++++++--- tests-clar/online/fetch.c | 18 +++++++++++++++--- tests-clar/online/fetchhead.c | 2 +- tests-clar/online/push.c | 2 +- tests-clar/online/push_util.h | 2 +- 15 files changed, 66 insertions(+), 46 deletions(-) diff --git a/examples/network/clone.c b/examples/network/clone.c index a09a94728..f1002656c 100644 --- a/examples/network/clone.c +++ b/examples/network/clone.c @@ -57,6 +57,7 @@ int do_clone(git_repository *repo, int argc, char **argv) git_repository *cloned_repo = NULL; git_clone_options clone_opts = GIT_CLONE_OPTIONS_INIT; git_checkout_opts checkout_opts = GIT_CHECKOUT_OPTS_INIT; + git_remote_callbacks callbacks = GIT_REMOTE_CALLBACKS_INIT; const char *url = argv[1]; const char *path = argv[2]; int error; @@ -74,8 +75,9 @@ int do_clone(git_repository *repo, int argc, char **argv) checkout_opts.progress_cb = checkout_progress; checkout_opts.progress_payload = &pd; clone_opts.checkout_opts = checkout_opts; - clone_opts.fetch_progress_cb = &fetch_progress; - clone_opts.fetch_progress_payload = &pd; + callbacks.transfer_progress = &fetch_progress; + callbacks.payload = &pd; + clone_opts.remote_callbacks = &callbacks; clone_opts.cred_acquire_cb = cred_acquire_cb; // Do the clone diff --git a/examples/network/fetch.c b/examples/network/fetch.c index ce016ce0b..1de223373 100644 --- a/examples/network/fetch.c +++ b/examples/network/fetch.c @@ -35,7 +35,7 @@ static void *download(void *ptr) // Download the packfile and index it. This function updates the // amount of received data and the indexer stats which lets you // inform the user about progress. - if (git_remote_download(data->remote, NULL, NULL) < 0) { + if (git_remote_download(data->remote) < 0) { data->ret = -1; goto exit; } diff --git a/include/git2/clone.h b/include/git2/clone.h index 580352ac1..122806ad5 100644 --- a/include/git2/clone.h +++ b/include/git2/clone.h @@ -69,8 +69,6 @@ typedef struct git_clone_options { git_checkout_opts checkout_opts; git_repository_init_options *init_options; int bare; - git_transfer_progress_callback fetch_progress_cb; - void *fetch_progress_payload; const char *remote_name; const char *pushurl; diff --git a/include/git2/remote.h b/include/git2/remote.h index fa8b378c6..2bde5e365 100644 --- a/include/git2/remote.h +++ b/include/git2/remote.h @@ -257,17 +257,9 @@ GIT_EXTERN(int) git_remote_ls(git_remote *remote, git_headlist_cb list_cb, void * The .idx file will be created and both it and the packfile with be * renamed to their final name. * - * @param remote the remote to download from - * @param progress_cb function to call with progress information. Be aware that - * this is called inline with network and indexing operations, so performance - * may be affected. - * @param payload payload for the progress callback * @return 0 or an error code */ -GIT_EXTERN(int) git_remote_download( - git_remote *remote, - git_transfer_progress_callback progress_cb, - void *payload); +GIT_EXTERN(int) git_remote_download(git_remote *remote); /** * Check whether the remote is connected @@ -403,6 +395,7 @@ struct git_remote_callbacks { unsigned int version; void (*progress)(const char *str, int len, void *data); int (*completion)(git_remote_completion_type type, void *data); + int (*transfer_progress)(const git_transfer_progress *stats, void *data); int (*update_tips)(const char *refname, const git_oid *a, const git_oid *b, void *data); void *payload; }; diff --git a/src/clone.c b/src/clone.c index ff251be1b..90d677bbb 100644 --- a/src/clone.c +++ b/src/clone.c @@ -380,8 +380,7 @@ static int setup_remotes_and_fetch( if ((retcode = git_remote_connect(origin, GIT_DIRECTION_FETCH)) < 0) goto on_error; - if ((retcode = git_remote_download(origin, options->fetch_progress_cb, - options->fetch_progress_payload)) < 0) + if ((retcode = git_remote_download(origin)) < 0) goto on_error; /* Create "origin/foo" branches for all remote branches */ diff --git a/src/fetch.c b/src/fetch.c index 03fad5fec..5d97913e8 100644 --- a/src/fetch.c +++ b/src/fetch.c @@ -119,15 +119,13 @@ int git_fetch_negotiate(git_remote *remote) remote->refs.length); } -int git_fetch_download_pack( - git_remote *remote, - git_transfer_progress_callback progress_cb, - void *progress_payload) +int git_fetch_download_pack(git_remote *remote) { git_transport *t = remote->transport; if(!remote->need_pack) return 0; - return t->download_pack(t, remote->repo, &remote->stats, progress_cb, progress_payload); + return t->download_pack(t, remote->repo, &remote->stats, + remote->callbacks.transfer_progress, remote->callbacks.payload); } diff --git a/src/fetch.h b/src/fetch.h index 059251d04..9605da1b5 100644 --- a/src/fetch.h +++ b/src/fetch.h @@ -11,10 +11,7 @@ int git_fetch_negotiate(git_remote *remote); -int git_fetch_download_pack( - git_remote *remote, - git_transfer_progress_callback progress_cb, - void *progress_payload); +int git_fetch_download_pack(git_remote *remote); int git_fetch__download_pack( git_transport *t, diff --git a/src/remote.c b/src/remote.c index 95b907ff1..e4696c4ec 100644 --- a/src/remote.c +++ b/src/remote.c @@ -742,10 +742,7 @@ static int remote_head_cmp(const void *_a, const void *_b) return git__strcmp_cb(a->name, b->name); } -int git_remote_download( - git_remote *remote, - git_transfer_progress_callback progress_cb, - void *progress_payload) +int git_remote_download(git_remote *remote) { int error; git_vector refs; @@ -767,7 +764,7 @@ int git_remote_download( if ((error = git_fetch_negotiate(remote)) < 0) return error; - return git_fetch_download_pack(remote, progress_cb, progress_payload); + return git_fetch_download_pack(remote); } static int remote_head_for_fetchspec_src(git_remote_head **out, git_vector *update_heads, const char *fetchspec_src) diff --git a/tests-clar/network/fetchlocal.c b/tests-clar/network/fetchlocal.c index 09335b3df..28c7115bf 100644 --- a/tests-clar/network/fetchlocal.c +++ b/tests-clar/network/fetchlocal.c @@ -25,13 +25,18 @@ void test_network_fetchlocal__complete(void) git_strarray refnames = {0}; const char *url = cl_git_fixture_url("testrepo.git"); + git_remote_callbacks callbacks = GIT_REMOTE_CALLBACKS_INIT; + + callbacks.transfer_progress = transfer_cb; + callbacks.payload = &callcount; cl_set_cleanup(&cleanup_local_repo, "foo"); cl_git_pass(git_repository_init(&repo, "foo", true)); cl_git_pass(git_remote_create(&origin, repo, GIT_REMOTE_ORIGIN, url)); + git_remote_set_callbacks(origin, &callbacks); cl_git_pass(git_remote_connect(origin, GIT_DIRECTION_FETCH)); - cl_git_pass(git_remote_download(origin, transfer_cb, &callcount)); + cl_git_pass(git_remote_download(origin)); cl_git_pass(git_remote_update_tips(origin)); cl_git_pass(git_reference_list(&refnames, repo)); @@ -56,6 +61,10 @@ void test_network_fetchlocal__partial(void) int callcount = 0; git_strarray refnames = {0}; const char *url; + git_remote_callbacks callbacks = GIT_REMOTE_CALLBACKS_INIT; + + callbacks.transfer_progress = transfer_cb; + callbacks.payload = &callcount; cl_set_cleanup(&cleanup_sandbox, NULL); cl_git_pass(git_reference_list(&refnames, repo)); @@ -63,8 +72,9 @@ void test_network_fetchlocal__partial(void) url = cl_git_fixture_url("testrepo.git"); cl_git_pass(git_remote_create(&origin, repo, GIT_REMOTE_ORIGIN, url)); + git_remote_set_callbacks(origin, &callbacks); cl_git_pass(git_remote_connect(origin, GIT_DIRECTION_FETCH)); - cl_git_pass(git_remote_download(origin, transfer_cb, &callcount)); + cl_git_pass(git_remote_download(origin)); cl_git_pass(git_remote_update_tips(origin)); git_strarray_free(&refnames); diff --git a/tests-clar/network/remote/local.c b/tests-clar/network/remote/local.c index c8edd37f5..6d658a2e4 100644 --- a/tests-clar/network/remote/local.c +++ b/tests-clar/network/remote/local.c @@ -123,7 +123,7 @@ void test_network_remote_local__shorthand_fetch_refspec0(void) cl_git_pass(git_remote_add_fetch(remote, refspec)); cl_git_pass(git_remote_add_fetch(remote, refspec2)); - cl_git_pass(git_remote_download(remote, NULL, NULL)); + cl_git_pass(git_remote_download(remote)); cl_git_pass(git_remote_update_tips(remote)); cl_git_pass(git_reference_lookup(&ref, repo, "refs/remotes/sloppy/master")); @@ -145,7 +145,7 @@ void test_network_remote_local__shorthand_fetch_refspec1(void) cl_git_pass(git_remote_add_fetch(remote, refspec)); cl_git_pass(git_remote_add_fetch(remote, refspec2)); - cl_git_pass(git_remote_download(remote, NULL, NULL)); + cl_git_pass(git_remote_download(remote)); cl_git_pass(git_remote_update_tips(remote)); cl_git_fail(git_reference_lookup(&ref, repo, "refs/remotes/master")); @@ -160,7 +160,7 @@ void test_network_remote_local__tagopt(void) connect_to_local_repository(cl_fixture("testrepo.git")); git_remote_set_autotag(remote, GIT_REMOTE_DOWNLOAD_TAGS_ALL); - cl_git_pass(git_remote_download(remote, NULL, NULL)); + cl_git_pass(git_remote_download(remote)); cl_git_pass(git_remote_update_tips(remote)); @@ -179,7 +179,7 @@ void test_network_remote_local__push_to_bare_remote(void) /* Get some commits */ connect_to_local_repository(cl_fixture("testrepo.git")); cl_git_pass(git_remote_add_fetch(remote, "master:master")); - cl_git_pass(git_remote_download(remote, NULL, NULL)); + cl_git_pass(git_remote_download(remote)); cl_git_pass(git_remote_update_tips(remote)); git_remote_disconnect(remote); @@ -215,7 +215,7 @@ void test_network_remote_local__push_to_non_bare_remote(void) /* Get some commits */ connect_to_local_repository(cl_fixture("testrepo.git")); cl_git_pass(git_remote_add_fetch(remote, "master:master")); - cl_git_pass(git_remote_download(remote, NULL, NULL)); + cl_git_pass(git_remote_download(remote)); cl_git_pass(git_remote_update_tips(remote)); git_remote_disconnect(remote); diff --git a/tests-clar/online/clone.c b/tests-clar/online/clone.c index dc5aa4150..bda260858 100644 --- a/tests-clar/online/clone.c +++ b/tests-clar/online/clone.c @@ -100,11 +100,15 @@ void test_online_clone__can_checkout_a_cloned_repo(void) bool checkout_progress_cb_was_called = false, fetch_progress_cb_was_called = false; + git_remote_callbacks callbacks = GIT_REMOTE_CALLBACKS_INIT; + g_options.checkout_opts.checkout_strategy = GIT_CHECKOUT_SAFE_CREATE; g_options.checkout_opts.progress_cb = &checkout_progress; g_options.checkout_opts.progress_payload = &checkout_progress_cb_was_called; - g_options.fetch_progress_cb = &fetch_progress; - g_options.fetch_progress_payload = &fetch_progress_cb_was_called; + + callbacks.transfer_progress = &fetch_progress; + callbacks.payload = &fetch_progress_cb_was_called; + g_options.remote_callbacks = &callbacks; cl_git_pass(git_clone(&g_repo, LIVE_REPO_URL, "./foo", &g_options)); @@ -199,6 +203,16 @@ static int cancel_at_half(const git_transfer_progress *stats, void *payload) void test_online_clone__can_cancel(void) { - g_options.fetch_progress_cb = cancel_at_half; + git_remote_callbacks callbacks = GIT_REMOTE_CALLBACKS_INIT; + + callbacks.transfer_progress = cancel_at_half; + g_options.remote_callbacks = &callbacks; + cl_git_fail_with(git_clone(&g_repo, LIVE_REPO_URL, "./foo", &g_options), GIT_EUSER); } + + + + + + diff --git a/tests-clar/online/fetch.c b/tests-clar/online/fetch.c index f76c6cff9..df1b2e288 100644 --- a/tests-clar/online/fetch.c +++ b/tests-clar/online/fetch.c @@ -38,14 +38,16 @@ static void do_fetch(const char *url, git_remote_autotag_option_t flag, int n) git_remote_callbacks callbacks = GIT_REMOTE_CALLBACKS_INIT; size_t bytes_received = 0; + callbacks.transfer_progress = progress; callbacks.update_tips = update_tips; + callbacks.payload = &bytes_received; counter = 0; cl_git_pass(git_remote_create(&remote, _repo, "test", url)); git_remote_set_callbacks(remote, &callbacks); git_remote_set_autotag(remote, flag); cl_git_pass(git_remote_connect(remote, GIT_DIRECTION_FETCH)); - cl_git_pass(git_remote_download(remote, progress, &bytes_received)); + cl_git_pass(git_remote_download(remote)); cl_git_pass(git_remote_update_tips(remote)); git_remote_disconnect(remote); cl_assert_equal_i(counter, n); @@ -93,6 +95,7 @@ void test_online_fetch__doesnt_retrieve_a_pack_when_the_repository_is_up_to_date git_repository *_repository; bool invoked = false; git_remote *remote; + git_remote_callbacks callbacks = GIT_REMOTE_CALLBACKS_INIT; git_clone_options opts = GIT_CLONE_OPTIONS_INIT; opts.bare = true; @@ -107,7 +110,10 @@ void test_online_fetch__doesnt_retrieve_a_pack_when_the_repository_is_up_to_date cl_assert_equal_i(false, invoked); - cl_git_pass(git_remote_download(remote, &transferProgressCallback, &invoked)); + callbacks.transfer_progress = &transferProgressCallback; + callbacks.payload = &invoked; + git_remote_set_callbacks(remote, &callbacks); + cl_git_pass(git_remote_download(remote)); cl_assert_equal_i(false, invoked); @@ -131,11 +137,17 @@ void test_online_fetch__can_cancel(void) { git_remote *remote; size_t bytes_received = 0; + git_remote_callbacks callbacks = GIT_REMOTE_CALLBACKS_INIT; cl_git_pass(git_remote_create(&remote, _repo, "test", "http://github.com/libgit2/TestGitRepository.git")); + + callbacks.transfer_progress = cancel_at_half; + callbacks.payload = &bytes_received; + git_remote_set_callbacks(remote, &callbacks); + cl_git_pass(git_remote_connect(remote, GIT_DIRECTION_FETCH)); - cl_git_fail_with(git_remote_download(remote, cancel_at_half, &bytes_received), GIT_EUSER); + cl_git_fail_with(git_remote_download(remote), GIT_EUSER); git_remote_disconnect(remote); git_remote_free(remote); } diff --git a/tests-clar/online/fetchhead.c b/tests-clar/online/fetchhead.c index 58717eef8..5d9eb1318 100644 --- a/tests-clar/online/fetchhead.c +++ b/tests-clar/online/fetchhead.c @@ -48,7 +48,7 @@ static void fetchhead_test_fetch(const char *fetchspec, const char *expected_fet } cl_git_pass(git_remote_connect(remote, GIT_DIRECTION_FETCH)); - cl_git_pass(git_remote_download(remote, NULL, NULL)); + cl_git_pass(git_remote_download(remote)); cl_git_pass(git_remote_update_tips(remote)); git_remote_disconnect(remote); git_remote_free(remote); diff --git a/tests-clar/online/push.c b/tests-clar/online/push.c index 6a4a9b281..d0d4ed05b 100644 --- a/tests-clar/online/push.c +++ b/tests-clar/online/push.c @@ -326,7 +326,7 @@ void test_online_push__initialize(void) /* Now that we've deleted everything, fetch from the remote */ cl_git_pass(git_remote_connect(_remote, GIT_DIRECTION_FETCH)); - cl_git_pass(git_remote_download(_remote, NULL, NULL)); + cl_git_pass(git_remote_download(_remote)); cl_git_pass(git_remote_update_tips(_remote)); git_remote_disconnect(_remote); } else diff --git a/tests-clar/online/push_util.h b/tests-clar/online/push_util.h index 759122aa6..659c6dd54 100644 --- a/tests-clar/online/push_util.h +++ b/tests-clar/online/push_util.h @@ -12,7 +12,7 @@ extern const git_oid OID_ZERO; * @param data pointer to a record_callbacks_data instance */ #define RECORD_CALLBACKS_INIT(data) \ - { GIT_REMOTE_CALLBACKS_VERSION, NULL, NULL, record_update_tips_cb, data } + { GIT_REMOTE_CALLBACKS_VERSION, NULL, NULL, NULL, record_update_tips_cb, data } typedef struct { char *name; From e3c131c544bc79573ebefab4931b5ca89836ace1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Mon, 16 Sep 2013 05:02:25 +0200 Subject: [PATCH 23/37] remote: move the credentials callback to the struct Move this one as well, letting us have a single way of setting the callbacks for the remote, and removing fields from the clone options. --- examples/network/clone.c | 2 +- examples/network/fetch.c | 2 +- examples/network/ls-remote.c | 4 +++- include/git2/clone.h | 5 ----- include/git2/remote.h | 1 + src/clone.c | 2 -- src/remote.c | 13 +------------ src/remote.h | 2 -- src/transports/local.c | 2 +- tests-clar/online/clone.c | 13 +++++++++---- tests-clar/online/push.c | 3 ++- tests-clar/online/push_util.h | 2 +- 12 files changed, 20 insertions(+), 31 deletions(-) diff --git a/examples/network/clone.c b/examples/network/clone.c index f1002656c..f553c4077 100644 --- a/examples/network/clone.c +++ b/examples/network/clone.c @@ -76,9 +76,9 @@ int do_clone(git_repository *repo, int argc, char **argv) checkout_opts.progress_payload = &pd; clone_opts.checkout_opts = checkout_opts; callbacks.transfer_progress = &fetch_progress; + callbacks.credentials = cred_acquire_cb; callbacks.payload = &pd; clone_opts.remote_callbacks = &callbacks; - clone_opts.cred_acquire_cb = cred_acquire_cb; // Do the clone error = git_clone(&cloned_repo, url, path, &clone_opts); diff --git a/examples/network/fetch.c b/examples/network/fetch.c index 1de223373..0c545ad7e 100644 --- a/examples/network/fetch.c +++ b/examples/network/fetch.c @@ -91,8 +91,8 @@ int fetch(git_repository *repo, int argc, char **argv) // Set up the callbacks (only update_tips for now) callbacks.update_tips = &update_cb; callbacks.progress = &progress_cb; + callbacks.credentials = cred_acquire_cb; git_remote_set_callbacks(remote, &callbacks); - git_remote_set_cred_acquire_cb(remote, &cred_acquire_cb, NULL); // Set up the information for the background worker thread data.remote = remote; diff --git a/examples/network/ls-remote.c b/examples/network/ls-remote.c index b22ac47a0..b65759ed3 100644 --- a/examples/network/ls-remote.c +++ b/examples/network/ls-remote.c @@ -18,6 +18,7 @@ static int use_remote(git_repository *repo, char *name) { git_remote *remote = NULL; int error; + git_remote_callbacks callbacks = GIT_REMOTE_CALLBACKS_INIT; // Find the remote by name error = git_remote_load(&remote, repo, name); @@ -27,7 +28,8 @@ static int use_remote(git_repository *repo, char *name) goto cleanup; } - git_remote_set_cred_acquire_cb(remote, &cred_acquire_cb, NULL); + callbacks.credentials = cred_acquire_cb; + git_remote_set_callbacks(remote, &callbacks); error = git_remote_connect(remote, GIT_DIRECTION_FETCH); if (error < 0) diff --git a/include/git2/clone.h b/include/git2/clone.h index 122806ad5..38c759f6e 100644 --- a/include/git2/clone.h +++ b/include/git2/clone.h @@ -48,9 +48,6 @@ GIT_BEGIN_DECL * results in the same behavior as GIT_REMOTE_DEFAULT_FETCH. * - `push_spec` is the fetch specification to be used for pushing. NULL means * use the same spec as for fetching. - * - `cred_acquire_cb` is a callback to be used if credentials are required - * during the initial fetch. - * - `cred_acquire_payload` is the payload for the above callback. * - `transport_flags` is flags used to create transport if no transport is * provided. * - `transport` is a custom transport to be used for the initial fetch. NULL @@ -74,8 +71,6 @@ typedef struct git_clone_options { const char *pushurl; const char *fetch_spec; const char *push_spec; - git_cred_acquire_cb cred_acquire_cb; - void *cred_acquire_payload; git_transport_flags_t transport_flags; git_transport *transport; git_remote_callbacks *remote_callbacks; diff --git a/include/git2/remote.h b/include/git2/remote.h index 2bde5e365..83ad195f4 100644 --- a/include/git2/remote.h +++ b/include/git2/remote.h @@ -395,6 +395,7 @@ struct git_remote_callbacks { unsigned int version; void (*progress)(const char *str, int len, void *data); int (*completion)(git_remote_completion_type type, void *data); + int (*credentials)(git_cred **cred, const char *url, const char *username_from_url, unsigned int allowed_types, void *data); int (*transfer_progress)(const git_transfer_progress *stats, void *data); int (*update_tips)(const char *refname, const git_oid *a, const git_oid *b, void *data); void *payload; diff --git a/src/clone.c b/src/clone.c index 90d677bbb..8385fb264 100644 --- a/src/clone.c +++ b/src/clone.c @@ -310,8 +310,6 @@ static int create_and_configure_origin( if ((error = git_remote_create(&origin, repo, options->remote_name, url)) < 0) goto on_error; - git_remote_set_cred_acquire_cb(origin, options->cred_acquire_cb, - options->cred_acquire_payload); git_remote_set_autotag(origin, options->remote_autotag); /* * Don't write FETCH_HEAD, we'll check out the remote tracking diff --git a/src/remote.c b/src/remote.c index e4696c4ec..2d0321eb3 100644 --- a/src/remote.c +++ b/src/remote.c @@ -591,7 +591,7 @@ int git_remote_connect(git_remote *remote, git_direction direction) if (!remote->check_cert) flags |= GIT_TRANSPORTFLAGS_NO_CHECK_CERT; - if (t->connect(t, url, remote->cred_acquire_cb, remote->cred_acquire_payload, direction, flags) < 0) + if (t->connect(t, url, remote->callbacks.credentials, remote->callbacks.payload, direction, flags) < 0) goto on_error; remote->transport = t; @@ -1152,17 +1152,6 @@ int git_remote_set_callbacks(git_remote *remote, git_remote_callbacks *callbacks return 0; } -void git_remote_set_cred_acquire_cb( - git_remote *remote, - git_cred_acquire_cb cred_acquire_cb, - void *payload) -{ - assert(remote); - - remote->cred_acquire_cb = cred_acquire_cb; - remote->cred_acquire_payload = payload; -} - int git_remote_set_transport(git_remote *remote, git_transport *transport) { assert(remote && transport); diff --git a/src/remote.h b/src/remote.h index dce4803ed..269584d96 100644 --- a/src/remote.h +++ b/src/remote.h @@ -21,8 +21,6 @@ struct git_remote { char *pushurl; git_vector refs; git_vector refspecs; - git_cred_acquire_cb cred_acquire_cb; - void *cred_acquire_payload; git_transport *transport; git_repository *repo; git_remote_callbacks callbacks; diff --git a/src/transports/local.c b/src/transports/local.c index 9ebea979c..3c1f98804 100644 --- a/src/transports/local.c +++ b/src/transports/local.c @@ -434,7 +434,7 @@ static int local_push( if (!url || t->parent.close(&t->parent) < 0 || t->parent.connect(&t->parent, url, - push->remote->cred_acquire_cb, NULL, GIT_DIRECTION_PUSH, flags)) + push->remote->callbacks.credentials, NULL, GIT_DIRECTION_PUSH, flags)) goto on_error; } diff --git a/tests-clar/online/clone.c b/tests-clar/online/clone.c index bda260858..b82cbcd46 100644 --- a/tests-clar/online/clone.c +++ b/tests-clar/online/clone.c @@ -155,11 +155,13 @@ void test_online_clone__credentials(void) cl_getenv("GITTEST_REMOTE_USER"), cl_getenv("GITTEST_REMOTE_PASS") }; + git_remote_callbacks callbacks = GIT_REMOTE_CALLBACKS_INIT; if (!remote_url) return; - g_options.cred_acquire_cb = git_cred_userpass; - g_options.cred_acquire_payload = &user_pass; + callbacks.credentials = git_cred_userpass; + callbacks.payload = &user_pass; + g_options.remote_callbacks = &callbacks; cl_git_pass(git_clone(&g_repo, remote_url, "./foo", &g_options)); git_repository_free(g_repo); g_repo = NULL; @@ -172,8 +174,11 @@ void test_online_clone__bitbucket_style(void) "libgit2", "libgit2" }; - g_options.cred_acquire_cb = git_cred_userpass; - g_options.cred_acquire_payload = &user_pass; + git_remote_callbacks callbacks = GIT_REMOTE_CALLBACKS_INIT; + + callbacks.credentials = git_cred_userpass; + callbacks.payload = &user_pass; + g_options.remote_callbacks = &callbacks; cl_git_pass(git_clone(&g_repo, BB_REPO_URL, "./foo", &g_options)); git_repository_free(g_repo); g_repo = NULL; diff --git a/tests-clar/online/push.c b/tests-clar/online/push.c index d0d4ed05b..05cef56e7 100644 --- a/tests-clar/online/push.c +++ b/tests-clar/online/push.c @@ -17,6 +17,8 @@ static char *_remote_url; static char *_remote_user; static char *_remote_pass; +static int cred_acquire_cb(git_cred **, const char *, const char *, unsigned int, void *); + static git_remote *_remote; static bool _cred_acquire_called; static record_callbacks_data _record_cbs_data = {{ 0 }}; @@ -294,7 +296,6 @@ void test_online_push__initialize(void) if (_remote_url) { cl_git_pass(git_remote_create(&_remote, _repo, "test", _remote_url)); - git_remote_set_cred_acquire_cb(_remote, cred_acquire_cb, &_cred_acquire_called); record_callbacks_data_clear(&_record_cbs_data); git_remote_set_callbacks(_remote, &_record_cbs); diff --git a/tests-clar/online/push_util.h b/tests-clar/online/push_util.h index 659c6dd54..64f02cf2f 100644 --- a/tests-clar/online/push_util.h +++ b/tests-clar/online/push_util.h @@ -12,7 +12,7 @@ extern const git_oid OID_ZERO; * @param data pointer to a record_callbacks_data instance */ #define RECORD_CALLBACKS_INIT(data) \ - { GIT_REMOTE_CALLBACKS_VERSION, NULL, NULL, NULL, record_update_tips_cb, data } + { GIT_REMOTE_CALLBACKS_VERSION, NULL, NULL, cred_acquire_cb, NULL, record_update_tips_cb, data } typedef struct { char *name; From d19870d947eef17008ae0b4b7ebc9e9d0038a770 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Mon, 16 Sep 2013 05:10:55 +0200 Subject: [PATCH 24/37] clone: implement git_clone_into This allows you to set up the repository and remote as you which to have them before performing the clone operation. --- include/git2/clone.h | 16 ++++++++ src/clone.c | 78 +++++++++++++++++++++++++++++++-------- tests-clar/online/clone.c | 39 ++++++++++++++++++++ 3 files changed, 118 insertions(+), 15 deletions(-) diff --git a/include/git2/clone.h b/include/git2/clone.h index 38c759f6e..c3936f6b0 100644 --- a/include/git2/clone.h +++ b/include/git2/clone.h @@ -99,6 +99,22 @@ GIT_EXTERN(int) git_clone( const char *local_path, const git_clone_options *options); +/** + * Clone into a repository + * + * After creating the repository and remote and configuring them for + * paths and callbacks respectively, you can call this function to + * perform the clone operation and optionally checkout files. + * + * @param repo the repository to use + * @param remote the remote repository to clone from + * @param co_opts options to use during checkout + * @param branch the branch to checkout after the clone, pass NULL for the remote's + * default branch + * @return 0 on success or an error code + */ +GIT_EXTERN(int) git_clone_into(git_repository *repo, git_remote *remote, git_checkout_opts *co_opts, const char *branch); + /** @} */ GIT_END_DECL #endif diff --git a/src/clone.c b/src/clone.c index 8385fb264..436fdff43 100644 --- a/src/clone.c +++ b/src/clone.c @@ -270,23 +270,23 @@ cleanup: static int update_head_to_branch( git_repository *repo, - const git_clone_options *options) + const char *remote_name, + const char *branch) { int retcode; git_buf remote_branch_name = GIT_BUF_INIT; git_reference* remote_ref = NULL; - assert(options->checkout_branch); + assert(remote_name && branch); if ((retcode = git_buf_printf(&remote_branch_name, GIT_REFS_REMOTES_DIR "%s/%s", - options->remote_name, options->checkout_branch)) < 0 ) + remote_name, branch)) < 0 ) goto cleanup; if ((retcode = git_reference_lookup(&remote_ref, repo, git_buf_cstr(&remote_branch_name))) < 0) goto cleanup; - retcode = update_head_to_new_branch(repo, git_reference_target(remote_ref), - options->checkout_branch); + retcode = update_head_to_new_branch(repo, git_reference_target(remote_ref), branch); cleanup: git_reference_free(remote_ref); @@ -350,6 +350,23 @@ on_error: return error; } +static int do_fetch(git_remote *origin) +{ + int retcode; + + /* Connect and download everything */ + if ((retcode = git_remote_connect(origin, GIT_DIRECTION_FETCH)) < 0) + return retcode; + + if ((retcode = git_remote_download(origin)) < 0) + return retcode; + + /* Create "origin/foo" branches for all remote branches */ + if ((retcode = git_remote_update_tips(origin)) < 0) + return retcode; + + return 0; +} static int setup_remotes_and_fetch( git_repository *repo, @@ -374,20 +391,12 @@ static int setup_remotes_and_fetch( ((retcode = git_remote_add_fetch(origin, "refs/tags/*:refs/tags/*")) < 0)) goto on_error; - /* Connect and download everything */ - if ((retcode = git_remote_connect(origin, GIT_DIRECTION_FETCH)) < 0) - goto on_error; - - if ((retcode = git_remote_download(origin)) < 0) - goto on_error; - - /* Create "origin/foo" branches for all remote branches */ - if ((retcode = git_remote_update_tips(origin)) < 0) + if ((retcode = do_fetch(origin)) < 0) goto on_error; /* Point HEAD to the requested branch */ if (options->checkout_branch) - retcode = update_head_to_branch(repo, options); + retcode = update_head_to_branch(repo, options->remote_name, options->checkout_branch); /* Point HEAD to the same ref as the remote's head */ else retcode = update_head_to_remote(repo, origin); @@ -432,6 +441,45 @@ static void normalize_options(git_clone_options *dst, const git_clone_options *s } } +int git_clone_into(git_repository *repo, git_remote *remote, git_checkout_opts *co_opts, const char *branch) +{ + int error = 0, old_fetchhead; + size_t nspecs; + + assert(repo && remote); + + if (!git_repository_is_empty(repo)) { + giterr_set(GITERR_INVALID, "the repository is not empty"); + return -1; + } + + if ((error = git_remote_add_fetch(remote, "refs/tags/*:refs/tags/*")) < 0) + return error; + + old_fetchhead = git_remote_update_fetchhead(remote); + git_remote_set_update_fetchhead(remote, 0); + + if ((error = do_fetch(remote)) < 0) + goto cleanup; + + if (branch) + error = update_head_to_branch(repo, git_remote_name(remote), branch); + /* Point HEAD to the same ref as the remote's head */ + else + error = update_head_to_remote(repo, remote); + + if (!error && should_checkout(repo, git_repository_is_bare(repo), co_opts)) + error = git_checkout_head(repo, co_opts); + +cleanup: + git_remote_set_update_fetchhead(remote, old_fetchhead); + /* Remove the tags refspec */ + nspecs = git_remote_refspec_count(remote); + git_remote_remove_refspec(remote, nspecs); + + return error; +} + int git_clone( git_repository **out, const char *url, diff --git a/tests-clar/online/clone.c b/tests-clar/online/clone.c index b82cbcd46..9a64ba166 100644 --- a/tests-clar/online/clone.c +++ b/tests-clar/online/clone.c @@ -126,6 +126,45 @@ void test_online_clone__can_checkout_a_cloned_repo(void) git_buf_free(&path); } +void test_online_clone__clone_into(void) +{ + git_buf path = GIT_BUF_INIT; + git_remote *remote; + git_reference *head; + git_checkout_opts checkout_opts = GIT_CHECKOUT_OPTS_INIT; + git_remote_callbacks callbacks = GIT_REMOTE_CALLBACKS_INIT; + + bool checkout_progress_cb_was_called = false, + fetch_progress_cb_was_called = false; + + checkout_opts.checkout_strategy = GIT_CHECKOUT_SAFE_CREATE; + checkout_opts.progress_cb = &checkout_progress; + checkout_opts.progress_payload = &checkout_progress_cb_was_called; + + cl_git_pass(git_repository_init(&g_repo, "./foo", false)); + cl_git_pass(git_remote_create(&remote, g_repo, "origin", LIVE_REPO_URL)); + + callbacks.transfer_progress = &fetch_progress; + callbacks.payload = &fetch_progress_cb_was_called; + git_remote_set_callbacks(remote, &callbacks); + + cl_git_pass(git_clone_into(g_repo, remote, &checkout_opts, NULL)); + + cl_git_pass(git_buf_joinpath(&path, git_repository_workdir(g_repo), "master.txt")); + cl_assert_equal_i(true, git_path_isfile(git_buf_cstr(&path))); + + cl_git_pass(git_reference_lookup(&head, g_repo, "HEAD")); + cl_assert_equal_i(GIT_REF_SYMBOLIC, git_reference_type(head)); + cl_assert_equal_s("refs/heads/master", git_reference_symbolic_target(head)); + + cl_assert_equal_i(true, checkout_progress_cb_was_called); + cl_assert_equal_i(true, fetch_progress_cb_was_called); + + git_remote_free(remote); + git_reference_free(head); + git_buf_free(&path); +} + static int update_tips(const char *refname, const git_oid *a, const git_oid *b, void *payload) { int *callcount = (int*)payload; From fe3a40a4ff056400cde6e456211d6b5f2ec1008e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Mon, 16 Sep 2013 16:54:37 +0200 Subject: [PATCH 25/37] remote: add a convenience 'fetch' function. --- include/git2/remote.h | 11 +++++++++++ src/clone.c | 22 ++-------------------- src/remote.c | 18 ++++++++++++++++++ 3 files changed, 31 insertions(+), 20 deletions(-) diff --git a/include/git2/remote.h b/include/git2/remote.h index 83ad195f4..8c21870de 100644 --- a/include/git2/remote.h +++ b/include/git2/remote.h @@ -310,6 +310,17 @@ GIT_EXTERN(void) git_remote_free(git_remote *remote); */ GIT_EXTERN(int) git_remote_update_tips(git_remote *remote); +/** + * Download new data and update tips + * + * Convenience function to connect to a remote, download the data, + * disconnect and update the remote-tracking branches. + * + * @param remote the remote to fetch from + * @return 0 or an error code + */ +GIT_EXTERN(int) git_remote_fetch(git_remote *remote); + /** * Return whether a string is a valid remote URL * diff --git a/src/clone.c b/src/clone.c index 436fdff43..13f2a8eea 100644 --- a/src/clone.c +++ b/src/clone.c @@ -350,24 +350,6 @@ on_error: return error; } -static int do_fetch(git_remote *origin) -{ - int retcode; - - /* Connect and download everything */ - if ((retcode = git_remote_connect(origin, GIT_DIRECTION_FETCH)) < 0) - return retcode; - - if ((retcode = git_remote_download(origin)) < 0) - return retcode; - - /* Create "origin/foo" branches for all remote branches */ - if ((retcode = git_remote_update_tips(origin)) < 0) - return retcode; - - return 0; -} - static int setup_remotes_and_fetch( git_repository *repo, const char *url, @@ -391,7 +373,7 @@ static int setup_remotes_and_fetch( ((retcode = git_remote_add_fetch(origin, "refs/tags/*:refs/tags/*")) < 0)) goto on_error; - if ((retcode = do_fetch(origin)) < 0) + if ((retcode = git_remote_fetch(origin)) < 0) goto on_error; /* Point HEAD to the requested branch */ @@ -459,7 +441,7 @@ int git_clone_into(git_repository *repo, git_remote *remote, git_checkout_opts * old_fetchhead = git_remote_update_fetchhead(remote); git_remote_set_update_fetchhead(remote, 0); - if ((error = do_fetch(remote)) < 0) + if ((error = git_remote_fetch(remote)) < 0) goto cleanup; if (branch) diff --git a/src/remote.c b/src/remote.c index 2d0321eb3..ace886502 100644 --- a/src/remote.c +++ b/src/remote.c @@ -767,6 +767,24 @@ int git_remote_download(git_remote *remote) return git_fetch_download_pack(remote); } +int git_remote_fetch(git_remote *remote) +{ + int error; + + /* Connect and download everything */ + if ((error = git_remote_connect(remote, GIT_DIRECTION_FETCH)) < 0) + return error; + + if ((error = git_remote_download(remote)) < 0) + return error; + + /* We don't need to be connected anymore */ + git_remote_disconnect(remote); + + /* Create "remote/foo" branches for all remote branches */ + return git_remote_update_tips(remote); +} + static int remote_head_for_fetchspec_src(git_remote_head **out, git_vector *update_heads, const char *fetchspec_src) { unsigned int i; From c8dbec4803aa7d8300f19a97431bbf631ac5a392 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Mon, 16 Sep 2013 18:42:53 +0200 Subject: [PATCH 26/37] clone: remove the autotag option Downloading all tags is part of what makes it a clone instead of simply a fetch. --- include/git2/clone.h | 3 --- src/clone.c | 13 +++++-------- tests-clar/clone/nonetwork.c | 33 --------------------------------- 3 files changed, 5 insertions(+), 44 deletions(-) diff --git a/include/git2/clone.h b/include/git2/clone.h index c3936f6b0..c80bf9baa 100644 --- a/include/git2/clone.h +++ b/include/git2/clone.h @@ -54,8 +54,6 @@ GIT_BEGIN_DECL * means use the transport autodetected from the URL. * - `remote_callbacks` may be used to specify custom progress callbacks for * the origin remote before the fetch is initiated. - * - `remote_autotag` may be used to specify the autotag setting before the - * initial fetch. The default is GIT_REMOTE_DOWNLOAD_TAGS_ALL. * - `checkout_branch` gives the name of the branch to checkout. NULL means * use the remote's HEAD. */ @@ -74,7 +72,6 @@ typedef struct git_clone_options { git_transport_flags_t transport_flags; git_transport *transport; git_remote_callbacks *remote_callbacks; - git_remote_autotag_option_t remote_autotag; const char* checkout_branch; } git_clone_options; diff --git a/src/clone.c b/src/clone.c index 13f2a8eea..d720907cc 100644 --- a/src/clone.c +++ b/src/clone.c @@ -310,7 +310,6 @@ static int create_and_configure_origin( if ((error = git_remote_create(&origin, repo, options->remote_name, url)) < 0) goto on_error; - git_remote_set_autotag(origin, options->remote_autotag); /* * Don't write FETCH_HEAD, we'll check out the remote tracking * branch ourselves based on the server's default. @@ -364,13 +363,11 @@ static int setup_remotes_and_fetch( git_remote_set_update_fetchhead(origin, 0); - /* If the download_tags value has not been specified, then make sure to - * download tags as well. It is set here because we want to download tags - * on the initial clone, but do not want to persist the value in the - * configuration file. - */ - if (origin->download_tags == GIT_REMOTE_DOWNLOAD_TAGS_AUTO && - ((retcode = git_remote_add_fetch(origin, "refs/tags/*:refs/tags/*")) < 0)) + /* Make sure to download all tags as well. It is set here because + * we want to download tags on the initial clone, but do not + * want to persist the value in the configuration file. + */ + if((retcode = git_remote_add_fetch(origin, "refs/tags/*:refs/tags/*")) < 0) goto on_error; if ((retcode = git_remote_fetch(origin)) < 0) diff --git a/tests-clar/clone/nonetwork.c b/tests-clar/clone/nonetwork.c index 5b9faa645..382491610 100644 --- a/tests-clar/clone/nonetwork.c +++ b/tests-clar/clone/nonetwork.c @@ -171,39 +171,6 @@ void test_clone_nonetwork__custom_push_spec(void) cl_assert_equal_s("refs/heads/foo", git_refspec_dst(actual_fs)); } -void test_clone_nonetwork__custom_autotag(void) -{ - git_remote *origin; - git_strarray tags = {0}; - - g_options.remote_autotag = GIT_REMOTE_DOWNLOAD_TAGS_NONE; - cl_git_pass(git_clone(&g_repo, cl_git_fixture_url("testrepo.git"), "./foo", &g_options)); - - cl_git_pass(git_tag_list(&tags, g_repo)); - cl_assert_equal_sz(0, tags.count); - - cl_git_pass(git_remote_load(&origin, g_repo, "origin")); - cl_assert_equal_i(GIT_REMOTE_DOWNLOAD_TAGS_NONE, origin->download_tags); - - git_strarray_free(&tags); - git_remote_free(origin); -} - -void test_clone_nonetwork__custom_autotag_tags_all(void) -{ - git_strarray tags = {0}; - git_remote *origin; - - g_options.remote_autotag = GIT_REMOTE_DOWNLOAD_TAGS_ALL; - cl_git_pass(git_clone(&g_repo, cl_git_fixture_url("testrepo.git"), "./foo", &g_options)); - - cl_git_pass(git_remote_load(&origin, g_repo, "origin")); - cl_assert_equal_i(GIT_REMOTE_DOWNLOAD_TAGS_ALL, origin->download_tags); - - git_strarray_free(&tags); - git_remote_free(origin); -} - void test_clone_nonetwork__cope_with_already_existing_directory(void) { p_mkdir("./foo", GIT_DIR_MODE); From e3a92f0dfc28e423a2f06688338f81b8d17abaf6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 17 Sep 2013 05:31:34 +0200 Subject: [PATCH 27/37] clone: implement git_clone on top of git_clone_into Unify the code bases. --- src/clone.c | 77 +++++++++++++---------------------------------------- 1 file changed, 19 insertions(+), 58 deletions(-) diff --git a/src/clone.c b/src/clone.c index d720907cc..3df317045 100644 --- a/src/clone.c +++ b/src/clone.c @@ -310,12 +310,6 @@ static int create_and_configure_origin( if ((error = git_remote_create(&origin, repo, options->remote_name, url)) < 0) goto on_error; - /* - * Don't write FETCH_HEAD, we'll check out the remote tracking - * branch ourselves based on the server's default. - */ - git_remote_set_update_fetchhead(origin, 0); - if (options->remote_callbacks && (error = git_remote_set_callbacks(origin, options->remote_callbacks)) < 0) goto on_error; @@ -349,43 +343,6 @@ on_error: return error; } -static int setup_remotes_and_fetch( - git_repository *repo, - const char *url, - const git_clone_options *options) -{ - int retcode = GIT_ERROR; - git_remote *origin = NULL; - - /* Construct an origin remote */ - if ((retcode = create_and_configure_origin(&origin, repo, url, options)) < 0) - goto on_error; - - git_remote_set_update_fetchhead(origin, 0); - - /* Make sure to download all tags as well. It is set here because - * we want to download tags on the initial clone, but do not - * want to persist the value in the configuration file. - */ - if((retcode = git_remote_add_fetch(origin, "refs/tags/*:refs/tags/*")) < 0) - goto on_error; - - if ((retcode = git_remote_fetch(origin)) < 0) - goto on_error; - - /* Point HEAD to the requested branch */ - if (options->checkout_branch) - retcode = update_head_to_branch(repo, options->remote_name, options->checkout_branch); - /* Point HEAD to the same ref as the remote's head */ - else - retcode = update_head_to_remote(repo, origin); - -on_error: - git_remote_free(origin); - return retcode; -} - - static bool should_checkout( git_repository *repo, bool is_bare, @@ -467,6 +424,7 @@ int git_clone( { int retcode = GIT_ERROR; git_repository *repo = NULL; + git_remote *origin; git_clone_options normOptions; int remove_directory_on_failure = 0; git_repository_init_options initOptions = GIT_REPOSITORY_INIT_OPTIONS_INIT; @@ -486,24 +444,27 @@ int git_clone( /* Only remove the directory on failure if we create it */ remove_directory_on_failure = !git_path_exists(local_path); - if (!(retcode = git_repository_init_ext(&repo, local_path, normOptions.init_options))) { - if ((retcode = setup_remotes_and_fetch(repo, url, &normOptions)) < 0) { - /* Failed to fetch; clean up */ - git_repository_free(repo); + if ((retcode = git_repository_init_ext(&repo, local_path, normOptions.init_options)) < 0) + return retcode; - if (remove_directory_on_failure) - git_futils_rmdir_r(local_path, NULL, GIT_RMDIR_REMOVE_FILES); - else - git_futils_cleanupdir_r(local_path); + if ((retcode = create_and_configure_origin(&origin, repo, url, &normOptions)) < 0) + goto cleanup; - } else { - *out = repo; - retcode = 0; - } - } + retcode = git_clone_into(repo, origin, &normOptions.checkout_opts, normOptions.checkout_branch); + git_remote_free(origin); - if (!retcode && should_checkout(repo, normOptions.bare, &normOptions.checkout_opts)) - retcode = git_checkout_head(*out, &normOptions.checkout_opts); + if (retcode < 0) + goto cleanup; + + *out = repo; + return 0; + +cleanup: + git_repository_free(repo); + if (remove_directory_on_failure) + git_futils_rmdir_r(local_path, NULL, GIT_RMDIR_REMOVE_FILES); + else + git_futils_cleanupdir_r(local_path); return retcode; } From 6ac15eff6d173674d9f17e9d5ddb98997eb97cf4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Fri, 20 Sep 2013 22:34:05 +0200 Subject: [PATCH 28/37] clone: remove more options from basic clone The basic clone function is there to make it easy to create a "normal" clone. Remove a bunch of options that are about changing the remote's configuration. --- include/git2/clone.h | 7 ------ src/clone.c | 48 ++++------------------------------- tests-clar/clone/nonetwork.c | 49 ------------------------------------ 3 files changed, 5 insertions(+), 99 deletions(-) diff --git a/include/git2/clone.h b/include/git2/clone.h index c80bf9baa..0a89b5712 100644 --- a/include/git2/clone.h +++ b/include/git2/clone.h @@ -62,15 +62,8 @@ typedef struct git_clone_options { unsigned int version; git_checkout_opts checkout_opts; - git_repository_init_options *init_options; int bare; - const char *remote_name; - const char *pushurl; - const char *fetch_spec; - const char *push_spec; - git_transport_flags_t transport_flags; - git_transport *transport; git_remote_callbacks *remote_callbacks; const char* checkout_branch; } git_clone_options; diff --git a/src/clone.c b/src/clone.c index 3df317045..abea0bd8f 100644 --- a/src/clone.c +++ b/src/clone.c @@ -307,31 +307,13 @@ static int create_and_configure_origin( int error; git_remote *origin = NULL; - if ((error = git_remote_create(&origin, repo, options->remote_name, url)) < 0) + if ((error = git_remote_create(&origin, repo, "origin", url)) < 0) goto on_error; if (options->remote_callbacks && (error = git_remote_set_callbacks(origin, options->remote_callbacks)) < 0) goto on_error; - if (options->fetch_spec) { - git_remote_clear_refspecs(origin); - if ((error = git_remote_add_fetch(origin, options->fetch_spec)) < 0) - goto on_error; - } - - if (options->push_spec && - (error = git_remote_add_push(origin, options->push_spec)) < 0) - goto on_error; - - if (options->pushurl && - (error = git_remote_set_pushurl(origin, options->pushurl)) < 0) - goto on_error; - - if (options->transport_flags == GIT_TRANSPORTFLAGS_NO_CHECK_CERT) { - git_remote_check_cert(origin, 0); - } - if ((error = git_remote_save(origin)) < 0) goto on_error; @@ -360,23 +342,6 @@ static bool should_checkout( return !git_repository_head_unborn(repo); } -static void normalize_options(git_clone_options *dst, const git_clone_options *src, git_repository_init_options *initOptions) -{ - git_clone_options default_options = GIT_CLONE_OPTIONS_INIT; - if (!src) src = &default_options; - - *dst = *src; - - /* Provide defaults for null pointers */ - if (!dst->remote_name) dst->remote_name = "origin"; - if (!dst->init_options) { - dst->init_options = initOptions; - initOptions->flags = GIT_REPOSITORY_INIT_MKPATH; - if (dst->bare) - initOptions->flags |= GIT_REPOSITORY_INIT_BARE; - } -} - int git_clone_into(git_repository *repo, git_remote *remote, git_checkout_opts *co_opts, const char *branch) { int error = 0, old_fetchhead; @@ -425,14 +390,11 @@ int git_clone( int retcode = GIT_ERROR; git_repository *repo = NULL; git_remote *origin; - git_clone_options normOptions; int remove_directory_on_failure = 0; - git_repository_init_options initOptions = GIT_REPOSITORY_INIT_OPTIONS_INIT; assert(out && url && local_path); - normalize_options(&normOptions, options, &initOptions); - GITERR_CHECK_VERSION(&normOptions, GIT_CLONE_OPTIONS_VERSION, "git_clone_options"); + GITERR_CHECK_VERSION(options, GIT_CLONE_OPTIONS_VERSION, "git_clone_options"); /* Only clone to a new directory or an empty directory */ if (git_path_exists(local_path) && !git_path_is_empty_dir(local_path)) { @@ -444,13 +406,13 @@ int git_clone( /* Only remove the directory on failure if we create it */ remove_directory_on_failure = !git_path_exists(local_path); - if ((retcode = git_repository_init_ext(&repo, local_path, normOptions.init_options)) < 0) + if ((retcode = git_repository_init(&repo, local_path, options->bare)) < 0) return retcode; - if ((retcode = create_and_configure_origin(&origin, repo, url, &normOptions)) < 0) + if ((retcode = create_and_configure_origin(&origin, repo, url, options)) < 0) goto cleanup; - retcode = git_clone_into(repo, origin, &normOptions.checkout_opts, normOptions.checkout_branch); + retcode = git_clone_into(repo, origin, &options->checkout_opts, options->checkout_branch); git_remote_free(origin); if (retcode < 0) diff --git a/tests-clar/clone/nonetwork.c b/tests-clar/clone/nonetwork.c index 382491610..84654fa5a 100644 --- a/tests-clar/clone/nonetwork.c +++ b/tests-clar/clone/nonetwork.c @@ -122,55 +122,6 @@ void test_clone_nonetwork__fail_with_already_existing_but_non_empty_directory(vo cl_git_fail(git_clone(&g_repo, cl_git_fixture_url("testrepo.git"), "./foo", &g_options)); } -void test_clone_nonetwork__custom_origin_name(void) -{ - g_options.remote_name = "my_origin"; - cl_git_pass(git_clone(&g_repo, cl_git_fixture_url("testrepo.git"), "./foo", &g_options)); - - cl_git_pass(git_remote_load(&g_remote, g_repo, "my_origin")); -} - -void test_clone_nonetwork__custom_push_url(void) -{ - const char *url = "http://example.com"; - - g_options.pushurl = url; - cl_git_pass(git_clone(&g_repo, cl_git_fixture_url("testrepo.git"), "./foo", &g_options)); - - cl_git_pass(git_remote_load(&g_remote, g_repo, "origin")); - cl_assert_equal_s(url, git_remote_pushurl(g_remote)); -} - -void test_clone_nonetwork__custom_fetch_spec(void) -{ - const git_refspec *actual_fs; - const char *spec = "+refs/heads/master:refs/heads/foo"; - - g_options.fetch_spec = spec; - cl_git_pass(git_clone(&g_repo, cl_git_fixture_url("testrepo.git"), "./foo", &g_options)); - - cl_git_pass(git_remote_load(&g_remote, g_repo, "origin")); - actual_fs = git_remote_get_refspec(g_remote, 0); - cl_assert_equal_s("refs/heads/master", git_refspec_src(actual_fs)); - cl_assert_equal_s("refs/heads/foo", git_refspec_dst(actual_fs)); - - cl_git_pass(git_reference_lookup(&g_ref, g_repo, "refs/heads/foo")); -} - -void test_clone_nonetwork__custom_push_spec(void) -{ - const git_refspec *actual_fs; - const char *spec = "+refs/heads/master:refs/heads/foo"; - - g_options.push_spec = spec; - cl_git_pass(git_clone(&g_repo, cl_git_fixture_url("testrepo.git"), "./foo", &g_options)); - - cl_git_pass(git_remote_load(&g_remote, g_repo, "origin")); - actual_fs = git_remote_get_refspec(g_remote, git_remote_refspec_count(g_remote) - 1); - cl_assert_equal_s("refs/heads/master", git_refspec_src(actual_fs)); - cl_assert_equal_s("refs/heads/foo", git_refspec_dst(actual_fs)); -} - void test_clone_nonetwork__cope_with_already_existing_directory(void) { p_mkdir("./foo", GIT_DIR_MODE); From b9bf5d701dd6dfcfcb41d2655f59f7669cd50ef9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Fri, 20 Sep 2013 22:46:32 +0200 Subject: [PATCH 29/37] clone: re-add a way to ignore certificate errors This used to be done via transport flags, which was removed in a previous commit. --- include/git2/clone.h | 1 + src/clone.c | 3 +++ 2 files changed, 4 insertions(+) diff --git a/include/git2/clone.h b/include/git2/clone.h index 0a89b5712..cf759ab5c 100644 --- a/include/git2/clone.h +++ b/include/git2/clone.h @@ -63,6 +63,7 @@ typedef struct git_clone_options { git_checkout_opts checkout_opts; int bare; + int ignore_cert_errors; git_remote_callbacks *remote_callbacks; const char* checkout_branch; diff --git a/src/clone.c b/src/clone.c index abea0bd8f..904be1b57 100644 --- a/src/clone.c +++ b/src/clone.c @@ -310,6 +310,9 @@ static int create_and_configure_origin( if ((error = git_remote_create(&origin, repo, "origin", url)) < 0) goto on_error; + if (options->ignore_cert_errors) + git_remote_check_cert(origin, 0); + if (options->remote_callbacks && (error = git_remote_set_callbacks(origin, options->remote_callbacks)) < 0) goto on_error; From eec1c1fe1e9a12b57c4014be25ce4ed390c8e199 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Fri, 20 Sep 2013 22:49:20 +0200 Subject: [PATCH 30/37] clone: const-ify checkout options The removal of many options which lead to the direct usage of the user's checkout options means we should make sure they remain const. --- include/git2/checkout.h | 2 +- include/git2/clone.h | 2 +- src/checkout.c | 6 +++--- src/checkout.h | 2 +- src/clone.c | 4 ++-- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/include/git2/checkout.h b/include/git2/checkout.h index aa48069cd..844f0a9e2 100644 --- a/include/git2/checkout.h +++ b/include/git2/checkout.h @@ -255,7 +255,7 @@ typedef struct git_checkout_opts { */ GIT_EXTERN(int) git_checkout_head( git_repository *repo, - git_checkout_opts *opts); + const git_checkout_opts *opts); /** * Updates files in the working tree to match the content of the index. diff --git a/include/git2/clone.h b/include/git2/clone.h index cf759ab5c..a59de1bc7 100644 --- a/include/git2/clone.h +++ b/include/git2/clone.h @@ -104,7 +104,7 @@ GIT_EXTERN(int) git_clone( * default branch * @return 0 on success or an error code */ -GIT_EXTERN(int) git_clone_into(git_repository *repo, git_remote *remote, git_checkout_opts *co_opts, const char *branch); +GIT_EXTERN(int) git_clone_into(git_repository *repo, git_remote *remote, const git_checkout_opts *co_opts, const char *branch); /** @} */ GIT_END_DECL diff --git a/src/checkout.c b/src/checkout.c index 0e9d11bff..5d741d393 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -1119,7 +1119,7 @@ static void checkout_data_clear(checkout_data *data) static int checkout_data_init( checkout_data *data, git_iterator *target, - git_checkout_opts *proposed) + const git_checkout_opts *proposed) { int error = 0; git_repository *repo = git_iterator_owner(target); @@ -1229,7 +1229,7 @@ cleanup: int git_checkout_iterator( git_iterator *target, - git_checkout_opts *opts) + const git_checkout_opts *opts) { int error = 0; git_iterator *baseline = NULL, *workdir = NULL; @@ -1404,7 +1404,7 @@ int git_checkout_tree( int git_checkout_head( git_repository *repo, - git_checkout_opts *opts) + const git_checkout_opts *opts) { int error; git_tree *head = NULL; diff --git a/src/checkout.h b/src/checkout.h index b1dc80c38..6d7186860 100644 --- a/src/checkout.h +++ b/src/checkout.h @@ -19,6 +19,6 @@ */ extern int git_checkout_iterator( git_iterator *target, - git_checkout_opts *opts); + const git_checkout_opts *opts); #endif diff --git a/src/clone.c b/src/clone.c index 904be1b57..60525939f 100644 --- a/src/clone.c +++ b/src/clone.c @@ -331,7 +331,7 @@ on_error: static bool should_checkout( git_repository *repo, bool is_bare, - git_checkout_opts *opts) + const git_checkout_opts *opts) { if (is_bare) return false; @@ -345,7 +345,7 @@ static bool should_checkout( return !git_repository_head_unborn(repo); } -int git_clone_into(git_repository *repo, git_remote *remote, git_checkout_opts *co_opts, const char *branch) +int git_clone_into(git_repository *repo, git_remote *remote, const git_checkout_opts *co_opts, const char *branch) { int error = 0, old_fetchhead; size_t nspecs; From c833893c64d28d4c017fdbf90bbeb714314a8dd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Fri, 20 Sep 2013 22:57:01 +0200 Subject: [PATCH 31/37] clone: re-allow using a custom remote name This is a small thing that by itself doesn't quite justify making the user use clone_into. --- include/git2/clone.h | 22 +++++----------------- src/clone.c | 4 +++- tests-clar/clone/nonetwork.c | 9 +++++++++ 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/include/git2/clone.h b/include/git2/clone.h index a59de1bc7..bd602fb7a 100644 --- a/include/git2/clone.h +++ b/include/git2/clone.h @@ -35,25 +35,12 @@ GIT_BEGIN_DECL * set the `checkout_strategy` to GIT_CHECKOUT_DEFAULT. * - `bare` should be set to zero to create a standard repo, non-zero for * a bare repo - * - `fetch_progress_cb` is optional callback for fetch progress. Be aware that - * this is called inline with network and indexing operations, so performance - * may be affected. - * - `fetch_progress_payload` is payload for fetch_progress_cb + * - `ignore_cert_errors` should be set to 1 if errors validating the remote host's + * certificate should be ignored. * * ** "origin" remote options: ** * - `remote_name` is the name given to the "origin" remote. The default is * "origin". - * - `pushurl` is a URL to be used for pushing. NULL means use the fetch url. - * - `fetch_spec` is the fetch specification to be used for fetching. NULL - * results in the same behavior as GIT_REMOTE_DEFAULT_FETCH. - * - `push_spec` is the fetch specification to be used for pushing. NULL means - * use the same spec as for fetching. - * - `transport_flags` is flags used to create transport if no transport is - * provided. - * - `transport` is a custom transport to be used for the initial fetch. NULL - * means use the transport autodetected from the URL. - * - `remote_callbacks` may be used to specify custom progress callbacks for - * the origin remote before the fetch is initiated. * - `checkout_branch` gives the name of the branch to checkout. NULL means * use the remote's HEAD. */ @@ -62,10 +49,11 @@ typedef struct git_clone_options { unsigned int version; git_checkout_opts checkout_opts; + git_remote_callbacks *remote_callbacks; + int bare; int ignore_cert_errors; - - git_remote_callbacks *remote_callbacks; + const char *remote_name; const char* checkout_branch; } git_clone_options; diff --git a/src/clone.c b/src/clone.c index 60525939f..cad9ea1dc 100644 --- a/src/clone.c +++ b/src/clone.c @@ -306,8 +306,10 @@ static int create_and_configure_origin( { int error; git_remote *origin = NULL; + const char *name; - if ((error = git_remote_create(&origin, repo, "origin", url)) < 0) + name = options->remote_name ? options->remote_name : "origin"; + if ((error = git_remote_create(&origin, repo, name, url)) < 0) goto on_error; if (options->ignore_cert_errors) diff --git a/tests-clar/clone/nonetwork.c b/tests-clar/clone/nonetwork.c index 84654fa5a..c7a219259 100644 --- a/tests-clar/clone/nonetwork.c +++ b/tests-clar/clone/nonetwork.c @@ -122,6 +122,15 @@ void test_clone_nonetwork__fail_with_already_existing_but_non_empty_directory(vo cl_git_fail(git_clone(&g_repo, cl_git_fixture_url("testrepo.git"), "./foo", &g_options)); } +void test_clone_nonetwork__custom_origin_name(void) +{ + g_options.remote_name = "my_origin"; + cl_git_pass(git_clone(&g_repo, cl_git_fixture_url("testrepo.git"), "./foo", &g_options)); + + cl_git_pass(git_remote_load(&g_remote, g_repo, "my_origin")); +} + + void test_clone_nonetwork__cope_with_already_existing_directory(void) { p_mkdir("./foo", GIT_DIR_MODE); From fdc7e5e35efa072a145cf318457055f60c9b21eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Fri, 20 Sep 2013 23:14:12 +0200 Subject: [PATCH 32/37] clone: bring back NULL as defaults This wasremoved as part of the large culling a few commits ago. --- src/clone.c | 14 +++++++++----- tests-clar/clone/nonetwork.c | 6 ++++++ 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/clone.c b/src/clone.c index cad9ea1dc..1af6e393f 100644 --- a/src/clone.c +++ b/src/clone.c @@ -390,16 +390,20 @@ int git_clone( git_repository **out, const char *url, const char *local_path, - const git_clone_options *options) + const git_clone_options *_options) { int retcode = GIT_ERROR; git_repository *repo = NULL; git_remote *origin; + git_clone_options options = GIT_CLONE_OPTIONS_INIT; int remove_directory_on_failure = 0; assert(out && url && local_path); - GITERR_CHECK_VERSION(options, GIT_CLONE_OPTIONS_VERSION, "git_clone_options"); + if (_options) + memcpy(&options, _options, sizeof(git_clone_options)); + + GITERR_CHECK_VERSION(&options, GIT_CLONE_OPTIONS_VERSION, "git_clone_options"); /* Only clone to a new directory or an empty directory */ if (git_path_exists(local_path) && !git_path_is_empty_dir(local_path)) { @@ -411,13 +415,13 @@ int git_clone( /* Only remove the directory on failure if we create it */ remove_directory_on_failure = !git_path_exists(local_path); - if ((retcode = git_repository_init(&repo, local_path, options->bare)) < 0) + if ((retcode = git_repository_init(&repo, local_path, options.bare)) < 0) return retcode; - if ((retcode = create_and_configure_origin(&origin, repo, url, options)) < 0) + if ((retcode = create_and_configure_origin(&origin, repo, url, &options)) < 0) goto cleanup; - retcode = git_clone_into(repo, origin, &options->checkout_opts, options->checkout_branch); + retcode = git_clone_into(repo, origin, &options.checkout_opts, options.checkout_branch); git_remote_free(origin); if (retcode < 0) diff --git a/tests-clar/clone/nonetwork.c b/tests-clar/clone/nonetwork.c index c7a219259..9b666b3f7 100644 --- a/tests-clar/clone/nonetwork.c +++ b/tests-clar/clone/nonetwork.c @@ -130,6 +130,12 @@ void test_clone_nonetwork__custom_origin_name(void) cl_git_pass(git_remote_load(&g_remote, g_repo, "my_origin")); } +void test_clone_nonetwork__defaults(void) +{ + cl_git_pass(git_clone(&g_repo, cl_git_fixture_url("testrepo.git"), "./foo", NULL)); + cl_assert(g_repo); + cl_git_pass(git_remote_load(&g_remote, g_repo, "origin")); +} void test_clone_nonetwork__cope_with_already_existing_directory(void) { From 36a241acbb39aa46cf5f7807aebd92e45f2f5eaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Fri, 20 Sep 2013 23:14:52 +0200 Subject: [PATCH 33/37] clone: mention clone_into in the clone documentation Make the difference more explicit. --- include/git2/clone.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/include/git2/clone.h b/include/git2/clone.h index bd602fb7a..a341a413c 100644 --- a/include/git2/clone.h +++ b/include/git2/clone.h @@ -61,8 +61,11 @@ typedef struct git_clone_options { #define GIT_CLONE_OPTIONS_INIT {GIT_CLONE_OPTIONS_VERSION, {GIT_CHECKOUT_OPTS_VERSION, GIT_CHECKOUT_SAFE_CREATE}} /** - * Clone a remote repository, and checkout the branch pointed to by the remote - * HEAD. + * Clone a remote repository. + * + * This version handles the simple case. If you'd like to create the + * repository or remote with non-default settings, you can create and + * configure them and then use `git_clone_into()`. * * @param out pointer that will receive the resulting repository object * @param url the remote repository to clone From ffc97d51264f8af435ccf52d33a62a6925b174c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Fri, 20 Sep 2013 23:23:42 +0200 Subject: [PATCH 34/37] remote: add some comments to the callback struct Hopefully clear up what they're for. --- include/git2/remote.h | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/include/git2/remote.h b/include/git2/remote.h index 8c21870de..8145de180 100644 --- a/include/git2/remote.h +++ b/include/git2/remote.h @@ -400,15 +400,47 @@ typedef enum git_remote_completion_type { /** * The callback settings structure * - * Set the calbacks to be called by the remote. + * Set the callbacks to be called by the remote when informing the user + * about the progress of the network operations. */ struct git_remote_callbacks { unsigned int version; + /** + * Textual progress from the remote. Text send over the + * progress side-band will be passed to this function (this is + * the 'counting objects' output. + */ void (*progress)(const char *str, int len, void *data); + + /** + * Completion is called when different parts of the download + * process are done (currently unused). + */ int (*completion)(git_remote_completion_type type, void *data); + + /** + * This will be called if the remote host requires + * authentication in order to connect to it. + */ int (*credentials)(git_cred **cred, const char *url, const char *username_from_url, unsigned int allowed_types, void *data); + + /** + * During the download of new data, this will be regularly + * called with the current count of progress done by the + * indexer. + */ int (*transfer_progress)(const git_transfer_progress *stats, void *data); + + /** + * Each time a reference is updated locally, this function + * will be called with information about it. + */ int (*update_tips)(const char *refname, const git_oid *a, const git_oid *b, void *data); + + /** + * This will be passed to each of the callbacks in this struct + * as the last parameter. + */ void *payload; }; From 0e0cf78773bea0d06298ba3bf981a3be839041df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Wed, 2 Oct 2013 14:04:44 +0200 Subject: [PATCH 35/37] clone: put the callbacks struct directly in the clone options There's no need for this to be a pointer to somewhere else. --- examples/network/clone.c | 8 +++----- include/git2/clone.h | 4 ++-- include/git2/remote.h | 2 +- src/clone.c | 3 +-- src/remote.c | 4 ++-- tests-clar/clone/empty.c | 2 ++ tests-clar/clone/nonetwork.c | 2 ++ tests-clar/online/clone.c | 34 +++++++++++----------------------- tests-clar/online/fetchhead.c | 2 ++ 9 files changed, 26 insertions(+), 35 deletions(-) diff --git a/examples/network/clone.c b/examples/network/clone.c index f553c4077..db35bd7db 100644 --- a/examples/network/clone.c +++ b/examples/network/clone.c @@ -57,7 +57,6 @@ int do_clone(git_repository *repo, int argc, char **argv) git_repository *cloned_repo = NULL; git_clone_options clone_opts = GIT_CLONE_OPTIONS_INIT; git_checkout_opts checkout_opts = GIT_CHECKOUT_OPTS_INIT; - git_remote_callbacks callbacks = GIT_REMOTE_CALLBACKS_INIT; const char *url = argv[1]; const char *path = argv[2]; int error; @@ -75,10 +74,9 @@ int do_clone(git_repository *repo, int argc, char **argv) checkout_opts.progress_cb = checkout_progress; checkout_opts.progress_payload = &pd; clone_opts.checkout_opts = checkout_opts; - callbacks.transfer_progress = &fetch_progress; - callbacks.credentials = cred_acquire_cb; - callbacks.payload = &pd; - clone_opts.remote_callbacks = &callbacks; + clone_opts.remote_callbacks.transfer_progress = &fetch_progress; + clone_opts.remote_callbacks.credentials = cred_acquire_cb; + clone_opts.remote_callbacks.payload = &pd; // Do the clone error = git_clone(&cloned_repo, url, path, &clone_opts); diff --git a/include/git2/clone.h b/include/git2/clone.h index a341a413c..331cf92e7 100644 --- a/include/git2/clone.h +++ b/include/git2/clone.h @@ -49,7 +49,7 @@ typedef struct git_clone_options { unsigned int version; git_checkout_opts checkout_opts; - git_remote_callbacks *remote_callbacks; + git_remote_callbacks remote_callbacks; int bare; int ignore_cert_errors; @@ -58,7 +58,7 @@ typedef struct git_clone_options { } git_clone_options; #define GIT_CLONE_OPTIONS_VERSION 1 -#define GIT_CLONE_OPTIONS_INIT {GIT_CLONE_OPTIONS_VERSION, {GIT_CHECKOUT_OPTS_VERSION, GIT_CHECKOUT_SAFE_CREATE}} +#define GIT_CLONE_OPTIONS_INIT {GIT_CLONE_OPTIONS_VERSION, {GIT_CHECKOUT_OPTS_VERSION, GIT_CHECKOUT_SAFE_CREATE}, GIT_REMOTE_CALLBACKS_INIT} /** * Clone a remote repository. diff --git a/include/git2/remote.h b/include/git2/remote.h index 8145de180..9858634cc 100644 --- a/include/git2/remote.h +++ b/include/git2/remote.h @@ -457,7 +457,7 @@ struct git_remote_callbacks { * @param callbacks a pointer to the user's callback settings * @return 0 or an error code */ -GIT_EXTERN(int) git_remote_set_callbacks(git_remote *remote, git_remote_callbacks *callbacks); +GIT_EXTERN(int) git_remote_set_callbacks(git_remote *remote, const git_remote_callbacks *callbacks); /** * Get the statistics structure that is filled in by the fetch operation. diff --git a/src/clone.c b/src/clone.c index 1af6e393f..f3e365c07 100644 --- a/src/clone.c +++ b/src/clone.c @@ -315,8 +315,7 @@ static int create_and_configure_origin( if (options->ignore_cert_errors) git_remote_check_cert(origin, 0); - if (options->remote_callbacks && - (error = git_remote_set_callbacks(origin, options->remote_callbacks)) < 0) + if ((error = git_remote_set_callbacks(origin, &options->remote_callbacks)) < 0) goto on_error; if ((error = git_remote_save(origin)) < 0) diff --git a/src/remote.c b/src/remote.c index ace886502..ccedf2386 100644 --- a/src/remote.c +++ b/src/remote.c @@ -1153,7 +1153,7 @@ void git_remote_check_cert(git_remote *remote, int check) remote->check_cert = check; } -int git_remote_set_callbacks(git_remote *remote, git_remote_callbacks *callbacks) +int git_remote_set_callbacks(git_remote *remote, const git_remote_callbacks *callbacks) { assert(remote && callbacks); @@ -1162,7 +1162,7 @@ int git_remote_set_callbacks(git_remote *remote, git_remote_callbacks *callbacks memcpy(&remote->callbacks, callbacks, sizeof(git_remote_callbacks)); if (remote->transport && remote->transport->set_callbacks) - remote->transport->set_callbacks(remote->transport, + return remote->transport->set_callbacks(remote->transport, remote->callbacks.progress, NULL, remote->callbacks.payload); diff --git a/tests-clar/clone/empty.c b/tests-clar/clone/empty.c index d9dc24fde..6d19244cc 100644 --- a/tests-clar/clone/empty.c +++ b/tests-clar/clone/empty.c @@ -10,12 +10,14 @@ static git_repository *g_repo_cloned; void test_clone_empty__initialize(void) { git_repository *sandbox = cl_git_sandbox_init("empty_bare.git"); + git_remote_callbacks dummy_callbacks = GIT_REMOTE_CALLBACKS_INIT; cl_git_remove_placeholders(git_repository_path(sandbox), "dummy-marker.txt"); g_repo = NULL; memset(&g_options, 0, sizeof(git_clone_options)); g_options.version = GIT_CLONE_OPTIONS_VERSION; + g_options.remote_callbacks = dummy_callbacks; } void test_clone_empty__cleanup(void) diff --git a/tests-clar/clone/nonetwork.c b/tests-clar/clone/nonetwork.c index 9b666b3f7..4bcb5be1e 100644 --- a/tests-clar/clone/nonetwork.c +++ b/tests-clar/clone/nonetwork.c @@ -15,6 +15,7 @@ static git_remote* g_remote; void test_clone_nonetwork__initialize(void) { git_checkout_opts dummy_opts = GIT_CHECKOUT_OPTS_INIT; + git_remote_callbacks dummy_callbacks = GIT_REMOTE_CALLBACKS_INIT; g_repo = NULL; @@ -22,6 +23,7 @@ void test_clone_nonetwork__initialize(void) g_options.version = GIT_CLONE_OPTIONS_VERSION; g_options.checkout_opts = dummy_opts; g_options.checkout_opts.checkout_strategy = GIT_CHECKOUT_SAFE; + g_options.remote_callbacks = dummy_callbacks; } void test_clone_nonetwork__cleanup(void) diff --git a/tests-clar/online/clone.c b/tests-clar/online/clone.c index 9a64ba166..4a6ade52d 100644 --- a/tests-clar/online/clone.c +++ b/tests-clar/online/clone.c @@ -18,6 +18,7 @@ static git_clone_options g_options; void test_online_clone__initialize(void) { git_checkout_opts dummy_opts = GIT_CHECKOUT_OPTS_INIT; + git_remote_callbacks dummy_callbacks = GIT_REMOTE_CALLBACKS_INIT; g_repo = NULL; @@ -25,6 +26,7 @@ void test_online_clone__initialize(void) g_options.version = GIT_CLONE_OPTIONS_VERSION; g_options.checkout_opts = dummy_opts; g_options.checkout_opts.checkout_strategy = GIT_CHECKOUT_SAFE; + g_options.remote_callbacks = dummy_callbacks; } void test_online_clone__cleanup(void) @@ -100,15 +102,11 @@ void test_online_clone__can_checkout_a_cloned_repo(void) bool checkout_progress_cb_was_called = false, fetch_progress_cb_was_called = false; - git_remote_callbacks callbacks = GIT_REMOTE_CALLBACKS_INIT; - g_options.checkout_opts.checkout_strategy = GIT_CHECKOUT_SAFE_CREATE; g_options.checkout_opts.progress_cb = &checkout_progress; g_options.checkout_opts.progress_payload = &checkout_progress_cb_was_called; - - callbacks.transfer_progress = &fetch_progress; - callbacks.payload = &fetch_progress_cb_was_called; - g_options.remote_callbacks = &callbacks; + g_options.remote_callbacks.transfer_progress = &fetch_progress; + g_options.remote_callbacks.payload = &fetch_progress_cb_was_called; cl_git_pass(git_clone(&g_repo, LIVE_REPO_URL, "./foo", &g_options)); @@ -175,12 +173,10 @@ static int update_tips(const char *refname, const git_oid *a, const git_oid *b, void test_online_clone__custom_remote_callbacks(void) { - git_remote_callbacks remote_callbacks = GIT_REMOTE_CALLBACKS_INIT; int callcount = 0; - g_options.remote_callbacks = &remote_callbacks; - remote_callbacks.update_tips = update_tips; - remote_callbacks.payload = &callcount; + g_options.remote_callbacks.update_tips = update_tips; + g_options.remote_callbacks.payload = &callcount; cl_git_pass(git_clone(&g_repo, LIVE_REPO_URL, "./foo", &g_options)); cl_assert(callcount > 0); @@ -194,13 +190,11 @@ void test_online_clone__credentials(void) cl_getenv("GITTEST_REMOTE_USER"), cl_getenv("GITTEST_REMOTE_PASS") }; - git_remote_callbacks callbacks = GIT_REMOTE_CALLBACKS_INIT; if (!remote_url) return; - callbacks.credentials = git_cred_userpass; - callbacks.payload = &user_pass; - g_options.remote_callbacks = &callbacks; + g_options.remote_callbacks.credentials = git_cred_userpass; + g_options.remote_callbacks.payload = &user_pass; cl_git_pass(git_clone(&g_repo, remote_url, "./foo", &g_options)); git_repository_free(g_repo); g_repo = NULL; @@ -213,11 +207,8 @@ void test_online_clone__bitbucket_style(void) "libgit2", "libgit2" }; - git_remote_callbacks callbacks = GIT_REMOTE_CALLBACKS_INIT; - - callbacks.credentials = git_cred_userpass; - callbacks.payload = &user_pass; - g_options.remote_callbacks = &callbacks; + g_options.remote_callbacks.credentials = git_cred_userpass; + g_options.remote_callbacks.payload = &user_pass; cl_git_pass(git_clone(&g_repo, BB_REPO_URL, "./foo", &g_options)); git_repository_free(g_repo); g_repo = NULL; @@ -247,10 +238,7 @@ static int cancel_at_half(const git_transfer_progress *stats, void *payload) void test_online_clone__can_cancel(void) { - git_remote_callbacks callbacks = GIT_REMOTE_CALLBACKS_INIT; - - callbacks.transfer_progress = cancel_at_half; - g_options.remote_callbacks = &callbacks; + g_options.remote_callbacks.transfer_progress = cancel_at_half; cl_git_fail_with(git_clone(&g_repo, LIVE_REPO_URL, "./foo", &g_options), GIT_EUSER); } diff --git a/tests-clar/online/fetchhead.c b/tests-clar/online/fetchhead.c index 5d9eb1318..57b183f88 100644 --- a/tests-clar/online/fetchhead.c +++ b/tests-clar/online/fetchhead.c @@ -12,10 +12,12 @@ static git_clone_options g_options; void test_online_fetchhead__initialize(void) { + git_remote_callbacks dummy_callbacks = GIT_REMOTE_CALLBACKS_INIT; g_repo = NULL; memset(&g_options, 0, sizeof(git_clone_options)); g_options.version = GIT_CLONE_OPTIONS_VERSION; + g_options.remote_callbacks = dummy_callbacks; } void test_online_fetchhead__cleanup(void) From 41a6de289fecbec426d2d977c4d02da5456701ac Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Wed, 2 Oct 2013 14:45:57 -0700 Subject: [PATCH 36/37] HTTP: handle "relative" redirects --- src/netops.c | 10 ++++++++++ tests-clar/network/urlparse.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/src/netops.c b/src/netops.c index 3257e7749..7a61ef820 100644 --- a/src/netops.c +++ b/src/netops.c @@ -609,6 +609,9 @@ int gitno_connection_data_from_url( data->use_ssl = true; } + if (url[0] == '/') + default_port = data->use_ssl ? "443" : "80"; + if (!default_port) { giterr_set(GITERR_NET, "Unrecognized URL prefix"); goto cleanup; @@ -618,6 +621,13 @@ int gitno_connection_data_from_url( &data->host, &data->port, &data->user, &data->pass, url, default_port); + if (url[0] == '/') { + /* Relative redirect; reuse original host name and port */ + git__free(data->host); + data->host = original_host; + original_host = NULL; + } + if (!error) { const char *path = strchr(url, '/'); size_t pathlen = strlen(path); diff --git a/tests-clar/network/urlparse.c b/tests-clar/network/urlparse.c index 8892781ad..274d7e900 100644 --- a/tests-clar/network/urlparse.c +++ b/tests-clar/network/urlparse.c @@ -125,6 +125,34 @@ void test_network_urlparse__connection_data_http_downgrade(void) -1); } +void test_network_urlparse__connection_data_relative_redirect(void) +{ + cl_git_pass(gitno_connection_data_from_url(&conndata, + "http://foo.com/bar/baz/biff", NULL)); + cl_git_pass(gitno_connection_data_from_url(&conndata, + "/zap/baz/biff?bam", NULL)); + cl_assert_equal_s(conndata.host, "foo.com"); + cl_assert_equal_s(conndata.port, "80"); + cl_assert_equal_s(conndata.path, "/zap/baz/biff?bam"); + cl_assert_equal_p(conndata.user, NULL); + cl_assert_equal_p(conndata.pass, NULL); + cl_assert_equal_i(conndata.use_ssl, false); +} + +void test_network_urlparse__connection_data_relative_redirect_ssl(void) +{ + cl_git_pass(gitno_connection_data_from_url(&conndata, + "https://foo.com/bar/baz/biff", NULL)); + cl_git_pass(gitno_connection_data_from_url(&conndata, + "/zap/baz/biff?bam", NULL)); + cl_assert_equal_s(conndata.host, "foo.com"); + cl_assert_equal_s(conndata.port, "443"); + cl_assert_equal_s(conndata.path, "/zap/baz/biff?bam"); + cl_assert_equal_p(conndata.user, NULL); + cl_assert_equal_p(conndata.pass, NULL); + cl_assert_equal_i(conndata.use_ssl, true); +} + /* Run this under valgrind */ void test_network_urlparse__connection_data_cleanup(void) { From 598f069b998c42c12439f3f353b6d075905becba Mon Sep 17 00:00:00 2001 From: nulltoken Date: Wed, 2 Oct 2013 12:42:41 +0200 Subject: [PATCH 37/37] commit: Introduce git_commit_message_raw() --- include/git2/commit.h | 11 +++++++++++ src/commit.c | 23 ++++++++++++++++++----- src/commit.h | 2 +- tests-clar/commit/parse.c | 8 +++++++- 4 files changed, 37 insertions(+), 7 deletions(-) diff --git a/include/git2/commit.h b/include/git2/commit.h index 0eaf917bd..a08cf1c6c 100644 --- a/include/git2/commit.h +++ b/include/git2/commit.h @@ -100,11 +100,22 @@ GIT_EXTERN(const char *) git_commit_message_encoding(const git_commit *commit); /** * Get the full message of a commit. * + * The returned message will be slightly prettified by removing any + * potential leading newlines. + * * @param commit a previously loaded commit. * @return the message of a commit */ GIT_EXTERN(const char *) git_commit_message(const git_commit *commit); +/** + * Get the full raw message of a commit. + * + * @param commit a previously loaded commit. + * @return the raw message of a commit + */ +GIT_EXTERN(const char *) git_commit_message_raw(const git_commit *commit); + /** * Get the commit time (i.e. committer time) of a commit. * diff --git a/src/commit.c b/src/commit.c index ab475a8f8..91b60bbb2 100644 --- a/src/commit.c +++ b/src/commit.c @@ -29,7 +29,7 @@ void git_commit__free(void *_commit) git_signature_free(commit->committer); git__free(commit->raw_header); - git__free(commit->message); + git__free(commit->raw_message); git__free(commit->message_encoding); git__free(commit); @@ -240,13 +240,13 @@ int git_commit__parse(void *_commit, git_odb_object *odb_obj) buffer_end = buffer + git_odb_object_size(odb_obj); buffer += header_len; - while (buffer < buffer_end && *buffer == '\n') + if (*buffer == '\n') ++buffer; /* extract commit message */ if (buffer <= buffer_end) { - commit->message = git__strndup(buffer, buffer_end - buffer); - GITERR_CHECK_ALLOC(commit->message); + commit->raw_message = git__strndup(buffer, buffer_end - buffer); + GITERR_CHECK_ALLOC(commit->raw_message); } return 0; @@ -265,7 +265,7 @@ bad_buffer: GIT_COMMIT_GETTER(const git_signature *, author, commit->author) GIT_COMMIT_GETTER(const git_signature *, committer, commit->committer) -GIT_COMMIT_GETTER(const char *, message, commit->message) +GIT_COMMIT_GETTER(const char *, message_raw, commit->raw_message) GIT_COMMIT_GETTER(const char *, message_encoding, commit->message_encoding) GIT_COMMIT_GETTER(const char *, raw_header, commit->raw_header) GIT_COMMIT_GETTER(git_time_t, time, commit->committer->when.time) @@ -273,6 +273,19 @@ GIT_COMMIT_GETTER(int, time_offset, commit->committer->when.offset) GIT_COMMIT_GETTER(unsigned int, parentcount, (unsigned int)git_array_size(commit->parent_ids)) GIT_COMMIT_GETTER(const git_oid *, tree_id, &commit->tree_id); +const char *git_commit_message(const git_commit *commit) +{ + const char *message = commit->raw_message; + + assert(commit); + + /* trim leading newlines from raw message */ + while (*message && *message == '\n') + ++message; + + return message; +} + int git_commit_tree(git_tree **tree_out, const git_commit *commit) { assert(commit); diff --git a/src/commit.h b/src/commit.h index 22fc898a1..d452e2975 100644 --- a/src/commit.h +++ b/src/commit.h @@ -24,7 +24,7 @@ struct git_commit { git_signature *committer; char *message_encoding; - char *message; + char *raw_message; char *raw_header; }; diff --git a/tests-clar/commit/parse.c b/tests-clar/commit/parse.c index c191b3421..41e162440 100644 --- a/tests-clar/commit/parse.c +++ b/tests-clar/commit/parse.c @@ -382,9 +382,13 @@ committer Vicent Marti 1273848544 +0200\n\ This commit has a few LF at the start of the commit message"; const char *message = "This commit has a few LF at the start of the commit message"; - + const char *raw_message = +"\n\ +\n\ +This commit has a few LF at the start of the commit message"; cl_git_pass(parse_commit(&commit, buffer)); cl_assert_equal_s(message, git_commit_message(commit)); + cl_assert_equal_s(raw_message, git_commit_message_raw(commit)); git_commit__free(commit); } @@ -400,8 +404,10 @@ committer Vicent Marti 1273848544 +0200\n\ \n\ \n"; const char *message = ""; + const char *raw_message = "\n\n"; cl_git_pass(parse_commit(&commit, buffer)); cl_assert_equal_s(message, git_commit_message(commit)); + cl_assert_equal_s(raw_message, git_commit_message_raw(commit)); git_commit__free(commit); }