diff mbox series

[v3] builtin/clone: avoid failure with GIT_DEFAULT_HASH

Message ID 20200920223541.1299038-1-sandals@crustytoothpaste.net (mailing list archive)
State Accepted
Commit b28919c7bcfabad81896070e6b7b66be8310e6a8
Headers show
Series [v3] builtin/clone: avoid failure with GIT_DEFAULT_HASH | expand

Commit Message

brian m. carlson Sept. 20, 2020, 10:35 p.m. UTC
If a user is cloning a SHA-1 repository with GIT_DEFAULT_HASH set to
"sha256", then we can end up with a repository where the repository
format version is 0 but the extensions.objectformat key is set to
"sha256".  This is both wrong (the user has a SHA-1 repository) and
nonfunctional (because the extension cannot be used in a v0 repository).

This happens because in a clone, we initially set up the repository, and
then change its algorithm based on what the remote side tells us it's
using.  We've initially set up the repository as SHA-256 in this case,
and then later on reset the repository version without clearing the
extension.

We could just always set the extension in this case, but that would mean
that our SHA-1 repositories weren't compatible with older Git versions,
even though there's no reason why they shouldn't be.  And we also don't
want to initialize the repository as SHA-1 initially, since that means
if we're cloning an empty repository, we'll have failed to honor the
GIT_DEFAULT_HASH variable and will end up with a SHA-1 repository, not a
SHA-256 repository.

Neither of those are appealing, so let's tell the repository
initialization code if we're doing a reinit like this, and if so, to
clear the extension if we're using SHA-1.  This makes sure we produce a
valid and functional repository and doesn't break any of our other use
cases.

Reported-by: Matheus Tavares <matheus.bernardino@usp.br>
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
Diff-intervalle contre v2 :
1:  1becbbbb50 ! 1:  004a7b86f8 builtin/clone: avoid failure with GIT_DEFAULT_HASH
    @@ t/t5601-clone.sh: test_expect_success CASE_INSENSITIVE_FS 'colliding file detect
     +test_expect_success 'clone with GIT_DEFAULT_HASH' '
     +	(
     +		sane_unset GIT_DEFAULT_HASH &&
    -+		git init test
    ++		git init --object-format=sha1 test-sha1 &&
    ++		git init --object-format=sha256 test-sha256
     +	) &&
    -+	test_commit -C test foo &&
    -+	git clone test test-clone &&
    -+	git -C test-clone status
    ++	test_commit -C test-sha1 foo &&
    ++	test_commit -C test-sha256 foo &&
    ++	GIT_DEFAULT_HASH=sha1 git clone test-sha256 test-clone-sha256 &&
    ++	GIT_DEFAULT_HASH=sha256 git clone test-sha1 test-clone-sha1 &&
    ++	git -C test-clone-sha1 status &&
    ++	git -C test-clone-sha256 status
     +'
     +
      partial_clone_server () {

 builtin/clone.c   |  2 +-
 builtin/init-db.c |  6 ++++--
 cache.h           |  2 +-
 t/t5601-clone.sh  | 14 ++++++++++++++
 4 files changed, 20 insertions(+), 4 deletions(-)

Comments

Junio C Hamano Sept. 21, 2020, 4:27 a.m. UTC | #1
"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> +test_expect_success 'clone with GIT_DEFAULT_HASH' '
> +	(
> +		sane_unset GIT_DEFAULT_HASH &&
> +		git init --object-format=sha1 test-sha1 &&
> +		git init --object-format=sha256 test-sha256
> +	) &&
> +	test_commit -C test-sha1 foo &&
> +	test_commit -C test-sha256 foo &&

Unfortunately, the 'foo' commit is created in test-sha1, but the
next step to create 'foo' in test-sha256 fails with

        fatal: unknown repository extensions found:
                objectformat

> +	GIT_DEFAULT_HASH=sha1 git clone test-sha256 test-clone-sha256 &&
> +	GIT_DEFAULT_HASH=sha256 git clone test-sha1 test-clone-sha1 &&
> +	git -C test-clone-sha1 status &&
> +	git -C test-clone-sha256 status
> +'
> +
>  partial_clone_server () {
>  	       SERVER="$1" &&
>
brian m. carlson Sept. 22, 2020, 9:17 a.m. UTC | #2
On 2020-09-21 at 04:27:14, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> 
> > +test_expect_success 'clone with GIT_DEFAULT_HASH' '
> > +	(
> > +		sane_unset GIT_DEFAULT_HASH &&
> > +		git init --object-format=sha1 test-sha1 &&
> > +		git init --object-format=sha256 test-sha256
> > +	) &&
> > +	test_commit -C test-sha1 foo &&
> > +	test_commit -C test-sha256 foo &&
> 
> Unfortunately, the 'foo' commit is created in test-sha1, but the
> next step to create 'foo' in test-sha256 fails with
> 
>         fatal: unknown repository extensions found:
>                 objectformat

I'm not seeing that with this series based on master
(385c171a018f2747b329bcfa6be8eda1709e5abd).  I'm doing this:

  make -j6 all && (cd t && GIT_TEST_DEFAULT_HASH=sha256 ./t5601-*.sh --verbose)
  make -j6 all && (cd t && GIT_TEST_DEFAULT_HASH=sha1 ./t5601-*.sh --verbose)

And getting this output:

  Initialized empty Git repository in /home/bmc/checkouts/git/t/trash directory.t5601-clone/test-sha1/.git/
  Initialized empty Git repository in /home/bmc/checkouts/git/t/trash directory.t5601-clone/test-sha256/.git/
  [master (root-commit) 946e985] foo
   Author: A U Thor <author@example.com>
   1 file changed, 1 insertion(+)
   create mode 100644 foo.t
  [master (root-commit) ff872d8] foo
   Author: A U Thor <author@example.com>
   1 file changed, 1 insertion(+)
   create mode 100644 foo.t
  Cloning into 'test-clone-sha256'...
  done.
  Cloning into 'test-clone-sha1'...
  done.
  On branch master
  Your branch is up to date with 'origin/master'.
  
  nothing to commit, working tree clean
  On branch master
  Your branch is up to date with 'origin/master'.
  
  nothing to commit, working tree clean

Is there something I'm missing here?
Junio C Hamano Sept. 22, 2020, 4:27 p.m. UTC | #3
"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> On 2020-09-21 at 04:27:14, Junio C Hamano wrote:
>> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>> 
>> > +test_expect_success 'clone with GIT_DEFAULT_HASH' '
>> > +	(
>> > +		sane_unset GIT_DEFAULT_HASH &&
>> > +		git init --object-format=sha1 test-sha1 &&
>> > +		git init --object-format=sha256 test-sha256
>> > +	) &&
>> > +	test_commit -C test-sha1 foo &&
>> > +	test_commit -C test-sha256 foo &&
>> 
>> Unfortunately, the 'foo' commit is created in test-sha1, but the
>> next step to create 'foo' in test-sha256 fails with
>> 
>>         fatal: unknown repository extensions found:
>>                 objectformat
>
> I'm not seeing that with this series based on master
> (385c171a018f2747b329bcfa6be8eda1709e5abd).  I'm doing this:
>
>   make -j6 all && (cd t && GIT_TEST_DEFAULT_HASH=sha256 ./t5601-*.sh --verbose)
>   make -j6 all && (cd t && GIT_TEST_DEFAULT_HASH=sha1 ./t5601-*.sh --verbose)
>
> And getting this output:
>
>   Initialized empty Git repository in /home/bmc/checkouts/git/t/trash directory.t5601-clone/test-sha1/.git/
>   Initialized empty Git repository in /home/bmc/checkouts/git/t/trash directory.t5601-clone/test-sha256/.git/
>   [master (root-commit) 946e985] foo
>    Author: A U Thor <author@example.com>
>    1 file changed, 1 insertion(+)
>    create mode 100644 foo.t
>   [master (root-commit) ff872d8] foo
>    Author: A U Thor <author@example.com>
>    1 file changed, 1 insertion(+)
>    create mode 100644 foo.t
>   Cloning into 'test-clone-sha256'...
>   done.
>   Cloning into 'test-clone-sha1'...
>   done.
>   On branch master
>   Your branch is up to date with 'origin/master'.
>   
>   nothing to commit, working tree clean
>   On branch master
>   Your branch is up to date with 'origin/master'.
>   
>   nothing to commit, working tree clean
>
> Is there something I'm missing here?

I think the issue was because I queued the patch on the same base as
the previous round, which was v2.28.0.  Whe applied to 385c171a
(Fifteenth batch, 2020-09-18), I get the same as the above.

Sorry about the noise.  Will queue again.
diff mbox series

Patch

diff --git a/builtin/clone.c b/builtin/clone.c
index b087ee40c2..925a2e3dd6 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1235,7 +1235,7 @@  int cmd_clone(int argc, const char **argv, const char *prefix)
 		 * Now that we know what algorithm the remote side is using,
 		 * let's set ours to the same thing.
 		 */
-		initialize_repository_version(hash_algo);
+		initialize_repository_version(hash_algo, 1);
 		repo_set_hash_algo(the_repository, hash_algo);
 
 		mapped_refs = wanted_peer_refs(refs, &remote->fetch);
diff --git a/builtin/init-db.c b/builtin/init-db.c
index cd3e760541..01bc648d41 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -179,7 +179,7 @@  static int needs_work_tree_config(const char *git_dir, const char *work_tree)
 	return 1;
 }
 
-void initialize_repository_version(int hash_algo)
+void initialize_repository_version(int hash_algo, int reinit)
 {
 	char repo_version_string[10];
 	int repo_version = GIT_REPO_VERSION;
@@ -195,6 +195,8 @@  void initialize_repository_version(int hash_algo)
 	if (hash_algo != GIT_HASH_SHA1)
 		git_config_set("extensions.objectformat",
 			       hash_algos[hash_algo].name);
+	else if (reinit)
+		git_config_set_gently("extensions.objectformat", NULL);
 }
 
 static int create_default_files(const char *template_path,
@@ -277,7 +279,7 @@  static int create_default_files(const char *template_path,
 		free(ref);
 	}
 
-	initialize_repository_version(fmt->hash_algo);
+	initialize_repository_version(fmt->hash_algo, 0);
 
 	/* Check filemode trustability */
 	path = git_path_buf(&buf, "config");
diff --git a/cache.h b/cache.h
index cee8aa5dc3..c0072d43b1 100644
--- a/cache.h
+++ b/cache.h
@@ -629,7 +629,7 @@  int path_inside_repo(const char *prefix, const char *path);
 int init_db(const char *git_dir, const char *real_git_dir,
 	    const char *template_dir, int hash_algo,
 	    const char *initial_branch, unsigned int flags);
-void initialize_repository_version(int hash_algo);
+void initialize_repository_version(int hash_algo, int reinit);
 
 void sanitize_stdfds(void);
 int daemonize(void);
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 15fb64c18d..b6c8312da1 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -631,6 +631,20 @@  test_expect_success CASE_INSENSITIVE_FS 'colliding file detection' '
 	test_i18ngrep "the following paths have collided" icasefs/warning
 '
 
+test_expect_success 'clone with GIT_DEFAULT_HASH' '
+	(
+		sane_unset GIT_DEFAULT_HASH &&
+		git init --object-format=sha1 test-sha1 &&
+		git init --object-format=sha256 test-sha256
+	) &&
+	test_commit -C test-sha1 foo &&
+	test_commit -C test-sha256 foo &&
+	GIT_DEFAULT_HASH=sha1 git clone test-sha256 test-clone-sha256 &&
+	GIT_DEFAULT_HASH=sha256 git clone test-sha1 test-clone-sha1 &&
+	git -C test-clone-sha1 status &&
+	git -C test-clone-sha256 status
+'
+
 partial_clone_server () {
 	       SERVER="$1" &&