Re: [PATCH 2/3] Teach read_tree_recursive() how to traverse into submodules
- From
- Lars Hjemli <hjemli@gmail.com>
- Date
- Jan 18, 2009, 16:37 UTC
- Message-ID
- <8c5c35580901180837i6e835d98ob8875ce1b8ad3011@mail.gmail.com>
- In-Reply-To
- <49735530.4090901@lsrfire.ath.cx>
On Sun, Jan 18, 2009 at 17:13, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:
Show 13 quoted lines
> Lars Hjemli schrieb: >> The traversal of submodules is only triggered if the current submodule >> HEAD commit object is accessible. To this end, read_tree_recursive() >> will try to insert the submodule odb as an alternate odb but the lack >> of such an odb is not treated as an error since it is then assumed that >> the user is not interested in the submodule content. However, if the >> submodule odb is found it is treated as an error if the HEAD commit >> object is missing. > > Callers of read_tree_recursive() specify a tree to traverse. > Unconditionally using the HEAD of submodules feels a bit restrictive, > but I don't use submodules, so I have no idea what I'm actually talking > about here. :)
For bare repositories (where the submodule repo is added to objects/info/alternates), following the tree of the linked commit is the only option. And for non-bare repositories with the submodule checked out, I think we should honor the users choice of checked out HEAD in the submodule (especially since we don't have any other way to specify which submodule commit to follow).
Show 42 quoted lines
>
>> int read_tree_recursive(struct tree *tree,
>> const char *base, int baselen,
>> int stage, const char **match,
>> @@ -132,6 +188,30 @@ int read_tree_recursive(struct tree *tree,
>> return -1;
>> continue;
>> }
>> + if (S_ISGITLINK(entry.mode) && get_traverse_gitlinks()) {
>> + int retval;
>> + char *newbase;
>> + struct tree *subtree;
>> + unsigned int pathlen = tree_entry_len(entry.path, entry.sha1);
>> +
>> + newbase = xmalloc(baselen + 1 + pathlen);
>> + memcpy(newbase, base, baselen);
>> + memcpy(newbase + baselen, entry.path, pathlen);
>> + newbase[baselen + pathlen] = 0;
>> + if (!traverse_gitlink(newbase, entry.sha1, &subtree)) {
>> + free(newbase);
>> + continue;
>> + }
>> + newbase[baselen + pathlen] = '/';
>> + retval = read_tree_recursive(subtree,
>> + newbase,
>> + baselen + pathlen + 1,
>> + stage, match, fn, context);
>> + free(newbase);
>> + if (retval)
>> + return -1;
>> + continue;
>> + }
>> }
>> return 0;
>> }
>
> You don't need to call get_traverse_gitlinks() in the if statement above
> if you make all read_tree_recursive() callback functions return 0 for
> gitlinks that they don't want to follow and READ_TREE_RECURSIVE for
> those they do. It's cleaner without the static variable and its
> accessors and more flexible, too: the callbacks might decide to traverse
> only certain submodules.I like the idea, but it will require thorough review of all read_tree_recursive() consumers. So now we've got three different approaches: * me: global setting * dscho: parameter to read_tree_recursive() * you: accept the return value from the callback function
Junio, what would you prefer?
-- larsh