| Message ID | 9204e36b7e9035c4cdda018d7ced8e8ca7acc8bc.1620758049.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Directory traversal fixes | expand |
On 5/11/21 2:34 PM, Elijah Newren via GitGitGadget wrote: > From: Elijah Newren <newren@gmail.com> > > Signed-off-by: Elijah Newren <newren@gmail.com> > --- > dir.c | 43 +++++-- > t/t7063-status-untracked-cache.sh | 205 ++++++++++++++++++------------ > t/t7519-status-fsmonitor.sh | 8 +- > 3 files changed, 155 insertions(+), 101 deletions(-) > > diff --git a/dir.c b/dir.c > index 3474e67e8f3c..122fcbffdf89 100644 > --- a/dir.c > +++ b/dir.c > @@ -2760,15 +2760,34 @@ static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *d > return root; > } > > +static void trace2_read_directory_statistics(struct dir_struct *dir, > + struct repository *repo, > + const char *path) > +{ > + if (!dir->untracked) > + return; > + trace2_data_string("read_directory", repo, "path", path); I'm probably just nit-picking here, but should this look more like: if (path && *path) trace2_data_string(...) if (!dir->untracked) return; Then when you add the visitied fields in the next commit, you'll have the path with them (when present). (and it would let you optionally avoid the tmp strbuf in the caller.) Jeff
On Tue, May 11, 2021 at 12:06 PM Jeff Hostetler <git@jeffhostetler.com> wrote: > > On 5/11/21 2:34 PM, Elijah Newren via GitGitGadget wrote: > > From: Elijah Newren <newren@gmail.com> > > > > Signed-off-by: Elijah Newren <newren@gmail.com> > > --- > > dir.c | 43 +++++-- > > t/t7063-status-untracked-cache.sh | 205 ++++++++++++++++++------------ > > t/t7519-status-fsmonitor.sh | 8 +- > > 3 files changed, 155 insertions(+), 101 deletions(-) > > > > diff --git a/dir.c b/dir.c > > index 3474e67e8f3c..122fcbffdf89 100644 > > --- a/dir.c > > +++ b/dir.c > > @@ -2760,15 +2760,34 @@ static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *d > > return root; > > } > > > > +static void trace2_read_directory_statistics(struct dir_struct *dir, > > + struct repository *repo, > > + const char *path) > > +{ > > + if (!dir->untracked) > > + return; > > + trace2_data_string("read_directory", repo, "path", path); > > I'm probably just nit-picking here, but should this look more like: nit-picking and questions are totally fine. :-) Thanks for reviewing. > > if (path && *path) > trace2_data_string(...) path is always non-NULL (it'd be an error to call read_directory() with a NULL path). So the first part of the check isn't meaningful for this particular code. The second half is interesting. Do we want to omit the path when it happens to be the toplevel directory (the case where !*path)? The original trace_performance_leave() calls certainly didn't, and I was just trying to provide the same info they do, as you suggested. I guess people could determine the path by knowing that the code doesn't print it when it's empty, but do we want trace2 users to need to read the code to figure out statistics and info? > if (!dir->untracked) > return; > > Then when you add the visitied fields in the next commit, > you'll have the path with them (when present). There is always a path with them, it's just that the empty string denotes the toplevel directory. > (and it would let you optionally avoid the tmp strbuf in > the caller.) The path in read_directory() is not necessarily NUL-delimited, so attempting to use it as-is, or even with your checks, would cause us to possibly print garbage and do out-of-bounds reads. We need the tmp strbuf.
On 5/11/21 4:12 PM, Elijah Newren wrote: > On Tue, May 11, 2021 at 12:06 PM Jeff Hostetler <git@jeffhostetler.com> wrote: >> >> On 5/11/21 2:34 PM, Elijah Newren via GitGitGadget wrote: >>> From: Elijah Newren <newren@gmail.com> >>> >>> Signed-off-by: Elijah Newren <newren@gmail.com> >>> --- >>> dir.c | 43 +++++-- >>> t/t7063-status-untracked-cache.sh | 205 ++++++++++++++++++------------ >>> t/t7519-status-fsmonitor.sh | 8 +- >>> 3 files changed, 155 insertions(+), 101 deletions(-) >>> >>> diff --git a/dir.c b/dir.c >>> index 3474e67e8f3c..122fcbffdf89 100644 >>> --- a/dir.c >>> +++ b/dir.c >>> @@ -2760,15 +2760,34 @@ static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *d >>> return root; >>> } >>> >>> +static void trace2_read_directory_statistics(struct dir_struct *dir, >>> + struct repository *repo, >>> + const char *path) >>> +{ >>> + if (!dir->untracked) >>> + return; >>> + trace2_data_string("read_directory", repo, "path", path); >> >> I'm probably just nit-picking here, but should this look more like: > > nit-picking and questions are totally fine. :-) Thanks for reviewing. > >> >> if (path && *path) >> trace2_data_string(...) > > path is always non-NULL (it'd be an error to call read_directory() > with a NULL path). So the first part of the check isn't meaningful > for this particular code. The second half is interesting. Do we want > to omit the path when it happens to be the toplevel directory (the > case where !*path)? The original trace_performance_leave() calls > certainly didn't, and I was just trying to provide the same info they > do, as you suggested. I guess people could determine the path by > knowing that the code doesn't print it when it's empty, but do we want > trace2 users to need to read the code to figure out statistics and > info? that's fine. it might be easier to just always print it (even if blank) so that post-processors know that rather than have to assume it. > >> if (!dir->untracked) >> return; >> >> Then when you add the visitied fields in the next commit, >> you'll have the path with them (when present). > > There is always a path with them, it's just that the empty string > denotes the toplevel directory. > >> (and it would let you optionally avoid the tmp strbuf in >> the caller.) > > The path in read_directory() is not necessarily NUL-delimited, so > attempting to use it as-is, or even with your checks, would cause us > to possibly print garbage and do out-of-bounds reads. We need the tmp > strbuf. > I just meant, "if (!len) pass NULL, else build and pass tmp.buf". but i'm nit-picking again. Jeff
On Tue, May 11, 2021 at 4:12 PM Jeff Hostetler <git@jeffhostetler.com> wrote: > > On 5/11/21 4:12 PM, Elijah Newren wrote: > > On Tue, May 11, 2021 at 12:06 PM Jeff Hostetler <git@jeffhostetler.com> wrote: > >> > >> On 5/11/21 2:34 PM, Elijah Newren via GitGitGadget wrote: > >>> From: Elijah Newren <newren@gmail.com> > >>> > >>> Signed-off-by: Elijah Newren <newren@gmail.com> > >>> --- > >>> dir.c | 43 +++++-- > >>> t/t7063-status-untracked-cache.sh | 205 ++++++++++++++++++------------ > >>> t/t7519-status-fsmonitor.sh | 8 +- > >>> 3 files changed, 155 insertions(+), 101 deletions(-) > >>> > >>> diff --git a/dir.c b/dir.c > >>> index 3474e67e8f3c..122fcbffdf89 100644 > >>> --- a/dir.c > >>> +++ b/dir.c > >>> @@ -2760,15 +2760,34 @@ static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *d > >>> return root; > >>> } > >>> > >>> +static void trace2_read_directory_statistics(struct dir_struct *dir, > >>> + struct repository *repo, > >>> + const char *path) > >>> +{ > >>> + if (!dir->untracked) > >>> + return; > >>> + trace2_data_string("read_directory", repo, "path", path); > >> > >> I'm probably just nit-picking here, but should this look more like: > > > > nit-picking and questions are totally fine. :-) Thanks for reviewing. > > > >> > >> if (path && *path) > >> trace2_data_string(...) > > > > path is always non-NULL (it'd be an error to call read_directory() > > with a NULL path). So the first part of the check isn't meaningful > > for this particular code. The second half is interesting. Do we want > > to omit the path when it happens to be the toplevel directory (the > > case where !*path)? The original trace_performance_leave() calls > > certainly didn't, and I was just trying to provide the same info they > > do, as you suggested. I guess people could determine the path by > > knowing that the code doesn't print it when it's empty, but do we want > > trace2 users to need to read the code to figure out statistics and > > info? > > that's fine. it might be easier to just always print it (even if > blank) so that post-processors know that rather than have to assume > it. > > > > >> if (!dir->untracked) > >> return; > >> > >> Then when you add the visitied fields in the next commit, > >> you'll have the path with them (when present). > > > > There is always a path with them, it's just that the empty string > > denotes the toplevel directory. > > > >> (and it would let you optionally avoid the tmp strbuf in > >> the caller.) > > > > The path in read_directory() is not necessarily NUL-delimited, so > > attempting to use it as-is, or even with your checks, would cause us > > to possibly print garbage and do out-of-bounds reads. We need the tmp > > strbuf. > > > > I just meant, "if (!len) pass NULL, else build and pass tmp.buf". Ah, gotcha, that's why you were checking non-NULL. However, what about the other case when len is nonzero. Let's say that len = 8 and path points at "filename*%&#)aWholeBunchOfTotalGarbageAfterTheRealFilenameThatShouldNotBeReadOrIncluded\0\0\0\0\0\0\0\0\0\0" ? How do you make it print "filename" and only "filename" without the other stuff without using the tmp strbuf?
On 5/11/21 8:44 PM, Elijah Newren wrote: > On Tue, May 11, 2021 at 4:12 PM Jeff Hostetler <git@jeffhostetler.com> wrote: >> >> On 5/11/21 4:12 PM, Elijah Newren wrote: >>> On Tue, May 11, 2021 at 12:06 PM Jeff Hostetler <git@jeffhostetler.com> wrote: >>>> >>>> On 5/11/21 2:34 PM, Elijah Newren via GitGitGadget wrote: >>>>> From: Elijah Newren <newren@gmail.com> >>>>> >>>>> Signed-off-by: Elijah Newren <newren@gmail.com> >>>>> --- >>>>> dir.c | 43 +++++-- >>>>> t/t7063-status-untracked-cache.sh | 205 ++++++++++++++++++------------ >>>>> t/t7519-status-fsmonitor.sh | 8 +- >>>>> 3 files changed, 155 insertions(+), 101 deletions(-) >>>>> >>>>> diff --git a/dir.c b/dir.c >>>>> index 3474e67e8f3c..122fcbffdf89 100644 >>>>> --- a/dir.c >>>>> +++ b/dir.c >>>>> @@ -2760,15 +2760,34 @@ static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *d >>>>> return root; >>>>> } >>>>> >>>>> +static void trace2_read_directory_statistics(struct dir_struct *dir, >>>>> + struct repository *repo, >>>>> + const char *path) >>>>> +{ >>>>> + if (!dir->untracked) >>>>> + return; >>>>> + trace2_data_string("read_directory", repo, "path", path); >>>> >>>> I'm probably just nit-picking here, but should this look more like: >>> >>> nit-picking and questions are totally fine. :-) Thanks for reviewing. >>> >>>> >>>> if (path && *path) >>>> trace2_data_string(...) >>> >>> path is always non-NULL (it'd be an error to call read_directory() >>> with a NULL path). So the first part of the check isn't meaningful >>> for this particular code. The second half is interesting. Do we want >>> to omit the path when it happens to be the toplevel directory (the >>> case where !*path)? The original trace_performance_leave() calls >>> certainly didn't, and I was just trying to provide the same info they >>> do, as you suggested. I guess people could determine the path by >>> knowing that the code doesn't print it when it's empty, but do we want >>> trace2 users to need to read the code to figure out statistics and >>> info? >> >> that's fine. it might be easier to just always print it (even if >> blank) so that post-processors know that rather than have to assume >> it. >> >>> >>>> if (!dir->untracked) >>>> return; >>>> >>>> Then when you add the visitied fields in the next commit, >>>> you'll have the path with them (when present). >>> >>> There is always a path with them, it's just that the empty string >>> denotes the toplevel directory. >>> >>>> (and it would let you optionally avoid the tmp strbuf in >>>> the caller.) >>> >>> The path in read_directory() is not necessarily NUL-delimited, so >>> attempting to use it as-is, or even with your checks, would cause us >>> to possibly print garbage and do out-of-bounds reads. We need the tmp >>> strbuf. >>> >> >> I just meant, "if (!len) pass NULL, else build and pass tmp.buf". > > Ah, gotcha, that's why you were checking non-NULL. > > However, what about the other case when len is nonzero. Let's say > that len = 8 and path points at > "filename*%&#)aWholeBunchOfTotalGarbageAfterTheRealFilenameThatShouldNotBeReadOrIncluded\0\0\0\0\0\0\0\0\0\0" > ? > > How do you make it print "filename" and only "filename" without the > other stuff without using the tmp strbuf? > I was still saying to use the "strbuf tmp" in the non-zero len case, but just pass NULL (or "") for the len==0 case. Alternatively, since `trace2_read_directory_statistics() a static local function, we could move all of the path manipulation into it. static void emit_stats( struct dir_struct *dir, struct repository *repo, const char* path_buf, size_t path_len) { if (!path_len) trace2_data_string("read_directory", repo, "path", ""); else { struct strbuf tmp = STRBUF_INIT; strbuf_add(&tmp, path_buf, path_len); trace2_data_string("read_directory", repo, "path", tmp.buf); strbuf_release(&tmp); } ... the rest of intmax stats ... } BTW, could we also rename your stats function? I've been trying to keep the "trace2_" prefix reserved for the Trace2 API. Thanks, Jeff
On Wed, May 12, 2021 at 5:26 AM Jeff Hostetler <git@jeffhostetler.com> wrote: > > On 5/11/21 8:44 PM, Elijah Newren wrote: > > On Tue, May 11, 2021 at 4:12 PM Jeff Hostetler <git@jeffhostetler.com> wrote: > >> > >> On 5/11/21 4:12 PM, Elijah Newren wrote: > >>> On Tue, May 11, 2021 at 12:06 PM Jeff Hostetler <git@jeffhostetler.com> wrote: > >>>> > >>>> On 5/11/21 2:34 PM, Elijah Newren via GitGitGadget wrote: > >>>>> From: Elijah Newren <newren@gmail.com> > >>>>> > >>>>> Signed-off-by: Elijah Newren <newren@gmail.com> > >>>>> --- > >>>>> dir.c | 43 +++++-- > >>>>> t/t7063-status-untracked-cache.sh | 205 ++++++++++++++++++------------ > >>>>> t/t7519-status-fsmonitor.sh | 8 +- > >>>>> 3 files changed, 155 insertions(+), 101 deletions(-) > >>>>> > >>>>> diff --git a/dir.c b/dir.c > >>>>> index 3474e67e8f3c..122fcbffdf89 100644 > >>>>> --- a/dir.c > >>>>> +++ b/dir.c > >>>>> @@ -2760,15 +2760,34 @@ static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *d > >>>>> return root; > >>>>> } > >>>>> > >>>>> +static void trace2_read_directory_statistics(struct dir_struct *dir, > >>>>> + struct repository *repo, > >>>>> + const char *path) > >>>>> +{ > >>>>> + if (!dir->untracked) > >>>>> + return; > >>>>> + trace2_data_string("read_directory", repo, "path", path); > >>>> > >>>> I'm probably just nit-picking here, but should this look more like: > >>> > >>> nit-picking and questions are totally fine. :-) Thanks for reviewing. > >>> > >>>> > >>>> if (path && *path) > >>>> trace2_data_string(...) > >>> > >>> path is always non-NULL (it'd be an error to call read_directory() > >>> with a NULL path). So the first part of the check isn't meaningful > >>> for this particular code. The second half is interesting. Do we want > >>> to omit the path when it happens to be the toplevel directory (the > >>> case where !*path)? The original trace_performance_leave() calls > >>> certainly didn't, and I was just trying to provide the same info they > >>> do, as you suggested. I guess people could determine the path by > >>> knowing that the code doesn't print it when it's empty, but do we want > >>> trace2 users to need to read the code to figure out statistics and > >>> info? > >> > >> that's fine. it might be easier to just always print it (even if > >> blank) so that post-processors know that rather than have to assume > >> it. > >> > >>> > >>>> if (!dir->untracked) > >>>> return; > >>>> > >>>> Then when you add the visitied fields in the next commit, > >>>> you'll have the path with them (when present). > >>> > >>> There is always a path with them, it's just that the empty string > >>> denotes the toplevel directory. > >>> > >>>> (and it would let you optionally avoid the tmp strbuf in > >>>> the caller.) > >>> > >>> The path in read_directory() is not necessarily NUL-delimited, so > >>> attempting to use it as-is, or even with your checks, would cause us > >>> to possibly print garbage and do out-of-bounds reads. We need the tmp > >>> strbuf. > >>> > >> > >> I just meant, "if (!len) pass NULL, else build and pass tmp.buf". > > > > Ah, gotcha, that's why you were checking non-NULL. > > > > However, what about the other case when len is nonzero. Let's say > > that len = 8 and path points at > > "filename*%&#)aWholeBunchOfTotalGarbageAfterTheRealFilenameThatShouldNotBeReadOrIncluded\0\0\0\0\0\0\0\0\0\0" > > ? > > > > How do you make it print "filename" and only "filename" without the > > other stuff without using the tmp strbuf? > > > > I was still saying to use the "strbuf tmp" in the non-zero len case, > but just pass NULL (or "") for the len==0 case. Ah, now I see what you were saying. Sorry for not getting it earlier. > Alternatively, since `trace2_read_directory_statistics() a static > local function, we could move all of the path manipulation into it. > > static void emit_stats( > struct dir_struct *dir, > struct repository *repo, > const char* path_buf, > size_t path_len) > { > if (!path_len) > trace2_data_string("read_directory", repo, > "path", ""); > else { > struct strbuf tmp = STRBUF_INIT; > strbuf_add(&tmp, path_buf, path_len); > trace2_data_string("read_directory", repo, > "path", tmp.buf); > strbuf_release(&tmp); > } > ... the rest of intmax stats ... > } Makes sense. > BTW, could we also rename your stats function? I've been trying > to keep the "trace2_" prefix reserved for the Trace2 API. Sure, will do.
diff --git a/dir.c b/dir.c index 3474e67e8f3c..122fcbffdf89 100644 --- a/dir.c +++ b/dir.c @@ -2760,15 +2760,34 @@ static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *d return root; } +static void trace2_read_directory_statistics(struct dir_struct *dir, + struct repository *repo, + const char *path) +{ + if (!dir->untracked) + return; + trace2_data_string("read_directory", repo, "path", path); + trace2_data_intmax("read_directory", repo, + "node-creation", dir->untracked->dir_created); + trace2_data_intmax("read_directory", repo, + "gitignore-invalidation", + dir->untracked->gitignore_invalidated); + trace2_data_intmax("read_directory", repo, + "directory-invalidation", + dir->untracked->dir_invalidated); + trace2_data_intmax("read_directory", repo, + "opendir", dir->untracked->dir_opened); +} + int read_directory(struct dir_struct *dir, struct index_state *istate, const char *path, int len, const struct pathspec *pathspec) { struct untracked_cache_dir *untracked; - trace_performance_enter(); + trace2_region_enter("dir", "read_directory", istate->repo); if (has_symlink_leading_path(path, len)) { - trace_performance_leave("read directory %.*s", len, path); + trace2_region_leave("dir", "read_directory", istate->repo); return dir->nr; } @@ -2784,23 +2803,20 @@ int read_directory(struct dir_struct *dir, struct index_state *istate, QSORT(dir->entries, dir->nr, cmp_dir_entry); QSORT(dir->ignored, dir->ignored_nr, cmp_dir_entry); - trace_performance_leave("read directory %.*s", len, path); + if (trace2_is_enabled()) { + struct strbuf tmp = STRBUF_INIT; + strbuf_add(&tmp, path, len); + trace2_read_directory_statistics(dir, istate->repo, tmp.buf); + strbuf_release(&tmp); + } + + trace2_region_leave("dir", "read_directory", istate->repo); if (dir->untracked) { static int force_untracked_cache = -1; - static struct trace_key trace_untracked_stats = TRACE_KEY_INIT(UNTRACKED_STATS); if (force_untracked_cache < 0) force_untracked_cache = git_env_bool("GIT_FORCE_UNTRACKED_CACHE", 0); - trace_printf_key(&trace_untracked_stats, - "node creation: %u\n" - "gitignore invalidation: %u\n" - "directory invalidation: %u\n" - "opendir: %u\n", - dir->untracked->dir_created, - dir->untracked->gitignore_invalidated, - dir->untracked->dir_invalidated, - dir->untracked->dir_opened); if (force_untracked_cache && dir->untracked == istate->untracked && (dir->untracked->dir_opened || @@ -2811,6 +2827,7 @@ int read_directory(struct dir_struct *dir, struct index_state *istate, FREE_AND_NULL(dir->untracked); } } + return dir->nr; } diff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh index accefde72fb1..9710d33b3cd6 100755 --- a/t/t7063-status-untracked-cache.sh +++ b/t/t7063-status-untracked-cache.sh @@ -57,6 +57,19 @@ iuc () { return $ret } +get_relevant_traces () { + # From the GIT_TRACE2_PERF data of the form + # $TIME $FILE:$LINE | d0 | main | data | r1 | ? | ? | read_directo | $RELEVANT_STAT + # extract the $RELEVANT_STAT fields. We don't care about region_enter + # or region_leave, or stats for things outside read_directory. + INPUT_FILE=$1 + OUTPUT_FILE=$2 + grep data.*read_directo $INPUT_FILE | + cut -d "|" -f 9 \ + >"$OUTPUT_FILE" +} + + test_lazy_prereq UNTRACKED_CACHE ' { git update-index --test-untracked-cache; ret=$?; } && test $ret -ne 1 @@ -129,19 +142,21 @@ EOF test_expect_success 'status first time (empty cache)' ' avoid_racy && - : >../trace && - GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace" \ + : >../trace.output && + GIT_TRACE2_PERF="$TRASH_DIRECTORY/trace.output" \ git status --porcelain >../actual && iuc status --porcelain >../status.iuc && test_cmp ../status.expect ../status.iuc && test_cmp ../status.expect ../actual && + get_relevant_traces ../trace.output ../trace.relevant && cat >../trace.expect <<EOF && -node creation: 3 -gitignore invalidation: 1 -directory invalidation: 0 -opendir: 4 + ....path: + ....node-creation:3 + ....gitignore-invalidation:1 + ....directory-invalidation:0 + ....opendir:4 EOF - test_cmp ../trace.expect ../trace + test_cmp ../trace.expect ../trace.relevant ' test_expect_success 'untracked cache after first status' ' @@ -151,19 +166,21 @@ test_expect_success 'untracked cache after first status' ' test_expect_success 'status second time (fully populated cache)' ' avoid_racy && - : >../trace && - GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace" \ + : >../trace.output && + GIT_TRACE2_PERF="$TRASH_DIRECTORY/trace.output" \ git status --porcelain >../actual && iuc status --porcelain >../status.iuc && test_cmp ../status.expect ../status.iuc && test_cmp ../status.expect ../actual && + get_relevant_traces ../trace.output ../trace.relevant && cat >../trace.expect <<EOF && -node creation: 0 -gitignore invalidation: 0 -directory invalidation: 0 -opendir: 0 + ....path: + ....node-creation:0 + ....gitignore-invalidation:0 + ....directory-invalidation:0 + ....opendir:0 EOF - test_cmp ../trace.expect ../trace + test_cmp ../trace.expect ../trace.relevant ' test_expect_success 'untracked cache after second status' ' @@ -174,8 +191,8 @@ test_expect_success 'untracked cache after second status' ' test_expect_success 'modify in root directory, one dir invalidation' ' avoid_racy && : >four && - : >../trace && - GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace" \ + : >../trace.output && + GIT_TRACE2_PERF="$TRASH_DIRECTORY/trace.output" \ git status --porcelain >../actual && iuc status --porcelain >../status.iuc && cat >../status.expect <<EOF && @@ -189,13 +206,15 @@ A two EOF test_cmp ../status.expect ../status.iuc && test_cmp ../status.expect ../actual && + get_relevant_traces ../trace.output ../trace.relevant && cat >../trace.expect <<EOF && -node creation: 0 -gitignore invalidation: 0 -directory invalidation: 1 -opendir: 1 + ....path: + ....node-creation:0 + ....gitignore-invalidation:0 + ....directory-invalidation:1 + ....opendir:1 EOF - test_cmp ../trace.expect ../trace + test_cmp ../trace.expect ../trace.relevant ' @@ -223,8 +242,8 @@ EOF test_expect_success 'new .gitignore invalidates recursively' ' avoid_racy && echo four >.gitignore && - : >../trace && - GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace" \ + : >../trace.output && + GIT_TRACE2_PERF="$TRASH_DIRECTORY/trace.output" \ git status --porcelain >../actual && iuc status --porcelain >../status.iuc && cat >../status.expect <<EOF && @@ -238,13 +257,15 @@ A two EOF test_cmp ../status.expect ../status.iuc && test_cmp ../status.expect ../actual && + get_relevant_traces ../trace.output ../trace.relevant && cat >../trace.expect <<EOF && -node creation: 0 -gitignore invalidation: 1 -directory invalidation: 1 -opendir: 4 + ....path: + ....node-creation:0 + ....gitignore-invalidation:1 + ....directory-invalidation:1 + ....opendir:4 EOF - test_cmp ../trace.expect ../trace + test_cmp ../trace.expect ../trace.relevant ' @@ -272,8 +293,8 @@ EOF test_expect_success 'new info/exclude invalidates everything' ' avoid_racy && echo three >>.git/info/exclude && - : >../trace && - GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace" \ + : >../trace.output && + GIT_TRACE2_PERF="$TRASH_DIRECTORY/trace.output" \ git status --porcelain >../actual && iuc status --porcelain >../status.iuc && cat >../status.expect <<EOF && @@ -285,13 +306,15 @@ A two EOF test_cmp ../status.expect ../status.iuc && test_cmp ../status.expect ../actual && + get_relevant_traces ../trace.output ../trace.relevant && cat >../trace.expect <<EOF && -node creation: 0 -gitignore invalidation: 1 -directory invalidation: 0 -opendir: 4 + ....path: + ....node-creation:0 + ....gitignore-invalidation:1 + ....directory-invalidation:0 + ....opendir:4 EOF - test_cmp ../trace.expect ../trace + test_cmp ../trace.expect ../trace.relevant ' test_expect_success 'verify untracked cache dump' ' @@ -330,8 +353,8 @@ EOF ' test_expect_success 'status after the move' ' - : >../trace && - GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace" \ + : >../trace.output && + GIT_TRACE2_PERF="$TRASH_DIRECTORY/trace.output" \ git status --porcelain >../actual && iuc status --porcelain >../status.iuc && cat >../status.expect <<EOF && @@ -343,13 +366,15 @@ A one EOF test_cmp ../status.expect ../status.iuc && test_cmp ../status.expect ../actual && + get_relevant_traces ../trace.output ../trace.relevant && cat >../trace.expect <<EOF && -node creation: 0 -gitignore invalidation: 0 -directory invalidation: 0 -opendir: 1 + ....path: + ....node-creation:0 + ....gitignore-invalidation:0 + ....directory-invalidation:0 + ....opendir:1 EOF - test_cmp ../trace.expect ../trace + test_cmp ../trace.expect ../trace.relevant ' test_expect_success 'verify untracked cache dump' ' @@ -389,8 +414,8 @@ EOF ' test_expect_success 'status after the move' ' - : >../trace && - GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace" \ + : >../trace.output && + GIT_TRACE2_PERF="$TRASH_DIRECTORY/trace.output" \ git status --porcelain >../actual && iuc status --porcelain >../status.iuc && cat >../status.expect <<EOF && @@ -402,13 +427,15 @@ A two EOF test_cmp ../status.expect ../status.iuc && test_cmp ../status.expect ../actual && + get_relevant_traces ../trace.output ../trace.relevant && cat >../trace.expect <<EOF && -node creation: 0 -gitignore invalidation: 0 -directory invalidation: 0 -opendir: 1 + ....path: + ....node-creation:0 + ....gitignore-invalidation:0 + ....directory-invalidation:0 + ....opendir:1 EOF - test_cmp ../trace.expect ../trace + test_cmp ../trace.expect ../trace.relevant ' test_expect_success 'verify untracked cache dump' ' @@ -438,8 +465,8 @@ test_expect_success 'set up for sparse checkout testing' ' ' test_expect_success 'status after commit' ' - : >../trace && - GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace" \ + : >../trace.output && + GIT_TRACE2_PERF="$TRASH_DIRECTORY/trace.output" \ git status --porcelain >../actual && iuc status --porcelain >../status.iuc && cat >../status.expect <<EOF && @@ -448,13 +475,15 @@ test_expect_success 'status after commit' ' EOF test_cmp ../status.expect ../status.iuc && test_cmp ../status.expect ../actual && + get_relevant_traces ../trace.output ../trace.relevant && cat >../trace.expect <<EOF && -node creation: 0 -gitignore invalidation: 0 -directory invalidation: 0 -opendir: 2 + ....path: + ....node-creation:0 + ....gitignore-invalidation:0 + ....directory-invalidation:0 + ....opendir:2 EOF - test_cmp ../trace.expect ../trace + test_cmp ../trace.expect ../trace.relevant ' test_expect_success 'untracked cache correct after commit' ' @@ -496,9 +525,9 @@ test_expect_success 'create/modify files, some of which are gitignored' ' ' test_expect_success 'test sparse status with untracked cache' ' - : >../trace && + : >../trace.output && avoid_racy && - GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace" \ + GIT_TRACE2_PERF="$TRASH_DIRECTORY/trace.output" \ git status --porcelain >../status.actual && iuc status --porcelain >../status.iuc && cat >../status.expect <<EOF && @@ -509,13 +538,15 @@ test_expect_success 'test sparse status with untracked cache' ' EOF test_cmp ../status.expect ../status.iuc && test_cmp ../status.expect ../status.actual && + get_relevant_traces ../trace.output ../trace.relevant && cat >../trace.expect <<EOF && -node creation: 0 -gitignore invalidation: 1 -directory invalidation: 2 -opendir: 2 + ....path: + ....node-creation:0 + ....gitignore-invalidation:1 + ....directory-invalidation:2 + ....opendir:2 EOF - test_cmp ../trace.expect ../trace + test_cmp ../trace.expect ../trace.relevant ' test_expect_success 'untracked cache correct after status' ' @@ -539,8 +570,8 @@ EOF test_expect_success 'test sparse status again with untracked cache' ' avoid_racy && - : >../trace && - GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace" \ + : >../trace.output && + GIT_TRACE2_PERF="$TRASH_DIRECTORY/trace.output" \ git status --porcelain >../status.actual && iuc status --porcelain >../status.iuc && cat >../status.expect <<EOF && @@ -551,13 +582,15 @@ test_expect_success 'test sparse status again with untracked cache' ' EOF test_cmp ../status.expect ../status.iuc && test_cmp ../status.expect ../status.actual && + get_relevant_traces ../trace.output ../trace.relevant && cat >../trace.expect <<EOF && -node creation: 0 -gitignore invalidation: 0 -directory invalidation: 0 -opendir: 0 + ....path: + ....node-creation:0 + ....gitignore-invalidation:0 + ....directory-invalidation:0 + ....opendir:0 EOF - test_cmp ../trace.expect ../trace + test_cmp ../trace.expect ../trace.relevant ' test_expect_success 'set up for test of subdir and sparse checkouts' ' @@ -568,8 +601,8 @@ test_expect_success 'set up for test of subdir and sparse checkouts' ' test_expect_success 'test sparse status with untracked cache and subdir' ' avoid_racy && - : >../trace && - GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace" \ + : >../trace.output && + GIT_TRACE2_PERF="$TRASH_DIRECTORY/trace.output" \ git status --porcelain >../status.actual && iuc status --porcelain >../status.iuc && cat >../status.expect <<EOF && @@ -581,13 +614,15 @@ test_expect_success 'test sparse status with untracked cache and subdir' ' EOF test_cmp ../status.expect ../status.iuc && test_cmp ../status.expect ../status.actual && + get_relevant_traces ../trace.output ../trace.relevant && cat >../trace.expect <<EOF && -node creation: 2 -gitignore invalidation: 0 -directory invalidation: 1 -opendir: 3 + ....path: + ....node-creation:2 + ....gitignore-invalidation:0 + ....directory-invalidation:1 + ....opendir:3 EOF - test_cmp ../trace.expect ../trace + test_cmp ../trace.expect ../trace.relevant ' test_expect_success 'verify untracked cache dump (sparse/subdirs)' ' @@ -616,19 +651,21 @@ EOF test_expect_success 'test sparse status again with untracked cache and subdir' ' avoid_racy && - : >../trace && - GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace" \ + : >../trace.output && + GIT_TRACE2_PERF="$TRASH_DIRECTORY/trace.output" \ git status --porcelain >../status.actual && iuc status --porcelain >../status.iuc && test_cmp ../status.expect ../status.iuc && test_cmp ../status.expect ../status.actual && + get_relevant_traces ../trace.output ../trace.relevant && cat >../trace.expect <<EOF && -node creation: 0 -gitignore invalidation: 0 -directory invalidation: 0 -opendir: 0 + ....path: + ....node-creation:0 + ....gitignore-invalidation:0 + ....directory-invalidation:0 + ....opendir:0 EOF - test_cmp ../trace.expect ../trace + test_cmp ../trace.expect ../trace.relevant ' test_expect_success 'move entry in subdir from untracked to cached' ' diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh index 45d025f96010..637391c6ce46 100755 --- a/t/t7519-status-fsmonitor.sh +++ b/t/t7519-status-fsmonitor.sh @@ -334,7 +334,7 @@ test_expect_success UNTRACKED_CACHE 'ignore .git changes when invalidating UNTR' git config core.fsmonitor .git/hooks/fsmonitor-test && git update-index --untracked-cache && git update-index --fsmonitor && - GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace-before" \ + GIT_TRACE2_PERF="$TRASH_DIRECTORY/trace-before" \ git status && test-tool dump-untracked-cache >../before ) && @@ -346,12 +346,12 @@ test_expect_success UNTRACKED_CACHE 'ignore .git changes when invalidating UNTR' EOF ( cd dot-git && - GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace-after" \ + GIT_TRACE2_PERF="$TRASH_DIRECTORY/trace-after" \ git status && test-tool dump-untracked-cache >../after ) && - grep "directory invalidation" trace-before >>before && - grep "directory invalidation" trace-after >>after && + grep "directory-invalidation" trace-before | cut -d"|" -f 9 >>before && + grep "directory-invalidation" trace-after | cut -d"|" -f 9 >>after && # UNTR extension unchanged, dir invalidation count unchanged test_cmp before after '