git/sequencer.c, branch v2.40.2

Merge branch 'pw/rebase-i-parse-fix'

2023-03-01T00:38:47Z

Fixes to code that parses the todo file used in "rebase -i". * pw/rebase-i-parse-fix: rebase -i: fix parsing of "fixup -C" rebase -i: match whole word in is_command()

Merge branch 'pw/rebase-i-validate-labels-early'

2023-02-27T18:08:56Z

An invalid label or ref in the "rebase -i" todo file used to trigger an runtime error. SUch an error is now diagnosed while the todo file is parsed. * pw/rebase-i-validate-labels-early: rebase -i: check labels and refs when parsing todo list

rebase -i: fix parsing of "fixup -C"

2023-02-23T22:25:50Z

If the user omits the space between "-C" and the commit in a fixup command then it is parsed as an ordinary fixup and the commit message is not updated as it should be. Fix this by making the space between "-C" and "" optional as it is for the "merge" command. Note that set_replace_editor() is changed to set $GIT_SEQUENCE_EDITOR instead of $EDITOR in order to be able to replace the todo list and reword commits with $FAKE_COMMIT_MESSAGE. This is safe as all the existing users are using set_replace_editor() to replace the todo list. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano

rebase -i: match whole word in is_command()

2023-02-23T22:25:48Z

When matching an unabbreviated command is_command() only does a prefix match which means it parses "pickled" as TODO_PICK. parse_insn_line() does error out because is_command() only advances as far as the end of "pick" so it looks like the command name is not followed by a space but the error message is "missing arguments for pick" rather than telling the user that the "pickled" is not a valid command. Fix this by ensuring the match is follow by whitespace or the end of the string as we already do for abbreviated commands. The (*bol = p) at the end of the condition is a bit cute for my taste but I decided to leave it be for now. Rather than add new tests the existing tests for bad commands are adapted to use a bad command name that triggers the prefix matching bug. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano

Merge branch 'ab/hook-api-with-stdin'

2023-02-22T22:55:45Z

Extend the run-hooks API to allow feeding data from the standard input when running the hook script(s). * ab/hook-api-with-stdin: hook: support a --to-stdin= option sequencer: use the new hook API for the simpler "post-rewrite" call hook API: support passing stdin to hooks, convert am's 'post-rewrite' run-command: allow stdin for run_processes_parallel run-command.c: remove dead assignment in while-loop

rebase -i: check labels and refs when parsing todo list

2023-02-21T17:18:37Z

Check that the argument to the "label" and "update-ref" commands is a valid refname when the todo list is parsed rather than waiting until the command is executed. This means that the user can deal with any errors at the beginning of the rebase rather than having it stop halfway through due to a typo in a label name. The "update-ref" command is changed to reject single level refs as it is all to easy to type "update-ref branch" which is incorrect rather than "update-ref refs/heads/branch" Note that it is not straight forward to check the arguments to "reset" and "merge" commands as they may be any revision, not just a refname and we do not have an equivalent of check_refname_format() for revisions. Helped-by: Derrick Stolee Signed-off-by: Phillip Wood Acked-by: Derrick Stolee Signed-off-by: Junio C Hamano

sequencer: use the new hook API for the simpler "post-rewrite" call

2023-02-08T20:50:03Z

Change the invocation of the "post-rewrite" hook added in 795160457db (sequencer (rebase -i): run the post-rewrite hook, if needed, 2017-01-02) to use the new hook API. This leaves the more complex "post-rewrite" invocation added in a87a6f3c98e (commit: move post-rewrite code to libgit, 2017-11-17) here in sequencer.c unconverted. Here we can pass in a file's via the "in" file descriptor, in that case we don't have a file, but will need to write_in_full() to an "in" provide by the API. Support for that will be added to the hook API in the future, but we're not there yet. Signed-off-by: Emily Shaffer Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

sequencer.c: always free() the "msgbuf" in do_pick_commit()

2023-02-07T00:03:52Z

In [1] the strbuf_release(&msgbuf) was moved into this do_pick_commit(), but didn't take into account the case of [2], where we'd return before the strbuf_release(&msgbuf). Then when the "fixup" support was added in [3] this leak got worse, as in this error case we added another place where we'd "return" before reaching the strbuf_release(). This changes the behavior so that we'll call update_abort_safety_file() in these cases where we'd previously "return", but as noted in [4] "update_abort_safety_file() is a no-op when rebasing and you're changing code that is only run when rebasing.". Here "no-op" refers to the early return in update_abort_safety_file() if git_path_seq_dir() doesn't exist. 1. 452202c74b8 (sequencer: stop releasing the strbuf in write_message(), 2016-10-21) 2. f241ff0d0a9 (prepare the builtins for a libified merge_recursive(), 2016-07-26) 3. 6e98de72c03 (sequencer (rebase -i): add support for the 'fixup' and 'squash' commands, 2017-01-02) 4. https://lore.kernel.org/git/bcace50b-a4c3-c468-94a3-4fe0c62b3671@dunelm.org.uk/ Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

builtin/revert.c: move free-ing of "revs" to replay_opts_release()

2023-02-07T00:03:52Z

In [1] and [2] I added the code being moved here to cmd_revert() and cmd_cherry_pick(), now that we've got a "replay_opts_release()" for the "struct replay_opts" it should know how to free these "revs", rather than having these users reach into the struct to free its individual members. 1. d1ec656d68f (cherry-pick: free "struct replay_opts" members, 2022-11-08) 2. fd74ac95ac3 (revert: free "struct replay_opts" members, 2022-07-01) Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

sequencer API users: fix get_replay_opts() leaks

2023-02-07T00:03:52Z

Make the replay_opts_release() function added in the preceding commit non-static, and use it for freeing the "struct replay_opts" constructed for "rebase" and "revert". To safely call our new replay_opts_release() we'll need to stop calling it in sequencer_remove_state(), and instead call it where we allocate the "struct replay_opts" itself. This is because in e.g. do_interactive_rebase() we construct a "struct replay_opts" with "get_replay_opts()", and then call "complete_action()". If we get far enough in that function without encountering errors we'll call "pick_commits()" which (indirectly) calls sequencer_remove_state() at the end. But if we encounter errors anywhere along the way we'd punt out early, and not free() the memory we allocated. Remembering whether we previously called sequencer_remove_state() would be a hassle. Using a FREE_AND_NULL() pattern would also work, as it would be safe to call replay_opts_release() repeatedly. But let's fix this properly instead, by having the owner of the data free() it. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano