| Message ID | d3b27394cad951a9f1412a04572439513d7e9f4d.1591821067.git.jonathantanmy@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | CDN offloading update | expand |
Jonathan Tan <jonathantanmy@google.com> writes: > diff --git a/http.c b/http.c > index 62aa995245..39cbd56702 100644 > --- a/http.c > +++ b/http.c > @@ -2270,9 +2270,9 @@ int finish_http_pack_request(struct http_pack_request *preq) > { > struct packed_git **lst; > struct packed_git *p = preq->target; > - char *tmp_idx; > - size_t len; > struct child_process ip = CHILD_PROCESS_INIT; > + int tmpfile_fd; > + int ret = 0; > > close_pack_index(p); > > @@ -2284,35 +2284,24 @@ int finish_http_pack_request(struct http_pack_request *preq) > lst = &((*lst)->next); > *lst = (*lst)->next; > > - if (!strip_suffix(preq->tmpfile.buf, ".pack.temp", &len)) > - BUG("pack tmpfile does not end in .pack.temp?"); > - tmp_idx = xstrfmt("%.*s.idx.temp", (int)len, preq->tmpfile.buf); > + tmpfile_fd = xopen(preq->tmpfile.buf, O_RDONLY); Shouldn't we catch the case where we cannot open the file for reading and return -1 to have the caller handle the error, just like the case where other errors are detected in this function? > argv_array_push(&ip.args, "index-pack"); > - argv_array_pushl(&ip.args, "-o", tmp_idx, NULL); > - argv_array_push(&ip.args, preq->tmpfile.buf); > + argv_array_push(&ip.args, "--stdin"); > ip.git_cmd = 1; > - ip.no_stdin = 1; > + ip.in = tmpfile_fd; > ip.no_stdout = 1; > > if (run_command(&ip)) { > - unlink(preq->tmpfile.buf); > - unlink(tmp_idx); > - free(tmp_idx); > - return -1; > - } > - > - unlink(sha1_pack_index_name(p->hash)); > - > - if (finalize_object_file(preq->tmpfile.buf, sha1_pack_name(p->hash)) > - || finalize_object_file(tmp_idx, sha1_pack_index_name(p->hash))) { > - free(tmp_idx); > - return -1; > + ret = -1; > + goto cleanup; > } > > install_packed_git(the_repository, p); > - free(tmp_idx); > - return 0; > +cleanup: > + close(tmpfile_fd); > + unlink(preq->tmpfile.buf); > + return ret; > } > > struct http_pack_request *new_http_pack_request(
diff --git a/http.c b/http.c index 62aa995245..39cbd56702 100644 --- a/http.c +++ b/http.c @@ -2270,9 +2270,9 @@ int finish_http_pack_request(struct http_pack_request *preq) { struct packed_git **lst; struct packed_git *p = preq->target; - char *tmp_idx; - size_t len; struct child_process ip = CHILD_PROCESS_INIT; + int tmpfile_fd; + int ret = 0; close_pack_index(p); @@ -2284,35 +2284,24 @@ int finish_http_pack_request(struct http_pack_request *preq) lst = &((*lst)->next); *lst = (*lst)->next; - if (!strip_suffix(preq->tmpfile.buf, ".pack.temp", &len)) - BUG("pack tmpfile does not end in .pack.temp?"); - tmp_idx = xstrfmt("%.*s.idx.temp", (int)len, preq->tmpfile.buf); + tmpfile_fd = xopen(preq->tmpfile.buf, O_RDONLY); argv_array_push(&ip.args, "index-pack"); - argv_array_pushl(&ip.args, "-o", tmp_idx, NULL); - argv_array_push(&ip.args, preq->tmpfile.buf); + argv_array_push(&ip.args, "--stdin"); ip.git_cmd = 1; - ip.no_stdin = 1; + ip.in = tmpfile_fd; ip.no_stdout = 1; if (run_command(&ip)) { - unlink(preq->tmpfile.buf); - unlink(tmp_idx); - free(tmp_idx); - return -1; - } - - unlink(sha1_pack_index_name(p->hash)); - - if (finalize_object_file(preq->tmpfile.buf, sha1_pack_name(p->hash)) - || finalize_object_file(tmp_idx, sha1_pack_index_name(p->hash))) { - free(tmp_idx); - return -1; + ret = -1; + goto cleanup; } install_packed_git(the_repository, p); - free(tmp_idx); - return 0; +cleanup: + close(tmpfile_fd); + unlink(preq->tmpfile.buf); + return ret; } struct http_pack_request *new_http_pack_request(
When Git fetches a pack using dumb HTTP, (among other things) it invokes index-pack on a ".pack.temp" packfile, specifying the filename as an argument. A future commit will require the aforementioned invocation of index-pack to also generate a "keep" file. To use this, we either have to use index-pack's naming convention (because --keep requires the pack's filename to end with ".pack") or to pass the pack through stdin. Of the two, it is simpler to pass the pack through stdin. Thus, teach http to pass --stdin to index-pack. As a bonus, the code is now simpler. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> --- http.c | 33 +++++++++++---------------------- 1 file changed, 11 insertions(+), 22 deletions(-)