On 2019.10.04 11:12, Johannes Schindelin wrote: > 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 Sorry about that. Fixed in V5, which will be out shortly. Is there a way to trigger the Azure pipeline apart from using GitGitGadget? > 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. Hmm, I wonder if Coccinelle could catch more CodingGuideline mistakes such as this? Although it seems there are a few exceptions to this rule, so maybe it's not a good fit in this particular case. > 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 > > > >
