Hi Josh, On Thu, 3 Oct 2019, Josh Steadmon wrote: > [...] > diff --git a/trace2/tr2_dst.c b/trace2/tr2_dst.c > index 5dda0ca1cd..af3405f179 100644 > --- a/trace2/tr2_dst.c > +++ b/trace2/tr2_dst.c > @@ -1,3 +1,5 @@ > +#include <dirent.h> > + This completely breaks the Windows build: In file included from trace2/tr2_dst.c:1: compat/win32/dirent.h:13:14: error: 'MAX_PATH' undeclared here (not in a function) 13 | char d_name[MAX_PATH * 3]; /* file name (* 3 for UTF-8 conversion) */ | ^~~~~~~~ See the full build log in all its glory here: https://dev.azure.com/gitgitgadget/git/_build/results?buildId=17409&view=logs&jobId=fd490c07-0b22-5182-fac9-6d67fe1e939b&taskId=ce91d5d6-0c55-50f5-8ab9-6695c03ab102&lineStart=252&lineEnd=255&colStart=1&colEnd=30 The fix is as easy as probably surprising: simply delete that `#include`. It is redundant anyway: https://github.com/git/git/blob/v2.23.0/git-compat-util.h#L184 Deleting that `#include` line from your patch not only lets the file compile cleanly on Windows, it also conforms to our coding style where `cache.h` or `git-compat-util.h` need to be the first `#include`. That rule's purpose is precisely to prevent platform-specific compile errors like the one shown above. Ciao, Johannes > #include "cache.h" > #include "trace2/tr2_dst.h" > #include "trace2/tr2_sid.h" > @@ -8,6 +10,19 @@ > */ > #define MAX_AUTO_ATTEMPTS 10 > > +/* > + * Sentinel file used to detect when we're overloading a directory with too many > + * trace files. > + */ > +#define OVERLOAD_SENTINEL_NAME "git-trace2-overload" > + > +/* > + * When set to zero, disables directory overload checking. Otherwise, controls > + * how many files we can write to a directory before entering overload mode. > + * This can be overridden via the TR2_SYSENV_MAX_FILES setting. > + */ > +static int tr2env_max_files = 0; > + > static int tr2_dst_want_warning(void) > { > static int tr2env_dst_debug = -1; > @@ -32,6 +47,67 @@ void tr2_dst_trace_disable(struct tr2_dst *dst) > dst->need_close = 0; > } > > +/* > + * Check to make sure we're not overloading the target directory with too many > + * files. First get the threshold (if present) from the config or envvar. If > + * it's zero or unset, disable this check. Next check for the presence of a > + * sentinel file, then check file count. If we are overloaded, create the > + * sentinel file if it doesn't already exist. > + * > + * We expect that some trace processing system is gradually collecting files > + * from the target directory; after it removes the sentinel file we'll start > + * writing traces again. > + */ > +static int tr2_dst_overloaded(const char *tgt_prefix) > +{ > + int file_count = 0, max_files = 0, ret = 0; > + const char *max_files_var; > + DIR *dirp; > + struct strbuf path = STRBUF_INIT, sentinel_path = STRBUF_INIT; > + struct stat statbuf; > + > + /* Get the config or envvar and decide if we should continue this check */ > + max_files_var = tr2_sysenv_get(TR2_SYSENV_MAX_FILES); > + if (max_files_var && *max_files_var && ((max_files = atoi(max_files_var)) >= 0)) > + tr2env_max_files = max_files; > + > + if (!tr2env_max_files) { > + ret = 0; > + goto cleanup; > + } > + > + strbuf_addstr(&path, tgt_prefix); > + if (!is_dir_sep(path.buf[path.len - 1])) { > + strbuf_addch(&path, '/'); > + } > + > + /* check sentinel */ > + strbuf_addbuf(&sentinel_path, &path); > + strbuf_addstr(&sentinel_path, OVERLOAD_SENTINEL_NAME); > + if (!stat(sentinel_path.buf, &statbuf)) { > + ret = 1; > + goto cleanup; > + } > + > + /* check file count */ > + dirp = opendir(path.buf); > + while (file_count < tr2env_max_files && dirp && readdir(dirp)) > + file_count++; > + if (dirp) > + closedir(dirp); > + > + if (file_count >= tr2env_max_files) { > + creat(sentinel_path.buf, 0666); > + ret = 1; > + goto cleanup; > + } > + > +cleanup: > + strbuf_release(&path); > + strbuf_release(&sentinel_path); > + return ret; > +} > + > static int tr2_dst_try_auto_path(struct tr2_dst *dst, const char *tgt_prefix) > { > int fd; > @@ -50,6 +126,16 @@ static int tr2_dst_try_auto_path(struct tr2_dst *dst, const char *tgt_prefix) > strbuf_addstr(&path, sid); > base_path_len = path.len; > > + if (tr2_dst_overloaded(tgt_prefix)) { > + strbuf_release(&path); > + if (tr2_dst_want_warning()) > + warning("trace2: not opening %s trace file due to too " > + "many files in target directory %s", > + tr2_sysenv_display_name(dst->sysenv_var), > + tgt_prefix); > + return 0; > + } > + > for (attempt_count = 0; attempt_count < MAX_AUTO_ATTEMPTS; attempt_count++) { > if (attempt_count > 0) { > strbuf_setlen(&path, base_path_len); > diff --git a/trace2/tr2_sysenv.c b/trace2/tr2_sysenv.c > index 5958cfc424..3c3792eca2 100644 > --- a/trace2/tr2_sysenv.c > +++ b/trace2/tr2_sysenv.c > @@ -49,6 +49,9 @@ static struct tr2_sysenv_entry tr2_sysenv_settings[] = { > "trace2.perftarget" }, > [TR2_SYSENV_PERF_BRIEF] = { "GIT_TRACE2_PERF_BRIEF", > "trace2.perfbrief" }, > + > + [TR2_SYSENV_MAX_FILES] = { "GIT_TRACE2_MAX_FILES", > + "trace2.maxfiles" }, > }; > /* clang-format on */ > > diff --git a/trace2/tr2_sysenv.h b/trace2/tr2_sysenv.h > index 8dd82a7a56..d4364a7b85 100644 > --- a/trace2/tr2_sysenv.h > +++ b/trace2/tr2_sysenv.h > @@ -24,6 +24,8 @@ enum tr2_sysenv_variable { > TR2_SYSENV_PERF, > TR2_SYSENV_PERF_BRIEF, > > + TR2_SYSENV_MAX_FILES, > + > TR2_SYSENV_MUST_BE_LAST > }; > > -- > 2.23.0.581.g78d2f28ef7-goog > >
