| Message ID | xmqqsfsjuw8m.fsf@gitster.g (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | rerere-train: modernise a bit | expand |
On 2/16/2022 2:05 AM, Junio C Hamano wrote: > The script wants to create a list of merges using "rev-list" and > filters commits that do not have more than one parent, but if we > always pass "--merges" to "rev-list", there is no need to filter. > > The command uses "git show --pretty=format:..." on a single commit > while generating progress reports, which means this title line is > left unterminated. It should have used --pretty=tformat:... > instead, or better yet, use the more modern --format=... to ensure > that the title line is properly terminated. I'm unfamiliar with the rerere-train.sh script, but the changes are pretty clearly achieving what you describe here. Thanks, -Stolee
On Tue, Feb 15, 2022 at 11:05:45PM -0800, Junio C Hamano wrote: > The script wants to create a list of merges using "rev-list" and > filters commits that do not have more than one parent, but if we > always pass "--merges" to "rev-list", there is no need to filter. > > The command uses "git show --pretty=format:..." on a single commit > while generating progress reports, which means this title line is > left unterminated. It should have used --pretty=tformat:... Yep, tformat is more correct semantically, but it's worth noting that there is no behavior change here. These commands behave the same git show -s --pretty=tformat:"Learning" HEAD git show -s --pretty=format:"Learning" HEAD I guess we automagically add a final newline somewhere, if it's missing. If there is a final newline ("Learning%n"), then the commands show different behavior. The subject (%s) can never have a newline, so that's not the case here. I'd add something like this (for the lack of knowing where exactly the implicit newline comes from): No harm was done because we implicitly add the trailing newline, but it should have used --pretty=tformat:... > instead, or better yet, use the more modern --format=... to ensure > that the title line is properly terminated. > > Signed-off-by: Junio C Hamano <gitster@pobox.com> > --- > contrib/rerere-train.sh | 9 ++------- > 1 file changed, 2 insertions(+), 7 deletions(-) > > diff --git c/contrib/rerere-train.sh w/contrib/rerere-train.sh > index 75125d6ae0..499b07e4a6 100755 > --- c/contrib/rerere-train.sh > +++ w/contrib/rerere-train.sh > @@ -66,14 +66,9 @@ original_HEAD=$(git rev-parse --verify HEAD) || { > > mkdir -p "$GIT_DIR/rr-cache" || exit > > -git rev-list --parents "$@" | > +git rev-list --parents --merges "$@" | > while read commit parent1 other_parents > do > - if test -z "$other_parents" > - then > - # Skip non-merges > - continue > - fi > git checkout -q "$parent1^0" > if git merge $other_parents >/dev/null 2>&1 > then > @@ -86,7 +81,7 @@ do > fi > if test -s "$GIT_DIR/MERGE_RR" > then > - git show -s --pretty=format:"Learning from %h %s" "$commit" > + git show -s --format="Learning from %h %s" "$commit" > git rerere > git checkout -q $commit -- . > git rerere
diff --git c/contrib/rerere-train.sh w/contrib/rerere-train.sh index 75125d6ae0..499b07e4a6 100755 --- c/contrib/rerere-train.sh +++ w/contrib/rerere-train.sh @@ -66,14 +66,9 @@ original_HEAD=$(git rev-parse --verify HEAD) || { mkdir -p "$GIT_DIR/rr-cache" || exit -git rev-list --parents "$@" | +git rev-list --parents --merges "$@" | while read commit parent1 other_parents do - if test -z "$other_parents" - then - # Skip non-merges - continue - fi git checkout -q "$parent1^0" if git merge $other_parents >/dev/null 2>&1 then @@ -86,7 +81,7 @@ do fi if test -s "$GIT_DIR/MERGE_RR" then - git show -s --pretty=format:"Learning from %h %s" "$commit" + git show -s --format="Learning from %h %s" "$commit" git rerere git checkout -q $commit -- . git rerere
The script wants to create a list of merges using "rev-list" and filters commits that do not have more than one parent, but if we always pass "--merges" to "rev-list", there is no need to filter. The command uses "git show --pretty=format:..." on a single commit while generating progress reports, which means this title line is left unterminated. It should have used --pretty=tformat:... instead, or better yet, use the more modern --format=... to ensure that the title line is properly terminated. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- contrib/rerere-train.sh | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)