Re: [RFC/PATCH 6/8] config: add core.untrackedCache
On Fri, Dec 4, 2015 at 6:54 PM, Torsten Bögershausen wrote: >> Current state of affairs: >> >> * Enable on a per-repo basis: git update-index --untracked-cache >> * Disable on a per-repo basis: git update-index --no-cache >> * Enable system-wide: N/A >> * Disable system-wide: N/A >> >> With this patch: >> >> * Enable on a per-repo basis: git update-index --untracked-cache OR >> "git config core.untrackedCache true" >> * Disable on a per-repo basis: git update-index --no-cache OR "git >> config core.untrackedCache false" >> * Enable system-wide: git config --global core.untrackedCache true >> * Disable system-wide: git config --global core.untrackedCache false >> * Caveat: The core.untrackedCache config has precidence over "git >> update-index" >> >> With the rest of the patches in this series: >> >> * Enable system-wide & per-repo the same, just tweak >> core.untrackedCache either for the local .git or --globally >> * If you want to test things either per-repo or globally just use >> "git update-index --test-untracked-cache" >> * If you want something exactly like the old --untracked-cache do: >> "git update-index --test-untracked-cache && git config >> core.untrackedCache true" >> >> I think applying this whole series makes sense. Enabling this feature >> doesn't work like anything else in Git, usually you just tweak a >> config option and thus can easily enable things either system-wide or >> per-repo (or any combination of the two), which makes both system >> administration and local configuration easy. >> >> A much saner UI for the CLI tools if we're going to ship git with >> tests for filesystem features is to separate the testing from the >> enabling of those features. > > My spontanous feeling: squash 6-8 together and add this nice explanation > to the commit message. Ok, I will do that then. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 6/8] config: add core.untrackedCache
> Current state of affairs: > > * Enable on a per-repo basis: git update-index --untracked-cache > * Disable on a per-repo basis: git update-index --no-cache > * Enable system-wide: N/A > * Disable system-wide: N/A > > With this patch: > > * Enable on a per-repo basis: git update-index --untracked-cache OR > "git config core.untrackedCache true" > * Disable on a per-repo basis: git update-index --no-cache OR "git > config core.untrackedCache false" > * Enable system-wide: git config --global core.untrackedCache true > * Disable system-wide: git config --global core.untrackedCache false > * Caveat: The core.untrackedCache config has precidence over "git > update-index" > > With the rest of the patches in this series: > > * Enable system-wide & per-repo the same, just tweak > core.untrackedCache either for the local .git or --globally > * If you want to test things either per-repo or globally just use > "git update-index --test-untracked-cache" > * If you want something exactly like the old --untracked-cache do: > "git update-index --test-untracked-cache && git config > core.untrackedCache true" > > I think applying this whole series makes sense. Enabling this feature > doesn't work like anything else in Git, usually you just tweak a > config option and thus can easily enable things either system-wide or > per-repo (or any combination of the two), which makes both system > administration and local configuration easy. > > A much saner UI for the CLI tools if we're going to ship git with > tests for filesystem features is to separate the testing from the > enabling of those features. My spontanous feeling: squash 6-8 together and add this nice explanation to the commit message. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 6/8] config: add core.untrackedCache
On Thu, Dec 3, 2015 at 5:10 PM, Torsten Bögershausen wrote: > [snip all good stuff] > > First of all: > Thanks for explaining it so well > > I now can see the point in having this patch. > (Do the commit messages reflect all this ? I need to re-read) Maybe not. I will have a look at improving them, but your re-reading is welcome. > The other question is: Do you have patch on a public repo ? Yes, here: https://github.com/chriscool/git/commits/uc-notifs8 > And, of course, can we add 1 or 2 test cases ? Yes I had planned to add tests for this. But it would be nice if I could know first if the last two patches are considered ok even though they are breaking compatibility a bit. In this case I could squash them with previous patches and only write tests for the resulting patches. > Thanks for pushing this forward. Thanks for your reviews, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 6/8] config: add core.untrackedCache
[snip all good stuff] First of all: Thanks for explaining it so well I now can see the point in having this patch. (Do the commit messages reflect all this ? I need to re-read) The other question is: Do you have patch on a public repo ? And, of course, can we add 1 or 2 test cases ? Thanks for pushing this forward. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 6/8] config: add core.untrackedCache
On Wed, Dec 2, 2015 at 8:12 AM, Torsten Bögershausen wrote: > On 12/01/2015 09:31 PM, Christian Couder wrote: >> >> When we know that mtime is fully supported by the environment, we >> might want the untracked cache to be always used by default without >> any mtime test or kernel version check being performed. > [Re-arranged some of the quotes for the clarity of my reply] [Also: Full disclosure, Christian is working on this for Booking.com, and I'm managing that project...] > I always want to test and verify that the untracked cache is working, > before I rely on it. Then with this patch you can just not use the core.untrackedCache=true option, or with the later patches in this series use "git update-index --test-untracked-cache && git config core.untrackedCache true". > I'm not sure if ever "we know" ? > How can we know without testing ? > I personaly can not say "I know" in all the different system I am using, Some users of Git can know that their mtime works, just like they know they deploy it on filesystems where say symlinks work. The current implementation of turning on this feature needs to be run on a per-repo basis and without the --force option includes mandatory tests, which a) makes it inconvenient to deploy across all Git repos on a set of machines b) Is needlessly paranoid as a default way to enable it. >> Also when we know that mtime is not supported by the environment, >> for example because the repo is shared over a network file system, >> then we might want 'git update-index --untracked-cache' to fail >> immediately instead of it testing if it works (because it might >> work on some systems using the repo over the network file system >> but not others). > > Same here. > >> Signed-off-by: Christian Couder >> --- >> Documentation/config.txt | 10 ++ >> Documentation/git-update-index.txt | 11 +-- >> builtin/update-index.c | 28 ++-- >> cache.h| 1 + >> config.c | 10 ++ >> contrib/completion/git-completion.bash | 1 + >> dir.c | 2 +- >> environment.c | 1 + >> wt-status.c| 9 + >> 9 files changed, 60 insertions(+), 13 deletions(-) >> >> diff --git a/Documentation/config.txt b/Documentation/config.txt >> index b4b0194..bf176ff 100644 >> --- a/Documentation/config.txt >> +++ b/Documentation/config.txt >> @@ -308,6 +308,16 @@ core.trustctime:: >> crawlers and some backup systems). >> See linkgit:git-update-index[1]. True by default. >> +core.untrackedCache:: >> + If unset or set to 'default' or 'check', untracked cache will >> + not be enabled by default and when >> + 'update-index --untracked-cache' is called, Git will test if >> + mtime is working properly before enabling it. If set to false, >> + Git will refuse to enable untracked cache even if >> + '--force-untracked-cache' is used. If set to true, Git will >> + blindly enabled untracked cache by default without testing if >> + it works. See linkgit:git-update-index[1]. >> + > > Please no. > The command line option should always be able to overwrite any settings > from a config file. If we keep this patch and not the rest in this series (which I think should also be applied) you'd either use the update-index way of changing the setting, or the config option. > Sorry, I may missing the big picture here. > What exactly should be achieved ? > > A config variable that should ask Git to always try to use the untracked > cache ? > Or a config variable that tells Git to never use the untracked cache ? > Or a combination ? > > core.untrackedCache:: > false: Never use the untracked cache ? > true: Always try to use the untracked cache ? >Try means: probe, and if the probing fails, record that if fails in > the index, >for this hostname/os/kernel/path (Don't remember all the details) > unset: As today, As discussed in the "[RFC/PATCH] config: add core.trustmtime" thread this feature is IMO needlessly paranoid about enabling itself. Current state of affairs: * Enable on a per-repo basis: git update-index --untracked-cache * Disable on a per-repo basis: git update-index --no-cache * Enable system-wide: N/A * Disable system-wide: N/A With this patch: * Enable on a per-repo basis: git update-index --untracked-cache OR "git config core.untrackedCache true" * Disable on a per-repo basis: git update-index --no-cache OR "git config core.untrackedCache false" * Enable system-wide: git config --global core.untrackedCache true * Disable system-wide: git config --global core.untrackedCache false * Caveat: The core.untrackedCache config has precidence over "git update-index" With the rest of the patches in this series: * Enable system-wide & per-repo the same, just tweak core.untrackedCache eithe
Re: [RFC/PATCH 6/8] config: add core.untrackedCache
On 12/01/2015 09:31 PM, Christian Couder wrote: When we know that mtime is fully supported by the environment, we might want the untracked cache to be always used by default without any mtime test or kernel version check being performed. I'm not sure if ever "we know" ? How can we know without testing ? I personaly can not say "I know" in all the different system I am using, so I always want to test and verify that the untracked cache is working, before I rely on it. Also when we know that mtime is not supported by the environment, for example because the repo is shared over a network file system, then we might want 'git update-index --untracked-cache' to fail immediately instead of it testing if it works (because it might work on some systems using the repo over the network file system but not others). Same here. Signed-off-by: Christian Couder --- Documentation/config.txt | 10 ++ Documentation/git-update-index.txt | 11 +-- builtin/update-index.c | 28 ++-- cache.h| 1 + config.c | 10 ++ contrib/completion/git-completion.bash | 1 + dir.c | 2 +- environment.c | 1 + wt-status.c| 9 + 9 files changed, 60 insertions(+), 13 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index b4b0194..bf176ff 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -308,6 +308,16 @@ core.trustctime:: crawlers and some backup systems). See linkgit:git-update-index[1]. True by default. +core.untrackedCache:: + If unset or set to 'default' or 'check', untracked cache will + not be enabled by default and when + 'update-index --untracked-cache' is called, Git will test if + mtime is working properly before enabling it. If set to false, + Git will refuse to enable untracked cache even if + '--force-untracked-cache' is used. If set to true, Git will + blindly enabled untracked cache by default without testing if + it works. See linkgit:git-update-index[1]. + Please no. The command line option should always be able to overwrite any settings from a config file. Sorry, I may missing the big picture here. What exactly should be achieved ? A config variable that should ask Git to always try to use the untracked cache ? Or a config variable that tells Git to never use the untracked cache ? Or a combination ? core.untrackedCache:: false: Never use the untracked cache ? true: Always try to use the untracked cache ? Try means: probe, and if the probing fails, record that if fails in the index, for this hostname/os/kernel/path (Don't remember all the details) unset: As today, -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 6/8] config: add core.untrackedCache
When we know that mtime is fully supported by the environment, we
might want the untracked cache to be always used by default without
any mtime test or kernel version check being performed.
Also when we know that mtime is not supported by the environment,
for example because the repo is shared over a network file system,
then we might want 'git update-index --untracked-cache' to fail
immediately instead of it testing if it works (because it might
work on some systems using the repo over the network file system
but not others).
Signed-off-by: Christian Couder
---
Documentation/config.txt | 10 ++
Documentation/git-update-index.txt | 11 +--
builtin/update-index.c | 28 ++--
cache.h| 1 +
config.c | 10 ++
contrib/completion/git-completion.bash | 1 +
dir.c | 2 +-
environment.c | 1 +
wt-status.c| 9 +
9 files changed, 60 insertions(+), 13 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index b4b0194..bf176ff 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -308,6 +308,16 @@ core.trustctime::
crawlers and some backup systems).
See linkgit:git-update-index[1]. True by default.
+core.untrackedCache::
+ If unset or set to 'default' or 'check', untracked cache will
+ not be enabled by default and when
+ 'update-index --untracked-cache' is called, Git will test if
+ mtime is working properly before enabling it. If set to false,
+ Git will refuse to enable untracked cache even if
+ '--force-untracked-cache' is used. If set to true, Git will
+ blindly enabled untracked cache by default without testing if
+ it works. See linkgit:git-update-index[1].
+
core.checkStat::
Determines which stat fields to match between the index
and work tree. The user can set this to 'default' or
diff --git a/Documentation/git-update-index.txt
b/Documentation/git-update-index.txt
index 0ff7396..4e6078b 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -177,7 +177,10 @@ may not support it yet.
up for commands that involve determining untracked files such
as `git status`. The underlying operating system and file
system must change `st_mtime` field of a directory if files
- are added or deleted in that directory.
+ are added or deleted in that directory. If you want to always
+ enable, or always disable, untracked cache, you can set the
+ `core.untrackedCache` configuration variable to 'true' or
+ 'false' respectively, (see linkgit:git-config[1]).
--test-untracked-cache::
Only perform tests on the working directory to make sure
@@ -190,7 +193,9 @@ may not support it yet.
For safety, `--untracked-cache` performs tests on the working
directory to make sure untracked cache can be used. These
tests can take a few seconds. `--force-untracked-cache` can be
- used to skip the tests.
+ used to skip the tests. It cannot enable untracked cache if
+ `core.untrackedCache` configuration variable is set to false
+ (see linkgit:git-config[1]).
\--::
Do not interpret any more arguments as options.
@@ -406,6 +411,8 @@ It can be useful when the inode change time is regularly
modified by
something outside Git (file system crawlers and backup systems use
ctime for marking files processed) (see linkgit:git-config[1]).
+Untracked cache look at `core.untrackedCache` configuration variable
+(see linkgit:git-config[1]).
SEE ALSO
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 2cbaee0..bf697a5 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1106,19 +1106,27 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
the_index.split_index = NULL;
the_index.cache_changed |= SOMETHING_CHANGED;
}
+ if (untracked_cache == 2 || (untracked_cache == 1 &&
use_untracked_cache == -1)) {
+ setup_work_tree();
+ if (!test_if_untracked_cache_is_supported())
+ return 1;
+ if (untracked_cache == 2)
+ return 0;
+ }
if (untracked_cache > 0) {
- if (untracked_cache < 3) {
- setup_work_tree();
- if (!test_if_untracked_cache_is_supported())
- return 1;
- if (untracked_cache == 2)
- return 0;
- }
+ if (!use_untracked_cache)
+ die("core.untrackedCache is set to false; "
+ "the untracked cache will not be enabled");
