Re: [PATCH v5 16/24] files-backend: replace submodule_allowed check in files_downcast()
From: Michael Haggerty <hidden>
Date: 2017-03-03 15:10:48
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?
quoted hunk ↗ jump to hunk
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 <redacted> --- 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