On 13-feb-2024 11:39:11, Junio C Hamano wrote:
> Rubén Justo <rjusto@xxxxxxxxx> writes:
>
> >> This one happens to be safe currently because "git tag" passes 2 in
> >> opts->padding, but I do not think this is needed.
> >
> > At first glance, I also thought this was not necessary.
> >
> > However, callers of run_column_filter() might forget to check the return
> > value, and the BUG() triggered by the underlying process could be buried
> > and ignored. Having the BUG() here, in the same process, makes it more
> > noticeable.
>
> The point of BUG() is to help developers catch the silly breakage
> before it excapes from the lab, and we can expect these careless
> developers to ignore the return value. But "column --padding=-1"
> invoked as a subprocess will show a human-readable error message
> to such a developer, so it is less important than the BUG() in the
> other place.
Thinking again about this; you are right. That BUG() in
run_column_filter() does not make sense in this series.
It is addressing a different error, and perhaps a solution could be:
--- >8 ---
Subject: [PATCH] tag: error when git-column fails
If the user asks for the list of tags to be displayed in columns
("--columns"), a child git-column process is used to format the output
as expected.
In a rare situation where we encounter a problem spawning that child
process, we will work erroneously.
Make noticeable we're having a problem executing git-column, so the user
can act accordingly.
Signed-off-by: Rubén Justo <rjusto@xxxxxxxxx>
---
builtin/tag.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index 37473ac21f..30532b76d5 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -530,7 +530,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
struct column_options copts;
memset(&copts, 0, sizeof(copts));
copts.padding = 2;
- run_column_filter(colopts, &copts);
+ if (run_column_filter(colopts, &copts))
+ die(_("could not start 'git column'")
}
filter.name_patterns = argv;
ret = list_tags(&filter, sorting, &format);
--
2.43.0
