Re: [RFC/PATCH 0/7] Rework git core for native submodules

Junio C Hamano wrote:

"Burden" refers to the extra work of looking for a file in the worktree, when this is completely unnecessary if you use a link object.

git's design follows along the lines of the UNIX philosophy: do one thing, do it well. git add/ rm/ mv have a very sharply defined task: they first lock the index file, read the_index using read_cache(), build a cache_entry struct using user-supplied data (this might involve worktree code from dir.c to recurse subdirectories, for example), write that cache_entry to the_index (removing existing entries with cache_tree_invalidate_path() if necessary), and finally write the_index to the index file, releasing the lock. Would you agree that any operation that doesn't follow along this line is unnatural? What then, does writing a special file in the worktree (aka .gitmodules) have to do with this entire process?

Does git diff/ commit/ add/ rm or any other command you can think of rely on a special file in the worktree (aka .gitmodules) to be checked out? Then why does git submodule require it? Isn't this a requirement that is inconsistent with the rest of git-core?

I should've said "means an easy route to get the existing UI to work with little or no additional code". Making the submodule information available to git-core is precisely what leads to this. In index_path(), you can inject a case for S_IFDIR to write a link object to the database, writing the sha1 to the supplied argument. This is not unnatural in any way, because we're just following along the lines of the S_IFLNK codepath, which writes a blob object to the database. Now index_path() is called by add_to_index(), which is the master function for adding anything to the index. Therefore, git add just works. git rm is much simpler: it calls remove_file_from_index() which in turn calls cache_tree_invalidate() and remove_index_entry_at(). Once the entry is removed from the cache, our job is done. The link object will be cleaned up at gc-time. git mv is just a combination of git rm and git add: it invalidates an existing entry and adds a new one with a different name.

There is no special .gitmodules to take care of.

Yes, I am aware. I'm piggy-banking on the mature parts of git-core to get functionality that I would otherwise have to write by hand. The current implementation needs to hand-code this, and hasn't done it yet (presumably because it's non-trivial).

This goes both ways: the information is both easier to read and write.
 I can easily create a link object from anywhere: index_path() or
cmd_edit_link().  To do this, I just have to call write_sha1_file()
with the buffer filled out and with the parameter link_type (which is
already defined).  To access the data in a link, I have to fill out a
tree_desc with "HEAD", an unpack_tree_opts with a custom callback, and
pass it to unpack_trees().  An example of a custom callback:
cmd_cat_link() which simply calls get_stat_data() to fill in the
SHA-1, and read_sha1_file() to read that object into a buffer.

I don't have to rely on a worktree with toplevel .gitmodules checked out. The information is easily readable/ writeable by default when I'm working with git, irrespective of my the state of my worktree.

Why must we use the git_config() API when it was never designed to do this? Why not leverage the mature part of git that was intended for this, where our new object fits in snugly?

Can you now present a counter-argument about why .gitmodules is _better_ suited for the task of managing submodules? (Hint: No, you can't; because it isn't). Can you give me an example of one thing that will become more complex if we were to follow my approach (I've already acknowledged foreach, so that's out)? Do you have any concrete objections to the new design, apart from the fact that it breaks what already "works"? Do you acknowledge that the new design will remove a lot of existing complexity (if "complex" is too subjective for you: result in a significant negative overall diffstat)?