On Mon, Feb 20, 2017 at 7:11 PM, Michael Haggerty <mhagger@xxxxxxxxxxxx> wrote: > On 02/18/2017 02:33 PM, Nguyễn Thái Ngọc Duy wrote: >> Since submodule or not is irrelevant to files-backend after the last >> patch, remove the parameter from files_downcast() and kill >> files_assert_main_repository(). >> >> PS. This implies that all ref operations are allowed for submodules. But >> we may need to look more closely to see if that's really true... > > I think you are jumping the gun here. > > Even after your changes, there is still a significant difference between > the main repository and submodules: we have access to the object > database for the main repository, but not for submodules. > > So, for example, the following don't work for submodules: > > * `parse_object()` is used to ensure that references are only pointed at > valid objects, and that branches are only pointed at commit objects. > > * `peel_object()` is used to write the peeled version of references in > `packed-refs`, and to peel references while they are being iterated > over. Since this doesn't work, I think you can't write `packed-refs` in > a submodule. > > These limitations, I think, imply that submodule references have to be > treated as read-only. Behind the scene submodule does add_submodule_odb(), which basically makes the submodule's odb an alternate of in-core odb. So odb access works. I was puzzled how submodules could by pass odb access at the beginning only to find that out. technical/api-ref-iteration.txt also mentions that you need to add_submodule_odb(), so I think it's deliberate (albeit hacky) design. > When I was working on a patch series similar to yours, I introduced a > boolean "main_repository" flag in `struct ref_store`, and use that > member to implement `files_assert_main_repository()`. That way > `files_ref_store::submodule` can still be removed, which is the more > important goal from a design standpoint. I could keep the submodule check back (and replace the submodule string in files_ref_store with just a flag). But I really think all backend functions work with submodule. Perhaps add some tests to exercise/verify that files-backend-on-submodule works? -- Duy