Re: [PATCH 0/3] push: detect local refspec errors early
On Wed, Mar 05, 2014 at 12:51:06PM -0800, Junio C Hamano wrote: > Jeff King writes: > > > We can't fully process the refspecs until we have talked to the other > > side, because they may involve matching refs from the remote; I don't > > think git even really looks at them until after we've connected. > > > > But I think there are some obvious cases, like a bogus left-hand side > > (i.e., what you have here) that cannot ever succeed, no matter what the > > other side has. We could sanity check the refspecs before doing anything > > else. > > The user's wallclock time is more important than machine cycles, > checking things we could check before having the user do things is a > good principle to follow. > > I wish that the solution did not have to involve doing the same > computation twice, but I do not think there is a clean way around > that in this codepath. Yeah, there are two inefficiencies here: 1. We parse the refspecs twice. In theory we could parse them once, feed the result to check_push_refspecs, then again to match_push_refspecs. That wouldn't be too hard a refactor. 2. We match the "src" side to local refs twice. Getting rid of this would involve splitting match_push_refs into two (analyzing the "src" half and the "dst" half), somehow storing the intermediate the two calls, and only contacting the remote after the first step is done. This is probably trickier. I'd be happy if somebody wanted to do those cleanups on top, but I don't personally have an interest in spending time on them. The amount of duplicated computation we're talking about here is not very much (and the number of refspecs tends to be small, anyway). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] push: detect local refspec errors early
Jeff King writes: > We can't fully process the refspecs until we have talked to the other > side, because they may involve matching refs from the remote; I don't > think git even really looks at them until after we've connected. > > But I think there are some obvious cases, like a bogus left-hand side > (i.e., what you have here) that cannot ever succeed, no matter what the > other side has. We could sanity check the refspecs before doing anything > else. The user's wallclock time is more important than machine cycles, checking things we could check before having the user do things is a good principle to follow. I wish that the solution did not have to involve doing the same computation twice, but I do not think there is a clean way around that in this codepath. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] push: detect local refspec errors early
On Wed, Mar 05, 2014 at 01:36:12PM +0400, Dmitry wrote: > Here's my usecase. I have created a BranchWithVeryLongName and I want > to have it pushed to origin. So I use this command with version > 1.8.1.2: > > git push origin BranchMistypedLongName > > (note that I mistyped the branch name). The following happens: > 1. git asks me for username and password > 2. it authenticates on the origin server > 3. it bails out with "error: sfc refspec BranchMistypedLongName does not > match any" > > Can't git perhaps check that the branch exists before it asks me for > credentials and just say there's no such branch? We can't fully process the refspecs until we have talked to the other side, because they may involve matching refs from the remote; I don't think git even really looks at them until after we've connected. But I think there are some obvious cases, like a bogus left-hand side (i.e., what you have here) that cannot ever succeed, no matter what the other side has. We could sanity check the refspecs before doing anything else. Here's a patch series that does that. [1/3]: match_explicit: hoist refspec lhs checks into their own function [2/3]: match_explicit_lhs: allow a "verify only" mode [3/3]: push: detect local refspec errors early -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html