From: Daniel Barkalow <hidden> Date: 2016-06-15 22:46:20
This series only supports the narrowest case of having the * in the middle
of a side of a refspec: having it as a full path component on each side.
Patches 1-3 centralize all of the parsing and matching rules to a pair of
functions; patch 4 makes the stored representation more convenient (and
serves as a distinguished bisection outcome for anything I missed that was
relying on the contents of struct refspec for patterns); and patch 5
extends the matching implementation and loosens the ref format
requirements to allow the * to be in the middle.
An easy followup would relax the restrictions further without requiring
any particularly tricky further changes.
Daniel Barkalow (5):
Make clone parse the default fetch refspec with the regular code
Use a single function to match names against patterns
Use the matching function to generate the match results
Keep '*' in pattern refspecs
Support '*' in the middle of a refspec
builtin-clone.c | 25 ++++++++--------
refs.c | 15 +++++----
remote.c | 78 +++++++++++++++++++++++++++++----------------------
t/t5511-refspec.sh | 12 ++++++++
4 files changed, 77 insertions(+), 53 deletions(-)
From: Jay Soffian <hidden> Date: 2016-06-15 22:46:20
On Thu, Mar 5, 2009 at 11:56 PM, Daniel Barkalow [off-list ref] wrote:
This series only supports the narrowest case of having the * in the middle
of a side of a refspec: having it as a full path component on each side.
Patches 1-3 centralize all of the parsing and matching rules to a pair of
functions; patch 4 makes the stored representation more convenient (and
serves as a distinguished bisection outcome for anything I missed that was
relying on the contents of struct refspec for patterns); and patch 5
extends the matching implementation and loosens the ref format
requirements to allow the * to be in the middle.
An easy followup would relax the restrictions further without requiring
any particularly tricky further changes.
This series and js/remote-improvements (e5dcbfd) in pu may not get
along completely. "git remote show" tries to show how the refspecs
expand out. And actually, that should be fine since builtin-remote now
uses the same code as fetch/push to expand the refs.
However, "git remote show -n" (-n means don't query the remote) makes
use of a new function, get_push_ref_states_noquery(), which more or
less tries to reverse the parsed refspec back into the original
string. That function relies on the current (before your patch)
refspec semantics and assumes if refspec.pattern is set, then the '*'
is at the end. So it will need tweaking.
j.
From: Daniel Barkalow <hidden> Date: 2016-06-15 22:46:20
On Fri, 6 Mar 2009, Jay Soffian wrote:
On Thu, Mar 5, 2009 at 11:56 PM, Daniel Barkalow [off-list ref] wrote:
quoted
This series only supports the narrowest case of having the * in the middle
of a side of a refspec: having it as a full path component on each side.
Patches 1-3 centralize all of the parsing and matching rules to a pair of
functions; patch 4 makes the stored representation more convenient (and
serves as a distinguished bisection outcome for anything I missed that was
relying on the contents of struct refspec for patterns); and patch 5
extends the matching implementation and loosens the ref format
requirements to allow the * to be in the middle.
An easy followup would relax the restrictions further without requiring
any particularly tricky further changes.
This series and js/remote-improvements (e5dcbfd) in pu may not get
along completely. "git remote show" tries to show how the refspecs
expand out. And actually, that should be fine since builtin-remote now
uses the same code as fetch/push to expand the refs.
However, "git remote show -n" (-n means don't query the remote) makes
use of a new function, get_push_ref_states_noquery(), which more or
less tries to reverse the parsed refspec back into the original
string. That function relies on the current (before your patch)
refspec semantics and assumes if refspec.pattern is set, then the '*'
is at the end. So it will need tweaking.
Actually, you should be able to just drop your "buf" and use spec->src and
spec->dst, since it just stores the original strings. So that should be
easy enough, although it might be good to go through a remote.c function
just in case it becomes more complicated later. On the other hand,
get_head_names() should probably get a patch like my 1/5 to have it use
the remote.c parser, or should use a constant "head mirror" refspec like
that tag_refspec already in remote.c
Do you have tests for "git remote show -n"? Merging my series (on top of
origin/master) and e5dcbfd and adding a final '*' to the string in
get_head_names() made everything pass for me, without doing anything about
the extra *s, but the output is clearly not quite right.
I'm not seeing anything that makes assumptions about the matching
semantics of pattern refspecs, just stuff about how the stored form
relates to the config-file form.
-Daniel
*This .sig left intentionally blank*
From: Jay Soffian <hidden> Date: 2016-06-15 22:46:20
On Fri, Mar 6, 2009 at 1:07 AM, Daniel Barkalow [off-list ref] wrote:
On Fri, 6 Mar 2009, Jay Soffian wrote:
Actually, you should be able to just drop your "buf" and use spec->src and
spec->dst, since it just stores the original strings. So that should be
easy enough, although it might be good to go through a remote.c function
just in case it becomes more complicated later. On the other hand,
get_head_names() should probably get a patch like my 1/5 to have it use
the remote.c parser, or should use a constant "head mirror" refspec like
that tag_refspec already in remote.c
Okay.
Do you have tests for "git remote show -n"?
Yes. Apparently not enough of them though if nothing is failing.
Merging my series (on top of
origin/master) and e5dcbfd and adding a final '*' to the string in
get_head_names() made everything pass for me, without doing anything about
the extra *s, but the output is clearly not quite right.
Hmm, alright.
I'm not seeing anything that makes assumptions about the matching
semantics of pattern refspecs, just stuff about how the stored form
relates to the config-file form.
Okay, that sounds right.
I assume your series will end up in pu soon enough, and I think my
series is about to hop to next. What's the right way to to have them
be happy together?
j.
From: Daniel Barkalow <hidden> Date: 2016-06-15 22:46:20
On Fri, 6 Mar 2009, Jay Soffian wrote:
On Fri, Mar 6, 2009 at 1:07 AM, Daniel Barkalow [off-list ref] wrote:
quoted
On Fri, 6 Mar 2009, Jay Soffian wrote:
Actually, you should be able to just drop your "buf" and use spec->src and
spec->dst, since it just stores the original strings. So that should be
easy enough, although it might be good to go through a remote.c function
just in case it becomes more complicated later. On the other hand,
get_head_names() should probably get a patch like my 1/5 to have it use
the remote.c parser, or should use a constant "head mirror" refspec like
that tag_refspec already in remote.c
Okay.
quoted
Do you have tests for "git remote show -n"?
Yes. Apparently not enough of them though if nothing is failing.
It only seems to be off by saying:
Local ref configured for 'git push' (status not queried):
refs/heads/** forces to refs/heads/**
so you didn't necessarily miss much, just the one thing I seem to have
broken.
quoted
Merging my series (on top of
origin/master) and e5dcbfd and adding a final '*' to the string in
get_head_names() made everything pass for me, without doing anything about
the extra *s, but the output is clearly not quite right.
Hmm, alright.
quoted
I'm not seeing anything that makes assumptions about the matching
semantics of pattern refspecs, just stuff about how the stored form
relates to the config-file form.
Okay, that sounds right.
I assume your series will end up in pu soon enough, and I think my
series is about to hop to next. What's the right way to to have them
be happy together?
The only "correctness of outcome" issue is the open-coded refspec
initialization, I think, which is probably actually cleaner to have as a
constant in remote.c anyway (unlike in builtin-clone, there's no
variability at all, so it might as well be a constant.
I can amend my series to avoid adding the extra * in the message when your
series graduates, and it should be clean enough to deal with if your
series wound up getting dropped later; it'll be the only change in that
file for my series, so I'd be able to drop it easily.
It'd be useful to have that message tested by your series, though, so I
can verify my series reliably without worrying about whether I
accidentally dropped both the fix and the test.
-Daniel
*This .sig left intentionally blank*
From: Jay Soffian <hidden> Date: 2016-06-15 22:46:20
On Fri, Mar 6, 2009 at 2:03 AM, Daniel Barkalow [off-list ref] wrote:
It'd be useful to have that message tested by your series, though, so I
can verify my series reliably without worrying about whether I
accidentally dropped both the fix and the test.
I'll send a patch for it in the next day or two.
j.