On Tue, Dec 8, 2015 at 11:43 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Junio C Hamano <gits...@pobox.com> writes:
>
>> Christian Couder <christian.cou...@gmail.com> writes:
>>
>>> 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 preforming tests (because it might work on
>>> some systems using the repo over the network file system but not
>>> others).
>>> ...
>> The logic in this paragraph is fuzzy to me.  Shouldn't the config
>> give a sensible default, that is overriden by command line options?

The problem is that "git update-index --[no-|force-]untracked-cache"
is not just changing things for the duration of the current command.
It is in fact changing the configuration of the repository, because it
is adding the UC (untracked cache) in the index and saving the index,
which will persist the use of the UC for the following commands.

So it is very different from something like "git status
--no-untracked-cache" that would perform a "git status" without using
the UC.

In fact "git update-index --[no-|force-]untracked-cache" is very bad
because it means that two repositories can be configured differently
even if they have the same config files.

>> I agree that it is insane to do a runtime check when the user says
>> "update-index --untracked-cache" to enable it, as the user _knows_
>> that enabling it would help (or the user _knows_ that she wants to
>> play with it).  Similarly, shouldn't the config be ignored when the
>> user says "update-index --no-untracked-cache" (hence removing the
>> untracked cache from the resulting index no matter the config is set
>> to)?  ...
>
> As I think about this more, it really seems to me that we shouldn't
> need to make this configuration variable that special.  Because I
> think it is a *BUG* in the current implementation to do the runtime
> check even when the user explicitly tells us to use untracked-cache,
> I'd imagine something along the lines of the following would be a
> lot simpler, easier to understand and a generally more useful
> bugfix:
>
>  1 Add one boolean configuration variable, core.untrackedCache, that
>    defaults to 'false'.
>
>  2 When creating an index file in an empty repository, enable the
>    untracked cache in the index (even without the user runninng
>    "update-index --untracked-cache") iff the configuration is set to
>    'true'.  No runtime check necessary.

I guess this means that when cloning a repo, it would not use the UC
unless either "git -c core.untrackedCache=true clone ..." is used, or
core.untrackedCache is set in the global config.

>  3 When working on an existing index file, unless the operation is
>    "update-index --[no-]untracked-cache", keep the current setting,
>    regardless of this configuration variable.  No runtime check
>    necessary.
>
>  4 "update-index --[no-]untracked-cache" should enable or disable
>    the untracked cache as the user tells us, regardless of the
>    configuration variable.  No runtime check necessary.

If you want only some repos to use the UC, you will set
core.untrackedCache in the repo config. Then after cloning such a
repo, you will copy the config file, and this will not be enough to
enable the UC. The original repo will use the UC but the cloned one
will not, and you might wonder why is "git status" slower in the
cloned repo despite the machines and the config being the same for
both repos?

And if you have set core.untrackedCache in the global config when you
clone, UC is enabled, but if you have just set it in the repo config
after the clone, it is not enabled.

This is quite bad in my opinion.

Also think about system administrators who have a lot of machines with
lots of repos everywhere on these machines and just upgrade Git from
let's say v2.4.0 to v2.8.0. They find out that using UC git status
will be faster and want to provide that to the machine's users knowing
that they only use recent Linux machines where mtime works well.
Shouldn't it be nice if they could just enable core.untrackedCache in
the global config files without having to also cd into every repo and
use "git update-index --untracked-cache" there?

If we consider that "git update-index --[no-|force-]untracked-cache"
should not have been created in the first place, and that the good
mechanism for this is a config variable, and that maybe "git
update-index --[no-|force-]untracked-cache" should just be deprecated,
then the important thing is to make the core.untrackedCache config
variable work in the best possible way.

> It is OK to then add an "auto-detect" on top of the above, that
> would only affect the second bullet point, like so:
>
>  2a When creating an index file in an empty repository, if the
>     configuration is set to 'auto', do the lengthy runtime check and
>     enable the untracked cache in the index (even without the user
>     runninng "update-index --untracked-cache").
>
> without changing any other in the first 4-bullet list.
>
> Am I missing some other requirements?

It is more about what is a good way to use this feature?

"git update-index --[no-|force-]untracked-cache" is a bad way, so
let's make it easy for people to not use it at all.

If people can just set core.untrackedCache in an existing repo without
the need to use "git update-index --untracked-cache" afterwards to
really enable it, and without having to remember to use it again after
cloning (supposing core.untrackedCache is not set globally where they
clone), it is just simpler, and as an added benefit we can deprecate
"git update-index --[no-|force-]untracked-cache".

Thanks,
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

Reply via email to