Hi,
On Sun, 18 Jan 2009, Lars Hjemli wrote:
> diff --git a/environment.c b/environment.c
> index e278bce..35cc557 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -53,6 +53,8 @@ static char *work_tree;
> static const char *git_dir;
> static char *git_object_dir, *git_index_file, *git_refs_dir, *git_graft_file;
>
> +static int traverse_gitlinks = 0;
> +
> static void setup_git_env(void)
> {
> git_dir = getenv(GIT_DIR_ENVIRONMENT);
> @@ -159,3 +161,13 @@ int set_git_dir(const char *path)
> setup_git_env();
> return 0;
> }
> +
> +int get_traverse_gitlinks()
> +{
> + return traverse_gitlinks;
> +}
> +
> +void set_traverse_gitlinks(int traverse)
> +{
> + traverse_gitlinks = traverse;
> +}
If you have full accessors anyway, it is much easier and cleaner to make
this a global variable to begin with.
However, environment.c is reserved for things that come from the config
and can be overridden by the user. That is certainly not the case for
traverse_gitlinks.
But let's think about it again: should traverse_gitlinks be a global
varible at all? I think not. It should be a per-call decision.
> diff --git a/tree.c b/tree.c
> index 03e782a..87cf309 100644
> --- a/tree.c
> +++ b/tree.c
> @@ -5,6 +5,7 @@
> #include "commit.h"
> #include "tag.h"
> #include "tree-walk.h"
> +#include "refs.h"
>
> const char *tree_type = "tree";
>
> @@ -89,6 +90,61 @@ static int match_tree_entry(const char *base, int baselen, const char *path, uns
> return 0;
> }
>
> +/* Try to add the objectdb of a submodule */
> +int add_gitlink_odb(char *relpath)
This wants to be static.
> +{
> + const char *odbpath;
> + struct stat st;
> +
> + odbpath = read_gitfile_gently(mkpath("%s/.git", relpath));
> + if (!odbpath)
> + odbpath = mkpath("%s/.git/objects", relpath);
> +
> + if (stat(odbpath, &st))
> + return 1;
> +
> + return add_alt_odb(odbpath);
> +}
> +
> +/* Check if we should recurse into the specified submodule */
> +int traverse_gitlink(char *path, const unsigned char *commit_sha1,
This, too.
> + struct tree **subtree)
> +{
> + unsigned char sha1[20];
> + int linked_odb = 0;
> + struct commit *commit;
> + void *buffer;
> + enum object_type type;
> + unsigned long size;
> +
> + hashcpy(sha1, commit_sha1);
> + if (!add_gitlink_odb(path)) {
> + linked_odb = 1;
> + if (resolve_gitlink_ref(path, "HEAD", sha1))
> + die("Unable to lookup HEAD in %s", path);
> + }
Why would you want to continue if add_gitlink_odb() did not find a checked
out submodule?
Seems you want to fall back to look in the superproject's object database.
But I think that is wrong, as I have a superproject with many platform
dependent submodules, only one of which is checked out, and for
convenience, the submodules all live in the superproject's repository.
But I might misunderstand your code.
> + commit = lookup_commit(sha1);
> + if (!commit)
> + die("traverse_gitlink(): internal error");
s/internal error/could not access commit '%s' of submodule '%s'",
sha1_to_hex(sha1), path);/
> @@ -132,6 +188,30 @@ int read_tree_recursive(struct tree *tree,
> return -1;
> continue;
> }
> + if (S_ISGITLINK(entry.mode) && get_traverse_gitlinks()) {
Like I said, traverse_gitlinks should be a flag to read_tree_recursive.
So preferably, you should add a parameter 'flags' and make that option an
enum.
> + int retval;
> + char *newbase;
> + struct tree *subtree;
> + unsigned int pathlen = tree_entry_len(entry.path, entry.sha1);
Nit: Long line.
> +
> + newbase = xmalloc(baselen + 1 + pathlen);
> + memcpy(newbase, base, baselen);
> + memcpy(newbase + baselen, entry.path, pathlen);
> + newbase[baselen + pathlen] = 0;
We have strbufs for that.
> + if (!traverse_gitlink(newbase, entry.sha1, &subtree)) {
> + free(newbase);
> + continue;
> + }
> + newbase[baselen + pathlen] = '/';
... to avoid this off-by-one.
> + retval = read_tree_recursive(subtree,
> + newbase,
> + baselen + pathlen + 1,
> + stage, match, fn, context);
> + free(newbase);
> + if (retval)
> + return -1;
> + continue;
> + }
> }
> return 0;
> }
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
