Subject: [PATCH v3 0/1] Write trace2 output to directories

From: Josh Steadmon <[email-masked]>
Date: Thu, 21 Mar 2019 14:09:50 -0700

Persistently enabling trace2 output is difficult because it requires
specifying a full filename. This series teaches tr2_dst_get_trace_fd()
to create files underneath a given directory provided as the target of
the GIT_TR2_* envvars.

Changes since V2:
* No longer appends a suffix to filenames on the first creation attempt.
* Renamed function & constant now that randomization is no longer used.
* Simplified strbuf usage.

Changes since V1:
* No longer supports timestamp templates.
* No longer supports filename prefixes.
* Always creates filenames based on the final component of the trace2
  SID.

Josh Steadmon (1):
  trace2: write to directory targets

 Documentation/technical/api-trace2.txt |  5 ++
 t/t0210-trace2-normal.sh               | 15 ++++++
 trace2/tr2_dst.c                       | 63 +++++++++++++++++++++++++-
 3 files changed, 81 insertions(+), 2 deletions(-)

Range-diff against v2:
1:  59d8c6511b ! 1:  ce5258610f trace2: write to directory targets
    @@ -23,8 +23,8 @@
      
     +	If the target already exists and is a directory, the traces will be
     +	written to files (one per process) underneath the given directory. They
    -+	will be named according to the last component of the SID followed by a
    -+	counter to avoid potential collisions.
    ++	will be named according to the last component of the SID (optionally
    ++	followed by a counter to avoid filename collisions).
     +
      `af_unix:[<socket_type>:]<absolute-pathname>`::
      
    @@ -37,7 +37,7 @@
      	test_cmp expect actual
      '
      
    -+test_expect_success 'randomized filename' '
    ++test_expect_success 'automatic filename' '
     +	test_when_finished "rm -r traces actual expect" &&
     +	mkdir traces &&
     +	GIT_TR2="$(pwd)/traces" test-tool trace2 001return 0 &&
    @@ -71,9 +71,9 @@
      #define TR2_ENVVAR_DST_DEBUG "GIT_TR2_DST_DEBUG"
      
     +/*
    -+ * How many attempts we will make at creating a random trace output path.
    ++ * How many attempts we will make at creating an automatically-named trace file.
     + */
    -+#define MAX_RANDOM_ATTEMPTS 10
    ++#define MAX_AUTO_ATTEMPTS 10
     +
      static int tr2_dst_want_warning(void)
      {
    @@ -82,45 +82,47 @@
      	dst->need_close = 0;
      }
      
    -+static int tr2_dst_try_random_path(struct tr2_dst *dst, const char *tgt_prefix)
    ++static int tr2_dst_try_auto_path(struct tr2_dst *dst, const char *tgt_prefix)
     +{
     +	int fd;
     +	const char *last_slash, *sid = tr2_sid_get();
    -+	struct strbuf base_path = STRBUF_INIT, final_path = STRBUF_INIT;
    ++	struct strbuf path = STRBUF_INIT;
    ++	size_t base_path_len;
     +	unsigned attempt_count;
     +
     +	last_slash = strrchr(sid, '/');
     +	if (last_slash)
     +		sid = last_slash + 1;
     +
    -+	strbuf_addstr(&base_path, tgt_prefix);
    -+	if (!is_dir_sep(base_path.buf[base_path.len - 1]))
    -+		strbuf_addch(&base_path, '/');
    -+	strbuf_addstr(&base_path, sid);
    ++	strbuf_addstr(&path, tgt_prefix);
    ++	if (!is_dir_sep(path.buf[path.len - 1]))
    ++		strbuf_addch(&path, '/');
    ++	strbuf_addstr(&path, sid);
    ++	base_path_len = path.len;
     +
    -+	for (attempt_count = 0; attempt_count < MAX_RANDOM_ATTEMPTS; attempt_count++) {
    -+		strbuf_reset(&final_path);
    -+		strbuf_addbuf(&final_path, &base_path);
    -+		strbuf_addf(&final_path, ".%d", attempt_count);
    ++	for (attempt_count = 0; attempt_count < MAX_AUTO_ATTEMPTS; attempt_count++) {
    ++		if (attempt_count > 0) {
    ++			strbuf_setlen(&path, base_path_len);
    ++			strbuf_addf(&path, ".%d", attempt_count);
    ++		}
     +
    -+		fd = open(final_path.buf, O_WRONLY | O_CREAT | O_EXCL, 0666);
    ++		fd = open(path.buf, O_WRONLY | O_CREAT | O_EXCL, 0666);
     +		if (fd != -1)
     +			break;
     +	}
     +
     +	if (fd == -1) {
     +		if (tr2_dst_want_warning())
    -+			warning("trace2: could not open '%s' for '%s' tracing: %s",
    -+				base_path.buf, dst->env_var_name, strerror(errno));
    ++			warning("trace2: could not open '%.*s' for '%s' tracing: %s",
    ++				(int) base_path_len, path.buf,
    ++				dst->env_var_name, strerror(errno));
     +
     +		tr2_dst_trace_disable(dst);
    -+		strbuf_release(&base_path);
    -+		strbuf_release(&final_path);
    ++		strbuf_release(&path);
     +		return 0;
     +	}
     +
    -+	strbuf_release(&base_path);
    -+	strbuf_release(&final_path);
    ++	strbuf_release(&path);
     +
     +	dst->fd = fd;
     +	dst->need_close = 1;
    @@ -140,7 +142,7 @@
     -		return tr2_dst_try_path(dst, tgt_value);
     +	if (is_absolute_path(tgt_value)) {
     +		if (is_directory(tgt_value))
    -+			return tr2_dst_try_random_path(dst, tgt_value);
    ++			return tr2_dst_try_auto_path(dst, tgt_value);
     +		else
     +			return tr2_dst_try_path(dst, tgt_value);
     +	}
-- 
2.21.0.225.g810b269d1ac-goog