On 3/21/2019 5:09 PM, Josh Steadmon wrote:
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);
      + }


Looks good.  Thanks.
Jeff

Reply via email to