diff mbox series

[19/44] builtin/clone: initialize hash algorithm properly

Message ID 20200513005424.81369-20-sandals@crustytoothpaste.net (mailing list archive)
State New, archived
Headers show
Series SHA-256 part 2/3: protocol functionality | expand

Commit Message

brian m. carlson May 13, 2020, 12:53 a.m. UTC
When performing a clone, we don't know what hash algorithm the other end
will support.  Currently, we don't support fetching data belonging to a
different algorithm, so we must know what algorithm the remote side is
using in order to properly initialize the repository.  We can know that
only after fetching the refs, so if the remote side has any references,
use that information to reinitialize the repository with the correct
hash algorithm information.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/clone.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Martin Ă…gren May 16, 2020, 10:48 a.m. UTC | #1
On Wed, 13 May 2020 at 02:57, brian m. carlson
<sandals@crustytoothpaste.net> wrote:
>
> When performing a clone, we don't know what hash algorithm the other end
> will support.  Currently, we don't support fetching data belonging to a
> different algorithm, so we must know what algorithm the remote side is
> using in order to properly initialize the repository.  We can know that
> only after fetching the refs, so if the remote side has any references,
> use that information to reinitialize the repository with the correct
> hash algorithm information.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  builtin/clone.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/builtin/clone.c b/builtin/clone.c
> index cb48a291ca..f27d38bc8e 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -1217,6 +1217,15 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>         refs = transport_get_remote_refs(transport, &ref_prefixes);
>
>         if (refs) {
> +               int hash_algo = hash_algo_by_ptr(transport_get_hash_algo(transport));
> +
> +               /*
> +                * Now that we know what algorithm the remote side is using,
> +                * let's set ours to the same thing.
> +                */
> +               initialize_repository_version(hash_algo);

This made me go "huh". It's not really new in this series, it's just
that from `initialize_repository_version(int)` I would have expected the
argument to be the, well, repository version, not a hash algo
identifier. But it all makes sense once you realize that the function is
"please initialize the repository version based on this stuff that I
give you" where, currently, the only input is a hash algo. (I see that
Han-Wen's reftable series adds another parameter here.)

> +               repo_set_hash_algo(the_repository, hash_algo);

I first wondered whether all calls to `repo_set_hash_algo()` would want
to be preceded by `initialize_repository_version()`, which might call
for the latter being called by the former. But I guess not. Various
users of `repo_set_hash_algo()` -- not that there would be a lot of them
-- might want to do similar updating and/or sanity checks, but the exact
details would differ.


Martin
diff mbox series

Patch

diff --git a/builtin/clone.c b/builtin/clone.c
index cb48a291ca..f27d38bc8e 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1217,6 +1217,15 @@  int cmd_clone(int argc, const char **argv, const char *prefix)
 	refs = transport_get_remote_refs(transport, &ref_prefixes);
 
 	if (refs) {
+		int hash_algo = hash_algo_by_ptr(transport_get_hash_algo(transport));
+
+		/*
+		 * Now that we know what algorithm the remote side is using,
+		 * let's set ours to the same thing.
+		 */
+		initialize_repository_version(hash_algo);
+		repo_set_hash_algo(the_repository, hash_algo);
+
 		mapped_refs = wanted_peer_refs(refs, &remote->fetch);
 		/*
 		 * transport_get_remote_refs() may return refs with null sha-1