The idea is to allow callers to redirect log-tree's output to a file without having to freopen() stdout (which would modify global state, a big no-no-no for library functions). I reviewed log-tree.c, graph.c, line-log.c, builtin/shortlog.c and builtin/log.c line by line to ensure that all calls that assumed stdout previously now use the `file` field instead, of course. I would welcome additional eyes to go over the code to confirm that I did not miss anything. This patch series ends up removing the freopen() call in the format-patch command, but that is only a by-product. The ulterior motive behind this series is to allow the sequencer to write a `patch` file as part of my endeavor to move large chunks of rebase -i into a builtin. In contrast to the previous iteration of this patch series, - the use_color = 0 setting was made contingent on use_color != ALWAYS - close_file = 1 was made to work in more circumstances, most notably when calling log_commit_tree() (and in builtin/log.c, where this function is called in a loop) - the changes to builtin/am.c were backed out completely (this is a can of worms I am not prepared to open for now) - I also taught shortlog to respect the --output=<file> option, because it was so easy to do - I added a test case to ensure that `log --output=<file>` works Johannes Schindelin (10): Prepare log/log-tree to reuse the diffopt.close_file attribute log-tree: respect diffopt's configured output file stream line-log: respect diffopt's configured output file stream graph: respect the diffopt.file setting shortlog: support outputting to streams other than stdout format-patch: explicitly switch off color when writing to files format-patch: avoid freopen() format-patch: use stdout directly shortlog: respect the --output=<file> setting Ensure that log respects --output=<file> builtin/log.c | 87 +++++++++++++++++++++++++++++------------------------ builtin/shortlog.c | 15 ++++++--- graph.c | 30 ++++++++++-------- line-log.c | 34 ++++++++++----------- log-tree.c | 69 ++++++++++++++++++++++-------------------- shortlog.h | 1 + t/t4201-shortlog.sh | 6 ++++ t/t4211-line-log.sh | 7 +++++ 8 files changed, 142 insertions(+), 107 deletions(-) Published-As: https://github.com/dscho/git/releases/tag/diffopt.file-v4 Interdiff vs v3: diff --git a/builtin/am.c b/builtin/am.c index 47d78aa..3dfe70b 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1433,16 +1433,12 @@ static void get_commit_info(struct am_state *state, struct commit *commit) /** * Writes `commit` as a patch to the state directory's "patch" file. */ -static int write_commit_patch(const struct am_state *state, struct commit *commit) +static void write_commit_patch(const struct am_state *state, struct commit *commit) { struct rev_info rev_info; FILE *fp; - int res; - fp = fopen(am_path(state, "patch"), "w"); - if (!fp) - return error(_("Could not open %s for writing"), - am_path(state, "patch")); + fp = xfopen(am_path(state, "patch"), "w"); init_revisions(&rev_info, NULL); rev_info.diff = 1; rev_info.abbrev = 0; @@ -1454,11 +1450,10 @@ static int write_commit_patch(const struct am_state *state, struct commit *commi DIFF_OPT_SET(&rev_info.diffopt, FULL_INDEX); rev_info.diffopt.use_color = 0; rev_info.diffopt.file = fp; + rev_info.diffopt.close_file = 1; add_pending_object(&rev_info, &commit->object, ""); diff_setup_done(&rev_info.diffopt); - res = log_tree_commit(&rev_info, commit); - fclose(fp); - return res; + log_tree_commit(&rev_info, commit); } /** @@ -1506,14 +1501,13 @@ static int parse_mail_rebase(struct am_state *state, const char *mail) unsigned char commit_sha1[GIT_SHA1_RAWSZ]; if (get_mail_commit_sha1(commit_sha1, mail) < 0) - return error(_("could not parse %s"), mail); + die(_("could not parse %s"), mail); commit = lookup_commit_or_die(commit_sha1, mail); get_commit_info(state, commit); - if (write_commit_patch(state, commit) < 0) - return -1; + write_commit_patch(state, commit); hashcpy(state->orig_commit, commit_sha1); write_state_text(state, "original-commit", sha1_to_hex(commit_sha1)); diff --git a/builtin/log.c b/builtin/log.c index 2bfcc43..2a42216 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -243,9 +243,10 @@ static struct itimerval early_output_timer; static void log_show_early(struct rev_info *revs, struct commit_list *list) { - int i = revs->early_output; + int i = revs->early_output, close_file = revs->diffopt.close_file; int show_header = 1; + revs->diffopt.close_file = 0; sort_in_topological_order(&list, revs->sort_order); while (list && i) { struct commit *commit = list->item; @@ -262,14 +263,19 @@ static void log_show_early(struct rev_info *revs, struct commit_list *list) case commit_ignore: break; case commit_error: + if (close_file) + fclose(revs->diffopt.file); return; } list = list->next; } /* Did we already get enough commits for the early output? */ - if (!i) + if (!i) { + if (close_file) + fclose(revs->diffopt.file); return; + } /* * ..if no, then repeat it twice a second until we @@ -331,7 +337,7 @@ static int cmd_log_walk(struct rev_info *rev) { struct commit *commit; int saved_nrl = 0; - int saved_dcctc = 0; + int saved_dcctc = 0, close_file = rev->diffopt.close_file; if (rev->early_output) setup_early_output(rev); @@ -347,6 +353,7 @@ static int cmd_log_walk(struct rev_info *rev) * and HAS_CHANGES being accumulated in rev->diffopt, so be careful to * retain that state information if replacing rev->diffopt in this loop */ + rev->diffopt.close_file = 0; while ((commit = get_revision(rev)) != NULL) { if (!log_tree_commit(rev, commit) && rev->max_count >= 0) /* @@ -367,6 +374,8 @@ static int cmd_log_walk(struct rev_info *rev) } rev->diffopt.degraded_cc_to_c = saved_dcctc; rev->diffopt.needed_rename_limit = saved_nrl; + if (close_file) + fclose(rev->diffopt.file); if (rev->diffopt.output_format & DIFF_FORMAT_CHECKDIFF && DIFF_OPT_TST(&rev->diffopt, CHECK_FAILED)) { @@ -1570,7 +1579,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) setup_pager(); if (output_directory) { - rev.diffopt.use_color = 0; + if (rev.diffopt.use_color != GIT_COLOR_ALWAYS) + rev.diffopt.use_color = 0; if (use_stdout) die(_("standard output, or directory, which one?")); if (mkdir(output_directory, 0777) < 0 && errno != EEXIST) diff --git a/builtin/shortlog.c b/builtin/shortlog.c index 39d74fe..be80547 100644 --- a/builtin/shortlog.c +++ b/builtin/shortlog.c @@ -229,7 +229,6 @@ void shortlog_init(struct shortlog *log) log->wrap = DEFAULT_WRAPLEN; log->in1 = DEFAULT_INDENT1; log->in2 = DEFAULT_INDENT2; - log->file = stdout; } int cmd_shortlog(int argc, const char **argv, const char *prefix) @@ -277,6 +276,7 @@ parse_done: log.user_format = rev.commit_format == CMIT_FMT_USERFORMAT; log.abbrev = rev.abbrev; + log.file = rev.diffopt.file; /* assume HEAD if from a tty */ if (!nongit && !rev.pending.nr && isatty(0)) @@ -290,6 +290,8 @@ parse_done: get_from_rev(&rev, &log); shortlog_output(&log); + if (log.file != stdout) + fclose(log.file); return 0; } diff --git a/log-tree.c b/log-tree.c index 530297d..cf24027 100644 --- a/log-tree.c +++ b/log-tree.c @@ -862,14 +862,12 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log int log_tree_commit(struct rev_info *opt, struct commit *commit) { struct log_info log; - int shown; - - if (opt->diffopt.close_file) - die("BUG: close_file is incompatible with log_tree_commit()"); + int shown, close_file = opt->diffopt.close_file; log.commit = commit; log.parent = NULL; opt->loginfo = &log; + opt->diffopt.close_file = 0; if (opt->line_level_traverse) return line_log_print(opt, commit); @@ -886,5 +884,7 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit) fprintf(opt->diffopt.file, "\n%s\n", opt->break_bar); opt->loginfo = NULL; maybe_flush_or_die(opt->diffopt.file, "stdout"); + if (close_file) + fclose(opt->diffopt.file); return shown; } diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh index a977365..bd699e1 100755 --- a/t/t4201-shortlog.sh +++ b/t/t4201-shortlog.sh @@ -184,4 +184,10 @@ test_expect_success 'shortlog with revision pseudo options' ' git shortlog --exclude=refs/heads/m* --all ' +test_expect_success 'shortlog with --output=<file>' ' + git shortlog --output=shortlog master >output && + test ! -s output && + test_line_count = 7 shortlog +' + test_done diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh index 4451127..9d87777 100755 --- a/t/t4211-line-log.sh +++ b/t/t4211-line-log.sh @@ -99,4 +99,11 @@ test_expect_success '-L with --first-parent and a merge' ' git log --first-parent -L 1,1:b.c ' +test_expect_success '-L with --output' ' + git checkout parallel-change && + git log --output=log -L :main:b.c >output && + test ! -s output && + test_line_count = 70 log +' + test_done -- 2.9.0.118.g0e1a633 base-commit: ab7797dbe95fff38d9265869ea367020046db118 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html