[PATCH 5 of 6] lfs: infer the blob store URL from an explicit push source or default-push

Yuya Nishihara yuya at tcha.org
Tue Apr 10 09:34:00 EDT 2018


On Mon, 9 Apr 2018 17:43:47 -0400, Matt Harbison wrote:
> 
> > On Apr 9, 2018, at 9:43 AM, Yuya Nishihara <yuya at tcha.org> wrote:
> > 
> >> On Mon, 09 Apr 2018 00:26:45 -0400, Matt Harbison wrote:
> >> # HG changeset patch
> >> # User Matt Harbison <matt_harbison at yahoo.com>
> >> # Date 1523211732 14400
> >> #      Sun Apr 08 14:22:12 2018 -0400
> >> # Node ID b8c871c097d3153d1eb71e0072cb7edf3356360c
> >> # Parent  2aecf5b7dfdaeb12a6f6ac151d40c3b60f789abc
> >> lfs: infer the blob store URL from an explicit push source or default-push
> >> 
> >> Same idea as pull, but the push command needs to hold onto the '_subtopath'
> >> field slightly longer.  Since this code has already resolved an explicit path or
> >> 'default-push' or 'default', it seems reasonable to make this simple tweak to
> >> avoid recalculating that.
> > 
> >> +        opargs = dict(opts.get('opargs', {})) # copy opargs since it may mutate
> >> +        opargs.setdefault('pushvars', []).extend(opts.get('pushvars', []))
> >> +
> >> +        pushop = exchange.push(repo, other, opts.get('force'), revs=revs,
> >> +                               newbranch=opts.get('new_branch'),
> >> +                               bookmarks=opts.get('bookmark', ()),
> >> +                               opargs=opargs)
> >> +        # _subtopath stays for this repo push to assist LFS server discovery
> >>     finally:
> >>         del repo._subtoppath
> >> 
> >> -    opargs = dict(opts.get('opargs', {})) # copy opargs since we may mutate it
> >> -    opargs.setdefault('pushvars', []).extend(opts.get('pushvars', []))
> >> -
> >> -    pushop = exchange.push(repo, other, opts.get('force'), revs=revs,
> >> -                           newbranch=opts.get('new_branch'),
> >> -                           bookmarks=opts.get('bookmark', ()),
> >> -                           opargs=opargs)
> > 
> > Can't we wrap e.g. exchange.push/pull() to temporarily replace
> > repo.lfsremoteblobstore based on the remote repository?
> 
> That works for exchange.push(), where the files are sent in a prepush hook.  That doesn’t work for pull -u, where the update occurs after the exchange.  Commands.postincoming() doesn’t have path info (that’s where repo._subtoppath comes in), and we can’t wrap commands.pull(), because that won’t help subrepos.
> 
> What about a hybrid approach of wrapping push(), but still looking at _subtoppath to handle pull?  That way, the core doesn’t have to change.

Ok, abusing _subtoppath would be no worse than the current state since the
situation is quite similar to subrepo pull. So maybe it's okay for now.

Can you try to fix the mess after the 4.6 release? For "pull -u" of
subrepos/lfs, we'll probably need

 a) an API to explicitly prefetch changes (recursively?) from the specified
    peer repository
 b) or, a mechanism to carry peer path/repository while updating (something
    like pull/pushop)
 c) or, a formal API to temporarily bind peer path/repository to the local
    repo (i.e. formalize _subtoppath in some way)


More information about the Mercurial-devel mailing list