Re: [PATCH v2] all: new command used for multi-repo operations
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 23, 2013, 16:52 UTC
- Message-ID
- <7v622nj0ys.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <1358928767-16283-1-git-send-email-hjemli@gmail.com>
Lars Hjemli <hjemli@gmail.com> writes:
Show 33 quoted lines
> +static int walk(struct strbuf *path, int argc, const char **argv)
> +{
> + DIR *dir;
> + struct dirent *ent;
> + struct stat st;
> + size_t len;
> +
> + dir = opendir(path->buf);
> + if (!dir)
> + return errno;
> + strbuf_addstr(path, "/");
> + len = path->len;
> + while ((ent = readdir(dir))) {
> + if (!strcmp(ent->d_name, ".") || !strcmp(ent->d_name, ".."))
> + continue;
> + if (!strcmp(ent->d_name, ".git")) {
> + strbuf_addstr(path, ent->d_name);
> + setenv(GIT_DIR_ENVIRONMENT, path->buf, 1);
> + strbuf_setlen(path, len - 1);
> + setenv(GIT_WORK_TREE_ENVIRONMENT, path->buf, 1);
> + handle_repo(path->buf, argv);
> + strbuf_addstr(path, "/");
> + continue;
> + }
> + strbuf_setlen(path, len);
> + strbuf_addstr(path, ent->d_name);
> + switch (DTYPE(ent)) {
> + case DT_UNKNOWN:
> + /* Use stat() instead of lstat(), since we want to
> + * know if we can follow this path into another
> + * directory - it's not important if it's actually
> + * a symlink which gets us there.
> + */This is wrong if you are on a platform that does have d_type, no? It may say it is a symbolic link, and until you stat you wouldn't know if it may lead to a directory. You can add "case DT_LNK:" that behaves the same as DT_UNKNOWN, I think.
Show 9 quoted lines
> + if (stat(path->buf, &st) || !S_ISDIR(st.st_mode)) > + break; > + /* fallthrough */ > + case DT_DIR: > + walk(path, argc, argv); > + break; > + } > + strbuf_setlen(path, len); > + }
But I still do not think this loop is correct. In a repository that has a working tree, you would learn that directory $D has $D/.git in it, feed $D to handle_repo(), and then descend into $D/.git/objects/, $D/.git/refs, and other random directories to see if you can find other repositories. That is just not right.
If this check were doing something like "The directory $D is worth handing to handle_repo() if it has all of the following: objects/, refs/ and HEAD that either points inside refs/ or 40-hex.", then it would make a lot more sense to me, including the part that goes on to check sibling directories. As a bonus side effect, it will give you a support for bare repositories for free.