Re: [PATCH 3/7] reftable/system: stop depending on "hash.h"
- From
Justin Tobler <jltobler@gmail.com>
- Date
- Nov 8, 2024, 01:10 UTC
- Message-ID
- <hhlsmyyqt2y6ue3abnlwjmrmsslewwjcoiu44aadp6nqkscict@m55ip4ibqgoq>
- In-Reply-To
- <b595668a5cdae0dff4a7271a3547c5821aa6e912.1729677003.git.ps@pks.im>
On 24/10/23 11:56AM, Patrick Steinhardt wrote:
Show 27 quoted lines
> diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
> index 3c6107c7ce5..7d86d920970 100644
> --- a/refs/reftable-backend.c
> +++ b/refs/reftable-backend.c
> @@ -15,6 +15,7 @@
> #include "../object.h"
> #include "../path.h"
> #include "../refs.h"
> +#include "../reftable/reftable-basics.h"
> #include "../reftable/reftable-stack.h"
> #include "../reftable/reftable-record.h"
> #include "../reftable/reftable-error.h"
> @@ -289,7 +290,16 @@ static struct ref_store *reftable_be_init(struct repository *repo,
> refs->store_flags = store_flags;
> refs->log_all_ref_updates = repo_settings_get_log_all_ref_updates(repo);
>
> - refs->write_options.hash_id = repo->hash_algo->format_id;
> + switch (repo->hash_algo->format_id) {
> + case GIT_SHA1_FORMAT_ID:
> + refs->write_options.hash_id = REFTABLE_HASH_SHA1;
> + break;
> + case GIT_SHA256_FORMAT_ID:
> + refs->write_options.hash_id = REFTABLE_HASH_SHA256;
> + break;
> + default:
> + BUG("unknown hash algorithm %d", repo->hash_algo->format_id);
> + }Here we define the mapping between the Git's format ID and the reftable hash external to the reftable library. This facilitates swapping uses of `GIT_*_FORMAT_ID` to `REFTABLE_HASH_*` as done in the rest of the patch which looks good.
> refs->write_options.default_permissions = calc_shared_perm(0666 & ~mask);
> refs->write_options.disable_auto_compact =
> !git_env_bool("GIT_TEST_REFTABLE_AUTOCOMPACTION", 1);[snip]
Show 9 quoted lines
> diff --git a/reftable/merged.h b/reftable/merged.h > index 89bd0c4b35b..13a5fe4154e 100644 > --- a/reftable/merged.h > +++ b/reftable/merged.h > @@ -10,11 +10,12 @@ license that can be found in the LICENSE file or at > #define MERGED_H > > #include "system.h" > +#include "basics.h"
Naive question, being that "merged.h" only depends on `reftable-basics.h:reftable_hash` and not any of the internal reftable basics components, would it be best to instead reference it directly? Or because "merged.h" is also internal do we also prefer to use the internal "basics.h"?
Probably doesn't really matter, but I was just curious if there was any reasoning. :)
-Justin