Re: [PATCH 09/11] refs: move submodule code out of files-backend.c

On Tue, Feb 14, 2017 at 6:35 AM, Stefan Beller [off-list ref] wrote:

quoted

+       git_dir = read_gitfile(buf.buf);

if buf.buf is a (git) directory as opposed to a git file,
we error out in read_gitfile. Did you mean to use
read_gitfile_gently here or rather even resolve_gitdir_gently ?

This is what strbuf_git_path_submodule() does. I don't know the
backstory so I'm going to keep it as it is to keep the behavior
exactly (or very close) as before. We can replace it with a better
version (with explanation and all).

quoted

+       if (git_dir) {

when not using the _gently version git_dir is always
non NULL here (or we're dead)?

quoted

+               strbuf_reset(&buf);
+               strbuf_addstr(&buf, git_dir);
+       }
+       if (!is_git_directory(buf.buf)) {
+               gitmodules_config();
+               sub = submodule_from_path(null_sha1, path);
+               if (!sub)
+                       goto done;
+               strbuf_reset(&buf);
+               strbuf_git_path(&buf, "%s/%s", "modules", sub->name);

You can inline "modules" into the format string?

Hm.. because this is strbuf_git_path_submodule() code. Perhaps it's
better to split it to a separate function and call it from here? Then
you can make more improvements on top that benefit everybody.

quoted

        } else {
                strbuf_addstr(&refs->gitdir, get_git_dir());
                strbuf_addstr(&refs->gitcommondir, get_git_common_dir());

@@ -1034,8 +1025,6 @@ static struct ref_store *files_ref_store_create(const char *submodule)
 static void files_assert_main_repository(struct files_ref_store *refs,
                                         const char *caller)
 {
-       if (refs->submodule)
-               die("BUG: %s called for a submodule", caller);
 }

In a followup we'd get rid of files_assert_main_repository
presumably?

Yes. Can't delete it now because I would need to touch all callers.
-- 
Duy