On Sun, 1 Nov 2009, Junio C Hamano wrote: > Daniel Barkalow <barkalow@xxxxxxxxxxxx> writes: > > > On Sat, 31 Oct 2009, Junio C Hamano wrote: > > ... > >> 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: > >> ... > > I think there's no benefit to allowing NULL for the remote; I think you > > can always get a struct remote for what you want to access. So it's > > probably just as well to require it, particularly because, as in the case > > of cmd_ls_remote() below, you'd need a special case to not get a struct > > remote. > > > > Is there any way in which the intended semantics of "transport_get(NULL, > > url)" is not the same as "transport_get(remote_get(url), url)"? > > (And, in the extended series, I make "transport_get(remote_get(url), > > NULL)" also mean the same thing as above, while "transport_get(NULL, > > NULL)" is obviously underspecified.) > > That was really my question to people who are involved in the transport > layer code. I didn't know how transport->url and transport->remote->url > are intended to relate to each other, for example, and that was why you > were on Cc list. In other words, you are the area expert, you tell me ;-) transport->remote->url[...] has all of the URLs for a remote; transport->url has the particular one this struct transport is supposed to contact. At least, that was my intent, but I think some of the code that uses struct transports didn't realize that remote_get() will work in any context where transport_get() will work. (And this might have been broken at the time). > Sverre seemed to think slightly differently; perhaps having worked on the > foreign vcs interface he has some other input. I think we agree that transport-helper.c ought to be able to deal with anything transport.c will put together. But I'd also like to tighten what transport.c will put together and I don't think he considered that possibility. > > Do things like git_path() fail cleanly if there was no git directory? If > > not, there should probably be tests of nongit on paths that actually need > > a git directory,... > > I don't know. Again, you tell me ;-) I'm not an expert on that part. But it looks like it misbehaves, returning ".git/foo" even when that path doesn't make sense. > It probably makes sesne as you outlined in the earlier part of your > response for the caller to give a bit more clue to the helper to help > making such a decision. Yeah, it's probably worth getting that in at this point. I'll write up some patches. -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