Re: [PATCH 2/3] http-push: when sending objects, don't PUT before MOVE
flat view
From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:45:58
Hi, On Sat, 17 Jan 2009, Ray Chuan wrote:
since 753bc911f489748a837ecb5ea4b5216220b24845, the opaquelocktocken
It would be nice to use the form <abbrev-sha1>(<oneline>) instead of a non-abbreviated SHA-1 that everybody who is interested has to look up, wasting time.
URI isn't used, so it doesn't make sense to PUT then MOVE. currently, git PUTs to /repo.git/objects/1a/1a2b..._opaquelocktoken:1234-....
First you say that the opaquelocktoken URI is not used, but here it looks like one?
on some platforms, ':' isn't allowed in filenames so this fails (assuming the server doesn't recognize it as opaquelocktoken scheme). in fact, i don't think this is the correct implementation of the scheme; 'opaquelocktoken: ' should come in front of the path.
It would be nice to make that a fact-backed commit message. IOW there has to be some documentation about the subject which you can quote, and which would give you a definitive answer to the question if it should be a prefix or not.
quoted hunk ↗ jump to hunk
diff --git a/src/git-1.6.1/http-push.c b/src/git-1.6.1/http-push.c index a646a49..838ff6f 100644 --- a/src/git-1.6.1/http-push.c +++ b/src/git-1.6.1/http-push.c@@ -31,7 +31,6 @@ enum XML_Status { /* DAV methods */ #define DAV_LOCK "LOCK" #define DAV_MKCOL "MKCOL" -#define DAV_MOVE "MOVE" #define DAV_PROPFIND "PROPFIND" #define DAV_PUT "PUT" #define DAV_UNLOCK "UNLOCK"@@ -104,7 +103,6 @@ enum transfer_state { NEED_PUSH, RUN_MKCOL, RUN_PUT, - RUN_MOVE, ABORTED, COMPLETE, };@@ -528,11 +526,6 @@ static void start_put(struct transfer_request *request) posn += 2; *(posn++) = '/'; strcpy(posn, hex + 2); - request->dest = xmalloc(strlen(request->url) + 14); - sprintf(request->dest, "Destination: %s", request->url); - posn += 38; - *(posn++) = '_'; - strcpy(posn, request->lock->token); slot = get_active_slot(); slot->callback_func = process_response;
Color me puzzled again. Why is this code no longer needed? Is this the lock you were talking about?
quoted hunk ↗ jump to hunk
@@ -705,23 +672,13 @@ static void finish_request(structtransfer_request *request) } } else if (request->state == RUN_PUT) { if (request->curl_result == CURLE_OK) { - start_move(request); - } else { - fprintf(stderr, "PUT %s failed, aborting (%d/%ld)\n", - sha1_to_hex(request->obj->sha1), - request->curl_result, request->http_code); - request->state = ABORTED; - aborted = 1; - }
... and here comes my first doubt that it would be a good idea to avoid "put && move"; what if "put" fails? Then you end up with a corrupt repository. Ciao, Dscho