| Message ID | 20230517192443.1149190-1-sandals@crustytoothpaste.net (mailing list archive) |
|---|---|
| Headers | show |
| Series | Fix empty SHA-256 clones with v0 and v1 | expand |
"brian m. carlson" <sandals@crustytoothpaste.net> writes: > We recently fixed empty clones with SHA-256 over protocol v2 by > honouring the hash algorithm specified even when no refs are present. > However, in doing so, we made it impossible to set up a v0 or v1 > repository by cloning from an empty SHA-256 repository. In doing so, we > also broke the Git LFS testsuite for SHA-256 repositories. > > This series introduces the dummy `capabilities^{}` entry for fetches and > clones from an empty repository for v0 and v1, just as we do for clones. > This is already supported by older versions of Git, as well as libgit2, > dulwich, and JGit. > > Changes since v2: > * Move advertisement of fake capabilities ref to a separate function to > avoid an extra strcmp. We want this in -rc2 if not -rc1 for the upcoming release, right? I've read the patch again and it all looked sensible. Thanks. > Changes since v1: > * Drop patch to honour GIT_DEFAULT_HASH > * Support all requests, not just HTTP. > * Add more tests. > * Fix NULL pointer dereference. > > brian m. carlson (1): > upload-pack: advertise capabilities when cloning empty repos > > t/t5551-http-fetch-smart.sh | 27 +++++++++++++++++++++++++++ > t/t5700-protocol-v1.sh | 31 +++++++++++++++++++++++++++++-- > upload-pack.c | 22 +++++++++++++++++----- > 3 files changed, 73 insertions(+), 7 deletions(-)
On 2023-05-17 at 21:48:39, Junio C Hamano wrote: > "brian m. carlson" <sandals@crustytoothpaste.net> writes: > > > We recently fixed empty clones with SHA-256 over protocol v2 by > > honouring the hash algorithm specified even when no refs are present. > > However, in doing so, we made it impossible to set up a v0 or v1 > > repository by cloning from an empty SHA-256 repository. In doing so, we > > also broke the Git LFS testsuite for SHA-256 repositories. > > > > This series introduces the dummy `capabilities^{}` entry for fetches and > > clones from an empty repository for v0 and v1, just as we do for clones. > > This is already supported by older versions of Git, as well as libgit2, > > dulwich, and JGit. > > > > Changes since v2: > > * Move advertisement of fake capabilities ref to a separate function to > > avoid an extra strcmp. > > We want this in -rc2 if not -rc1 for the upcoming release, right? > I've read the patch again and it all looked sensible. If it's possible, that would be great. I understand you just put out -rc0, and I apologize for the delay in getting back to you, but it would be ideal to avoid having the problem we're fixing in the release.
On Wed, May 17, 2023 at 07:24:42PM +0000, brian m. carlson wrote: > Changes since v2: > * Move advertisement of fake capabilities ref to a separate function to > avoid an extra strcmp. Thanks, the code change looks good to me. I'm still a little puzzled why we need http tests both in t5551 and t5700. Not too big a deal to have redundant tests, obviously, but I wonder if I am missing something. -Peff
On 2023-05-18 at 18:28:38, Jeff King wrote: > I'm still a little puzzled why we need http tests both in t5551 and > t5700. Not too big a deal to have redundant tests, obviously, but I > wonder if I am missing something. Technically, they work with v0 and v1, which are separate, and I wanted to test both, not just v0. I realize that practically, they are very similar, but I prefer to be a bit more thorough.
We recently fixed empty clones with SHA-256 over protocol v2 by honouring the hash algorithm specified even when no refs are present. However, in doing so, we made it impossible to set up a v0 or v1 repository by cloning from an empty SHA-256 repository. In doing so, we also broke the Git LFS testsuite for SHA-256 repositories. This series introduces the dummy `capabilities^{}` entry for fetches and clones from an empty repository for v0 and v1, just as we do for clones. This is already supported by older versions of Git, as well as libgit2, dulwich, and JGit. Changes since v2: * Move advertisement of fake capabilities ref to a separate function to avoid an extra strcmp. Changes since v1: * Drop patch to honour GIT_DEFAULT_HASH * Support all requests, not just HTTP. * Add more tests. * Fix NULL pointer dereference. brian m. carlson (1): upload-pack: advertise capabilities when cloning empty repos t/t5551-http-fetch-smart.sh | 27 +++++++++++++++++++++++++++ t/t5700-protocol-v1.sh | 31 +++++++++++++++++++++++++++++-- upload-pack.c | 22 +++++++++++++++++----- 3 files changed, 73 insertions(+), 7 deletions(-)