On Mon, Feb 20, 2017 at 6:34 PM, Michael Haggerty <mhagger@xxxxxxxxxxxx> wrote:
> On 02/18/2017 02:32 PM, Nguyễn Thái Ngọc Duy wrote:
>> Given $GIT_DIR and $GIT_COMMON_DIR, files-backend is now in charge of
>> deciding what goes where. The end goal is to pass $GIT_DIR only. A
>> refs "view" of a linked worktree is a logical ref store that combines
>> two files backends together.
>>
>> (*) Not entirely true since strbuf_git_path_submodule() still does path
>> translation underneath. But that's for another patch.
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
>> ---
>> refs/files-backend.c | 37 +++++++++++++++++++++++++++++++++----
>> 1 file changed, 33 insertions(+), 4 deletions(-)
>>
>> diff --git a/refs/files-backend.c b/refs/files-backend.c
>> index b599ddf92..dbcaf9bda 100644
>> --- a/refs/files-backend.c
>> +++ b/refs/files-backend.c
>> @@ -924,6 +924,9 @@ struct files_ref_store {
>> */
>> const char *submodule;
>>
>> + struct strbuf gitdir;
>> + struct strbuf gitcommondir;
>
> Is there a reason for these to be `strbuf`s rather than `const char *`?
> (One reason would be if you planned to use the `len` field, but I don't
> think you do so.)
Nope. I just didn't think about char *. It may have to lose "const"
though because in submodule case we may need a new allocation.
>
>> @@ -937,15 +940,33 @@ static void files_path(struct files_ref_store *refs, struct strbuf *sb,
>> {
>> struct strbuf tmp = STRBUF_INIT;
>> va_list vap;
>> + const char *ref;
>>
>> va_start(vap, fmt);
>> strbuf_vaddf(&tmp, fmt, vap);
>> va_end(vap);
>> - if (refs->submodule)
>> + if (refs->submodule) {
>> strbuf_git_path_submodule(sb, refs->submodule,
>> "%s", tmp.buf);
>> - else
>> - strbuf_git_path(sb, "%s", tmp.buf);
>> + } else if (!strcmp(tmp.buf, "packed-refs") ||
>> + !strcmp(tmp.buf, "logs")) { /* non refname path */
>> + strbuf_addf(sb, "%s/%s", refs->gitcommondir.buf, tmp.buf);
>> + } else if (skip_prefix(tmp.buf, "logs/", &ref)) { /* reflog */
>> + if (is_per_worktree_ref(ref))
>> + strbuf_addf(sb, "%s/%s", refs->gitdir.buf, tmp.buf);
>> + else
>> + strbuf_addf(sb, "%s/%s", refs->gitcommondir.buf, tmp.buf);
>
> This code would also be simpler if there were separate functions for
> packed-refs, loose references, and reflogs.
And maybe keep the path to packed-refs, the base path up to "logs" in
struct files_ref_store too (they will be calculated at ref store
init)? That way the files_packed_refs_path() does no calculation.
files_reflog_path() and files_ref_path() will just do string
concatenation, no fancy addf.
--
Duy