git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] t/lib-rebase.sh: support explicit 'pick' commands in 'fake_editor.sh'

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Aug 23, 2018, 20:53 UTC
Message-ID
<nycvar.QRO.7.76.6.1808232253220.73@tvgsbejvaqbjf.bet>
In-Reply-To
<20180823100915.22855-1-szeder.dev@gmail.com>
Hi Gábor,
On Thu, 23 Aug 2018, SZEDER Gábor wrote:
Show 36 quoted lines
> The verbose output of the test 'reword without issues functions as
> intended' in 't3423-rebase-reword.sh', added in a9279c6785 (sequencer:
> do not squash 'reword' commits when we hit conflicts, 2018-06-19),
> contains the following error output:
> 
>   sed: -e expression #1, char 2: extra characters after command
> 
> This error comes from within the 'fake-editor.sh' script created by
> 'lib-rebase.sh's set_fake_editor() function, and the root cause is the
> FAKE_LINES="pick 1 reword 2" variable in the test in question, in
> particular the "pick" word.  'fake-editor.sh' assumes 'pick' to be the
> default rebase command and doesn't support an explicit 'pick' command
> in FAKE_LINES.  As a result, 'pick' will be used instead of a line
> number when assembling the following 'sed' script:
> 
>   sed -n picks/^pick/pick/p
> 
> which triggers the aforementioned error.
> 
> Luckily, this didn't affect the test's correctness: the erroring 'sed'
> command doesn't write anything to the todo script, and processing the
> rest of FAKE_LINES generates the desired todo script, as if that
> 'pick' command were not there at all.
> 
> The minimal fix would be to remove the 'pick' word from FAKE_LINES,
> but that would leave us susceptible to similar issues in the future.
> 
> Instead, teach the fake-editor script to recognize an explicit 'pick'
> command, which is still a fairly trivial change.
> 
> In the future we might want to consider reinforcing this fake editor
> script with an &&-chain and stricter parsing of the FAKE_LINES
> variable (e.g. to error out when encountering unknown rebase commands
> or commands and line numbers in the wrong order).
> 
> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
ACK!

Thank you very much, Dscho

Show 21 quoted lines
> ---
>  t/lib-rebase.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
> index 25a77ee5cb..592865d019 100644
> --- a/t/lib-rebase.sh
> +++ b/t/lib-rebase.sh
> @@ -47,7 +47,7 @@ set_fake_editor () {
>  	action=pick
>  	for line in $FAKE_LINES; do
>  		case $line in
> -		squash|fixup|edit|reword|drop)
> +		pick|squash|fixup|edit|reword|drop)
>  			action="$line";;
>  		exec*)
>  			echo "$line" | sed 's/_/ /g' >> "$1";;
> -- 
> 2.19.0.rc0.136.gd2dd172e64
> 
> 
Previous: Junio C HamanoNext: Johannes Schindelin
Message 18 of 19 in “[BUG] git-rebase: reword squashes commits in case of merge-conflicts”
  1. chJun 11, 2018
  2. Jeff KingJun 12, 2018
  3. chJun 15, 2018
  4. Elijah NewrenJun 16, 2018
  5. Eric SunshineJun 17, 2018
  6. sequencer: do not squash 'reword' commits when we hit conflictsElijah Newren, Jun 17, 2018
  7. Phillip WoodJun 17, 2018
  8. Johannes SchindelinJun 17, 2018
  9. Phillip WoodJun 18, 2018
  10. Junio C HamanoJun 18, 2018
  11. Johannes SchindelinJun 18, 2018
  12. Phillip WoodJun 19, 2018
  13. sequencer: do not squash 'reword' commits when we hit conflictsPhillip Wood, Jun 19, 2018
  14. Elijah NewrenJun 19, 2018
  15. Junio C HamanoJun 19, 2018
  16. t/lib-rebase.sh: support explicit 'pick' commands in 'fake_editor.sh'SZEDER Gábor, Aug 23, 2018
  17. Junio C HamanoAug 23, 2018
  18. Johannes SchindelinAug 23, 2018
  19. Johannes SchindelinJun 17, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.