From: Johannes Schindelin <johannes.schindelin@gmx.de>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
Eric Sunshine <sunshine@sunshineco.com>
Subject: [PATCH v4 00/10] Let log-tree and friends respect diffopt's `file` field
Date: Wed, 22 Jun 2016 17:01:22 +0200 (CEST) [thread overview]
Message-ID: <cover.1466607667.git.johannes.schindelin@gmx.de> (raw)
In-Reply-To: <cover.1466505222.git.johannes.schindelin@gmx.de>
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
next prev parent reply other threads:[~2016-06-22 15:03 UTC|newest]
Thread overview: 67+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-18 10:03 [PATCH 0/5] Let log-tree and friends respect diffopt's `file` field Johannes Schindelin
2016-06-18 10:03 ` [PATCH 1/5] log-tree: respect diffopt's configured output file stream Johannes Schindelin
2016-06-18 10:03 ` [PATCH 2/5] line-log: " Johannes Schindelin
2016-06-18 10:04 ` [PATCH 3/5] graph: respect the diffopt.file setting Johannes Schindelin
2016-06-18 10:04 ` [PATCH 4/5] shortlog: support outputting to streams other than stdout Johannes Schindelin
2016-06-18 10:04 ` [PATCH 5/5] format-patch: avoid freopen() Johannes Schindelin
2016-06-19 20:01 ` Eric Sunshine
2016-06-20 6:26 ` Johannes Schindelin
2016-06-20 6:32 ` Eric Sunshine
2016-06-20 10:09 ` Johannes Schindelin
2016-06-20 16:03 ` Junio C Hamano
2016-06-21 7:15 ` Johannes Schindelin
2016-06-21 16:50 ` Junio C Hamano
2016-06-22 7:24 ` Johannes Schindelin
2016-06-22 15:49 ` Junio C Hamano
2016-06-22 16:14 ` Johannes Schindelin
2016-06-22 17:37 ` Junio C Hamano
2016-06-22 17:53 ` Junio C Hamano
2016-06-20 10:55 ` [PATCH v2 0/7] Let log-tree and friends respect diffopt's `file` field Johannes Schindelin
2016-06-20 10:55 ` [PATCH v2 1/7] log-tree: respect diffopt's configured output file stream Johannes Schindelin
2016-06-20 17:01 ` Junio C Hamano
2016-06-21 7:31 ` Johannes Schindelin
2016-06-21 7:38 ` Johannes Schindelin
2016-06-21 10:39 ` Johannes Schindelin
2016-06-20 10:55 ` [PATCH v2 2/7] line-log: " Johannes Schindelin
2016-06-20 10:55 ` [PATCH v2 3/7] graph: respect the diffopt.file setting Johannes Schindelin
2016-06-20 10:55 ` [PATCH v2 4/7] shortlog: support outputting to streams other than stdout Johannes Schindelin
2016-06-20 10:55 ` [PATCH v2 5/7] format-patch: explicitly switch off color when writing to files Johannes Schindelin
2016-06-20 10:55 ` [PATCH v2 6/7] format-patch: avoid freopen() Johannes Schindelin
2016-06-20 10:56 ` [PATCH v2 7/7] format-patch: use stdout directly Johannes Schindelin
2016-06-20 18:57 ` Junio C Hamano
2016-06-20 11:50 ` [PATCH v2 0/7] Let log-tree and friends respect diffopt's `file` field Johannes Schindelin
2016-06-21 10:34 ` [PATCH v3 0/9] " Johannes Schindelin
2016-06-21 10:34 ` [PATCH v3 1/9] am: stop ignoring errors reported by log_tree_diff() Johannes Schindelin
2016-06-21 18:59 ` Junio C Hamano
2016-06-22 12:21 ` Johannes Schindelin
2016-06-21 10:34 ` [PATCH v3 2/9] Disallow diffopt.close_file when using the log_tree machinery Johannes Schindelin
2016-06-21 18:14 ` Junio C Hamano
2016-06-21 19:05 ` Junio C Hamano
2016-06-21 19:32 ` Junio C Hamano
2016-06-22 15:17 ` Johannes Schindelin
2016-06-21 10:34 ` [PATCH v3 3/9] log-tree: respect diffopt's configured output file stream Johannes Schindelin
2016-06-21 10:35 ` [PATCH v3 4/9] line-log: " Johannes Schindelin
2016-06-21 10:35 ` [PATCH v3 5/9] graph: respect the diffopt.file setting Johannes Schindelin
2016-06-21 10:35 ` [PATCH v3 6/9] shortlog: support outputting to streams other than stdout Johannes Schindelin
2016-06-21 10:35 ` [PATCH v3 7/9] format-patch: explicitly switch off color when writing to files Johannes Schindelin
2016-06-21 10:35 ` [PATCH v3 8/9] format-patch: avoid freopen() Johannes Schindelin
2016-06-21 10:35 ` [PATCH v3 9/9] format-patch: use stdout directly Johannes Schindelin
2016-06-21 13:47 ` [PATCH v3 0/9] Let log-tree and friends respect diffopt's `file` field Paul Tan
2016-06-21 14:12 ` Johannes Schindelin
2016-06-22 9:23 ` Paul Tan
2016-06-22 15:01 ` Johannes Schindelin [this message]
2016-06-22 15:01 ` [PATCH v4 01/10] Prepare log/log-tree to reuse the diffopt.close_file attribute Johannes Schindelin
2016-06-24 20:56 ` Junio C Hamano
2016-06-26 6:56 ` Johannes Schindelin
2016-06-22 15:01 ` [PATCH v4 02/10] log-tree: respect diffopt's configured output file stream Johannes Schindelin
2016-06-22 15:01 ` [PATCH v4 03/10] line-log: " Johannes Schindelin
2016-06-22 15:01 ` [PATCH v4 04/10] graph: respect the diffopt.file setting Johannes Schindelin
2016-06-22 15:01 ` [PATCH v4 05/10] shortlog: support outputting to streams other than stdout Johannes Schindelin
2016-06-22 15:01 ` [PATCH v4 06/10] format-patch: explicitly switch off color when writing to files Johannes Schindelin
2016-06-24 22:01 ` Junio C Hamano
2016-06-26 6:49 ` Johannes Schindelin
2016-06-22 15:01 ` [PATCH v4 07/10] format-patch: avoid freopen() Johannes Schindelin
2016-06-22 15:02 ` [PATCH v4 08/10] format-patch: use stdout directly Johannes Schindelin
2016-06-24 22:03 ` Junio C Hamano
2016-06-22 15:02 ` [PATCH v4 09/10] shortlog: respect the --output=<file> setting Johannes Schindelin
2016-06-22 15:02 ` [PATCH v4 10/10] Ensure that log respects --output=<file> Johannes Schindelin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cover.1466607667.git.johannes.schindelin@gmx.de \
--to=johannes.schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=sunshine@sunshineco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).