Re: [PATCH v7 00/28] Remove submodule from files-backend.c

[Junio C Hamano]

Michael Haggerty  writes:

> On 03/26/2017 04:42 AM, Nguyễn Thái Ngọc Duy wrote:
>> v7 is mostly about style changes except the one bug in
>> test-ref-store.c, missing setup_git_directory().
>> 
>> There's one new patch, 03/28, which maps to the "if (!refs)" deletion
>> in the interdiff.
>> 
>> The one comment from v6 I haven't addressed in v7 is whether to delete
>> REF_STORE_READ. But if it is deleted, I think I'm doing it in
>> nd/worktree-kill-parse-ref, which is partly about cleanup refs code
>> anyway, to avoid another re-roll in this series.
>
> I reviewed v7 pretty carefully, and send a couple of minor comments. But
> with or without changes, it looks good to me and the whole series is
>
> Reviewed-by: Michael Haggerty 
>
> Thanks!

Thanks.  The "What's cooking" report at the end of the last month
marked this topic to wait for a few days for comments before moving
to 'next'.  Now it has been more than a few days, so I'll move it to
the normal "will merge to 'next'" category.

Re: [PATCH v7 00/28] Remove submodule from files-backend.c

[Michael Haggerty]

On 03/26/2017 04:42 AM, Nguyễn Thái Ngọc Duy wrote:
> v7 is mostly about style changes except the one bug in
> test-ref-store.c, missing setup_git_directory().
> 
> There's one new patch, 03/28, which maps to the "if (!refs)" deletion
> in the interdiff.
> 
> The one comment from v6 I haven't addressed in v7 is whether to delete
> REF_STORE_READ. But if it is deleted, I think I'm doing it in
> nd/worktree-kill-parse-ref, which is partly about cleanup refs code
> anyway, to avoid another re-roll in this series.

I reviewed v7 pretty carefully, and send a couple of minor comments. But
with or without changes, it looks good to me and the whole series is

Reviewed-by: Michael Haggerty 

Thanks!

Michael

[PATCH v7 00/28] Remove submodule from files-backend.c

[Nguyễn Thái Ngọc Duy]

v7 is mostly about style changes except the one bug in
test-ref-store.c, missing setup_git_directory().

There's one new patch, 03/28, which maps to the "if (!refs)" deletion
in the interdiff.

The one comment from v6 I haven't addressed in v7 is whether to delete
REF_STORE_READ. But if it is deleted, I think I'm doing it in
nd/worktree-kill-parse-ref, which is partly about cleanup refs code
anyway, to avoid another re-roll in this series.

diff --git a/refs.c b/refs.c
index 77a39f8b17..ec1f563824 100644
--- a/refs.c
+++ b/refs.c
@@ -1523,18 +1523,15 @@ static struct ref_store *ref_store_init(const char 
*gitdir,
 
 struct ref_store *get_main_ref_store(void)
 {
-   struct ref_store *refs;
-
if (main_ref_store)
return main_ref_store;
 
-   refs = ref_store_init(get_git_dir(),
- (REF_STORE_READ |
-  REF_STORE_WRITE |
-  REF_STORE_ODB |
-  REF_STORE_MAIN));
-   main_ref_store = refs;
-   return refs;
+   main_ref_store = ref_store_init(get_git_dir(),
+   (REF_STORE_READ |
+REF_STORE_WRITE |
+REF_STORE_ODB |
+REF_STORE_MAIN));
+   return main_ref_store;
 }
 
 /*
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 4242486118..a5b405436f 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1195,9 +1195,9 @@ static void files_reflog_path(struct files_ref_store 
*refs,
}
 }
 
-static void files_refname_path(struct files_ref_store *refs,
-  struct strbuf *sb,
-  const char *refname)
+static void files_ref_path(struct files_ref_store *refs,
+  struct strbuf *sb,
+  const char *refname)
 {
switch (ref_type(refname)) {
case REF_TYPE_PER_WORKTREE:
@@ -1283,7 +1283,7 @@ static void read_loose_refs(const char *dirname, struct 
ref_dir *dir)
struct strbuf path = STRBUF_INIT;
size_t path_baselen;
 
-   files_refname_path(refs, , dirname);
+   files_ref_path(refs, , dirname);
path_baselen = path.len;
 
d = opendir(path.buf);
@@ -1420,7 +1420,7 @@ static int files_read_raw_ref(struct ref_store *ref_store,
*type = 0;
strbuf_reset(_path);
 
-   files_refname_path(refs, _path, refname);
+   files_ref_path(refs, _path, refname);
 
path = sb_path.buf;
 
@@ -1608,7 +1608,7 @@ static int lock_raw_ref(struct files_ref_store *refs,
*lock_p = lock = xcalloc(1, sizeof(*lock));
 
lock->ref_name = xstrdup(refname);
-   files_refname_path(refs, _file, refname);
+   files_ref_path(refs, _file, refname);
 
 retry:
switch (safe_create_leading_directories(ref_file.buf)) {
@@ -1949,8 +1949,6 @@ static struct ref_iterator *files_ref_iterator_begin(
refs = files_downcast(ref_store,
  REF_STORE_READ | (ref_paranoia ? 0 : 
REF_STORE_ODB),
  "ref_iterator_begin");
-   if (!refs)
-   return empty_ref_iterator_begin();
 
iter = xcalloc(1, sizeof(*iter));
ref_iterator = >base;
@@ -2086,7 +2084,7 @@ static struct ref_lock *lock_ref_sha1_basic(struct 
files_ref_store *refs,
if (flags & REF_DELETING)
resolve_flags |= RESOLVE_REF_ALLOW_BAD_NAME;
 
-   files_refname_path(refs, _file, refname);
+   files_ref_path(refs, _file, refname);
resolved = !!refs_resolve_ref_unsafe(>base,
 refname, resolve_flags,
 lock->old_oid.hash, type);
@@ -2387,7 +2385,7 @@ static void try_remove_empty_parents(struct 
files_ref_store *refs,
strbuf_setlen(, q - buf.buf);
 
strbuf_reset();
-   files_refname_path(refs, , buf.buf);
+   files_ref_path(refs, , buf.buf);
if ((flags & REMOVE_EMPTY_PARENTS_REF) && rmdir(sb.buf))
flags &= ~REMOVE_EMPTY_PARENTS_REF;
 
@@ -2709,7 +2707,7 @@ static int files_rename_ref(struct ref_store *ref_store,
struct strbuf path = STRBUF_INIT;
int result;
 
-   files_refname_path(refs, , newrefname);
+   files_ref_path(refs, , newrefname);
result = remove_empty_directories();
strbuf_release();
 
@@ -2935,10 +2933,10 @@ static int log_ref_write_fd(int fd, const unsigned char 
*old_sha1,
return 0;
 }
 
-int files_log_ref_write(struct files_ref_store *refs,
-   const char *refname, const unsigned char *old_sha1,
-   const unsigned char *new_sha1, const char *msg,
-   int flags,