format-patch: support --output option

We've never intended to support diff's --output option in format-patch. And until baa4adc66a (parse-options: disable option abbreviation with PARSE_OPT_KEEP_UNKNOWN, 2019-01-27), it was impossible to trigger. We first parse the format-patch options before handing the remainder off to setup_revisions(). Before that commit, we'd accept "--output=foo" as an abbreviation for "--output-directory=foo". But afterwards, we don't check abbreviations, and --output gets passed to the diff code. This results in nonsense behavior and bugs. The diff code will have opened a filehandle at rev.diffopt.file, but we'll overwrite that with our own handles that we open for each individual patch file. So the --output file will always just be empty. But worse, the diff code also sets rev.diffopt.close_file, so log_tree_commit() will close the filehandle itself. And then the main loop in cmd_format_patch() will try to close it again, resulting in a double-free. The simplest solution would be to just disallow --output with format-patch, as nobody ever intended it to work. However, we have accidentally documented it (because format-patch includes diff-options). And it does work with "git log", which writes the whole output to the specified file. It's easy enough to make that work for format-patch, too: it's really the same as --stdout, but pointed at a specific file. We can detect the use of the --output option by the "close_file" flag (note that we can't use rev.diffopt.file, since the diff setup will otherwise set it to stdout). So we just need to unset that flag, but don't have to do anything else. Our situation is otherwise exactly like --stdout (note that we don't fclose() the file, but nor does the stdout case; exiting the program takes care of that for us). Reported-by: Johannes Postler <johannes.postler@txture.io> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Nov 4, 2020 at 14:28 UTC dc1672dd10dbd7dbc16aa00fe51551295cc51fdc
2 files changed +32 -5
builtin/log.c
+9 -2
@@ -1942,11 +1942,18 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
1942 if (rev.show_notes)
1943 load_display_notes(&rev.notes_opt);
1944
1945 - if (use_stdout + !!output_directory > 1)
1946 - die(_("--stdout and --output-directory are mutually exclusive"));
1945 + if (use_stdout + rev.diffopt.close_file + !!output_directory > 1)
1946 + die(_("--stdout, --output, and --output-directory are mutually exclusive"));
1947
1948 if (use_stdout) {
1949 setup_pager();
1950 + } else if (rev.diffopt.close_file) {
1951 + /*
1952 + * The diff code parsed --output; it has already opened the
1953 + * file, but but we must instruct it not to close after each
1954 + * diff.
1955 + */
1956 + rev.diffopt.close_file = 0;
1957 } else {
1958 int saved;
1959
t/t4014-format-patch.sh
+23 -3
@@ -1920,18 +1920,38 @@ test_expect_success 'format-patch -o overrides format.outputDirectory' '
1920 '
1921
1922 test_expect_success 'format-patch forbids multiple outputs' '
1923 - rm -fr outdir &&
1923 + rm -fr outfile outdir &&
1924 test_must_fail \
1925 - git format-patch --stdout --output-directory=outdir
1925 + git format-patch --stdout --output-directory=outdir &&
1926 + test_must_fail \
1927 + git format-patch --stdout --output=outfile &&
1928 + test_must_fail \
1929 + git format-patch --output=outfile --output-directory=outdir
1930 '
1931
1932 test_expect_success 'configured outdir does not conflict with output options' '
1929 - rm -fr outdir &&
1933 + rm -fr outfile outdir &&
1934 test_config format.outputDirectory outdir &&
1935 git format-patch --stdout &&
1936 + test_path_is_missing outdir &&
1937 + git format-patch --output=outfile &&
1938 test_path_is_missing outdir
1939 '
1940
1941 +test_expect_success 'format-patch --output' '
1942 + rm -fr outfile &&
1943 + git format-patch -3 --stdout HEAD >expect &&
1944 + git format-patch -3 --output=outfile HEAD &&
1945 + test_cmp expect outfile
1946 +'
1947 +
1948 +test_expect_success 'format-patch --cover-letter --output' '
1949 + rm -fr outfile &&
1950 + git format-patch --cover-letter -3 --stdout HEAD >expect &&
1951 + git format-patch --cover-letter -3 --output=outfile HEAD &&
1952 + test_cmp expect outfile
1953 +'
1954 +
1955 test_expect_success 'format-patch --base' '
1956 git checkout patchid &&
1957