Thread (3 messages) flat view 3 messages, 3 authors, 2016-06-15

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

From: Daniel Barkalow <hidden>
Date: 2016-06-15 22:46:24

Possibly related (same subject, not in this thread)

On Mon, 16 Mar 2009, Junio C Hamano wrote:

Amos King [off-list ref] writes:
quoted
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.
quoted hunk ↗ jump to hunk
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*
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help