On Mon, 16 Mar 2009, Junio C Hamano wrote:
> Amos King <amos.l.king@xxxxxxxxx> writes:
>
> > Junio,
> >
> > I'm working with Mike on the http auth stuff, and I was testing out
> > your patch. I can get it to work for fetch but push is giving me some
> > grief. Looking through the code I noticed that online 219 of
> > http-push.c that http_init is being called with NULL instead of a
> > remote. If I pass in the remote then there is no remotre-url. I've
> > been digging around and can't find where or when that is being set.
> > It has been a while since I worked with C but I'd love to jump in and
> > help out here. Can you point me in the right direction to get the
> > remote->url[0] set for the http_auth_init to use?
>
> Daniel is the primary culprit who introduced the transport abstraction,
> and I think he muttered something about his work-in-progress that involves
> in some change in the API. Perhaps he has some insights here?
>
> Naah. I forgot that the transport abstraction on the fetch side is much
> more integrated but curl_transport_push() simply launches http-push.c that
> has a world on its own. Worse yet, "remote" in http-push.c is not even
> the "struct remote"; it is something private to http-push.c called "struct
> repo".
>
> I am not sure how much work would be involved in converting (or if it is
> even worth to convert) http-push.c to fit better into the transport API,
> but if that is feasible, it might be a better longer-term solution.
I think it would be a good thing to do. With the new transport push
method, it could even get support for updating the tracking refs that
track the refs you changed, since I broke that out of the git-native
protocol code and into the transport code.
My guess is that converting http-push to be called in-process would
actually be pretty easy, because the two sides don't look at the same data
currently. (Some things were tricky with fetch, because the same process
sometimes wants to do fetches multiple times, for getting tags; this isn't
as big a deal.)
I think it should just be a matter of breaking http-push's main() into a
function that deals with the http-push command line and a function that
does the work, setting up a header file like send-pack.h, and changing
transport.c to just call the function.
> Right now, builtin-push.c does all the remote inspection and when
> http-push is called, the latter gets the information at the lowest level
> only; the higher level information such as what nickname was used by the
> user to initiate the "git push" process and whether the refspecs came from
> the command line or from the config are all lost, which is quite sad.
What I did short-term for the send-pack version was introduce an optional
command line argument, "--remote", that names the remote used, so the
other program could get the configuration as well. It's a pretty easy
step, but I don't think it's too worthwhile in this case.
> But as a much lower impact interim solution, I suspect that you can fake a
> minimally usable remote. The http_push() codepath only cares about
> remote->http_proxy and remote->url settings as far as I can tell, so
> perhaps you can start (with a big warning that the remote you are creating
> is a fake one) by filling the absolute minimum?
>
> That is, something along these lines (this comes on top of an obvious
> patch that renames existing "remote" variable in http-push. to "repo").
>
> diff --git a/http-push.c b/http-push.c
> index dfbb247..f04ac74 100644
> --- a/http-push.c
> +++ b/http-push.c
> @@ -2197,6 +2197,7 @@ int main(int argc, char **argv)
> int new_refs;
> struct ref *ref;
> char *rewritten_url = NULL;
> + struct remote *remote;
>
> git_extract_argv0_path(argv[0]);
>
> @@ -2258,12 +2259,14 @@ int main(int argc, char **argv)
> if (!repo->url)
> usage(http_push_usage);
>
> + remote = remote_get(repo->url);
> +
> if (delete_branch && nr_refspec != 1)
> die("You must specify only one branch name when deleting a remote branch");
>
> memset(remote_dir_exists, -1, 256);
>
> - http_init(NULL);
> + http_init(remote);
>
> no_pragma_header = curl_slist_append(no_pragma_header, "Pragma:");
This should work, although it obviously won't figure out the proxy for the
configuration for the remote that was actually used.
-Daniel
*This .sig left intentionally blank*
--
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
