Re: [RFC PATCH v2] builtin/shortlog: explicitly set hash algo when there is no repo

From: Wolfgang Müller <wolf@oriole.systems>
Date: Oct 16, 2024, 09:48 UTC
Message-ID: <w6vzuumbd5vrdluvnheescdgw36teuj6k5anlyiphvpr6slcgc@2gkwe2ye26a6>
In-Reply-To: <Zw9_46fbvbGhjmYw@pks.im>

On 2024-10-16 10:57, Patrick Steinhardt wrote:

Show 5 quoted lines

> Given that we do set `log.abbrev` I think we should be hitting code
> paths in git-shortlog(1) that use it. `git shortlog --format=%h` for
> example would use `log.abbrev`, wouldn't it? It would be nice to
> figure out whether this can be made to misbehave based on which object
> hash we have in the input.

I dove into the code again and now I'm fairly sure custom formatting is only ever done when in a repository. shortlog_output() itself, called at the end of cmd_shortlog(), doesn't do any formatting, only possibly wrapping the lines already present in the shortlog struct.

That struct is filled either by read_from_stdin() or get_from_rev(). The latter is only ever called when in a repository:

Show 9 quoted lines

>	if (!nongit && !rev.pending.nr && isatty(0))
>		add_head_to_pending(&rev);
>	if (rev.pending.nr == 0) {
>		if (isatty(0))
>			fprintf(stderr, _("(reading log message from standard input)\n"));
>		read_from_stdin(&log);
>	}
>	else
>		get_from_rev(&rev, &log);

get_from_rev() handles formatting through shortlog_add_commit(), directly formatting the commit into the oneline buffer that is later added to the shortlog struct:

Show 11 quoted lines

> 	if (!log->summary) {
> 		if (log->user_format)
> 			pretty_print_commit(&ctx, commit, &oneline);
> 		else
> 			repo_format_commit_message(the_repository, commit,
> 						   "%s", &oneline, &ctx);
> 	}
>	oneline_str = oneline.len ? oneline.buf : "<none>";
>
>	insert_records_from_trailers(log, &dups, commit, &ctx, oneline_str);
>	insert_records_from_format(log, &dups, commit, &ctx, oneline_str);

read_from_stdin(), in comparison, skips all lines from stdin not matching either "Author: " or "Commit: ", and then adds only the author/committer name and the subject to its log:

Show 16 quoted lines

>		const char *v;
>		if (!skip_prefix(ident.buf, match[0], &v) &&
>		    !skip_prefix(ident.buf, match[1], &v))
>			continue;
>		while (strbuf_getline_lf(&oneline, stdin) != EOF &&
>		       oneline.len)
>			; /* discard headers */
>		while (strbuf_getline_lf(&oneline, stdin) != EOF &&
>		       !oneline.len)
>			; /* discard blanks */
>
>		strbuf_reset(&mapped_ident);
>		if (parse_ident(log, &mapped_ident, v) < 0)
>			continue;
>
>		insert_one_record(log, mapped_ident.buf, oneline.buf);

So whilst we parse all the relevant options like --abbrev and --format, we take a shortcut through read_from_stdin() and never get to apply a custom format. Commit hashes from stdin are discarded.

I'm not sure a test case for different hash algorithms would test anything meaningful here, unless the plan in the future is to have git-shortlog(1) support formatting when reading from stdin.

-- 
Wolf