[PATCH 1 of 3 RFC] scmutil: introduce the flagstate class to suppliment missing flag support

[Matt Harbison]

On Wed, 06 Jul 2016 13:35:44 -0400, Matt Mackall <mpm at selenic.com> wrote:

> On Sun, 2016-07-03 at 17:44 -0400, Matt Harbison wrote:
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison at yahoo.com>
>> # Date 1467571904 14400
>> #      Sun Jul 03 14:51:44 2016 -0400
>> # Node ID e9fce4275ce6b26b941f47044744015e90367e7b
>> # Parent  de4a80a2b45c6fcae0948ac12872dd8a61ced26a
>> scmutil: introduce the flagstate class to suppliment missing flag  
>> support
>>
>> This will allow manipulation of the executable bit when filesystems  
>> don't
>> track
>> it natively.  This file isn't tracked in any form, and will likely be  
>> small
>> since it can be purged completely when wdir is cleaned- the manifest  
>> tracks
>> the
>> flags in permanent history.  It is intended only for platforms that  
>> don't have
>> native flag support, and will noop on platforms that do for simplicity.
>> Obviously a tree copy in either direction between filesystems with and  
>> without
>> flag support will lose the flag state, but that seems like an edge case  
>> that
>> isn't worth having two sources of data that can get out of sync on Unix.
>>
>> We could in theory also support designating a file as a symlink, so I  
>> kept
>> this
>> generic.  Since the file isn't tracked, and the flags are stored  
>> symbolically,
>> it should be extensible without changing the file format.  So I didn't  
>> bother
>> with a version identifier.  I have no idea if there would be any  
>> encoding
>> concerns (I saw references to encode/decode in the fncache class when I  
>> was
>> trying to find a suitable bit of code to model), but I wouldn't think  
>> so.
>
> No, filenames are not (re-)encoded.
>
>>
>> I couldn't find an existing suitable file format, and I really wanted  
>> to just
>> munge the flags on dirstate to get `commit`, `update -C` and `revert`  
>> handling
>> for free.  But dirstate doesn't know about the flag building function  
>> used in
>> context.py,
>
> That's intentional, because context is at a higher abstraction level.  
> Dirstate
> doesn't even know that Mercurial only recognizes two mode bits and  
> stores 32.
>
>>  and seems to be populated by what the OS reports, though I couldn't
>> figure out where exactly.
>
> Here: https://www.selenic.com/hg/file/tip/mercurial/dirstate.py#l493
>
> Note the call to stat and how the results are passed on to _addpath.

Thanks.  I saw the former, but missed that _addpath was setting _dirty, so  
the changes weren't being written.  That made me think something somewhere  
else was also setting this mode.

> Note also the trick we do with the _rangemask to deal with weird sizes  
> and
> times. We could use a similar trick on the mode field to get some  
> protected bits
> for storing our virtual mode bits.
>
>> I wasn't looking to do a dirstate transaction with this, but the tests  
>> were
>> totally unstable from run to run without the normallookup() call.  I'm  
>> not
>> sure
>> if we need to backup the file, but that's what the fncache code was  
>> doing.
>
> Whether or not this is part of the dirstate file, it's logically a  
> dirstate
> transaction. Getting it into the dirstate proper will avoid a number of
> headaches.

Agreed.  One of the headaches with dirstate (or any tracking file), is how  
to handle {util,vfs}.setflags().  It seems wrong that either would know  
about dirstate (or another file).  In my functioning dirstate based WIP, I  
changed localrepo.wwrite() so that `update -C` and `revert` end up with  
the proper bits:

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -988,6 +988,7 @@
              self.wvfs.write(filename, data,  
backgroundclose=backgroundclose)
              if 'x' in flags:
                  self.wvfs.setflags(filename, False, True)
+            self.dirstate.setflags(filename, False, 'x' in flags)
          return len(data)

      def wwritedata(self, filename, data):

But it also seems wrong that setflags has to be called on two different  
things side by side.  OTOH, there are places where dirstate will never be  
available (like convert's svn sink).  Thoughts?

> --Mathematics is the supreme nostalgia of our time.