Hi,
On Sun, 25 Jan 2009, Lars Hjemli wrote:
> When the callback function invoked from read_tree_recursive() returns
> the value `READ_TREE_RECURSIVE` for a gitlink entry, the traversal will
> now continue into the tree connected to the gitlinked commit.
\n
> This functionality can be used to allow inter-repository operations, but
> since the current users of read_tree_recursive() does not yet support
> such operations, they have been modified where necessary to make sure
> that they never return READ_TREE_RECURSIVE for gitlink entries (hence no
> change in behaviour should be introduces by this patch alone).
s/\(introduce\)s/\1d/
> diff --git a/archive.c b/archive.c
> index 9ac455d..e6de039 100644
> --- a/archive.c
> +++ b/archive.c
> @@ -132,7 +132,7 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,
> err = write_entry(args, sha1, path.buf, path.len, mode, NULL, 0);
> if (err)
> return err;
> - return READ_TREE_RECURSIVE;
> + return (S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0);
You do not need the parentheses around the conditional:
$ git grep 'return (.*?' *.c | wc -l
14
gene099@racer:~/git (rebase-i-p)$ git grep 'return [^(]*?' *.c | wc -l
41
Note that the 14 matches include 9 false positives.
> diff --git a/builtin-ls-tree.c b/builtin-ls-tree.c
> index 5b63e6e..fca4631 100644
> --- a/builtin-ls-tree.c
> +++ b/builtin-ls-tree.c
> @@ -68,13 +68,8 @@ static int show_tree(const unsigned char *sha1, const char *base, int baselen,
> *
> * Something similar to this incomplete example:
> *
> - if (show_subprojects(base, baselen, pathname)) {
> - struct child_process ls_tree;
> -
> - ls_tree.dir = base;
> - ls_tree.argv = ls-tree;
I wondered how that could ever have compiled...
Until I inspected the file (which is different in junio/next from what you
based your patch on; your patch is vs junio/master).
> @@ -131,6 +131,34 @@ int read_tree_recursive(struct tree *tree,
> if (retval)
> return -1;
> continue;
> + } else if (S_ISGITLINK(entry.mode)) {
> + int retval;
> + struct strbuf path;
s/;/ = STRBUF_INIT;/
> + unsigned int entrylen;
> + struct commit *commit;
> +
> + entrylen = tree_entry_len(entry.path, entry.sha1);
> + strbuf_init(&path, baselen + entrylen + 1);
> + strbuf_add(&path, base, baselen);
> + strbuf_add(&path, entry.path, entrylen);
> + strbuf_addch(&path, '/');
Why not
strbuf_addf(&path, "%.*s%.*s/", baselen, base,
entrylen, entry.path);
> +
> + commit = lookup_commit(entry.sha1);
> + if (!commit)
> + die("Commit %s in submodule path %s not found",
> + sha1_to_hex(entry.sha1), path.buf);
> +
> + if (parse_commit(commit))
> + die("Invalid commit %s in submodule path %s",
> + sha1_to_hex(entry.sha1), path.buf);
> +
> + retval = read_tree_recursive(commit->tree,
> + path.buf, path.len,
> + stage, match, fn, context);
> + strbuf_release(&path);
> + if (retval)
> + return -1;
> + continue;
I'd also place a comment above read_tree_recursive() stating that this
function tries to traverse into submodules when READ_TREE_RECURSIVE is
returned for submodule entries, but no attempt is made at including
alternate object directories. (And it must be that way: think bare
repositories -- they cannot just try to include a subdirectory's
.git/objects/..)
Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
