On Sun, Jan 25, 2009 at 12:43, Johannes Schindelin
<Johannes.Schindelin@xxxxxx> wrote:
> On Sun, 25 Jan 2009, Lars Hjemli wrote:
>
>> 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/
Thanks
>
>> 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.
Ok, will fix.
>
>> 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).
Yes, sorry for not mentioning that.
>
>> @@ -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;/
I skipped the STRBUF_INIT since I used strbuf_init() below, but...
>
>> + 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);
...with this cute fix the STRBUF_INIT is required. Will fix.
Thanks for the review.
--
larsh
--
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
