* Re: [PATCH 6/5] Move fetch_ref from http-push.c and http-walker.c to http.c
From: Mike Hommey @ 2007-12-11 6:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vve75hl72.fsf@gitster.siamese.dyndns.org>
On Mon, Dec 10, 2007 at 09:09:05PM -0800, Junio C Hamano wrote:
> Mike Hommey <mh@glandium.org> writes:
>
> > Make the necessary changes to be ok with their difference, and rename the
> > function http_fetch_ref.
>
> Sorry, but I cannot parse "ok with their difference" part...
What about "to fit with their difference" ?
Mike
^ permalink raw reply
* [Replacement PATCH 2/4] Use strbuf in http code
From: Mike Hommey @ 2007-12-11 6:25 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <20071211061620.GA8047@glandium.org>
Also, replace whitespaces with tabs in some places
Signed-off-by: Mike Hommey <mh@glandium.org>
---
http-push.c | 187 +++++++++++++++++++--------------------------------------
http-walker.c | 60 +++++++------------
http.c | 34 ++++------
http.h | 11 ++--
transport.c | 18 ++----
5 files changed, 109 insertions(+), 201 deletions(-)
diff --git a/http-push.c b/http-push.c
index 78283b4..a69d0e3 100644
--- a/http-push.c
+++ b/http-push.c
@@ -495,10 +495,10 @@ static void start_put(struct transfer_request *request)
memset(&stream, 0, sizeof(stream));
deflateInit(&stream, zlib_compression_level);
size = deflateBound(&stream, len + hdrlen);
- request->buffer.buffer = xmalloc(size);
+ strbuf_init(&request->buffer.buf, size);
/* Compress it */
- stream.next_out = request->buffer.buffer;
+ stream.next_out = (unsigned char *)request->buffer.buf.buf;
stream.avail_out = size;
/* First header.. */
@@ -515,8 +515,7 @@ static void start_put(struct transfer_request *request)
deflateEnd(&stream);
free(unpacked);
- request->buffer.size = stream.total_out;
- request->buffer.posn = 0;
+ request->buffer.buf.len = stream.total_out;
request->url = xmalloc(strlen(remote->url) +
strlen(request->lock->token) + 51);
@@ -538,7 +537,7 @@ static void start_put(struct transfer_request *request)
slot->callback_func = process_response;
slot->callback_data = request;
curl_easy_setopt(slot->curl, CURLOPT_INFILE, &request->buffer);
- curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, request->buffer.size);
+ curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, request->buffer.buf.len);
curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, fread_buffer);
curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_null);
curl_easy_setopt(slot->curl, CURLOPT_CUSTOMREQUEST, DAV_PUT);
@@ -1003,18 +1002,13 @@ static int fetch_indices(void)
{
unsigned char sha1[20];
char *url;
- struct buffer buffer;
+ struct strbuf buffer = STRBUF_INIT;
char *data;
int i = 0;
struct active_request_slot *slot;
struct slot_results results;
- data = xcalloc(1, 4096);
- buffer.size = 4096;
- buffer.posn = 0;
- buffer.buffer = data;
-
if (push_verbosely)
fprintf(stderr, "Getting pack list\n");
@@ -1030,7 +1024,7 @@ static int fetch_indices(void)
if (start_active_slot(slot)) {
run_active_slot(slot);
if (results.curl_result != CURLE_OK) {
- free(buffer.buffer);
+ strbuf_release(&buffer);
free(url);
if (results.http_code == 404)
return 0;
@@ -1038,18 +1032,18 @@ static int fetch_indices(void)
return error("%s", curl_errorstr);
}
} else {
- free(buffer.buffer);
+ strbuf_release(&buffer);
free(url);
return error("Unable to start request");
}
free(url);
- data = buffer.buffer;
- while (i < buffer.posn) {
+ data = buffer.buf;
+ while (i < buffer.len) {
switch (data[i]) {
case 'P':
i++;
- if (i + 52 < buffer.posn &&
+ if (i + 52 < buffer.len &&
!prefixcmp(data + i, " pack-") &&
!prefixcmp(data + i + 46, ".pack\n")) {
get_sha1_hex(data + i + 6, sha1);
@@ -1064,7 +1058,7 @@ static int fetch_indices(void)
i++;
}
- free(buffer.buffer);
+ strbuf_release(&buffer);
return 0;
}
@@ -1115,16 +1109,11 @@ static char *quote_ref_url(const char *base, const char *ref)
int fetch_ref(char *ref, unsigned char *sha1)
{
- char *url;
- char hex[42];
- struct buffer buffer;
+ char *url;
+ struct strbuf buffer = STRBUF_INIT;
char *base = remote->url;
struct active_request_slot *slot;
struct slot_results results;
- buffer.size = 41;
- buffer.posn = 0;
- buffer.buffer = hex;
- hex[41] = '\0';
url = quote_ref_url(base, ref);
slot = get_active_slot();
@@ -1142,9 +1131,10 @@ int fetch_ref(char *ref, unsigned char *sha1)
return error("Unable to start request");
}
- hex[40] = '\0';
- get_sha1_hex(hex, sha1);
- return 0;
+ strbuf_rtrim(&buffer);
+ if (buffer.len != 40)
+ return 1;
+ return get_sha1_hex(buffer.buf, sha1);
}
static void one_remote_object(const char *hex)
@@ -1267,10 +1257,8 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
{
struct active_request_slot *slot;
struct slot_results results;
- struct buffer out_buffer;
- struct buffer in_buffer;
- char *out_data;
- char *in_data;
+ struct buffer out_buffer = { 0, STRBUF_INIT };
+ struct strbuf in_buffer = STRBUF_INIT;
char *url;
char *ep;
char timeout_header[25];
@@ -1312,16 +1300,7 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
ep = strchr(ep + 1, '/');
}
- out_buffer.size = strlen(LOCK_REQUEST) + strlen(git_default_email) - 2;
- out_data = xmalloc(out_buffer.size + 1);
- snprintf(out_data, out_buffer.size + 1, LOCK_REQUEST, git_default_email);
- out_buffer.posn = 0;
- out_buffer.buffer = out_data;
-
- in_buffer.size = 4096;
- in_data = xmalloc(in_buffer.size);
- in_buffer.posn = 0;
- in_buffer.buffer = in_data;
+ strbuf_addf(&out_buffer.buf, LOCK_REQUEST, git_default_email);
sprintf(timeout_header, "Timeout: Second-%ld", timeout);
dav_headers = curl_slist_append(dav_headers, timeout_header);
@@ -1330,7 +1309,7 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
slot = get_active_slot();
slot->results = &results;
curl_easy_setopt(slot->curl, CURLOPT_INFILE, &out_buffer);
- curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.size);
+ curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.buf.len);
curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, fread_buffer);
curl_easy_setopt(slot->curl, CURLOPT_FILE, &in_buffer);
curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
@@ -1354,8 +1333,8 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
XML_SetElementHandler(parser, xml_start_tag,
xml_end_tag);
XML_SetCharacterDataHandler(parser, xml_cdata);
- result = XML_Parse(parser, in_buffer.buffer,
- in_buffer.posn, 1);
+ result = XML_Parse(parser, in_buffer.buf,
+ in_buffer.len, 1);
free(ctx.name);
if (result != XML_STATUS_OK) {
fprintf(stderr, "XML error: %s\n",
@@ -1369,8 +1348,8 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
}
curl_slist_free_all(dav_headers);
- free(out_data);
- free(in_data);
+ strbuf_release(&out_buffer.buf);
+ strbuf_release(&in_buffer);
if (lock->token == NULL || lock->timeout <= 0) {
if (lock->token != NULL)
@@ -1521,10 +1500,8 @@ static void remote_ls(const char *path, int flags,
char *url = xmalloc(strlen(remote->url) + strlen(path) + 1);
struct active_request_slot *slot;
struct slot_results results;
- struct buffer in_buffer;
- struct buffer out_buffer;
- char *in_data;
- char *out_data;
+ struct strbuf in_buffer = STRBUF_INIT;
+ struct buffer out_buffer = { 0, STRBUF_INIT };
XML_Parser parser = XML_ParserCreate(NULL);
enum XML_Status result;
struct curl_slist *dav_headers = NULL;
@@ -1540,16 +1517,7 @@ static void remote_ls(const char *path, int flags,
sprintf(url, "%s%s", remote->url, path);
- out_buffer.size = strlen(PROPFIND_ALL_REQUEST);
- out_data = xmalloc(out_buffer.size + 1);
- snprintf(out_data, out_buffer.size + 1, PROPFIND_ALL_REQUEST);
- out_buffer.posn = 0;
- out_buffer.buffer = out_data;
-
- in_buffer.size = 4096;
- in_data = xmalloc(in_buffer.size);
- in_buffer.posn = 0;
- in_buffer.buffer = in_data;
+ strbuf_addf(&out_buffer.buf, PROPFIND_ALL_REQUEST);
dav_headers = curl_slist_append(dav_headers, "Depth: 1");
dav_headers = curl_slist_append(dav_headers, "Content-Type: text/xml");
@@ -1557,7 +1525,7 @@ static void remote_ls(const char *path, int flags,
slot = get_active_slot();
slot->results = &results;
curl_easy_setopt(slot->curl, CURLOPT_INFILE, &out_buffer);
- curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.size);
+ curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.buf.len);
curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, fread_buffer);
curl_easy_setopt(slot->curl, CURLOPT_FILE, &in_buffer);
curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
@@ -1578,8 +1546,8 @@ static void remote_ls(const char *path, int flags,
XML_SetElementHandler(parser, xml_start_tag,
xml_end_tag);
XML_SetCharacterDataHandler(parser, xml_cdata);
- result = XML_Parse(parser, in_buffer.buffer,
- in_buffer.posn, 1);
+ result = XML_Parse(parser, in_buffer.buf,
+ in_buffer.len, 1);
free(ctx.name);
if (result != XML_STATUS_OK) {
@@ -1594,8 +1562,8 @@ static void remote_ls(const char *path, int flags,
free(ls.path);
free(url);
- free(out_data);
- free(in_buffer.buffer);
+ strbuf_release(&out_buffer.buf);
+ strbuf_release(&in_buffer);
curl_slist_free_all(dav_headers);
}
@@ -1616,29 +1584,15 @@ static int locking_available(void)
{
struct active_request_slot *slot;
struct slot_results results;
- struct buffer in_buffer;
- struct buffer out_buffer;
- char *in_data;
- char *out_data;
+ struct strbuf in_buffer = STRBUF_INIT;
+ struct buffer out_buffer = { 0, STRBUF_INIT };
XML_Parser parser = XML_ParserCreate(NULL);
enum XML_Status result;
struct curl_slist *dav_headers = NULL;
struct xml_ctx ctx;
int lock_flags = 0;
- out_buffer.size =
- strlen(PROPFIND_SUPPORTEDLOCK_REQUEST) +
- strlen(remote->url) - 2;
- out_data = xmalloc(out_buffer.size + 1);
- snprintf(out_data, out_buffer.size + 1,
- PROPFIND_SUPPORTEDLOCK_REQUEST, remote->url);
- out_buffer.posn = 0;
- out_buffer.buffer = out_data;
-
- in_buffer.size = 4096;
- in_data = xmalloc(in_buffer.size);
- in_buffer.posn = 0;
- in_buffer.buffer = in_data;
+ strbuf_addf(&out_buffer.buf, PROPFIND_SUPPORTEDLOCK_REQUEST, remote->url);
dav_headers = curl_slist_append(dav_headers, "Depth: 0");
dav_headers = curl_slist_append(dav_headers, "Content-Type: text/xml");
@@ -1646,7 +1600,7 @@ static int locking_available(void)
slot = get_active_slot();
slot->results = &results;
curl_easy_setopt(slot->curl, CURLOPT_INFILE, &out_buffer);
- curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.size);
+ curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.buf.len);
curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, fread_buffer);
curl_easy_setopt(slot->curl, CURLOPT_FILE, &in_buffer);
curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
@@ -1666,8 +1620,8 @@ static int locking_available(void)
XML_SetUserData(parser, &ctx);
XML_SetElementHandler(parser, xml_start_tag,
xml_end_tag);
- result = XML_Parse(parser, in_buffer.buffer,
- in_buffer.posn, 1);
+ result = XML_Parse(parser, in_buffer.buf,
+ in_buffer.len, 1);
free(ctx.name);
if (result != XML_STATUS_OK) {
@@ -1681,8 +1635,8 @@ static int locking_available(void)
fprintf(stderr, "Unable to start PROPFIND request\n");
}
- free(out_data);
- free(in_buffer.buffer);
+ strbuf_release(&out_buffer.buf);
+ strbuf_release(&in_buffer);
curl_slist_free_all(dav_headers);
return lock_flags;
@@ -1800,30 +1754,20 @@ static int update_remote(unsigned char *sha1, struct remote_lock *lock)
{
struct active_request_slot *slot;
struct slot_results results;
- char *out_data;
char *if_header;
- struct buffer out_buffer;
+ struct buffer out_buffer = { 0, STRBUF_INIT };
struct curl_slist *dav_headers = NULL;
- int i;
if_header = xmalloc(strlen(lock->token) + 25);
sprintf(if_header, "If: (<opaquelocktoken:%s>)", lock->token);
dav_headers = curl_slist_append(dav_headers, if_header);
- out_buffer.size = 41;
- out_data = xmalloc(out_buffer.size + 1);
- i = snprintf(out_data, out_buffer.size + 1, "%s\n", sha1_to_hex(sha1));
- if (i != out_buffer.size) {
- fprintf(stderr, "Unable to initialize PUT request body\n");
- return 0;
- }
- out_buffer.posn = 0;
- out_buffer.buffer = out_data;
+ strbuf_addf(&out_buffer.buf, "%s\n", sha1_to_hex(sha1));
slot = get_active_slot();
slot->results = &results;
curl_easy_setopt(slot->curl, CURLOPT_INFILE, &out_buffer);
- curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.size);
+ curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, out_buffer.buf.len);
curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, fread_buffer);
curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_null);
curl_easy_setopt(slot->curl, CURLOPT_CUSTOMREQUEST, DAV_PUT);
@@ -1834,7 +1778,7 @@ static int update_remote(unsigned char *sha1, struct remote_lock *lock)
if (start_active_slot(slot)) {
run_active_slot(slot);
- free(out_data);
+ strbuf_release(&out_buffer.buf);
free(if_header);
if (results.curl_result != CURLE_OK) {
fprintf(stderr,
@@ -1844,7 +1788,7 @@ static int update_remote(unsigned char *sha1, struct remote_lock *lock)
return 0;
}
} else {
- free(out_data);
+ strbuf_release(&out_buffer.buf);
free(if_header);
fprintf(stderr, "Unable to start PUT request\n");
return 0;
@@ -2001,7 +1945,7 @@ static void mark_edges_uninteresting(struct commit_list *list)
static void add_remote_info_ref(struct remote_ls_ctx *ls)
{
- struct buffer *buf = (struct buffer *)ls->userData;
+ struct strbuf *buf = (struct strbuf *)ls->userData;
unsigned char remote_sha1[20];
struct object *o;
int len;
@@ -2046,17 +1990,14 @@ static void add_remote_info_ref(struct remote_ls_ctx *ls)
static void update_remote_info_refs(struct remote_lock *lock)
{
- struct buffer buffer;
+ struct buffer buffer = { 0, STRBUF_INIT };
struct active_request_slot *slot;
struct slot_results results;
char *if_header;
struct curl_slist *dav_headers = NULL;
- buffer.buffer = xcalloc(1, 4096);
- buffer.size = 4096;
- buffer.posn = 0;
remote_ls("refs/", (PROCESS_FILES | RECURSIVE),
- add_remote_info_ref, &buffer);
+ add_remote_info_ref, &buffer.buf);
if (!aborted) {
if_header = xmalloc(strlen(lock->token) + 25);
sprintf(if_header, "If: (<opaquelocktoken:%s>)", lock->token);
@@ -2065,7 +2006,7 @@ static void update_remote_info_refs(struct remote_lock *lock)
slot = get_active_slot();
slot->results = &results;
curl_easy_setopt(slot->curl, CURLOPT_INFILE, &buffer);
- curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, buffer.posn);
+ curl_easy_setopt(slot->curl, CURLOPT_INFILESIZE, buffer.buf.len);
curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, fread_buffer);
curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_null);
curl_easy_setopt(slot->curl, CURLOPT_CUSTOMREQUEST, DAV_PUT);
@@ -2074,8 +2015,6 @@ static void update_remote_info_refs(struct remote_lock *lock)
curl_easy_setopt(slot->curl, CURLOPT_PUT, 1);
curl_easy_setopt(slot->curl, CURLOPT_URL, lock->url);
- buffer.posn = 0;
-
if (start_active_slot(slot)) {
run_active_slot(slot);
if (results.curl_result != CURLE_OK) {
@@ -2086,7 +2025,7 @@ static void update_remote_info_refs(struct remote_lock *lock)
}
free(if_header);
}
- free(buffer.buffer);
+ strbuf_release(&buffer.buf);
}
static int remote_exists(const char *path)
@@ -2097,12 +2036,12 @@ static int remote_exists(const char *path)
sprintf(url, "%s%s", remote->url, path);
- slot = get_active_slot();
+ slot = get_active_slot();
slot->results = &results;
- curl_easy_setopt(slot->curl, CURLOPT_URL, url);
- curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1);
+ curl_easy_setopt(slot->curl, CURLOPT_URL, url);
+ curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1);
- if (start_active_slot(slot)) {
+ if (start_active_slot(slot)) {
run_active_slot(slot);
if (results.http_code == 404)
return 0;
@@ -2120,17 +2059,13 @@ static int remote_exists(const char *path)
static void fetch_symref(const char *path, char **symref, unsigned char *sha1)
{
char *url;
- struct buffer buffer;
+ struct strbuf buffer = STRBUF_INIT;
struct active_request_slot *slot;
struct slot_results results;
url = xmalloc(strlen(remote->url) + strlen(path) + 1);
sprintf(url, "%s%s", remote->url, path);
- buffer.size = 4096;
- buffer.posn = 0;
- buffer.buffer = xmalloc(buffer.size);
-
slot = get_active_slot();
slot->results = &results;
curl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);
@@ -2153,17 +2088,17 @@ static void fetch_symref(const char *path, char **symref, unsigned char *sha1)
*symref = NULL;
hashclr(sha1);
- if (buffer.posn == 0)
+ if (buffer.len == 0)
return;
/* If it's a symref, set the refname; otherwise try for a sha1 */
- if (!prefixcmp((char *)buffer.buffer, "ref: ")) {
- *symref = xmemdupz((char *)buffer.buffer + 5, buffer.posn - 6);
+ if (!prefixcmp((char *)buffer.buf, "ref: ")) {
+ *symref = xmemdupz((char *)buffer.buf + 5, buffer.len - 6);
} else {
- get_sha1_hex(buffer.buffer, sha1);
+ get_sha1_hex(buffer.buf, sha1);
}
- free(buffer.buffer);
+ strbuf_release(&buffer);
}
static int verify_merge_base(unsigned char *head_sha1, unsigned char *branch_sha1)
diff --git a/http-walker.c b/http-walker.c
index a3fb596..d9a5f1e 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -48,7 +48,7 @@ struct alternates_request {
struct walker *walker;
const char *base;
char *url;
- struct buffer *buffer;
+ struct strbuf *buffer;
struct active_request_slot *slot;
int http_specific;
};
@@ -475,7 +475,7 @@ static void process_alternates_response(void *callback_data)
if (alt_req->http_specific) {
if (slot->curl_result != CURLE_OK ||
- !alt_req->buffer->posn) {
+ !alt_req->buffer->len) {
/* Try reusing the slot to get non-http alternates */
alt_req->http_specific = 0;
@@ -503,12 +503,12 @@ static void process_alternates_response(void *callback_data)
}
fwrite_buffer(&null_byte, 1, 1, alt_req->buffer);
- alt_req->buffer->posn--;
- data = alt_req->buffer->buffer;
+ alt_req->buffer->len--;
+ data = alt_req->buffer->buf;
- while (i < alt_req->buffer->posn) {
+ while (i < alt_req->buffer->len) {
int posn = i;
- while (posn < alt_req->buffer->posn && data[posn] != '\n')
+ while (posn < alt_req->buffer->len && data[posn] != '\n')
posn++;
if (data[posn] == '\n') {
int okay = 0;
@@ -596,9 +596,8 @@ static void process_alternates_response(void *callback_data)
static void fetch_alternates(struct walker *walker, const char *base)
{
- struct buffer buffer;
+ struct strbuf buffer = STRBUF_INIT;
char *url;
- char *data;
struct active_request_slot *slot;
struct alternates_request alt_req;
struct walker_data *cdata = walker->data;
@@ -619,11 +618,6 @@ static void fetch_alternates(struct walker *walker, const char *base)
/* Start the fetch */
cdata->got_alternates = 0;
- data = xmalloc(4096);
- buffer.size = 4096;
- buffer.posn = 0;
- buffer.buffer = data;
-
if (walker->get_verbosely)
fprintf(stderr, "Getting alternates list for %s\n", base);
@@ -652,7 +646,7 @@ static void fetch_alternates(struct walker *walker, const char *base)
else
cdata->got_alternates = -1;
- free(data);
+ strbuf_release(&buffer);
free(url);
}
@@ -660,7 +654,7 @@ static int fetch_indices(struct walker *walker, struct alt_base *repo)
{
unsigned char sha1[20];
char *url;
- struct buffer buffer;
+ struct strbuf buffer = STRBUF_INIT;
char *data;
int i = 0;
@@ -670,11 +664,6 @@ static int fetch_indices(struct walker *walker, struct alt_base *repo)
if (repo->got_indices)
return 0;
- data = xmalloc(4096);
- buffer.size = 4096;
- buffer.posn = 0;
- buffer.buffer = data;
-
if (walker->get_verbosely)
fprintf(stderr, "Getting pack list for %s\n", repo->base);
@@ -690,28 +679,27 @@ static int fetch_indices(struct walker *walker, struct alt_base *repo)
if (start_active_slot(slot)) {
run_active_slot(slot);
if (results.curl_result != CURLE_OK) {
+ strbuf_release(&buffer);
if (missing_target(&results)) {
repo->got_indices = 1;
- free(buffer.buffer);
return 0;
} else {
repo->got_indices = 0;
- free(buffer.buffer);
return error("%s", curl_errorstr);
}
}
} else {
repo->got_indices = 0;
- free(buffer.buffer);
+ strbuf_release(&buffer);
return error("Unable to start request");
}
- data = buffer.buffer;
- while (i < buffer.posn) {
+ data = buffer.buf;
+ while (i < buffer.len) {
switch (data[i]) {
case 'P':
i++;
- if (i + 52 <= buffer.posn &&
+ if (i + 52 <= buffer.len &&
!prefixcmp(data + i, " pack-") &&
!prefixcmp(data + i + 46, ".pack\n")) {
get_sha1_hex(data + i + 6, sha1);
@@ -720,13 +708,13 @@ static int fetch_indices(struct walker *walker, struct alt_base *repo)
break;
}
default:
- while (i < buffer.posn && data[i] != '\n')
+ while (i < buffer.len && data[i] != '\n')
i++;
}
i++;
}
- free(buffer.buffer);
+ strbuf_release(&buffer);
repo->got_indices = 1;
return 0;
}
@@ -958,17 +946,12 @@ static char *quote_ref_url(const char *base, const char *ref)
static int fetch_ref(struct walker *walker, char *ref, unsigned char *sha1)
{
- char *url;
- char hex[42];
- struct buffer buffer;
+ char *url;
+ struct strbuf buffer = STRBUF_INIT;
struct walker_data *data = walker->data;
const char *base = data->alt->base;
struct active_request_slot *slot;
struct slot_results results;
- buffer.size = 41;
- buffer.posn = 0;
- buffer.buffer = hex;
- hex[41] = '\0';
url = quote_ref_url(base, ref);
slot = get_active_slot();
@@ -986,9 +969,10 @@ static int fetch_ref(struct walker *walker, char *ref, unsigned char *sha1)
return error("Unable to start request");
}
- hex[40] = '\0';
- get_sha1_hex(hex, sha1);
- return 0;
+ strbuf_rtrim(&buffer);
+ if (buffer.len != 40)
+ return 1;
+ return get_sha1_hex(buffer.buf, sha1);
}
static void cleanup(struct walker *walker)
diff --git a/http.c b/http.c
index 146f626..2d0b46d 100644
--- a/http.c
+++ b/http.c
@@ -34,31 +34,25 @@ size_t fread_buffer(void *ptr, size_t eltsize, size_t nmemb,
struct buffer *buffer)
{
size_t size = eltsize * nmemb;
- if (size > buffer->size - buffer->posn)
- size = buffer->size - buffer->posn;
- memcpy(ptr, (char *) buffer->buffer + buffer->posn, size);
+ if (size > buffer->buf.len - buffer->posn)
+ size = buffer->buf.len - buffer->posn;
+ memcpy(ptr, (char *) buffer->buf.buf + buffer->posn, size);
buffer->posn += size;
+
return size;
}
size_t fwrite_buffer(const void *ptr, size_t eltsize,
- size_t nmemb, struct buffer *buffer)
+ size_t nmemb, struct strbuf *buffer)
{
size_t size = eltsize * nmemb;
- if (size > buffer->size - buffer->posn) {
- buffer->size = buffer->size * 3 / 2;
- if (buffer->size < buffer->posn + size)
- buffer->size = buffer->posn + size;
- buffer->buffer = xrealloc(buffer->buffer, buffer->size);
- }
- memcpy((char *) buffer->buffer + buffer->posn, ptr, size);
- buffer->posn += size;
+ strbuf_add(buffer, ptr, size);
data_received++;
return size;
}
size_t fwrite_null(const void *ptr, size_t eltsize,
- size_t nmemb, struct buffer *buffer)
+ size_t nmemb, struct strbuf *buffer)
{
data_received++;
return eltsize * nmemb;
@@ -508,8 +502,8 @@ void run_active_slot(struct active_request_slot *slot)
static void closedown_active_slot(struct active_request_slot *slot)
{
- active_requests--;
- slot->in_use = 0;
+ active_requests--;
+ slot->in_use = 0;
}
void release_active_slot(struct active_request_slot *slot)
@@ -530,7 +524,7 @@ void release_active_slot(struct active_request_slot *slot)
static void finish_active_slot(struct active_request_slot *slot)
{
closedown_active_slot(slot);
- curl_easy_getinfo(slot->curl, CURLINFO_HTTP_CODE, &slot->http_code);
+ curl_easy_getinfo(slot->curl, CURLINFO_HTTP_CODE, &slot->http_code);
if (slot->finished != NULL)
(*slot->finished) = 1;
@@ -541,10 +535,10 @@ static void finish_active_slot(struct active_request_slot *slot)
slot->results->http_code = slot->http_code;
}
- /* Run callback if appropriate */
- if (slot->callback_func != NULL) {
- slot->callback_func(slot->callback_data);
- }
+ /* Run callback if appropriate */
+ if (slot->callback_func != NULL) {
+ slot->callback_func(slot->callback_data);
+ }
}
void finish_all_active_slots(void)
diff --git a/http.h b/http.h
index fe1b0d1..a0fb4cf 100644
--- a/http.h
+++ b/http.h
@@ -6,6 +6,8 @@
#include <curl/curl.h>
#include <curl/easy.h>
+#include "strbuf.h"
+
#if LIBCURL_VERSION_NUM >= 0x071000
#define USE_CURL_MULTI
#define DEFAULT_MAX_REQUESTS 5
@@ -48,18 +50,17 @@ struct active_request_slot
struct buffer
{
- size_t posn;
- size_t size;
- void *buffer;
+ size_t posn;
+ struct strbuf buf;
};
/* Curl request read/write callbacks */
extern size_t fread_buffer(void *ptr, size_t eltsize, size_t nmemb,
struct buffer *buffer);
extern size_t fwrite_buffer(const void *ptr, size_t eltsize,
- size_t nmemb, struct buffer *buffer);
+ size_t nmemb, struct strbuf *buffer);
extern size_t fwrite_null(const void *ptr, size_t eltsize,
- size_t nmemb, struct buffer *buffer);
+ size_t nmemb, struct strbuf *buffer);
/* Slot lifecycle functions */
extern struct active_request_slot *get_active_slot(void);
diff --git a/transport.c b/transport.c
index 58e66f6..22234e8 100644
--- a/transport.c
+++ b/transport.c
@@ -441,7 +441,7 @@ static int missing__target(int code, int result)
static struct ref *get_refs_via_curl(struct transport *transport)
{
- struct buffer buffer;
+ struct strbuf buffer = STRBUF_INIT;
char *data, *start, *mid;
char *ref_name;
char *refs_url;
@@ -454,11 +454,6 @@ static struct ref *get_refs_via_curl(struct transport *transport)
struct ref *ref = NULL;
struct ref *last_ref = NULL;
- data = xmalloc(4096);
- buffer.size = 4096;
- buffer.posn = 0;
- buffer.buffer = data;
-
refs_url = xmalloc(strlen(transport->url) + 11);
sprintf(refs_url, "%s/info/refs", transport->url);
@@ -477,27 +472,26 @@ static struct ref *get_refs_via_curl(struct transport *transport)
if (start_active_slot(slot)) {
run_active_slot(slot);
if (results.curl_result != CURLE_OK) {
+ strbuf_release(&buffer);
if (missing_target(&results)) {
- free(buffer.buffer);
return NULL;
} else {
- free(buffer.buffer);
error("%s", curl_errorstr);
return NULL;
}
}
} else {
- free(buffer.buffer);
+ strbuf_release(&buffer);
error("Unable to start request");
return NULL;
}
http_cleanup();
- data = buffer.buffer;
+ data = buffer.buf;
start = NULL;
mid = data;
- while (i < buffer.posn) {
+ while (i < buffer.len) {
if (!start)
start = &data[i];
if (data[i] == '\t')
@@ -520,7 +514,7 @@ static struct ref *get_refs_via_curl(struct transport *transport)
i++;
}
- free(buffer.buffer);
+ strbuf_release(&buffer);
return refs;
}
--
1.5.3.7.1161.g14b8
^ permalink raw reply related
* Re: v1.5.4 plans
From: Jeff King @ 2007-12-11 6:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Eric Wong, git
In-Reply-To: <20071211061743.GA21718@coredump.intra.peff.net>
Subject: [PATCH 1/2] Support GIT_PAGER_IN_USE environment variable
When deciding whether or not to turn on automatic color
support, git_config_colorbool checks whether stdout is a
tty. However, because we run a pager, if stdout is not a
tty, we must check whether it is because we started the
pager. This used to be done by checking the pager_in_use
variable.
This variable was set only when the git program being run
started the pager; there was no way for an external program
running git indicate that it had already started a pager.
This patch allows a program to set GIT_PAGER_IN_USE to a
true value to indicate that even though stdout is not a tty,
it is because a pager is being used.
Signed-off-by: Jeff King <peff@peff.net>
---
A few notes:
We could also just put the color.pager logic in git-svn, or in Git.pm,
and have it impact the stdout_is_tty argument; but the whole point of
--get-colorbool is to consolidate that logic.
We convert pager_in_use to a function; we could also just set the
variable early on, but I think this lazy evaluation is more robust.
This might have uses besides --get-colorbool (e.g., wrapper scripts
which start their own pager can still have git sub-commands understand
whether to turn on color).
cache.h | 2 +-
color.c | 2 +-
environment.c | 1 -
| 15 ++++++++++++++-
4 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/cache.h b/cache.h
index 1bcb3df..27d90fe 100644
--- a/cache.h
+++ b/cache.h
@@ -608,7 +608,7 @@ extern int write_or_whine_pipe(int fd, const void *buf, size_t count, const char
/* pager.c */
extern void setup_pager(void);
extern char *pager_program;
-extern int pager_in_use;
+extern int pager_in_use(void);
extern int pager_use_color;
extern char *editor_program;
diff --git a/color.c b/color.c
index 7bd424a..7f66c29 100644
--- a/color.c
+++ b/color.c
@@ -135,7 +135,7 @@ int git_config_colorbool(const char *var, const char *value, int stdout_is_tty)
auto_color:
if (stdout_is_tty < 0)
stdout_is_tty = isatty(1);
- if (stdout_is_tty || (pager_in_use && pager_use_color)) {
+ if (stdout_is_tty || (pager_in_use() && pager_use_color)) {
char *term = getenv("TERM");
if (term && strcmp(term, "dumb"))
return 1;
diff --git a/environment.c b/environment.c
index f3e3d41..18a1c4e 100644
--- a/environment.c
+++ b/environment.c
@@ -31,7 +31,6 @@ size_t packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE;
size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT;
size_t delta_base_cache_limit = 16 * 1024 * 1024;
char *pager_program;
-int pager_in_use;
int pager_use_color = 1;
char *editor_program;
char *excludes_file;
--git a/pager.c b/pager.c
index fb7a1a6..0376953 100644
--- a/pager.c
+++ b/pager.c
@@ -5,6 +5,8 @@
* something different on Windows, for example.
*/
+static int spawned_pager;
+
static void run_pager(const char *pager)
{
/*
@@ -41,7 +43,7 @@ void setup_pager(void)
else if (!*pager || !strcmp(pager, "cat"))
return;
- pager_in_use = 1; /* means we are emitting to terminal */
+ spawned_pager = 1; /* means we are emitting to terminal */
if (pipe(fd) < 0)
return;
@@ -70,3 +72,14 @@ void setup_pager(void)
die("unable to execute pager '%s'", pager);
exit(255);
}
+
+int pager_in_use(void)
+{
+ const char *env;
+
+ if (spawned_pager)
+ return 1;
+
+ env = getenv("GIT_PAGER_IN_USE");
+ return env ? git_config_bool("GIT_PAGER_IN_USE", env) : 0;
+}
--
1.5.3.7.2230.g796d07-dirty
^ permalink raw reply related
* [PATCH 2/2] git-svn: get color config from --get-colorbool
From: Jeff King @ 2007-12-11 6:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Eric Wong, git
In-Reply-To: <20071211061743.GA21718@coredump.intra.peff.net>
git-config recently learned a --get-colorbool option. By
using it, we will get the same color=auto behavior that
other git commands have.
Specifically, this fixes the case where "color.diff = true"
meant "always" in git-svn, but "auto" in other programs.
Signed-off-by: Jeff King <peff@peff.net>
---
git-svn.perl | 35 ++---------------------------------
1 files changed, 2 insertions(+), 33 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 9f884eb..1c42c55 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -3969,39 +3969,7 @@ sub cmt_showable {
}
sub log_use_color {
- return 1 if $color;
- my ($dc, $dcvar);
- $dcvar = 'color.diff';
- $dc = `git-config --get $dcvar`;
- if ($dc eq '') {
- # nothing at all; fallback to "diff.color"
- $dcvar = 'diff.color';
- $dc = `git-config --get $dcvar`;
- }
- chomp($dc);
- if ($dc eq 'auto') {
- my $pc;
- $pc = `git-config --get color.pager`;
- if ($pc eq '') {
- # does not have it -- fallback to pager.color
- $pc = `git-config --bool --get pager.color`;
- }
- else {
- $pc = `git-config --bool --get color.pager`;
- if ($?) {
- $pc = 'false';
- }
- }
- chomp($pc);
- if (-t *STDOUT || (defined $pager && $pc eq 'true')) {
- return ($ENV{TERM} && $ENV{TERM} ne 'dumb');
- }
- return 0;
- }
- return 0 if $dc eq 'never';
- return 1 if $dc eq 'always';
- chomp($dc = `git-config --bool --get $dcvar`);
- return ($dc eq 'true');
+ return $color || Git->repository->get_colorbool('color.diff');
}
sub git_svn_log_cmd {
@@ -4060,6 +4028,7 @@ sub config_pager {
} elsif (length $pager == 0 || $pager eq 'cat') {
$pager = undef;
}
+ $ENV{GIT_PAGER_IN_USE} = defined($pager);
}
sub run_pager {
--
1.5.3.7.2230.g796d07-dirty
^ permalink raw reply related
* Re: [PATCH] Adding menu for Emacs git.el
From: Junio C Hamano @ 2007-12-11 6:36 UTC (permalink / raw)
To: Alexandre Julliard; +Cc: Remi Vanicat, git
In-Reply-To: <877ijwfh6z.dlv@vanicat.homelinux.org>
Remi Vanicat <vanicat@debian.org> writes:
> Adding three menus to the git-status-mode of git.el : One for marking
> and unmarking, one for what you do when you have a conflict, and the
> other one for all the rest.
>
> Signed-off-by: Rémi Vanicat <vanicat@debian.org>
> ---
>
> Alexandre Julliard <julliard@winehq.org> writes:
>
>> "=?utf-8?q?R=C3=A9mi=20Vanicat?=" <vanicat@debian.org>, Remi Vanicat
>> <vanicat@debian.org> writes:
>>
>>> Adding three menu to the git-status-mode of git.el : One for marking
>>> and unmarking, one for every thing you need when you have a conflict,
>>> and a last one for all the rest.
>>>
>>> Signed-off-by: Rémi Vanicat <vanicat@debian.org>
>>
>> It looks good to me. A couple of minor details:
>
> Here is the corrected patch
> [...]
>
>> BTW do you have a copyright assignment for Emacs?
> No, should I seek one ?
Alex, what is the current status of this patch?
^ permalink raw reply
* Re: v1.5.4 plans
From: Jeff King @ 2007-12-11 6:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vzlwhhli5.fsf@gitster.siamese.dyndns.org>
On Mon, Dec 10, 2007 at 09:02:26PM -0800, Junio C Hamano wrote:
> [PATCH] commit: do not add extra LF at the end of the summary.
>
> The scripted version relied on the nice "auto-strip the terminating LF"
> behaviour shell gives to "var=$(cmd)" construct, but we have to roll
> that ourselves.
This looks reasonable and generates the correct output as far as I can
tell, but...
> - log_tree_commit(&rev, commit);
> - printf("\n");
> + if (!log_tree_commit(&rev, commit)) {
> + struct strbuf buf = STRBUF_INIT;
> + pretty_print_commit(rev.commit_format, commit, &buf,
> + 0, NULL, NULL, 0, 0);
> + printf("%s\n", buf.buf);
> + strbuf_release(&buf);
> + }
We are duplicating the "!shown && ..." conditional branch from
log_tree_commit, which calls show_log. Why are we not calling show_log
instead of pretty_print_commit (I understand that show_log should end up
calling pretty_print_commit, but it is not immediately obvious that all
of the extra code in show_log is going to be ignored).
-Peff
^ permalink raw reply
* Re: v1.5.4 plans
From: Junio C Hamano @ 2007-12-11 6:47 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20071211063941.GB21718@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> We are duplicating the "!shown && ..." conditional branch from
> log_tree_commit, which calls show_log. Why are we not calling show_log
> instead of pretty_print_commit (I understand that show_log should end up
> calling pretty_print_commit, but it is not immediately obvious that all
> of the extra code in show_log is going to be ignored).
Exactly. I think show_log() has become too complex, and when we want a
oneline userformat, pretty-print-commit is more appropriate to use.
Actually, when I re-review the code, I think the part should use
format_commit_message() which is more to the point without any of the
other "more generic" parameter pretty-print-commit takes.
^ permalink raw reply
* Re: v1.5.4 plans
From: Jeff King @ 2007-12-11 6:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd4tdhgnd.fsf@gitster.siamese.dyndns.org>
On Mon, Dec 10, 2007 at 10:47:18PM -0800, Junio C Hamano wrote:
> > We are duplicating the "!shown && ..." conditional branch from
> > log_tree_commit, which calls show_log. Why are we not calling show_log
> > instead of pretty_print_commit (I understand that show_log should end up
> > calling pretty_print_commit, but it is not immediately obvious that all
> > of the extra code in show_log is going to be ignored).
>
> Exactly. I think show_log() has become too complex, and when we want a
> oneline userformat, pretty-print-commit is more appropriate to use.
> Actually, when I re-review the code, I think the part should use
> format_commit_message() which is more to the point without any of the
> other "more generic" parameter pretty-print-commit takes.
Perhaps it would be even more readable to simply split the printing of
the commit message and the diff. Tracking this bug was a bit confusing.
Ideally, print_summary would just say:
print_commit_message(commit);
print_diff(commit, "^commit");
where obviously each of those takes a few lines to say properly. But the
use of a combination "show the log and maybe the diff" function seems
like a shell holdover, trying to avoid spawning too many processes.
-Peff
^ permalink raw reply
* Re: "remote end hung up unexpectedly"
From: Andreas Ericsson @ 2007-12-11 6:54 UTC (permalink / raw)
To: Clifford Heath, Git Mailing List
In-Reply-To: <5F13DCA7-5072-4D76-89A7-7F05A5928FA2@gmail.com>
I'm bringing this back on the list. It's very uncharitable of you to not
do so yourself, as it prevents others with the same problem to find the
answer in the archives. Also, I don't run a private support-center for
git, so emailing me privately with questions regarding it is just plain
rude. Everyone's entitled to a second chance though, so read on below.
Clifford Heath wrote:
> On 11/12/2007, at 12:11 AM, Andreas Ericsson wrote:
>> How did you clone it? If you cloned via git:// protocol, you most likely
>> can't push back there. The git-daemon supports pushing, but does no
>> authentication.
>
> That was a problem - shame git didn't say "connection refused" instead,
> that would have been more obvious. The manpages are loaded with
> git-specific jargon - almost impenetrable for a newbie.
>
Git doesn't see that. It only knows it didn't get a proper response from
the other end.
>
>> Not really. You can do "git rebase --onto origin/master master". However,
>> since you merged origin/master earlier, a rebase will only tell you that
>> you're up-to-date.
>
> I don't think that anything I've done has affected the remote repository.
It hasn't. The only way you can update the remote repository, short of
editing it manually, is to use "git push". That's not strictly true, but
for the sake of this argument, it will suffice.
> At least, no changes are visible in the web view.
>
You can merge between local branches and still get the "up-to-date" message.
> Most of the instructions and manpages I've read show how to do things
> to local repositories and push changes back. Here's what I most recently
> tried, perhaps you can see where I went wrong. I've changed the project
> name for PROJECT, and the branch name with BRANCH
>
> git clone git+ssh://cjheath@repo.or.cz/srv/git/PROJECT.git
> cd PROJECT
> git checkout --track -b BRANCH origin/BRANCH
> git rebase origin/master
>
> At this point the local branch seems to have the content I want, so I tried
> to push the changes back in:
>
You mean, "at this point BRANCH seems to have the content I want"?
> git push
>
> Which replied:
>
> error: remote 'refs/heads/BRANCH' is not a strict subset of local ref
> 'refs/heads/BRANCH'. maybe you are not up-to-date and need to pull first?
> error: failed to push to 'git+ssh://cjheath@repo.or.cz/srv/git/PROJECT.git'
>
Yes. What you did caused history to be rewritten. Push is fast-forward[1]
only by default, to prevent published history from being modified, so when
you moved one line of development onto another you effectively changed its
ancestry.
If you do
git checkout BRANCH
git reset --hard origin/BRANCH
git merge origin/master
git push
you will achieve the desired end-result. If you really, really want a linear
history, you can do
git push -f origin BRANCH
but beware that this will cause errors for everyone fetching from you, and
for yourself if you fetch into multiple local clones of the same remote.
I suggest you sit down and really read through the git rebase man-page to
understand what it does and the precautions one must take when rewriting
history like that.
[1]fast-forward:
A fast-forward is a special type of merge where you have a
revision and you are "merging" another branch's changes that
happen to be a descendant of what you have. In such cases,
you do not make a new merge commit but instead just update
to his revision. This will happen frequently on a tracking
of a remote repository.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* [PATCH] Brown paper bag fix to previous send-email change
From: Mike Hommey @ 2007-12-11 6:55 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
My previous change led to the In-Reply-To header being <> when the given
value was empty. This fixes it.
Signed-off-by: Mike Hommey <mh@glandium.org>
---
I think I have an antispam problem somewhere with this patch, because even
keeping sending it, it never reached the list. And Junio didn't answer to my
private message about it, so I guess that was some server being smart with
the subject line. *sigh*
PS: sorry Junio if you get this patch multiple time.
git-send-email.perl | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index c0e1dd3..0e8c457 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -369,8 +369,10 @@ if ($thread && !defined $initial_reply_to && $prompting) {
$initial_reply_to = $_;
}
-$initial_reply_to =~ s/^\s*<?/</;
-$initial_reply_to =~ s/>?\s*$/>/;
+if ($initial_reply_to) {
+ $initial_reply_to =~ s/^\s*<?/</;
+ $initial_reply_to =~ s/>?\s*$/>/;
+}
if (!defined $smtp_server) {
foreach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) {
--
1.5.3.7.1161.g14b8
^ permalink raw reply related
* Re: v1.5.4 plans
From: Junio C Hamano @ 2007-12-11 7:00 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20071211065400.GA25985@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Perhaps it would be even more readable to simply split the printing of
> the commit message and the diff. Tracking this bug was a bit confusing.
> Ideally, print_summary would just say:
>
> print_commit_message(commit);
> print_diff(commit, "^commit");
>
> where obviously each of those takes a few lines to say properly. But the
> use of a combination "show the log and maybe the diff" function seems
> like a shell holdover, trying to avoid spawning too many processes.
Perhaps, but that's post 1.5.4. There is no short-hand to call the
single commit diff-tree and not give any commit header afair.
Anyway, format-commit-message() makes it much more readable ;-)
diff --git a/builtin-commit.c b/builtin-commit.c
index b6b81d5..9cb7589 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -660,12 +660,16 @@ static void print_summary(const char *prefix, const unsigned char *sha1)
rev.verbose_header = 1;
rev.show_root_diff = 1;
rev.commit_format = get_commit_format("format:%h: %s");
- rev.always_show_header = 1;
+ rev.always_show_header = 0;
printf("Created %scommit ", initial_commit ? "initial " : "");
- log_tree_commit(&rev, commit);
- printf("\n");
+ if (!log_tree_commit(&rev, commit)) {
+ struct strbuf buf = STRBUF_INIT;
+ format_commit_message(commit, "%h: %s", &buf);
+ printf("%s\n", buf.buf);
+ strbuf_release(&buf);
+ }
}
int git_commit_config(const char *k, const char *v)
^ permalink raw reply related
* Re: Something is broken in repack
From: Jon Smirl @ 2007-12-11 7:01 UTC (permalink / raw)
To: Nicolas Pitre, Junio C Hamano, gcc; +Cc: Git Mailing List
In-Reply-To: <9e4733910712102129v140c2affqf2e73e75855b61ea@mail.gmail.com>
Switching to the Google perftools malloc
http://goog-perftools.sourceforge.net/
10% 30 828M
20% 15 831M
30% 10 834M
40% 50 1014M
50% 80 1086M
60% 80 1500M
70% 200 1.53G
80% 200 1.85G
90% 260 1.87G
95% 520 1.97G
100% 1335 2.24G
Google allocator knocked 600MB off from memory use.
Memory consumption did not fall during the write out phase like it did with gcc.
Since all of this is with the same code except for changing the
threading split, those runs where memory consumption went to 4.5GB
with the gcc allocator must have triggered an extreme problem with
fragmentation.
Total CPU time 196 CPU minutes vs 190 for gcc. Google's claims of
being faster are not true.
So why does our threaded code take 20 CPU minutes longer (12%) to run
than the same code with a single thread? Clock time is obviously
faster. Are the threads working too close to each other in memory and
bouncing cache lines between the cores? Q6600 is just two E6600s in
the same package, the caches are not shared.
Why does the threaded code need 2.24GB (google allocator, 2.85GB gcc)
with 4 threads? But only need 950MB with one thread? Where's the extra
gigabyte going?
Is there another allocator to try? One that combines Google's
efficiency with gcc's speed?
On 12/11/07, Jon Smirl <jonsmirl@gmail.com> wrote:
> I added the gcc people to the CC, it's their repository. Maybe they
> can help up sort this out.
>
> On 12/11/07, Jon Smirl <jonsmirl@gmail.com> wrote:
> > On 12/10/07, Nicolas Pitre <nico@cam.org> wrote:
> > > On Mon, 10 Dec 2007, Jon Smirl wrote:
> > >
> > > > New run using same configuration. With the addition of the more
> > > > efficient load balancing patches and delta cache accounting.
> > > >
> > > > Seconds are wall clock time. They are lower since the patch made
> > > > threading better at using all four cores. I am stuck at 380-390% CPU
> > > > utilization for the git process.
> > > >
> > > > complete seconds RAM
> > > > 10% 60 900M (includes counting)
> > > > 20% 15 900M
> > > > 30% 15 900M
> > > > 40% 50 1.2G
> > > > 50% 80 1.3G
> > > > 60% 70 1.7G
> > > > 70% 140 1.8G
> > > > 80% 180 2.0G
> > > > 90% 280 2.2G
> > > > 95% 530 2.8G - 1,420 total to here, previous was 1,983
> > > > 100% 1390 2.85G
> > > > During the writing phase RAM fell to 1.6G
> > > > What is being freed in the writing phase??
> > >
> > > The cached delta results, but you put a cap of 256MB for them.
> > >
> > > Could you try again with that cache disabled entirely, with
> > > pack.deltacachesize = 1 (don't use 0 as that means unbounded).
> > >
> > > And then, while still keeping the delta cache disabled, could you try
> > > with pack.threads = 2, and pack.threads = 1 ?
> > >
> > > I'm sorry to ask you to do this but I don't have enough ram to even
> > > complete a repack with threads=2 so I'm reattempting single threaded at
> > > the moment. But I really wonder if the threading has such an effect on
> > > memory usage.
> >
> > I already have a threads = 1 running with this config. Binary and
> > config were same from threads=4 run.
> >
> > 10% 28min 950M
> > 40% 135min 950M
> > 50% 157min 900M
> > 60% 160min 830M
> > 100% 170min 830M
> >
> > Something is hurting bad with threads. 170 CPU minutes with one
> > thread, versus 195 CPU minutes with four threads.
> >
> > Is there a different memory allocator that can be used when
> > multithreaded on gcc? This whole problem may be coming from the memory
> > allocation function. git is hardly interacting at all on the thread
> > level so it's likely a problem in the C run-time.
> >
> > [core]
> > repositoryformatversion = 0
> > filemode = true
> > bare = false
> > logallrefupdates = true
> > [pack]
> > threads = 1
> > deltacachesize = 256M
> > windowmemory = 256M
> > deltacachelimit = 0
> > [remote "origin"]
> > url = git://git.infradead.org/gcc.git
> > fetch = +refs/heads/*:refs/remotes/origin/*
> > [branch "trunk"]
> > remote = origin
> > merge = refs/heads/trunk
> >
> >
> >
> >
> > >
> > >
> > >
> > > >
> > > > I have no explanation for the change in RAM usage. Two guesses come to
> > > > mind. Memory fragmentation. Or the change in the way the work was
> > > > split up altered RAM usage.
> > > >
> > > > Total CPU time was 195 minutes in 70 minutes clock time. About 70%
> > > > efficient. During the compress phase all four cores were active until
> > > > the last 90 seconds. Writing the objects took over 23 minutes CPU
> > > > bound on one core.
> > > >
> > > > New pack file is: 270,594,853
> > > > Old one was: 344,543,752
> > > > It still has 828,660 objects
> > >
> > > You mean the pack for the gcc repo is now less than 300MB? Wow.
> > >
> > >
> > > Nicolas
> > >
> >
> >
> > --
> > Jon Smirl
> > jonsmirl@gmail.com
> >
>
>
> --
> Jon Smirl
> jonsmirl@gmail.com
>
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: v1.5.4 plans
From: Jeff King @ 2007-12-11 7:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vtzmqhvgq.fsf@gitster.siamese.dyndns.org>
On Mon, Dec 10, 2007 at 05:27:17PM -0800, Junio C Hamano wrote:
> > I suspect there are still some bugs lurking in there, but it's hard
> > to say because I don't know what the behavior _should_ be (there are
> > some test cases in that email).
>
> The last time I looked at the "directory" side of builtin-clean.c, I had
> to quickly reach for my barf bag. I never use "git clean" without "-n"
> and I never ever use "git clean" with "-d"; I do not have any idea what
> behaviour when given "-d" would be useful. AFAIU, the scripted version
> did not have clear semantics either.
I had the same feeling. I am tempted to leave it, then. The
non-intuitive behavior I managed to trigger was:
- _only_ when using git pathspec matching like "git clean -n '*.ext'"
- confusing in a safe way (trying to remove 'dir.ext' with '*.ext'
will accidentally not happen, rather than accidentally happening)
So unless somebody complains, it is probably not a big problem,
and I think fixing it will require mucking with pathspec and dir
matching internals, which would be nice not to do right before v1.5.4.
OTOH, leaving something that is broken and just hoping nobody will
complain feels kind of wrong. :)
-Peff
^ permalink raw reply
* Re: v1.5.4 plans
From: Jeff King @ 2007-12-11 7:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7ijlhg0o.fsf@gitster.siamese.dyndns.org>
On Mon, Dec 10, 2007 at 11:00:55PM -0800, Junio C Hamano wrote:
> Perhaps, but that's post 1.5.4. There is no short-hand to call the
> single commit diff-tree and not give any commit header afair.
OK. I was going to add a "something like this..." patch to my last
email, but realized I had no idea how one would invoke the diff
machinery in such a way.
> Anyway, format-commit-message() makes it much more readable ;-)
The repetition of the format string is a little ugly. But otherwise,
Acked-by: Jeff King <peff@peff.net>
-Peff
^ permalink raw reply
* Re: [PATCH] Brown paper bag fix to previous send-email change
From: Junio C Hamano @ 2007-12-11 7:04 UTC (permalink / raw)
To: Mike Hommey; +Cc: git
In-Reply-To: <1197356116-12176-1-git-send-email-mh@glandium.org>
Mike Hommey <mh@glandium.org> writes:
> My previous change led to the In-Reply-To header being <> when the given
> value was empty. This fixes it.
>
> Signed-off-by: Mike Hommey <mh@glandium.org>
> ---
>
> I think I have an antispam problem somewhere with this patch, because even
> keeping sending it, it never reached the list. And Junio didn't answer to my
> private message about it, so I guess that was some server being smart with
> the subject line. *sigh*
>
> PS: sorry Junio if you get this patch multiple time.
No, I happened to have noticed the same and have an almost identical fix
(I said "defined $initial_reply_to") already queued in my tree.
^ permalink raw reply
* Re: v1.5.4 plans
From: Andreas Ericsson @ 2007-12-11 7:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vtzmqhvgq.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
>> - git-clean's handling of directory wildcards. I didn't get a response
>> to
>>
>> http://mid.gmane.org/20071206043247.GC5499@coredump.intra.peff.net
>>
>> I suspect there are still some bugs lurking in there, but it's hard
>> to say because I don't know what the behavior _should_ be (there are
>> some test cases in that email).
>
> The last time I looked at the "directory" side of builtin-clean.c, I had
> to quickly reach for my barf bag. I never use "git clean" without "-n"
> and I never ever use "git clean" with "-d"; I do not have any idea what
> behaviour when given "-d" would be useful.
When you have a trash directory without any tracked files, clean will not
by default descend into that directory and thus won't remove neither files
nor directory. I frequently use one for automated testing, much like git's
trash repository, but the only time I do "git clean -d" is when building
things on a release-server with the repository checked out. It's faster
than "make distclean", and not all of our projects have a Makefile to begin
with. Tacking "git clean -d" at the end of test-scripts makes it simple to
remove all excess cruft in one go.
So in short, git clean -d can be useful. I have no idea when "git clean dir"
would be though.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: "remote end hung up unexpectedly"
From: Clifford Heath @ 2007-12-11 7:15 UTC (permalink / raw)
To: Andreas Ericsson; +Cc: Git Mailing List
In-Reply-To: <475E3415.3030208@op5.se>
On 11/12/2007, at 5:54 PM, Andreas Ericsson wrote:
> I'm bringing this back on the list. It's very uncharitable of you
> to not
> do so yourself,
My humble apologies - I just hit reply. I've previously only encountered
majordomo configurations where the reply goes to the list (including
when I operated majordomo for ten years), so I made a mistake.
>>> Not really. You can do "git rebase --onto origin/master master".
>>> However,
>>> since you merged origin/master earlier, a rebase will only tell
>>> you that
>>> you're up-to-date.
>> I don't think that anything I've done has affected the remote
>> repository.
>
> It hasn't. The only way you can update the remote repository, short of
> editing it manually, is to use "git push". That's not strictly
> true, but
> for the sake of this argument, it will suffice.
Thanks, that's good confirmation.
>> Most of the instructions and manpages I've read show how to do things
>> to local repositories and push changes back. Here's what I most
>> recently
>> tried, perhaps you can see where I went wrong. I've changed the
>> project
>> name for PROJECT, and the branch name with BRANCH
>> git clone git+ssh://cjheath@repo.or.cz/srv/git/PROJECT.git
>> cd PROJECT
>> git checkout --track -b BRANCH origin/BRANCH
>> git rebase origin/master
>> At this point the local branch seems to have the content I want,
>> so I tried
>> to push the changes back in:
>
> You mean, "at this point BRANCH seems to have the content I want"?
No, I mean that the recent changes have been pulled from origin/master
and my tree looks like what I want the remote origin/BRANCH to look
like.
Though if I pushed, I don't know that its revision history would
necessarily
be in the preferred order.
>> git push
>> Which replied:
>> error: remote 'refs/heads/BRANCH' is not a strict subset of local
>> ref 'refs/heads/BRANCH'. maybe you are not up-to-date and need to
>> pull first?
>> error: failed to push to 'git+ssh://cjheath@repo.or.cz/srv/git/
>> PROJECT.git'
>
> Yes. What you did caused history to be rewritten. Push is fast-
> forward[1]
> only by default, to prevent published history from being modified,
> so when
> you moved one line of development onto another you effectively
> changed its
> ancestry.
Ok, I think I follow that. I can see I need to spend more time
reading the
man pages. Thanks for your help...
Clifford Heath.
> If you do
>
> git checkout BRANCH
> git reset --hard origin/BRANCH
> git merge origin/master
> git push
>
> you will achieve the desired end-result. If you really, really want
> a linear
> history, you can do
>
> git push -f origin BRANCH
>
> but beware that this will cause errors for everyone fetching from
> you, and
> for yourself if you fetch into multiple local clones of the same
> remote.
>
> I suggest you sit down and really read through the git rebase man-
> page to
> understand what it does and the precautions one must take when
> rewriting
> history like that.
>
>
> [1]fast-forward:
> A fast-forward is a special type of merge where you have a
> revision and you are "merging" another branch's changes that
> happen to be a descendant of what you have. In such cases,
> you do not make a new merge commit but instead just update
> to his revision. This will happen frequently on a tracking
> of a remote repository.
>
> --
> Andreas Ericsson andreas.ericsson@op5.se
> OP5 AB www.op5.se
> Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [PATCH 6/5] Move fetch_ref from http-push.c and http-walker.c to http.c
From: Andreas Ericsson @ 2007-12-11 7:22 UTC (permalink / raw)
To: Mike Hommey; +Cc: Junio C Hamano, git
In-Reply-To: <20071211062129.GA8372@glandium.org>
Mike Hommey wrote:
> On Mon, Dec 10, 2007 at 09:09:05PM -0800, Junio C Hamano wrote:
>> Mike Hommey <mh@glandium.org> writes:
>>
>>> Make the necessary changes to be ok with their difference, and rename the
>>> function http_fetch_ref.
>> Sorry, but I cannot parse "ok with their difference" part...
>
> What about "to fit with their difference" ?
>
Define "their" and "difference". Implicitly referring to another commit
message won't help cursory glances much (and I have no idea what you're
talking about since I didn't read the rest of your series very carefully).
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: Something is broken in repack
From: Andreas Ericsson @ 2007-12-11 7:34 UTC (permalink / raw)
To: Jon Smirl; +Cc: Nicolas Pitre, Junio C Hamano, gcc, Git Mailing List
In-Reply-To: <9e4733910712102301p5e6c4165v6afb32d157478828@mail.gmail.com>
Jon Smirl wrote:
> Switching to the Google perftools malloc
> http://goog-perftools.sourceforge.net/
>
> Google allocator knocked 600MB off from memory use.
> Memory consumption did not fall during the write out phase like it did with gcc.
>
> Since all of this is with the same code except for changing the
> threading split, those runs where memory consumption went to 4.5GB
> with the gcc allocator must have triggered an extreme problem with
> fragmentation.
>
> Total CPU time 196 CPU minutes vs 190 for gcc. Google's claims of
> being faster are not true.
>
Did you use the tcmalloc with heap checker/profiler, or tcmalloc_minimal?
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [PATCH] Brown paper bag fix to previous send-email change
From: Mike Hommey @ 2007-12-11 7:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v3au9hfuj.fsf@gitster.siamese.dyndns.org>
On Mon, Dec 10, 2007 at 11:04:36PM -0800, Junio C Hamano wrote:
> Mike Hommey <mh@glandium.org> writes:
>
> > My previous change led to the In-Reply-To header being <> when the given
> > value was empty. This fixes it.
> >
> > Signed-off-by: Mike Hommey <mh@glandium.org>
> > ---
> >
> > I think I have an antispam problem somewhere with this patch, because even
> > keeping sending it, it never reached the list. And Junio didn't answer to my
> > private message about it, so I guess that was some server being smart with
> > the subject line. *sigh*
> >
> > PS: sorry Junio if you get this patch multiple time.
>
> No, I happened to have noticed the same and have an almost identical fix
> (I said "defined $initial_reply_to") already queued in my tree.
Testing "defined $initial_reply_to" might not be enough, because $initial_reply_to
can be defined and empty.
Mike
^ permalink raw reply
* Re: [PATCH 6/5] Move fetch_ref from http-push.c and http-walker.c to http.c
From: Mike Hommey @ 2007-12-11 7:49 UTC (permalink / raw)
To: Andreas Ericsson; +Cc: Junio C Hamano, git
In-Reply-To: <475E3AA0.5010901@op5.se>
On Tue, Dec 11, 2007 at 08:22:08AM +0100, Andreas Ericsson wrote:
> Mike Hommey wrote:
>> On Mon, Dec 10, 2007 at 09:09:05PM -0800, Junio C Hamano wrote:
>>> Mike Hommey <mh@glandium.org> writes:
>>>
>>>> Make the necessary changes to be ok with their difference, and rename the
>>>> function http_fetch_ref.
>>> Sorry, but I cannot parse "ok with their difference" part...
>>
>> What about "to fit with their difference" ?
>>
>
> Define "their" and "difference". Implicitly referring to another commit
> message won't help cursory glances much (and I have no idea what you're
> talking about since I didn't read the rest of your series very carefully).
The rest of the series doesn't have anything to do with this one, except
that fetch_ref had been modified to use strbuf in both.
Their difference is that one would take a ref with "refs/" at the
beginning and the other wouldn't. I'll try to write a better commit
message later today.
Mike
^ permalink raw reply
* Re: [PATCH] Brown paper bag fix to previous send-email change
From: Junio C Hamano @ 2007-12-11 8:02 UTC (permalink / raw)
To: Mike Hommey; +Cc: git
In-Reply-To: <20071211074427.GA12959@glandium.org>
Mike Hommey <mh@glandium.org> writes:
> On Mon, Dec 10, 2007 at 11:04:36PM -0800, Junio C Hamano wrote:
>> Mike Hommey <mh@glandium.org> writes:
>>
>> > My previous change led to the In-Reply-To header being <> when the given
>> > value was empty. This fixes it.
>> >
>> > Signed-off-by: Mike Hommey <mh@glandium.org>
>> > ---
>> >
>> > I think I have an antispam problem somewhere with this patch, because even
>> > keeping sending it, it never reached the list. And Junio didn't answer to my
>> > private message about it, so I guess that was some server being smart with
>> > the subject line. *sigh*
>> >
>> > PS: sorry Junio if you get this patch multiple time.
>>
>> No, I happened to have noticed the same and have an almost identical fix
>> (I said "defined $initial_reply_to") already queued in my tree.
>
> Testing "defined $initial_reply_to" might not be enough, because $initial_reply_to
> can be defined and empty.
And if empty what does your regexp do? Produce "<>"? How's that
useful?
^ permalink raw reply
* Re: [PATCH] Brown paper bag fix to previous send-email change
From: Junio C Hamano @ 2007-12-11 8:06 UTC (permalink / raw)
To: Mike Hommey; +Cc: git
In-Reply-To: <20071211074427.GA12959@glandium.org>
Mike Hommey <mh@glandium.org> writes:
>> No, I happened to have noticed the same and have an almost identical fix
>> (I said "defined $initial_reply_to") already queued in my tree.
>
> Testing "defined $initial_reply_to" might not be enough, because $initial_reply_to
> can be defined and empty.
Ah, I see what you mean. Thanks.
^ permalink raw reply
* [PATCH] Fix clone not to ignore depth when performing a local clone
From: Charles Bailey @ 2007-12-11 6:47 UTC (permalink / raw)
To: git
When git-clone detects that it can perform a local clone it
follows a path that silently ignores the depth parameter.
Presumably if the user explicitly requests a shallow clone they
have a reason to prefer a space efficient clone of just the recent
history so bypass the local magic if the user specifies the depth
parameter.
Signed-off-by: Charles Bailey <charles@hashpling.org>
---
git-clone.sh | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/git-clone.sh b/git-clone.sh
index ecf9d89..fb124d8 100755
--- a/git-clone.sh
+++ b/git-clone.sh
@@ -205,7 +205,10 @@ fi
# it is local
if base=$(get_repo_base "$repo"); then
repo="$base"
- local=yes
+ if test -z "$depth"
+ then
+ local=yes
+ fi
fi
dir="$2"
--
1.5.3.7.2242.gcc945-dirty
^ permalink raw reply related
* Re: Building git-1.5.3.7 on HP-UX 11.00
From: Junio C Hamano @ 2007-12-11 8:26 UTC (permalink / raw)
To: H.Merijn Brand; +Cc: Johannes Schindelin, Andreas Ericsson, git, Sam Vilain
In-Reply-To: <20071210145123.7c34af6d@pc09.procura.nl>
"H.Merijn Brand" <h.m.brand@xs4all.nl> writes:
> Summary of the changes:
>
> 1 Added a section for HP-UX in the Makefile. Note that this will
> cover most of HP-UX, but might need several changes for both
> newer HP-UX versions and 64bit environments. Will come to that
> once I've got it all running
>
> 2 HP-UX does not have <sys/select.h>. I mentioned this before
Unfortunate. It is even in POSIX.
But nothing "make -DNO_SYS_SELECT=Unfortunately" cannot fix.
> 3 I am willing to believe that HP_UX' vsnprintf () is broken, or
> at least does not conform to the expectations in the GNU world,
> but chickening out like the way strbuf_addf () does is maybe a
> bit too rude, so I forced a minimum of 64bytes available. That
> fixes a lot!
>
> but it still breaks t4013 :(
I think Shawn had something similar for Solaris, and if it is a small
cost to help portability, it does not feel so bad.
> 4 'tr' doesn't like '\0', but wants '\000' instead.
> 5 'tr' cannot deal with character classes
Ok, that's odd (and not old fashioned, which makes even odder), but
"\000" is not too bad (unless we have to do that million places).
> 6 I don't know how to attack another HP specific problem: HP has
> a system command called 'patch' and it is *very* incompatible
> with GNU patch. Very.
I do not think there is any reason for us to rely on "GNU patch"
anymore. 4109 can have precomputed test vector instead of comparing our
output with what GNU patch of the day happens to do.
> 7 What do you expect for LOCALE's?
I am not opposed to a change that makes these tests to conditionally
skip on systems (not necessarily "platforms" but individual
"installations") that do not have locales necessary to run tests
installed, just like cvs or svn tests are skipped when they are not
installed. Especially you are not using iconv...
> + git-index-pack -o tmp.idx test-2-7f8ead892057e78576c0329a70cc83afb113f117.pack
> fatal: serious inflate inconsistency
That sounds like a broken zlib X-<.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox