[PATCH 2 of 2] lfs: migrate most file filtering from threshold to custom filter

[Boris Feld]

On Sun, 2018-01-14 at 17:26 +0900, Yuya Nishihara wrote:
> On Sun, 14 Jan 2018 00:22:05 -0500, Matt Harbison wrote:
> > On Sat, 13 Jan 2018 21:11:45 -0500, Yuya Nishihara <yuya at tcha.org>
> > wrote:
> > 
> > > On Sat, 13 Jan 2018 02:23:16 -0500, Matt Harbison wrote:
> > > > # HG changeset patch
> > > > # User Matt Harbison <matt_harbison at yahoo.com>
> > > > # Date 1514706889 18000
> > > > #      Sun Dec 31 02:54:49 2017 -0500
> > > > # Node ID 66976f55793cced57929dedc8993204be340c717
> > > > # Parent  868cc63bfe9d7d7f5b40bc8cd70175cf1a608a95
> > > > lfs: migrate most file filtering from threshold to custom
> > > > filter
> > > 
> > > Queued, thanks.
> > > 
> > > > -    threshold = repo.ui.configbytes('lfs', 'threshold')
> > > > +    trackspec = repo.ui.config('lfs', 'track')
> > > > 
> > > > -    repo.svfs.options['lfsthreshold'] = threshold
> > > > +    # deprecated config: lfs.threshold
> > > > +    threshold = repo.ui.configbytes('lfs', 'threshold')
> > > > +    if threshold:
> > > > +        trackspec = "(%s) | size('>%s')" % (trackspec,
> > > > threshold)
> > > 
> > > I've added fileset.parse(trackspec) to detect bad spec like '..)
> > > ..  
> > > (..', and
> > > s/%s/%d/ to make sure threshold is integer so no need of string
> > > escape.
> > 
> > Thanks.
> > 
> > I assume that your amend obsoleted something local only to your
> > system?   
> > The reason I ask is because I ran `hg extdiff --patch --hidden -r  
> > 'precursors(c780e06)' -r c780e06`, and it aborted with an empty
> > revision.   
> > It's the precursors() predicate coming up empty.  I ended up
> > using  
> > 'last(allprecursors(c780e06))', and that found what I
> > submitted.  But that  
> > seems inconsistent with the obsolete line of that logged revision
> > pointing  
> > directly to c780e06.
> 
> What I generally do is:
> 
>  hg import --obsolete mails  # yours->mine
>  hg amend                    # mine->committed
> 
> > At minimum, it would be nice to have a shorter way to spell that.
> 
> I think allprecursors() should be renamed to precursors(), and
> delete/rename
> the original precursors().
> 
> > But it
> > seems weird that the log predicate would care about revisions not
> > in the  
> > repo.  Boris- it looks like '{precursors}' will skip the obsolete
> > revision  
> > not on my system, and agrees with 'last(..)'.
> >  

I checked the precursors revset and the behavior is not the one
expected, thank you for pointing that.


I also realized that the revset is in Evolve and not in Core, which
makes it a good opportunity to upstream it.

I'm not sure if we should keep only the equivalent of allprecursors(),
what about having a predecessor() and allpredecessors() revsets.

predecessors() would returns the closests locally known predecessors,
like the {predecessors} template keyword while allpredecessors() would
returns all the locally known predecessors.

What do you think?