On Thu, Feb 9, 2017 at 1:07 PM, Michael Haggerty <mhagger@xxxxxxxxxxxx> wrote: > It is unquestionably a good goal to avoid parsing references outside of > `refs/files-backend.c`. But I'm not a fan of this approach. Yes. But in this context it was more of a guinea pig. I wanted something simple enough to code up show we can see what the approach looked like. Good thing I did it. > > There are two meanings of the concept of a "ref store", and I think this > change muddles them: > > 1. The references that happen to be *physically* stored in a particular > location, for example the `refs/bisect/*` references in a worktree. > > 2. The references that *logically* should be considered part of a > particular repository. This might require stitching together > references from multiple sources, for example `HEAD` and > `refs/bisect` from a worktree's own directory with other > references from the main repository. > > Either of these concepts can be implemented via the `ref_store` abstraction. > > The `ref_store` for a submodule should represent the references > logically visible from the submodule. The main program shouldn't care > whether the references are stored in a single physical location or > spread across multiple locations (for example, if the submodule were > itself a linked worktree). > > The `ref_store` that you want here for a worktree is not the worktree's > *logical* `ref_store`. You want the worktree's *physical* `ref_store`. Yep. > Mixing logical and physical reference stores together is a bad idea > (even if we were willing to ignore the fact that worktrees are not > submodules in the accepted sense of the word). > > ... > > I think the best solution would be to expose the concept of `ref_store` > in the public refs API. Then users of submodules would essentially do > > struct ref_store *refs = get_submodule_refs(submodule_path); > ... resolve_ref_recursively(refs, refname, 0, sha1, &flags) ... > ... for_each_ref(refs, fn, cb_data) ... > > whereas for a worktree you'd have to look up the `ref_store` instance > somewhere else (or maybe keep it as part of some worktree structure, if > there is one) but you would use it via the same API. Oh I was going to reply to Stefan about his comment to my (**) footnote. Something along the this line "Ideally we would introduce a new set of api, maybe with refs_ prefix, that takes a refs_store. Then submodule people can get a ref store somewhere and pass to it. Worktree people get maybe some other refs store for it. The "old" api like for_each_ref() is a thin wrapper around it, just like read_cache() vs read_index(&the_index). If the *_submodule does not see much use, we might as well kill it and use the generic refs_*". If I didn't misunderstood anything else, then I think we're on the same page. Now I need to see if I can get there in a reasonable time frame (so I can fix my "gc in worktree" problem properly) or I would need something temporary but not so hacky. I'll try to make this new api and see how it works out. If you think I should not do it right away, for whatever reason, stop me now. -- Duy