On Wed, Dec 11, 2024 at 02:25:03PM +1100, Mark Jamsek wrote:
> Thanks for finding this; I did test with the GOT_AUTHOR environment
> variable unset, but I have `author` set in my got.git/got.conf file!
> 
> I agree it shouldn't be a fatal error; however, I think it's better to
> explicitly check for the no author case. I'm not sure if it's a good
> idea to attempt another malloc if get_author() has already failed with
> its strdup(3) or asprintf(3) calls because of ENOMEM. So the below diffs
> produce the same behaviour as your fix except we check get_author()'s
> GOT_ERR_COMMIT_NO_AUTHOR failure case.
> 
> I've also removed the author validation code from get_author() for two
> reasons: tog's not the place to validate the author; and you've reminded
> me of my TODO note to make get_author() a library routine, but I think
> the validation should be decoupled from the retrieval. So the API will
> be something like:
> 
> got_author_retrieve(char **, struct got_repository *, struct got_worktree *);
> got_author_validate(char *);
> 
> In any case, the first commit removes valid_author(), and the second
> sets wctx->wt_author to the empty string if get_author() fails because
> we were unable to find an author.

ok