git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jonathan Tan <jonathantanmy@google.com>
To: gitster@pobox.com
Cc: jonathantanmy@google.com, git@vger.kernel.org
Subject: Re: [PATCH 1/8] http: use --stdin when getting dumb HTTP pack
Date: Mon,  1 Jun 2020 13:37:10 -0700	[thread overview]
Message-ID: <20200601203710.222245-1-jonathantanmy@google.com> (raw)
In-Reply-To: <xmqqmu5qxs6e.fsf@gitster.c.googlers.com>

> Jonathan Tan <jonathantanmy@google.com> writes:
> 
> > When Git fetches a pack using dumb HTTP, it reuses the server's name for
> > the packfile (which incorporates a hash), which is different from the
> > behavior of fetch-pack and receive-pack.
> 
> My first two reads of the above mistakenly thought that for some
> reason the packfile has the URL to the server encoded in its name,
> but that is not what you meant by "the server's name".  You rather
> meant "the name the server stores the packfile under", "the name the
> server gave the packfile", "it reuses the name the server uses for
> the packfile".  The last rephrase may be the easiest to understand.

OK - I'll use that.

> > A subsequent patch will allow downloading packs over HTTP(S) as part of
> > a fetch. These downloads will not necessarily be from a Git repository,
> > and thus may not have a hash as part of its name.
> 
> A location that is not necessarily a Git repository can still honor
> the naming convention, so I find this a bit weak argument.  After
> all, the procedure to prepare such a CDN backed file would be using
> Git and the (git) "natural" name for the resulting packfile is
> easily available to it, isn't it?
> 
> I am not necessarily against loosening the limitation of the shape
> of the filename, but we may want to say the reason why we want to
> name the packfile on the CDN side differently from how Git would
> naturally name them.  What benefit would we get out from geing able
> to do so?  Perhaps it makes arrangements such as "you can fetch
> 'pack-v1.0.pack' to become reasonably up-to-date if you at least
> have the version v1.0 software", "if the last time you fetched from
> us was 2020-05-20 or after, you can fetch 'pack-2020-05-20.pack' and
> be assured that you aren't missing anything", etc.?  Such a "why
> would somebody want to name the packfile differently" would make a
> more convincing justification.

I didn't want to unnecessarily exclude features like signed URLs which
may change the way the URL is - for example, in Google Cloud Storage,
the signed part is a suffix [1]. I'll include this in the commit
message.

Having said that, after rereading my patch:

 (1) I'm not sure anymore if the restriction is that there must be a
     hash in the filename. It might be just that the filename
     must end in ".pack.temp". (Having said that, if the filename was
     not named "<hash>.pack.temp", it would look different to the rest
     of the contents of "objects/pack/", which may or may not be fine.)

 (2) The filename restriction in question is on the local filename, not
     the URL. We could do any manipulation we want on the URL (e.g. by
     appending ".pack.temp").

And one idea that came up at $DAYJOB is that if we're using a suffix of
the URL as the filename, there may be a clash of names anyway, so we
might as well use the hash instead (which is reported by the server).
I'll take a further look - maybe this patch will no longer be needed.

[1] https://cloud.google.com/storage/docs/access-control/signed-urls

  reply	other threads:[~2020-06-01 20:37 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-29 22:30 [PATCH 0/8] CDN offloading update Jonathan Tan
2020-05-29 22:30 ` [PATCH 1/8] http: use --stdin when getting dumb HTTP pack Jonathan Tan
2020-05-29 23:00   ` Junio C Hamano
2020-06-01 20:37     ` Jonathan Tan [this message]
2020-05-29 22:30 ` [PATCH 2/8] http: improve documentation of http_pack_request Jonathan Tan
2020-05-29 22:30 ` [PATCH 3/8] http-fetch: support fetching packfiles by URL Jonathan Tan
2020-05-29 23:25   ` Junio C Hamano
2020-06-01 20:54     ` Jonathan Tan
2020-05-29 22:30 ` [PATCH 4/8] Documentation: order protocol v2 sections Jonathan Tan
2020-05-29 23:32   ` Junio C Hamano
2020-06-01 20:57     ` Jonathan Tan
2020-05-29 22:30 ` [PATCH 5/8] Documentation: add Packfile URIs design doc Jonathan Tan
2020-05-30  0:15   ` Junio C Hamano
2020-05-30  0:22     ` Junio C Hamano
2020-06-01 23:10       ` Jonathan Tan
2020-06-01 23:07     ` Jonathan Tan
2020-06-10  1:14     ` Jonathan Tan
2020-06-10 17:16       ` Junio C Hamano
2020-06-10 18:04         ` Jonathan Tan
2020-05-29 22:30 ` [PATCH 6/8] upload-pack: refactor reading of pack-objects out Jonathan Tan
2020-05-29 22:30 ` [PATCH 7/8] fetch-pack: support more than one pack lockfile Jonathan Tan
2020-05-29 22:30 ` [PATCH 8/8] upload-pack: send part of packfile response as uri Jonathan Tan
2020-05-31 16:59   ` Junio C Hamano
2020-05-31 17:35   ` Junio C Hamano
2020-06-01 23:20     ` Jonathan Tan
2020-06-01 20:00   ` Jonathan Nieder
2020-06-10 20:57 ` [PATCH v2 0/9] CDN offloading update Jonathan Tan
2020-06-10 20:57   ` [PATCH v2 1/9] http: use --stdin when indexing dumb HTTP pack Jonathan Tan
2020-06-11  1:10     ` Junio C Hamano
2020-06-10 20:57   ` [PATCH v2 2/9] http: refactor finish_http_pack_request() Jonathan Tan
2020-06-10 20:57   ` [PATCH v2 3/9] http-fetch: refactor into function Jonathan Tan
2020-06-10 20:57   ` [PATCH v2 4/9] http-fetch: support fetching packfiles by URL Jonathan Tan
2020-06-11  1:30     ` Junio C Hamano
2020-06-10 20:57   ` [PATCH v2 5/9] Documentation: order protocol v2 sections Jonathan Tan
2020-06-10 20:57   ` [PATCH v2 6/9] Documentation: add Packfile URIs design doc Jonathan Tan
2020-06-11  1:55     ` Junio C Hamano
2020-11-25  9:15     ` Ævar Arnfjörð Bjarmason
2020-11-25 19:09       ` Jonathan Tan
2020-12-01 12:48         ` Ævar Arnfjörð Bjarmason
2020-06-10 20:57   ` [PATCH v2 7/9] upload-pack: refactor reading of pack-objects out Jonathan Tan
2020-06-10 20:57   ` [PATCH v2 8/9] fetch-pack: support more than one pack lockfile Jonathan Tan
2020-06-11  1:41     ` Junio C Hamano
2020-06-10 20:57   ` [PATCH v2 9/9] upload-pack: send part of packfile response as uri Jonathan Tan
2020-06-10 23:16   ` [PATCH v2 0/9] CDN offloading update Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200601203710.222245-1-jonathantanmy@google.com \
    --to=jonathantanmy@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).