Samium Gromoff <_deepfire@xxxxxxxxxxxxxxxxx> writes:
> Attached is the SEGV bugreport I sent to debian.
Thanks for a report.
There are two issues.
* transport-helper.c::get_helper() assumes that the transport structure
always has its remote member filled and it can get name out of it.
This is the segv in the report.
* Even if we work around the above issue, the helper subprocess (in this
case, remote-curl helper) insists on being inside a git repository, To
satisfy ls-remote request, you do not have to be in one.
Attached is a minimum fix/work around, but this is done without being very
familiar with the current assumptions in the codepaths involved.
Issues I want area experts to consider before coming up with the final fix
are:
- Should we fix get_helper() in transport-helper.c, instead of touching
ls-remote.c like this patch does?
This issue really boils down to this question: is it valid for a
transport to have NULL in its remote field, and should all the code
that touch transport be prepared to deal with such a transport
structure?
- When helping to handle ls-remote request, there is no need for the
helper to know anything about the local state. We probably shouldn't
even run setup_git_directory_gently() at all in this case. But when
helping other kinds of request, the helper does need to know where our
repository is.
In general, what should the initial environment for helpers be? Should
they assume that they have to figure out where the git repository is
themselves (in other words, should they assume they cannot rely on
anything the caller does before they are called? Would the caller
generally have done the usual repo discovery (including chdir() to the
toplevel), and there are some set of assumptions they can make? If so
what are they?
builtin-ls-remote.c | 2 +-
remote-curl.c | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/builtin-ls-remote.c b/builtin-ls-remote.c
index 78a88f7..a8d5613 100644
--- a/builtin-ls-remote.c
+++ b/builtin-ls-remote.c
@@ -86,7 +86,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
pattern[j - i] = p;
}
}
- remote = nongit ? NULL : remote_get(dest);
+ remote = remote_get(dest);
if (remote && !remote->url_nr)
die("remote %s has no configured URL", dest);
transport = transport_get(remote, remote ? remote->url[0] : dest);
diff --git a/remote-curl.c b/remote-curl.c
index ad6a163..7c83f77 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -81,8 +81,9 @@ int main(int argc, const char **argv)
struct strbuf buf = STRBUF_INIT;
const char *url;
struct walker *walker = NULL;
+ int nongit = 0;
- setup_git_directory();
+ setup_git_directory_gently(&nongit);
if (argc < 2) {
fprintf(stderr, "Remote needed\n");
return 1;
--
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