Re: [PATCH v7 19/28] files-backend: replace submodule_allowed check in files_downcast()

> files-backend.c is unlearning submodules. Instead of having a specific
> check for submodules to see what operation is allowed, files backend
> now takes a set of flags at init. Each operation will check if the
> required flags is present before performing.
> 
> For now we have four flags: read, write and odb access. Main ref store
> has all flags, obviously, while submodule stores are read-only and have
> access to odb (*).
> 
> The "main" flag stays because many functions in the backend calls
> frontend ones without a ref store, so these functions always target the
> main ref store. Ideally the flag should be gone after ref-store-aware
> api is in place and used by backends.
> 
> (*) Submodule code needs for_each_ref. Try take REF_STORE_ODB flag
> out. At least t3404 would fail. The "have access to odb" in submodule is
> a bit hacky since we don't know from he whether add_submodule_odb() has
> been called.
> 
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  refs.c               | 15 +++++++---
>  refs/files-backend.c | 81 +++++++++++++++++++++++++++++++++-------------------
>  refs/refs-internal.h |  9 +++++-
>  3 files changed, 71 insertions(+), 34 deletions(-)
> 
> diff --git a/refs.c b/refs.c
> index d72b48a430..241b4227b2 100644
> --- a/refs.c
> +++ b/refs.c
> [...]
> @@ -1481,7 +1486,9 @@ struct ref_store *get_ref_store(const char *submodule)
>  		return NULL;
>  	}
>  
> -	refs = ref_store_init(submodule_sb.buf);
> +	/* pretend that add_submodule_odb() has been called */

> +	refs = ref_store_init(submodule_sb.buf,
> +			      REF_STORE_READ | REF_STORE_ODB);
>  	register_submodule_ref_store(refs, submodule);
>  
>  	strbuf_release(&submodule_sb);
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 490f05a6f4..d97a924860 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> [...]
> @@ -994,13 +997,17 @@ static struct ref_store *files_ref_store_create(const char *gitdir)
>  }
>  
>  /*
> - * Die if refs is for a submodule (i.e., not for the main repository).
> - * caller is used in any necessary error messages.
> + * Die if refs is not the main ref store. caller is used in any
> + * necessary error messages.
>   */
>  static void files_assert_main_repository(struct files_ref_store *refs,
>  					 const char *caller)
>  {
> -	/* This function is to be fixed up in the next patch */
> +	if (refs->store_flags & REF_STORE_MAIN)
> +		return;
> +
> +	die("BUG: unallowed operation (%s), only works "
> +	    "on main ref store\n", caller);

>  }
>  
>  /*
> @@ -1009,9 +1016,9 @@ static void files_assert_main_repository(struct files_ref_store *refs,
>   * files_ref_store is for a submodule (i.e., not for the main
>   * repository). caller is used in any necessary error messages.
>   */
> -static struct files_ref_store *files_downcast(
> -		struct ref_store *ref_store, int submodule_allowed,
> -		const char *caller)
> +static struct files_ref_store *files_downcast(struct ref_store *ref_store,
> +					      unsigned int required_flags,
> +					      const char *caller)

>  {
>  	struct files_ref_store *refs;
>  
> @@ -1021,8 +1028,9 @@ static struct files_ref_store *files_downcast(
>  
>  	refs = (struct files_ref_store *)ref_store;
>  
> -	if (!submodule_allowed)
> -		files_assert_main_repository(refs, caller);
> +	if ((refs->store_flags & required_flags) != required_flags)
> +		die("BUG: unallowed operation (%s), requires %x, has %x\n",
> +		    caller, required_flags, refs->store_flags);