"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > This adds the reftable library, and hooks it up as a ref backend. I do not see the promised .gitattributes addition; please add one early in the series, before the step in which you start adding files that want different whitespace rules, so that I do not have to apply with "git am --whitespace=nowarn". It is rather unfortunate that you included Jonathan's reftable doc patch in the series---it has separately been queued and merged already in 'next'. I'll fork hn/reftable on top of the reftable doc topic of Jonathan's, while dropping the extra copy [4/6] in this series, to cope with this (an alternative would be to revert the one in 'next' and take this series as-is). > * spots marked with XXX in the Git source code. What follows is my notes while "git diff master...hn/reftable" and looking for those lines, except for the ones in the reftable implementation proper (for which you would be a better judge). The init-db call in clone hardcodes DEFAULT_REF_STORAGE and that should be OK at least for now. Eventually the code would want to read from the system-wide and the per-user configuration files to figure it out, but I do not think the config subsystem is prepared to read the configuration before making this call to init_db() right now---let's leave such a complication until after the basics is done. After all, "git init && git add remote && git pull" can be used as a rough approximate for "git clone"; as long as "git init" can be told what backend to use, that would be sufficient to get us going. It sounds a bit too magical to convert the repository by running "git init" again on an already existing repository. But a user who runs "git init" in an existing repository and asks to use settings that are different from what the repository uses from the command line cannot be expecting the command to error out when the settings do not match (i.e. as a mechanism to check the setting)---the only sensible way to interpret such an invocation is as a request to convert the existing repository. Again, I do not think it is a huge loss to do without such an automatic conversion in the initial deployment. In refs.c, I am puzzled by the fact that a call to ref_store_init() in the get_submodule_ref_store() needs to pass DEFAULT_REF_STORAGE, or any format for that matter, at all. Wouldn't the gitdir argument sufficient for ref_store_init() to learn what backend the repository uses? IOW, I am not sure why the caller needs to pass the backend format. The way r->ref_storage_format is used by get_main_ref_store(), at least when the field is set, looks a lot more sensible. Would it be possible that get_submodule_ref_store() function would also want to have, not just a submodule as a mere "string", but an instance of a repository for that submodule? Speaking of the use of ref_store_init() and r->ref_storage_format in get_main_ref_store(), when is the storage_format field unspecified? Is there a chicken-and-egg problem around here? The same comments apply to the use of the default format in get_worktree_ref_store(), but do we foresee to "add" a worktree that uses one ref backend off of a repository that uses a different ref backend? Even if we do, it probably makes sense to default to the same format as the primary worktree uses, I would think. It is still unclear to me why ref_store_init() wants the format in the first place, though, given that it takes a path to the .git/ directory (or its equivalent), and ref_store_init() should be able to figuire it out without being told. I think it is fine to define the textual name of the default ref storage in refs.h Thanks.
