Hi Danny,
On Sat, 8 Apr 2017, Danny Sauer wrote:
> Make git log's `--use-mailmap` argument works if the GIT_DIR &
> GIT_WORK_TREE env vars are set and git is run from outside of work tree.
> Without the NEED_WORK_TREE set on the log subcommand, .mailmap is
> silently not found.
A laudable goal. How about adding a test case, say, to t/t4203-mailmap.sh,
to ensure that Git won't regress on your fix?
> diff --git a/git.c b/git.c
> index 8ff44f0..e147f01 100644
> --- a/git.c
> +++ b/git.c
> @@ -440,7 +440,7 @@ static struct cmd_struct commands[] = {
> { "init", cmd_init_db },
> { "init-db", cmd_init_db },
> { "interpret-trailers", cmd_interpret_trailers, RUN_SETUP_GENTLY },
> - { "log", cmd_log, RUN_SETUP },
> + { "log", cmd_log, RUN_SETUP | NEED_WORK_TREE },
> { "ls-files", cmd_ls_files, RUN_SETUP | SUPPORT_SUPER_PREFIX },
> { "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY },
> { "ls-tree", cmd_ls_tree, RUN_SETUP },
This may work for you, but it does not work for me, as I often call `git
log` in a bare repository. And that call works, and it should keep
working.
Instead, I think, you need to figure out why the .mailmap file is not read
correctly when you use the GIT_DIR & GIT_WORK_TREE approach. My vague
hunch is that you need to replace the ".mailmap" in read_mailmap()
(defined in mailmap.c):
err |= read_mailmap_file(map, ".mailmap", repo_abbrev);
by something like mkpath("%s/%s", get_git_work_tree(), ".mailmap"), but
probably only after testing that we're not in a bare repository (which
would also fix a bug, I suspect, as `git log` in a bare repository
probably heeds a .mailmap file in the current directory, which is
incorrect). I.e. something like:
if (!is_bare_repository()) {
const char *path = mkpath("%s/%s",
get_git_work_tree(), ".mailmap")
err |= read_mailmap_file(map, path, repo_abbrev);
}
But you really want to add the test case first, with
`test_expect_failure`, to demonstrate what is currently broken, and then
triumphantly setting it to `test_expect_success` with your patch.
Ciao,
Johannes
