Stefan Beller <sbel...@google.com> writes:
> @@ -283,11 +286,22 @@ static void strip_trailing_slashes(char *dir)
> static int add_one_reference(struct string_list_item *item, void *cb_data)
> {
> char *ref_git;
> - const char *repo;
> + const char *repo, *ref_git_s;
> + int *required = cb_data;
> struct strbuf alternate = STRBUF_INIT;
>
> - /* Beware: read_gitfile(), real_path() and mkpath() return static
> buffer */
> - ref_git = xstrdup(real_path(item->string));
> + ref_git_s = *required ?
> + real_path(item->string) :
> + real_path_if_valid(item->string);
> + if (!ref_git_s) {
> + warning(_("Not using proposed alternate %s"), item->string);
If I am reading the implementation of real_path_internal()
correctly, the most relevant reason that an "if-able" repository
cannot be used causes real_path_if_valid() to return NULL.
Let's say your superproject borrows from ~/w/super, whose submodule
repositories (if cloned) are directly in "~/w/super/.git/modules/".
When you clone a submodule X for your superproject, you allow it to
borrow from "~/w/super/.git/modules/X" if there is one available.
I think both real_path() and real_path_if_valid() happily returns
the real path to "~/w/super/.git/modules/" with "X" concatenated at
the end, as long as that 'modules' directory exists, even when there
is no X inside.
Using if_valid() is still necessary to avoid a totally bogus path to
cause real_path() to barf. I am just saying that this warning is
not so interesting, and a real check, "do we have a repository
there? If not, don't add it as an alternate" must be somewhere
else.
> + return 0;
> + } else
> + /*
> + * Beware: read_gitfile(), real_path() and mkpath()
> + * return static buffer
> + */
> + ref_git = xstrdup(ref_git_s);
>
> repo = read_gitfile(ref_git);
> if (!repo)
> @@ -304,7 +318,8 @@ static int add_one_reference(struct string_list_item
> *item, void *cb_data)
> } else if (!is_directory(mkpath("%s/objects", ref_git))) {
> struct strbuf sb = STRBUF_INIT;
> if (get_common_dir(&sb, ref_git))
> - die(_("reference repository '%s' as a linked checkout
> is not supported yet."),
> + die(_("reference repository '%s' as a "
> + "linked checkout is not supported yet."),
> item->string);
And curiously, this is not such a check. This is an unrelated change.
> die(_("reference repository '%s' is not a local repository."),
> item->string);
You need to arrange this die() not to trigger when you are asked to
borrow from there if there is one to borrow from. I see a few other
die() following this check to avoid using shallow repositories and
repositories with grafts, but I think they all should turn into a
"Nah, this is unusable, and I was asked to borrow _only_ if the
repository is usable, so I won't use it."
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
