* [PATCH 0/2] contrib/git-resurrect.sh: make it hash-agnostic
@ 2020-10-08 6:44 Denton Liu
2020-10-08 6:44 ` [PATCH 1/2] contrib/git-resurrect.sh: indent with tabs Denton Liu
2020-10-08 6:44 ` [PATCH 2/2] contrib/git-resurrect.sh: use hash-agnostic OID pattern Denton Liu
0 siblings, 2 replies; 11+ messages in thread
From: Denton Liu @ 2020-10-08 6:44 UTC (permalink / raw)
To: Git Mailing List
Since Git supports hashes other than SHA-1, the hardcoded $_x40 used to
match OIDs is no longer correct. Replace it with a hash-agnostic
version.
While we're at it fix some indent issues.
There are some other style issues in the script but I'll leave those as
out-of-scope.
Denton Liu (2):
contrib/git-resurrect.sh: indent with tabs
contrib/git-resurrect.sh: use hash-agnostic OID pattern
contrib/git-resurrect.sh | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
--
2.29.0.rc0.261.g7178c9af9c
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] contrib/git-resurrect.sh: indent with tabs
2020-10-08 6:44 [PATCH 0/2] contrib/git-resurrect.sh: make it hash-agnostic Denton Liu
@ 2020-10-08 6:44 ` Denton Liu
2020-10-08 17:32 ` Junio C Hamano
2020-10-08 6:44 ` [PATCH 2/2] contrib/git-resurrect.sh: use hash-agnostic OID pattern Denton Liu
1 sibling, 1 reply; 11+ messages in thread
From: Denton Liu @ 2020-10-08 6:44 UTC (permalink / raw)
To: Git Mailing List
In the git-resurrect script, there are a few lines that are mistakenly
indented with spaces. Replace these lines with tabs.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
contrib/git-resurrect.sh | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/contrib/git-resurrect.sh b/contrib/git-resurrect.sh
index 8c171dd959..57a77c03f9 100755
--- a/contrib/git-resurrect.sh
+++ b/contrib/git-resurrect.sh
@@ -27,7 +27,7 @@ n,dry-run don't recreate the branch"
search_reflog () {
sed -ne 's~^\([^ ]*\) .* checkout: moving from '"$1"' .*~\1~p' \
- < "$GIT_DIR"/logs/HEAD
+ < "$GIT_DIR"/logs/HEAD
}
search_reflog_merges () {
@@ -41,9 +41,9 @@ _x40="[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]"
_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
search_merges () {
- git rev-list --all --grep="Merge branch '$1'" \
- --pretty=tformat:"%P %s" |
- sed -ne "/^$_x40 \($_x40\) Merge .*/ {s//\1/p;$early_exit}"
+ git rev-list --all --grep="Merge branch '$1'" \
+ --pretty=tformat:"%P %s" |
+ sed -ne "/^$_x40 \($_x40\) Merge .*/ {s//\1/p;$early_exit}"
}
search_merge_targets () {
--
2.29.0.rc0.261.g7178c9af9c
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] contrib/git-resurrect.sh: use hash-agnostic OID pattern
2020-10-08 6:44 [PATCH 0/2] contrib/git-resurrect.sh: make it hash-agnostic Denton Liu
2020-10-08 6:44 ` [PATCH 1/2] contrib/git-resurrect.sh: indent with tabs Denton Liu
@ 2020-10-08 6:44 ` Denton Liu
2020-10-08 16:13 ` Jeff King
1 sibling, 1 reply; 11+ messages in thread
From: Denton Liu @ 2020-10-08 6:44 UTC (permalink / raw)
To: Git Mailing List
Since Git now supports hashes other than SHA-1, the hash length isn't
guaranteed to be 40 characters. Replace $_x40 with a hash-agnostic OID
pattern.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
contrib/git-resurrect.sh | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/contrib/git-resurrect.sh b/contrib/git-resurrect.sh
index 57a77c03f9..d843df3afd 100755
--- a/contrib/git-resurrect.sh
+++ b/contrib/git-resurrect.sh
@@ -37,19 +37,18 @@ search_reflog_merges () {
)
}
-_x40="[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]"
-_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
+oid_pattern=$(git hash-object --stdin </dev/null | sed -e 's/./[0-9a-f]/g')
search_merges () {
git rev-list --all --grep="Merge branch '$1'" \
--pretty=tformat:"%P %s" |
- sed -ne "/^$_x40 \($_x40\) Merge .*/ {s//\1/p;$early_exit}"
+ sed -ne "/^$oid_pattern \($oid_pattern\) Merge .*/ {s//\1/p;$early_exit}"
}
search_merge_targets () {
git rev-list --all --grep="Merge branch '[^']*' into $branch\$" \
--pretty=tformat:"%H %s" --all |
- sed -ne "/^\($_x40\) Merge .*/ {s//\1/p;$early_exit} "
+ sed -ne "/^\($oid_pattern\) Merge .*/ {s//\1/p;$early_exit} "
}
dry_run=
--
2.29.0.rc0.261.g7178c9af9c
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] contrib/git-resurrect.sh: use hash-agnostic OID pattern
2020-10-08 6:44 ` [PATCH 2/2] contrib/git-resurrect.sh: use hash-agnostic OID pattern Denton Liu
@ 2020-10-08 16:13 ` Jeff King
2020-10-08 18:29 ` Junio C Hamano
2020-10-08 22:11 ` brian m. carlson
0 siblings, 2 replies; 11+ messages in thread
From: Jeff King @ 2020-10-08 16:13 UTC (permalink / raw)
To: Denton Liu; +Cc: Git Mailing List
On Wed, Oct 07, 2020 at 11:44:40PM -0700, Denton Liu wrote:
> diff --git a/contrib/git-resurrect.sh b/contrib/git-resurrect.sh
> index 57a77c03f9..d843df3afd 100755
> --- a/contrib/git-resurrect.sh
> +++ b/contrib/git-resurrect.sh
> @@ -37,19 +37,18 @@ search_reflog_merges () {
> )
> }
>
> -_x40="[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]"
> -_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
> +oid_pattern=$(git hash-object --stdin </dev/null | sed -e 's/./[0-9a-f]/g')
This looks correct, although...
> search_merges () {
> git rev-list --all --grep="Merge branch '$1'" \
> --pretty=tformat:"%P %s" |
> - sed -ne "/^$_x40 \($_x40\) Merge .*/ {s//\1/p;$early_exit}"
> + sed -ne "/^$oid_pattern \($oid_pattern\) Merge .*/ {s//\1/p;$early_exit}"
> }
>
> search_merge_targets () {
> git rev-list --all --grep="Merge branch '[^']*' into $branch\$" \
> --pretty=tformat:"%H %s" --all |
> - sed -ne "/^\($_x40\) Merge .*/ {s//\1/p;$early_exit} "
> + sed -ne "/^\($oid_pattern\) Merge .*/ {s//\1/p;$early_exit} "
> }
in both cases we are matching output we asked for, so we really matching
[0-9a-f]\+ would be correct and sufficient. That's a little simpler. I
don't feel too strongly either way, though.
Side note: It's a shame that there is no way to convince rev-list not
to print the "commit ..." header, which is really what we're avoiding
here. We probably should have suppressed it with user-formats when
they were introduced, but it's too late to make that change. I wonder
if it would be worth adding a command-line option, though. I've often
had to hack around this when parsing rev-list output (and sometimes
even resort to using git-log if it's a one-off).
-Peff
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] contrib/git-resurrect.sh: indent with tabs
2020-10-08 6:44 ` [PATCH 1/2] contrib/git-resurrect.sh: indent with tabs Denton Liu
@ 2020-10-08 17:32 ` Junio C Hamano
2020-10-08 18:48 ` Junio C Hamano
0 siblings, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2020-10-08 17:32 UTC (permalink / raw)
To: Denton Liu; +Cc: Git Mailing List
Denton Liu <liu.denton@gmail.com> writes:
> In the git-resurrect script, there are a few lines that are mistakenly
> indented with spaces. Replace these lines with tabs.
>
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
> contrib/git-resurrect.sh | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/contrib/git-resurrect.sh b/contrib/git-resurrect.sh
> index 8c171dd959..57a77c03f9 100755
> --- a/contrib/git-resurrect.sh
> +++ b/contrib/git-resurrect.sh
> @@ -27,7 +27,7 @@ n,dry-run don't recreate the branch"
>
> search_reflog () {
> sed -ne 's~^\([^ ]*\) .* checkout: moving from '"$1"' .*~\1~p' \
> - < "$GIT_DIR"/logs/HEAD
> + < "$GIT_DIR"/logs/HEAD
> }
Unless this is done in preparation for a more meaningful change
(like not reading from on-disk logs/HEAD and instead reading from
"git reflog" output in order to help our future migration to the
reftable backend), I do not think the reviewers' time is worth it
for cosmetic changes on this script.
> search_reflog_merges () {
> @@ -41,9 +41,9 @@ _x40="[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]"
> _x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
>
> search_merges () {
> - git rev-list --all --grep="Merge branch '$1'" \
> - --pretty=tformat:"%P %s" |
> - sed -ne "/^$_x40 \($_x40\) Merge .*/ {s//\1/p;$early_exit}"
> + git rev-list --all --grep="Merge branch '$1'" \
> + --pretty=tformat:"%P %s" |
> + sed -ne "/^$_x40 \($_x40\) Merge .*/ {s//\1/p;$early_exit}"
> }
>
> search_merge_targets () {
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] contrib/git-resurrect.sh: use hash-agnostic OID pattern
2020-10-08 16:13 ` Jeff King
@ 2020-10-08 18:29 ` Junio C Hamano
2020-10-08 19:53 ` Jeff King
2020-10-08 22:11 ` brian m. carlson
1 sibling, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2020-10-08 18:29 UTC (permalink / raw)
To: Jeff King; +Cc: Denton Liu, Git Mailing List
Jeff King <peff@peff.net> writes:
> On Wed, Oct 07, 2020 at 11:44:40PM -0700, Denton Liu wrote:
>
>> diff --git a/contrib/git-resurrect.sh b/contrib/git-resurrect.sh
>> index 57a77c03f9..d843df3afd 100755
>> --- a/contrib/git-resurrect.sh
>> +++ b/contrib/git-resurrect.sh
>> @@ -37,19 +37,18 @@ search_reflog_merges () {
>> )
>> }
>>
>> -_x40="[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]"
>> -_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
>> +oid_pattern=$(git hash-object --stdin </dev/null | sed -e 's/./[0-9a-f]/g')
> ...
> in both cases we are matching output we asked for, so we really matching
> [0-9a-f]\+ would be correct and sufficient. That's a little simpler. I
> don't feel too strongly either way, though.
Yes. I think use of _x40 is overrated.
> Side note: It's a shame that there is no way to convince rev-list not
> to print the "commit ..." header, which is really what we're avoiding
> here. We probably should have suppressed it with user-formats when
> they were introduced, but it's too late to make that change. I wonder
> if it would be worth adding a command-line option, though. I've often
> had to hack around this when parsing rev-list output (and sometimes
> even resort to using git-log if it's a one-off).
Or make "git log" without frills as fast as rev-list, perhaps?
What extra things do we do that makes "log" inherently slower than
"rev-list"?
I do not mind a new option (e.g. --no-header) to "rev-list", though.
Thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] contrib/git-resurrect.sh: indent with tabs
2020-10-08 17:32 ` Junio C Hamano
@ 2020-10-08 18:48 ` Junio C Hamano
0 siblings, 0 replies; 11+ messages in thread
From: Junio C Hamano @ 2020-10-08 18:48 UTC (permalink / raw)
To: Denton Liu; +Cc: Git Mailing List
Junio C Hamano <gitster@pobox.com> writes:
> Denton Liu <liu.denton@gmail.com> writes:
>
>> In the git-resurrect script, there are a few lines that are mistakenly
>> indented with spaces. Replace these lines with tabs.
>>
>> Signed-off-by: Denton Liu <liu.denton@gmail.com>
>> ---
>> contrib/git-resurrect.sh | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/contrib/git-resurrect.sh b/contrib/git-resurrect.sh
>> index 8c171dd959..57a77c03f9 100755
>> --- a/contrib/git-resurrect.sh
>> +++ b/contrib/git-resurrect.sh
>> @@ -27,7 +27,7 @@ n,dry-run don't recreate the branch"
>>
>> search_reflog () {
>> sed -ne 's~^\([^ ]*\) .* checkout: moving from '"$1"' .*~\1~p' \
>> - < "$GIT_DIR"/logs/HEAD
>> + < "$GIT_DIR"/logs/HEAD
>> }
>
> Unless this is done in preparation for a more meaningful change
> (like not reading from on-disk logs/HEAD and instead reading from
> "git reflog" output in order to help our future migration to the
> reftable backend), I do not think the reviewers' time is worth it
> for cosmetic changes on this script.
>
OK, so the "meaningful" part is to prepare for SHA-256 world. That
sort-of makes sense.
But don't we want to remove the SP after the redirection operator
on the same line, while we are looking at it?
>> search_reflog_merges () {
>> @@ -41,9 +41,9 @@ _x40="[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]"
>> _x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
>>
>> search_merges () {
>> - git rev-list --all --grep="Merge branch '$1'" \
>> - --pretty=tformat:"%P %s" |
>> - sed -ne "/^$_x40 \($_x40\) Merge .*/ {s//\1/p;$early_exit}"
>> + git rev-list --all --grep="Merge branch '$1'" \
>> + --pretty=tformat:"%P %s" |
>> + sed -ne "/^$_x40 \($_x40\) Merge .*/ {s//\1/p;$early_exit}"
>> }
>>
>> search_merge_targets () {
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] contrib/git-resurrect.sh: use hash-agnostic OID pattern
2020-10-08 18:29 ` Junio C Hamano
@ 2020-10-08 19:53 ` Jeff King
0 siblings, 0 replies; 11+ messages in thread
From: Jeff King @ 2020-10-08 19:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Denton Liu, Git Mailing List
On Thu, Oct 08, 2020 at 11:29:34AM -0700, Junio C Hamano wrote:
> > Side note: It's a shame that there is no way to convince rev-list not
> > to print the "commit ..." header, which is really what we're avoiding
> > here. We probably should have suppressed it with user-formats when
> > they were introduced, but it's too late to make that change. I wonder
> > if it would be worth adding a command-line option, though. I've often
> > had to hack around this when parsing rev-list output (and sometimes
> > even resort to using git-log if it's a one-off).
>
> Or make "git log" without frills as fast as rev-list, perhaps?
>
> What extra things do we do that makes "log" inherently slower than
> "rev-list"?
It's not the speed of log that is a problem, but just that I usually try
to use plumbing when scripting. So I often reach for rev-list first.
I do think for just listing commit hashes that log is slower, though.
One reason is that when there's a commit-graph, it's not as good at
avoiding reading the commit objects. E.g.:
$ time git rev-list HEAD >/dev/null
real 0m0.031s
user 0m0.027s
sys 0m0.004s
$ time git -c core.commitgraph=false rev-list HEAD >/dev/null
real 0m0.362s
user 0m0.345s
sys 0m0.016s
$ time git log --format=%H HEAD >/dev/null
real 0m0.371s
user 0m0.355s
sys 0m0.016s
So running "git log" takes about the same time as rev-list if we disable
the commit-graph. Which makes sense. The pretty-print code is aggressive
about loading the object contents, even if we end up not needing it
(because really, everything _except_ userformat does need it, and even
userformat usually needs it).
I think it would be nice to make the formatting code smarter about
reporting exactly which parts it needs.
> I do not mind a new option (e.g. --no-header) to "rev-list", though.
I took a brief look at this earlier today and it was more awkward than I
expected. The "commit <oid>" header might also have other stuff attached
to it (revision marks, parents, and who even knew we had a "--timestamp"
option?). It's not clear where those things should go if we suppress the
header (for oneline, they just get stuck in front of the oneline; would
that be OK for userformats, too?).
-Peff
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] contrib/git-resurrect.sh: use hash-agnostic OID pattern
2020-10-08 16:13 ` Jeff King
2020-10-08 18:29 ` Junio C Hamano
@ 2020-10-08 22:11 ` brian m. carlson
2020-10-09 7:55 ` Andreas Schwab
1 sibling, 1 reply; 11+ messages in thread
From: brian m. carlson @ 2020-10-08 22:11 UTC (permalink / raw)
To: Jeff King; +Cc: Denton Liu, Git Mailing List
[-- Attachment #1: Type: text/plain, Size: 2862 bytes --]
On 2020-10-08 at 16:13:45, Jeff King wrote:
> On Wed, Oct 07, 2020 at 11:44:40PM -0700, Denton Liu wrote:
>
> > diff --git a/contrib/git-resurrect.sh b/contrib/git-resurrect.sh
> > index 57a77c03f9..d843df3afd 100755
> > --- a/contrib/git-resurrect.sh
> > +++ b/contrib/git-resurrect.sh
> > @@ -37,19 +37,18 @@ search_reflog_merges () {
> > )
> > }
> >
> > -_x40="[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]"
> > -_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
> > +oid_pattern=$(git hash-object --stdin </dev/null | sed -e 's/./[0-9a-f]/g')
>
> This looks correct, although...
We could write this more simply as this:
oid_pattern=$(git hash-object /dev/null | sed -e 's/./[0-9a-f]/g')
I'm almost certain that works just fine, even on Windows, and I think
the hashing code may be able to optimize better if we avoid --stdin
(since it knows the size ahead of time). The performance benefit, if
any, won't be that significant, though, since we're only hashing 7
bytes.
This is, of course, not worth a reroll in itself.
> > search_merges () {
> > git rev-list --all --grep="Merge branch '$1'" \
> > --pretty=tformat:"%P %s" |
> > - sed -ne "/^$_x40 \($_x40\) Merge .*/ {s//\1/p;$early_exit}"
> > + sed -ne "/^$oid_pattern \($oid_pattern\) Merge .*/ {s//\1/p;$early_exit}"
> > }
> >
> > search_merge_targets () {
> > git rev-list --all --grep="Merge branch '[^']*' into $branch\$" \
> > --pretty=tformat:"%H %s" --all |
> > - sed -ne "/^\($_x40\) Merge .*/ {s//\1/p;$early_exit} "
> > + sed -ne "/^\($oid_pattern\) Merge .*/ {s//\1/p;$early_exit} "
> > }
>
> in both cases we are matching output we asked for, so we really matching
> [0-9a-f]\+ would be correct and sufficient. That's a little simpler. I
> don't feel too strongly either way, though.
The problem here is that we'd need to write [0-9a-f][0-9a-f]* because
this is a BRE and a backslashed + here is a GNU extension.
I'm fine with this patch (and patch 1) as it stands or with that change.
I don't think this can be ambiguous, although if we think it can be
(even with silly user behavior), then we should adopt this solution.
> Side note: It's a shame that there is no way to convince rev-list not
> to print the "commit ..." header, which is really what we're avoiding
> here. We probably should have suppressed it with user-formats when
> they were introduced, but it's too late to make that change. I wonder
> if it would be worth adding a command-line option, though. I've often
> had to hack around this when parsing rev-list output (and sometimes
> even resort to using git-log if it's a one-off).
Yeah, I use git log for scripting more often than I'd like because
rev-list can't remove the header. That would indeed be a welcome
feature.
--
brian m. carlson: Houston, Texas, US
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] contrib/git-resurrect.sh: use hash-agnostic OID pattern
2020-10-08 22:11 ` brian m. carlson
@ 2020-10-09 7:55 ` Andreas Schwab
2020-10-09 11:53 ` Jeff King
0 siblings, 1 reply; 11+ messages in thread
From: Andreas Schwab @ 2020-10-09 7:55 UTC (permalink / raw)
To: brian m. carlson; +Cc: Jeff King, Denton Liu, Git Mailing List
On Okt 08 2020, brian m. carlson wrote:
> On 2020-10-08 at 16:13:45, Jeff King wrote:
>> On Wed, Oct 07, 2020 at 11:44:40PM -0700, Denton Liu wrote:
>> > search_merges () {
>> > git rev-list --all --grep="Merge branch '$1'" \
>> > --pretty=tformat:"%P %s" |
>> > - sed -ne "/^$_x40 \($_x40\) Merge .*/ {s//\1/p;$early_exit}"
>> > + sed -ne "/^$oid_pattern \($oid_pattern\) Merge .*/ {s//\1/p;$early_exit}"
>> > }
>> >
>> > search_merge_targets () {
>> > git rev-list --all --grep="Merge branch '[^']*' into $branch\$" \
>> > --pretty=tformat:"%H %s" --all |
>> > - sed -ne "/^\($_x40\) Merge .*/ {s//\1/p;$early_exit} "
>> > + sed -ne "/^\($oid_pattern\) Merge .*/ {s//\1/p;$early_exit} "
>> > }
>>
>> in both cases we are matching output we asked for, so we really matching
>> [0-9a-f]\+ would be correct and sufficient. That's a little simpler. I
>> don't feel too strongly either way, though.
>
> The problem here is that we'd need to write [0-9a-f][0-9a-f]* because
> this is a BRE and a backslashed + here is a GNU extension.
I wonder why --pretty uses %s when it is filtered out again anyway?
(There is also a duplicate --all.)
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] contrib/git-resurrect.sh: use hash-agnostic OID pattern
2020-10-09 7:55 ` Andreas Schwab
@ 2020-10-09 11:53 ` Jeff King
0 siblings, 0 replies; 11+ messages in thread
From: Jeff King @ 2020-10-09 11:53 UTC (permalink / raw)
To: Andreas Schwab; +Cc: brian m. carlson, Denton Liu, Git Mailing List
On Fri, Oct 09, 2020 at 09:55:53AM +0200, Andreas Schwab wrote:
> >> > search_merges () {
> >> > git rev-list --all --grep="Merge branch '$1'" \
> >> > --pretty=tformat:"%P %s" |
> >> > - sed -ne "/^$_x40 \($_x40\) Merge .*/ {s//\1/p;$early_exit}"
> >> > + sed -ne "/^$oid_pattern \($oid_pattern\) Merge .*/ {s//\1/p;$early_exit}"
> [...]
> I wonder why --pretty uses %s when it is filtered out again anyway?
> (There is also a duplicate --all.)
It does confirm that the commit in question is a (likely) merge commit,
and not one that happens to have "Merge branch 'foo'" in the body, which
the earlier --grep would have hit. But it doesn't actually check for
'foo' in the sed match, so it would be fooled by a commit message like:
Merge branch 'bar'
* bar:
Merge branch 'foo'
If we wanted to tighten that up, then sed should match the branch name.
If we're willing to accept the looseness, the whole thing could probably
be:
git rev-list --all --grep="Merge branch '$1'" \
--min-parents 2 --parents ${early_exit:-"-1"} |
awk '{print $3}'
I'm happy either way, but I'm not sure anybody overly cares. Let's not
derail Denton's actual fix to make the script work in a sha256 world.
-Peff
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-10-09 11:53 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-08 6:44 [PATCH 0/2] contrib/git-resurrect.sh: make it hash-agnostic Denton Liu
2020-10-08 6:44 ` [PATCH 1/2] contrib/git-resurrect.sh: indent with tabs Denton Liu
2020-10-08 17:32 ` Junio C Hamano
2020-10-08 18:48 ` Junio C Hamano
2020-10-08 6:44 ` [PATCH 2/2] contrib/git-resurrect.sh: use hash-agnostic OID pattern Denton Liu
2020-10-08 16:13 ` Jeff King
2020-10-08 18:29 ` Junio C Hamano
2020-10-08 19:53 ` Jeff King
2020-10-08 22:11 ` brian m. carlson
2020-10-09 7:55 ` Andreas Schwab
2020-10-09 11:53 ` Jeff King
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).