Re: [PATCH][v2] http authentication via prompts (with correct line lengths)

[Amos King <amos.l.king@gmail.com>]

I got push working.  I wanted to ask one thing before I submit.  I
know that it is nice to keep changes local.  There is a method in
remote.c called add_url, but it is not exposed.  Right now to keep the
changes local I moved the internals of that method inline, but is it
ok for me to expose that method instead?

Amos

PS - Junio sorry for the double send.  I didn't notice that we weren't
on the list.

On Tue, Mar 17, 2009 at 11:24 AM, Daniel Barkalow <barkalow@iabervon.org> wrote:
> On Mon, 16 Mar 2009, Junio C Hamano wrote:
>
>> Amos King <amos.l.king@gmail.com> 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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Amos King
http://dirtyInformation.com
http://github.com/Adkron
--
Looking for something to do? Visit http://ImThere.com