On Wed, Apr 15, 2020 at 08:31:39AM -0700, Junio C Hamano wrote: > "Vasil Dimov via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > > > From: Vasil Dimov <vd@xxxxxxxxxxx> > > > > `git range-diff` calls `git log` internally and tries to parse its > > output. But `git log` output can be customized by the user in their > > git config and for certain configurations either an error will be > > returned by `git range-diff` or it will crash. > > > > To fix this explicitly set the output format of the internally > > executed `git log` with `--pretty=medium`. Because that cancels > > `--notes`, add explicitly `--notes` at the end. > > Good finding. > > Shouldn't we also disable customizations that come from the > configuration variables like diff.external, diff.<driver>.command? If range-diff were a script, I would say it should be using the "rev-list | diff-tree --stdin" plumbing under the hood, rather than "log". The read_patches() function does let callers pass options to git-log, but I don't _think_ this is exposed to the user. We only allow a few --notes options to be passed, and we should be able to apply those to diff-tree. So converting it to use plumbing might be an option. Though I think there is another bug: $ git rev-list HEAD | git diff-tree --stdin --pretty=medium --notes commit 8f3d9f354286745c751374f5f1fcafee6b3f3136 git: notes.c:1308: format_display_notes: Assertion `display_notes_trees' failed. Aborted Another option would be for range-diff to directly call the revision-traversal plumbing itself. There may be complications there, though (or else it would have been done from the outset). We should fix that assertion regardless, though. -Peff
