[PATCH 1 of 3] hgweb: use an extensible list of file to check for refrash

[Yuya Nishihara]

On Wed, 01 Jul 2015 12:26:07 -0700, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david at fb.com>
> # Date 1435735130 25200
> #      Wed Jul 01 00:18:50 2015 -0700
> # Node ID 7e3ad479d9bda6560885f08a1df63d386639b2e1
> # Parent  c76e8d14383a44a740d986d87db6f58276fb57e8
> hgweb: use an extensible list of file to check for refrash
> 
> The refresh feature was explicitly testing if '00changelog.i' and 'phasesroots'
> changed. This is overlooking other important information like bookmarks and
> obstore (bookmark have their own hack to work around it).
> 
> We move to a more extensible system with a list of file of interest that will be
> used to build the repo state. The system should probably move in a more central
> place so that the command server and other system are able to use it.
> Extensions write will also be able to add entry to ensure that changes to
> extension data are properly detected.
> 
> Also the current key (mtime, size) is notably weak for bookmarks and phases
> whose files can easily change content without effect on their size.
> 
> Still, this patch seems like a valuable minimal first step.
> 
> diff --git a/mercurial/hgweb/hgweb_mod.py b/mercurial/hgweb/hgweb_mod.py
> --- a/mercurial/hgweb/hgweb_mod.py
> +++ b/mercurial/hgweb/hgweb_mod.py
> @@ -24,10 +24,17 @@ perms = {
>      'listkeys': 'pull',
>      'unbundle': 'push',
>      'pushkey': 'push',
>  }
>  
> +## files of interrest
> +# used to check if the repository has changed looking at mtime and size of
> +# theses files.  This should probably be relocated a bit higher in core
> +foi = [('spath', None),
> +       ('spath', 'phaseroots'), # ! phase can change content at the same size
> +      ]
> +
>  def makebreadcrumb(url, prefix=''):
>      '''Return a 'URL breadcrumb' list
>  
>      A 'URL breadcrumb' is a list of URL-name pairs,
>      corresponding to each of the path items on a URL.
> @@ -118,14 +125,20 @@ class hgweb(object):
>              return repo.filtered(viewconfig)
>          else:
>              return repo.filtered('served')
>  
>      def refresh(self, request=None):
> -        st = get_stat(self.repo.spath)
> -        pst = get_stat(self.repo.spath, 'phaseroots')
> -        # changelog mtime and size, phaseroots mtime and size
> -        repostate = ((st.st_mtime, st.st_size), (pst.st_mtime, pst.st_size))
> +        repostate = []
> +        # file of interrests mtime and size
> +        for meth, fname in foi:
> +            pathfunc = getattr(self.repo, meth)

'repo.spath' isn't a function.

> +            if fname is None:
> +                st = get_stat(pathfunc)
> +            else:
> +                st = get_stat(pathfunc, fname)

I think the "fname is None" case is misleading because get_stat(spath) means
get_stat(spath, '00changelog.i'). Can we make fname a mandatory parameter?

Actually, I misread it as fstat('.hg/store') and wondered why it had to
check the mtime of .hg/store/phaseroots even though the mtime of .hg/store
should be changed at repo.lock().