From: Jameson Miller <firstname.lastname@example.org> To: Stefan Beller <email@example.com> Cc: "firstname.lastname@example.org" <email@example.com>, Samuel Lijin <firstname.lastname@example.org>, Brandon Williams <email@example.com>, Junio C Hamano <firstname.lastname@example.org>, Jeff King <email@example.com>, Jameson Miller <firstname.lastname@example.org> Subject: Re: [PATCH v1 1/1] dir: teach status to show ignored directories Date: Fri, 11 Aug 2017 13:48:41 -0400 Message-ID: <email@example.com> (raw) In-Reply-To: <CAGZ79kb8cWR1P=1FSkfZ72Ai63T0WapwtCCN3gwqrWE_=MzOLg@mail.gmail.com> On 08/10/2017 04:03 PM, Stefan Beller wrote: > On Thu, Aug 10, 2017 at 11:49 AM, Jameson Miller > <firstname.lastname@example.org> wrote: > > Welcome to the Git mailing list. :) Thank you for the welcome and the review! I will include the suggested code changes in the next patch version. >> Teach Git to optionally show ignored directories when showing all >> untracked files. The git status command exposes the options to report >> ignored and/or untracked files. However, when reporting all untracked >> files (--untracked-files=all), all individual ignored files are reported >> as well. It is not currently possible to get the reporting behavior of >> the --ignored flag, while also reporting all untracked files. > Trying to understand this based off the documentation for > --untracked=all and --ignored, I realize that the documentation > for --ignored seems to be lacking as I do not understand what the > --ignored behavior is in combination with --untracked=[all, normal, no] > The set of files listed by "--ignored" changes when different values are given to "--untracked-files". If would be nice to be able to make the ignored output independent of the untracked settings. This patch attempts to do that while maintaining backward compatibility with the existing behavior. When "--ignored" is used by itself ("--untracked-files=normal"), ignored directories (both directories that explicitly match a directory ignore pattern -and- directories containing only ignored files) are listed. Individual files within are *not* listed. When "--ignored" is used with "--untracked-files=all", git always lists the individual files within the ignored directories and DOES NOT collapse the output to just the containing (explicitly or implicitly ignored) directory. This can cause a massive performance problem when there are a lot of ignored files in a well-defined set of ignored directories. For example, on Windows, Visual Studio creates a bin/ and obj/ directory inside your project where it writes all .obj files. Normal usage is to explicitly ignore those 2 directory names in your .gitignores (rather than or in addition to *.obj). We just want to see the "bin/" and "obj/" paths regardless of which "--untracked" flag is passed in. We want to see the paths that explicitly match an ignore pattern. This means that if a directory only contains ignored files, but the directory itself is not explicitly ignored, then we want to see the individual files. This is slightly different than the current behavior of "--ignored". I am open to suggestions on how to present the options to control this behavior. >> This >> change exposes a flag to report all untracked files while not showing >> individual files in ignored directories. > By the description up to here, it sounds as if you want to introduce > mode for --untracked, e.g. "normal-adjusted-for-ignored" (it's a bad > suggestion)? However the patch seems to add an orthogonal flag, > such that > > status --no-ignored --untracked=no --show-ignored-directory > > would also be possible. What is a reasonable expectation for > the output of such? The current patch does add another flag. This flag only has meaning if the "--ignored" and "--untracked=all" flags are also specified. Another option I had considered is to let the "--ignored" flag take an argument. Then, we could express this new behavior through (for example) a "--ignored=exact" flag to reflect the fact that this new option returns paths that match the ignore pattern, and does not collapse directories that contain only ignored files. >> Motivation: >> Our application (Visual Studio) needs all untracked files listed >> individually, but does not need all ignored files listed individually. > For parsing output, I would strongly recommend --porcelain[=2], > but that is a tangent. > >> Reporting all ignored files can affect the time it takes for status >> to run. For a representative repository, here are some measurements >> showing a large perf improvement for this scenario: >> >> | Command | Reported ignored entries | Time (s) | >> | ------- | ------------------------ | -------- | >> | 1 | 0 | 1.3 | >> | 2 | 1024 | 4.2 | >> | 3 | 174904 | 7.5 | >> | 4 | 1046 | 1.6 | >> >> Commands: >> 1) status >> 2) status --ignored >> 3) status --ignored --untracked-files=all >> 4) status --ignored --untracked-files=all --show-ignored-directory >> (2) is --untracked-files=normal I'd presume, such that this flag >> can be understood as a tweak to "normal" based on the similar size >> between 2 and 4? (The timing improvement from 2 to 4 is huge though). (2) is --untracked-files=normal. Although the count of ignored files similar between 2 and 4, I consider this flag more of a tweak on 3, as we want the untracked files reported with the "--untracked=all" flag. The counts between 2 and 4 are similar in this case because most of the ignored files are contained in ignored directories. Our application calls status including the following flags: --porcelain=v2 --ignored --untracked-files=all --ignore-submodules=none This means we have bad performance compared to just "git status" when there is a large number of files in ignored directories With this new behavior, our application would move from case 3 to case 4 for this repository. You also point out the timing difference between case 2 and 4. I think there is an optimization we can make when running "git status --ignored" logic that will improve the the timing here. Currently, the logic in dir.c is iterating over all files contained in an ignored directory to see if it is empty or not. This is not necessary - we should be able to stop after finding any file. I plan to follow up on this in a different patch. >> This changes exposes a --show-ignored-directory flag to the git status > s/changes/change/ > >> command. This flag is utilized when running git status with the >> --ignored and --untracked-files options to not list ignored individual >> ignored files contained in directories that match an ignore pattern. >> >> Part of the perf improvement comes from the tweak to >> read_directory_recursive to stop scanning the file system after it >> encounters the first file. When a directory is ignored, all it needs to >> determine is if the directory is empty or not. The logic currently keeps >> scanning the file system until it finds an untracked file. However, as >> the directory is ignored, all the contained contents are also marked >> excluded. For ignored directories that contain a large number of files, >> this can take some time. > I think it is possible to ignore a directory and still track files in it, what > are the implications of this flag on a tracked (and changed) file in an > ignored dir? It is possible to have tracked files in an ignored directory. The behavior with this patch is to show individual files in the ignored directory. This patch includes a test covering this scenario. I think this behavior is reasonable for this case - I am interested if there are other opinions on this. > > What happens to empty directories that match an ignore pattern? > The "git status" command would not show empty directories. From the lower level implementation in dir.c, I think it *should* depend on the DIR_HIDE_EMPTY_DIRECTORIES flag, but does not currently. I will fix this with the next patch version. >> @@ -1362,6 +1363,8 @@ int cmd_status(int argc, const char **argv, const char *prefix) >> N_("ignore changes to submodules, optional when: all, dirty, untracked. (Default: all)"), >> PARSE_OPT_OPTARG, NULL, (intptr_t)"all" }, >> OPT_COLUMN(0, "column", &s.colopts, N_("list untracked files in columns")), >> + OPT_BOOL(0, "show-ignored-directory", &show_ignored_directory, > Is it possible to directly read into s.show_ignored_directory here? > >> +test_expect_success 'Verify behavior of status on folders with ignored files' ' >> + test_when_finished "git clean -fdx" && >> + git status --porcelain=v2 --ignored --untracked-files=all --show-ignored-directory >output && >> + test_i18ncmp expect output >> +' >> + >> +# Test status bahavior on folder with tracked and ignored files > behavior > >> +cat >expect <<\EOF >> +? expect >> +? output >> +! dir/tracked_ignored/ignored_1.ign >> +! dir/tracked_ignored/ignored_2.ign >> +! tracked_ignored/ignored_1.ign >> +! tracked_ignored/ignored_2.ign >> +EOF > I think our latest 'best style' is to include these heredocs into the test.
next prev parent reply index Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-08-10 18:49 [PATCH v1 0/1] Teach " Jameson Miller 2017-08-10 18:49 ` [PATCH v1 1/1] dir: teach " Jameson Miller 2017-08-10 20:03 ` Stefan Beller 2017-08-11 17:48 ` Jameson Miller [this message] 2017-08-14 21:05 ` Stefan Beller 2017-08-11 17:39 ` Brandon Williams 2017-08-11 18:29 ` Jameson Miller 2017-09-18 17:24 ` [PATCH v2] Improve performance of git status --ignored Jameson Miller 2017-09-18 17:24 ` Jameson Miller 2017-09-19 3:27 ` Junio C Hamano 2017-09-19 17:52 ` Brandon Williams
Reply instructions: You may reply publically to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style List information: http://vger.kernel.org/majordomo-info.html * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
firstname.lastname@example.org mailing list mirror (one of many) Archives are clonable: git clone --mirror https://public-inbox.org/git git clone --mirror http://ou63pmih66umazou.onion/git git clone --mirror http://czquwvybam4bgbro.onion/git git clone --mirror http://hjrcffqmbrq6wope.onion/git Newsgroups are available over NNTP: nntp://news.public-inbox.org/inbox.comp.version-control.git nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git nntp://news.gmane.org/gmane.comp.version-control.git note: .onion URLs require Tor: https://www.torproject.org/ or Tor2web: https://www.tor2web.org/ AGPL code for this site: git clone https://public-inbox.org/ public-inbox