From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Mon, 20 Feb 2017 12:11:01 GMT
Subject: Re: [PATCH v4 14/15] files-backend: remove submodule_allowed from files_downcast()
Message-ID: <25fcb527-595a-7865-41e3-ee7c4c1ad668@alum.mit.edu>
In-Reply-To: <20170218133303.3682-15-pclouds@gmail.com>

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.

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.

Michael