| Message ID | 20241017093528.18157-1-wolf@oriole.systems (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | b33001645e9a1b042f5bc843b76e0e8a28a91223 |
| Headers | show |
| Series | [v4] builtin/shortlog: explicitly set hash algo when there is no repo | expand |
On Thu, Oct 17, 2024 at 11:35:28AM +0200, Wolfgang Müller wrote: > builtin/shortlog.c | 12 ++++++++++++ > t/t4201-shortlog.sh | 4 ++++ > 2 files changed, 16 insertions(+) This version looks great. Thanks, Wolfgang -- let's start merging this one down. Thanks, Taylor
On 2024-10-17 16:10, Taylor Blau wrote: > This version looks great. Thanks, Wolfgang -- let's start merging this > one down. Thanks! And thanks to the community as well for the help and feedback, as well as the thorough discussion. It was fun to contribute, and I feel like I learned a lot :-)
diff --git a/builtin/shortlog.c b/builtin/shortlog.c index 3ed5c46078..c86b75d981 100644 --- a/builtin/shortlog.c +++ b/builtin/shortlog.c @@ -407,6 +407,18 @@ int cmd_shortlog(int argc, struct parse_opt_ctx_t ctx; + /* + * NEEDSWORK: Later on we'll call parse_revision_opt which relies on + * the hash algorithm being set but since we are operating outside of a + * Git repository we cannot determine one. This is only needed because + * parse_revision_opt expects hexsz for --abbrev which is irrelevant + * for shortlog outside of a git repository. For now explicitly set + * SHA1, but ideally the parsing machinery would be split between + * git/nongit so that we do not have to do this. + */ + if (nongit && !the_hash_algo) + repo_set_hash_algo(the_repository, GIT_HASH_SHA1); + git_config(git_default_config, NULL); shortlog_init(&log); repo_init_revisions(the_repository, &rev, prefix); diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh index c20c885724..5b5d3b637c 100755 --- a/t/t4201-shortlog.sh +++ b/t/t4201-shortlog.sh @@ -143,6 +143,10 @@ fuzz() test_grep "too many arguments" out ' +test_expect_success 'shortlog --author from non-git directory does not segfault' ' + nongit git shortlog --author=author </dev/null +' + test_expect_success 'shortlog should add newline when input line matches wraplen' ' cat >expect <<\EOF && A U Thor (2):
Whilst git-shortlog(1) does not explicitly need any repository information when run without reference to one, it still parses some of its arguments with parse_revision_opt() which assumes that the hash algorithm is set. However, in c8aed5e8da (repository: stop setting SHA1 as the default object hash, 2024-05-07) we stopped setting up a default hash algorithm and instead require commands to set it up explicitly. This was done for most other commands like in ab274909d4 (builtin/diff: explicitly set hash algo when there is no repo, 2024-05-07) but was missed for builtin/shortlog, making git-shortlog(1) segfault outside of a repository when given arguments like --author that trigger a call to parse_revision_opt(). Fix this for now by explicitly setting the hash algorithm to SHA1. Also add a regression test for the segfault. Thanks-to: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Wolfgang Müller <wolf@oriole.systems> --- Here's v4 with the trailers fixed and a small improvement to the test. The commit including the additional test has been dropped. Thanks! builtin/shortlog.c | 12 ++++++++++++ t/t4201-shortlog.sh | 4 ++++ 2 files changed, 16 insertions(+) Range-diff against v3: 1: 4813b458ac ! 1: 1a2959c0de builtin/shortlog: explicitly set hash algo when there is no repo @@ Commit message Fix this for now by explicitly setting the hash algorithm to SHA1. Also add a regression test for the segfault. - Signed-off-by: Wolfgang Müller <wolf@oriole.systems> Thanks-to: Eric Sunshine <sunshine@sunshineco.com> + Signed-off-by: Wolfgang Müller <wolf@oriole.systems> ## builtin/shortlog.c ## @@ builtin/shortlog.c: int cmd_shortlog(int argc, @@ t/t4201-shortlog.sh: fuzz() ' +test_expect_success 'shortlog --author from non-git directory does not segfault' ' -+ echo | nongit git shortlog --author=author ++ nongit git shortlog --author=author </dev/null +' + test_expect_success 'shortlog should add newline when input line matches wraplen' ' 2: 9dfdc7510f < -: ---------- shortlog: Test reading a log from a SHA256 repo in a non-git directory