Re: [PATCH v5 16/24] files-backend: replace submodule_allowed check in files_downcast()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 02/22/2017 03:04 PM, Nguyễn Thái Ngọc Duy wrote:
> 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 (*).

I'm surprised that you have implemented a "read" flag. Do you expect
ever to support ref-stores that are unreadable? In other words, couldn't
"read" just be assumed for any ref store?

> 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@xxxxxxxxx>
> ---
>  refs.c               | 15 ++++++---
>  refs/files-backend.c | 86 +++++++++++++++++++++++++++++++++-------------------
>  refs/refs-internal.h |  9 +++++-
>  3 files changed, 73 insertions(+), 37 deletions(-)
> 
> diff --git a/refs.c b/refs.c
> index 67acae60c..2dc97891a 100644
> --- a/refs.c
> +++ b/refs.c
> [...]
> @@ -1821,10 +1829,14 @@ static enum peel_status peel_entry(struct ref_entry *entry, int repeel)
>  static int files_peel_ref(struct ref_store *ref_store,
>  			  const char *refname, unsigned char *sha1)
>  {
> -	struct files_ref_store *refs = files_downcast(ref_store, 0, "peel_ref");
> +	struct files_ref_store *refs =
> +		files_downcast(ref_store, REF_STORE_READ | REF_STORE_ODB,
> +			       "peel_ref");
>  	int flag;
>  	unsigned char base[20];
>  
> +	files_assert_main_repository(refs, "peel_ref");

Instead of this call, couldn't you add `REF_STORE_MAIN` to the flags
passed to `files_downcase()`?

> +
>  	if (current_ref_iter && current_ref_iter->refname == refname) {
>  		struct object_id peeled;
>  
> [...]

Michael




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]