[PATCH v3 1/1] bugreport: include +i in outfile suffix as needed

[Jacob Stopak <jacob@initialcommit.io>]

When the -s flag is absent, git bugreport includes the current hour and
minute values in the default bugreport filename (and diagnostics zip
filename if --diagnose is supplied).

If a user runs the bugreport command more than once within a minute, a
filename conflict with an existing file occurs and the program errors,
since the new output filename was already used for the previous file. If
the user waits anywhere from 1 to 60 seconds (depending on when during
the minute the first command was run) the command works again with no
error since the default filename is now unique, and multiple bug reports
are able to be created with default settings.

This is a minor thing but can cause confusion for first time users of
the bugreport command, who are likely to run it multiple times in quick
succession to learn how it works, (like I did). Or users who quickly
fill in a few details before closing and creating a new one.

Add a '+i' into the bugreport filename suffix where 'i' is an integer
starting at 1 and growing as needed until a unique filename is obtained.

This leads to default output filenames like:

git-bugreport-%Y-%m-%d-%H%M+1.txt
git-bugreport-%Y-%m-%d-%H%M+2.txt
...
git-bugreport-%Y-%m-%d-%H%M+i.txt

This means the user will end up with multiple bugreport files being
created if they run the command multiple times quickly, but that feels
more intuitive and consistent than an error arbitrarily occuring within
a minute, especially given that the time window in which the error
currently occurs is variable as described above.

If --diagnose is supplied, match the incremented suffix of the
diagnostics zip file to the bugreport.

Signed-off-by: Jacob Stopak <jacob@initialcommit.io>
---
 builtin/bugreport.c | 83 +++++++++++++++++++++++++++++++--------------
 1 file changed, 57 insertions(+), 26 deletions(-)

diff --git a/builtin/bugreport.c b/builtin/bugreport.c
index d2ae5c305d..ed65735873 100644
--- a/builtin/bugreport.c
+++ b/builtin/bugreport.c
@@ -11,6 +11,7 @@
 #include "diagnose.h"
 #include "object-file.h"
 #include "setup.h"
+#include "dir.h"
 
 static void get_system_info(struct strbuf *sys_info)
 {
@@ -97,20 +98,41 @@ static void get_header(struct strbuf *buf, const char *title)
 	strbuf_addf(buf, "\n\n[%s]\n", title);
 }
 
+static void build_path(struct strbuf *buf, const char *dir_path,
+		       const char *prefix, const char *suffix,
+		       time_t t, int *i, const char *ext)
+{
+	struct tm tm;
+
+	strbuf_reset(buf);
+	strbuf_addstr(buf, dir_path);
+	strbuf_complete(buf, '/');
+
+	strbuf_addstr(buf, prefix);
+	strbuf_addftime(buf, suffix, localtime_r(&t, &tm), 0, 0);
+
+	if (*i > 0)
+		strbuf_addf(buf, "+%d", *i);
+
+	strbuf_addstr(buf, ext);
+
+	(*i)++;
+}
+
 int cmd_bugreport(int argc, const char **argv, const char *prefix)
 {
 	struct strbuf buffer = STRBUF_INIT;
 	struct strbuf report_path = STRBUF_INIT;
 	int report = -1;
 	time_t now = time(NULL);
-	struct tm tm;
 	enum diagnose_mode diagnose = DIAGNOSE_NONE;
 	char *option_output = NULL;
-	char *option_suffix = "%Y-%m-%d-%H%M";
+	char *option_suffix = "";
+	int option_suffix_is_from_user = 0;
 	const char *user_relative_path = NULL;
 	char *prefixed_filename;
-	size_t output_path_len;
 	int ret;
+	int i = 0;
 
 	const struct option bugreport_options[] = {
 		OPT_CALLBACK_F(0, "diagnose", &diagnose, N_("mode"),
@@ -126,16 +148,16 @@ int cmd_bugreport(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, bugreport_options,
 			     bugreport_usage, 0);
 
+	if (!strlen(option_suffix))
+		option_suffix = "%Y-%m-%d-%H%M";
+	else
+		option_suffix_is_from_user = 1;
+
 	/* Prepare the path to put the result */
 	prefixed_filename = prefix_filename(prefix,
 					    option_output ? option_output : "");
-	strbuf_addstr(&report_path, prefixed_filename);
-	strbuf_complete(&report_path, '/');
-	output_path_len = report_path.len;
-
-	strbuf_addstr(&report_path, "git-bugreport-");
-	strbuf_addftime(&report_path, option_suffix, localtime_r(&now, &tm), 0, 0);
-	strbuf_addstr(&report_path, ".txt");
+	build_path(&report_path, prefixed_filename, "git-bugreport-",
+		   option_suffix, now, &i, ".txt");
 
 	switch (safe_create_leading_directories(report_path.buf)) {
 	case SCLD_OK:
@@ -146,20 +168,6 @@ int cmd_bugreport(int argc, const char **argv, const char *prefix)
 		    report_path.buf);
 	}
 
-	/* Prepare diagnostics, if requested */
-	if (diagnose != DIAGNOSE_NONE) {
-		struct strbuf zip_path = STRBUF_INIT;
-		strbuf_add(&zip_path, report_path.buf, output_path_len);
-		strbuf_addstr(&zip_path, "git-diagnostics-");
-		strbuf_addftime(&zip_path, option_suffix, localtime_r(&now, &tm), 0, 0);
-		strbuf_addstr(&zip_path, ".zip");
-
-		if (create_diagnostics_archive(&zip_path, diagnose))
-			die_errno(_("unable to create diagnostics archive %s"), zip_path.buf);
-
-		strbuf_release(&zip_path);
-	}
-
 	/* Prepare the report contents */
 	get_bug_template(&buffer);
 
@@ -169,14 +177,37 @@ int cmd_bugreport(int argc, const char **argv, const char *prefix)
 	get_header(&buffer, _("Enabled Hooks"));
 	get_populated_hooks(&buffer, !startup_info->have_repository);
 
-	/* fopen doesn't offer us an O_EXCL alternative, except with glibc. */
-	report = xopen(report_path.buf, O_CREAT | O_EXCL | O_WRONLY, 0666);
+	again:
+		/* fopen doesn't offer us an O_EXCL alternative, except with glibc. */
+		report = open(report_path.buf, O_CREAT | O_EXCL | O_WRONLY, 0666);
+		if (report < 0 && errno == EEXIST && !option_suffix_is_from_user) {
+			build_path(&report_path, prefixed_filename,
+				   "git-bugreport-", option_suffix, now, &i,
+				   ".txt");
+			goto again;
+		} else if (report < 0) {
+			die_errno(_("unable to open '%s'"), report_path.buf);
+		}
 
 	if (write_in_full(report, buffer.buf, buffer.len) < 0)
 		die_errno(_("unable to write to %s"), report_path.buf);
 
 	close(report);
 
+	/* Prepare diagnostics, if requested */
+	if (diagnose != DIAGNOSE_NONE) {
+		struct strbuf zip_path = STRBUF_INIT;
+		i--; /* Undo last increment to match zipfile suffix to bugreport */
+		build_path(&zip_path, prefixed_filename, "git-diagnostics-",
+			   option_suffix, now, &i, ".zip");
+
+		if (create_diagnostics_archive(&zip_path, diagnose))
+			die_errno(_("unable to create diagnostics archive %s"),
+				  zip_path.buf);
+
+		strbuf_release(&zip_path);
+	}
+
 	/*
 	 * We want to print the path relative to the user, but we still need the
 	 * path relative to us to give to the editor.
-- 
2.42.0.297.g36452639b8