Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)

> On Sun, 30 Nov 2008, Nguyen Thai Ngoc Duy wrote:
>
>  > On 11/29/08, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
>  > > On 11/29/08, Daniel Barkalow <barkalow@iabervon.org> wrote:
>  > >  >  If there's any need for this to be distinguished from "assume unchanged",
>  > >  >  I think it should be used with, not instead of, the CE_VALID bit; and it
>  > >  >  could probably use some bit in the stat info section, since we don't need
>  > >  >  stat info if we know by assumption that the entry is valid.
>  > >
>  > >
>  > > Interesting. I'll think more about this.
>  > >
>  >
>  > As I said, CE_VALID implies all files are present.
>
>
> My first question is whether this actually should be true. Going back to
>  the message for 5f73076c1a9b4b8dc94f77eac98eb558d25e33c0, it sounds like
>  the CE_VALID code is designed to be safe and sort of correct even if the
>  files are not actually unchanged; I don't think it would be out-of-spec
>  for CE_VALID to (1) always produce output as if the working tree contained
>  what the index contains, while (2) refusing to make any changes to working
>  tree files that do not actually match the index. As it is now, (2) is
>  explicitly true, but (1) is left vague-- commands may fail entirely or
>  produce different output if CE_VALID is set in the index for a file that
>  has changes in the working tree, but not in any particular way.

>  Now, it might be necessary for CE_NO_CHECKOUT to differ from CE_VALID in
>  some ways in (2): if a file is CE_NO_CHECKOUT and absent, code which would
>  modify it could probably just report sucess, while CE_VALID on a file
>  with changes should probably report failure. On the other hand, that could
>  just as easily be at the porcelain layer, with the porcelain instructing
>  the plumbing to change the index without changing the working tree for
>  those files outside the sparse checkout, and the plumbing would report
>  errors if the porcelain did not do this.

>  > I could make CE_NO_CHECKOUT to be used with CE_VALID, but I would need
>  > to check all CE_VALID code path to make sure the behaviour remains if
>  > CE_NO_CHECKOUT is absent. It's just more intrusive.
>
>
> I would expect all code that has a CE_VALID path to do something actually
>  wrong if it took the non-CE_VALID code path on CE_NO_CHECKOUT and there
>  was no CE_NO_CHECKOUT code path. So I'd expect that your patch is
>  insufficient to the extent that CE_NO_CHECKOUT doesn't imply CE_VALID
>  (since there is very little in the way of CE_NO_CHECKOUT-specific
>  handling in your patch).

>  The only case I can think of where NO_CHECKOUT is more like !VALID than
>  VALID is with respect to whether we can report the content in the index by
>  looking in the filesystem instead of in the database; I don't think this
>  is an intentional optimization anywhere, and I think it would be a likely
>  source of bugs if it were (e.g., it would have to know about files which
>  are up-to-date with respect to stat info, but which have been "smudged" on
>  disk and therefore don't match byte-for-byte with the database).

>  Actually,
>  it might be most accurate to treat --no-checkout as being CE_VALID with a
>  smudge filter of "rm". If the combination of CE_VALID and on-disk
>  conversion works (which is likely to be the common pattern for Windows
>  users, who need autocrlf and have a slow lstat(), and is therefore
>  maintained), surely this combination would work for CE_NO_CHECKOUT.

>  > I have nothing against storing CE_NO_CHECKOUT in stat info except that
>  > it seems inappropriate/hidden place to do. ce_flags is more obvious
>  > choice. I haven't looked closely to stat info code in read-cache.c
>  > though.
>
>
> It should be pretty clean to check CE_VALID when reading an entry from
>  disk and remap bits from it to additional flags in memory. I wouldn't
>  suggest overlaying them in memory, but there's also no shortage of space
>  for flags in memory.