Hi Peff
On 28/08/2023 22:47, Jeff King wrote:
In sequencer_get_last_command(), we don't ever look at the repository
parameter. It _should_ be used when calling into git_path_* functions,
but the one we use here is declared with the non-REPO variant of
GIT_PATH_FUNC(), and so just uses the_repository internally.
We could change the path helper to use REPO_GIT_PATH_FUNC(), but doing
so piecemeal is not great. There are 41 uses of GIT_PATH_FUNC() in
sequencer.c,
Wow, I knew there were quite a few but I hadn't realized there were that
many. Changing them all to take a struct repository will be a big change
and will make struct repo_cache_path a lot larger.
and inconsistently switching one makes the code more
confusing. Likewise, this one function is used in half a dozen other
spots, all of which would need to start passing in a repository argument
(with rippling effects up the call stack).
So let's punt on that for now and just silence any -Wunused-parameter
warning.
Note that we could also drop this parameter entirely, as the function is
always called directly, and not as a callback that has to conform to
some external interface. But since we'd eventually want to use the
repository parameter, let's leave it in place to avoid disrupting the
callers twice.
I think that makes sense as we're going to need that argument
eventually. I was curious as to why this function takes a repository
argument. When the function was added in 4a72486de97 (fix
cherry-pick/revert status after commit, 2019-04-16) it called
parse_insn_line() which takes a repository argument. It was refactored
in ed5b1ca10b (status: do not report errors in sequencer/todo,
2019-06-27) and I failed to notice that the repository was unused
afterwards.
Best Wishes
Phillip
Signed-off-by: Jeff King <peff@xxxxxxxx>
---
sequencer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sequencer.c b/sequencer.c
index 82dc3e160e..41fd79d215 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2649,7 +2649,7 @@ static int parse_insn_line(struct repository *r, struct todo_item *item,
return item->commit ? 0 : -1;
}
-int sequencer_get_last_command(struct repository *r, enum replay_action *action)
+int sequencer_get_last_command(struct repository *r UNUSED, enum replay_action *action)
{
const char *todo_file, *bol;
struct strbuf buf = STRBUF_INIT;
