mbox series

[v2,0/9] CDN offloading update

Message ID cover.1591821067.git.jonathantanmy@google.com (mailing list archive)
Headers show
Series CDN offloading update | expand

Message

Jonathan Tan June 10, 2020, 8:57 p.m. UTC 
Thanks, Junio, for taking a look at this.

This version is based on cc/upload-pack-data-2. I thought of retaining
the base from the original version, but trying out the rebase showed
that there were some code movements that would clutter the range-diff
anyway, so I went ahead with the rebase.

I tried to avoid the need for the first patch, but it turns out that
index-pack can operate with custom names for .pack and .idx
(".pack.temp" and ".idx.temp", as used by http-fetch) but not .keep, and
we need .keep, so changing to --stdin instead of using custom names is
necessary anyway. I've updated the commit message to use that
justification.

The main change is Junio's suggestion [1] to refactor into smaller
functions. Quoting from [1]:

>  - Whether we are fetching a single packfile from a URL, or walking
>    to fetch all the packfiles in the repository at a given URL
> 
>  - Whether packfiles taken from outer space are marked with the
>    "keep" bit
> 
>  - Whether the obtained packfile(s) are internally "installed"
>    to the running process

To which we can add:

 4. Whether to close the index mmap
 5. Whether to delete the packfile from the given list

The 3rd, 4th, and 5th concerns were refactored in patch 2, and the
others were refactored in patch 4.

I also updated the documentation to hopefully make it clearer that the
client should download all packfiles, including the inline packfile,
before performing the connectivity check.

[1] https://lore.kernel.org/git/xmqqeer2xr0f.fsf@gitster.c.googlers.com/

Jonathan Tan (9):
  http: use --stdin when indexing dumb HTTP pack
  http: refactor finish_http_pack_request()
  http-fetch: refactor into function
  http-fetch: support fetching packfiles by URL
  Documentation: order protocol v2 sections
  Documentation: add Packfile URIs design doc
  upload-pack: refactor reading of pack-objects out
  fetch-pack: support more than one pack lockfile
  upload-pack: send part of packfile response as uri

 Documentation/git-http-fetch.txt         |   9 +-
 Documentation/technical/packfile-uri.txt |  78 +++++++++++
 Documentation/technical/protocol-v2.txt  |  48 +++++--
 builtin/fetch-pack.c                     |  17 ++-
 builtin/pack-objects.c                   |  76 +++++++++++
 connected.c                              |   8 +-
 fetch-pack.c                             | 137 +++++++++++++++++---
 fetch-pack.h                             |   2 +-
 http-fetch.c                             | 126 +++++++++++++-----
 http-push.c                              |   8 +-
 http-walker.c                            |   5 +-
 http.c                                   |  82 ++++++------
 http.h                                   |  24 +++-
 t/t5550-http-fetch-dumb.sh               |  30 +++++
 t/t5702-protocol-v2.sh                   |  88 +++++++++++++
 transport-helper.c                       |   5 +-
 transport.c                              |  14 +-
 transport.h                              |   6 +-
 upload-pack.c                            | 157 +++++++++++++++++------
 19 files changed, 752 insertions(+), 168 deletions(-)
 create mode 100644 Documentation/technical/packfile-uri.txt

Comments

Junio C Hamano June 10, 2020, 11:16 p.m. UTC | #1 
Jonathan Tan <jonathantanmy@google.com> writes:

> Thanks, Junio, for taking a look at this.
>
> This version is based on cc/upload-pack-data-2. I thought of retaining
> the base from the original version, but trying out the rebase showed
> that there were some code movements that would clutter the range-diff
> anyway, so I went ahead with the rebase.

I think rebasing is not just justifiable, but is the right thing to
do in this case, as the topics fairly heavily steps each other's toe.

Thanks.