Patchwork [3,of,4,V3] largefiles: add support for 'largefiles://' url scheme

login
register
mail settings
Submitter Boris Feld
Date Jan. 8, 2018, 9:16 p.m.
Message ID <18db7b5d796d31fbced9.1515446207@FB>
Download mbox | patch
Permalink /patch/26618/
State Accepted
Headers show

Comments

Boris Feld - Jan. 8, 2018, 9:16 p.m.
# HG changeset patch
# User Boris Feld <boris.feld@octobus.net>
# Date 1513861077 -3600
#      Thu Dec 21 13:57:57 2017 +0100
# Node ID 18db7b5d796d31fbced9d41dd50a61d0e62e6fcb
# Parent  a503a19221d6c6113ad1e3add9eb084be3177daf
# EXP-Topic largefile-url
# Available At https://bitbucket.org/octobus/mercurial-devel/
#              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 18db7b5d796d
largefiles: add support for 'largefiles://' url scheme

This changesets allows Mercurial to transparently download content from the
configured largefile store. This handle all authentication and largefile
protocol details.

The target usecase is to leverage largefile infrastructure for clone bundle. See
next changeset for details
Yuya Nishihara - Jan. 9, 2018, 12:08 p.m.
On Mon, 08 Jan 2018 22:16:47 +0100, Boris Feld wrote:
> # HG changeset patch
> # User Boris Feld <boris.feld@octobus.net>
> # Date 1513861077 -3600
> #      Thu Dec 21 13:57:57 2017 +0100
> # Node ID 18db7b5d796d31fbced9d41dd50a61d0e62e6fcb
> # Parent  a503a19221d6c6113ad1e3add9eb084be3177daf
> # EXP-Topic largefile-url
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 18db7b5d796d
> largefiles: add support for 'largefiles://' url scheme

> +_lfscheme = 'largefile://'
> +def openlargefile(orig, ui, url_, data=''):

Nit: s/data=''/data=None/ according to the signature of url.open().

> +    if url_.startswith(_lfscheme):
> +        if data:
> +            msg = "cannot use data on a 'largefile://' url"
> +            raise error.ProgrammingError(msg)

If the error can be triggered by user (by e.g. setting crafted repo path),
it shouldn't be a ProgrammingError.

> +def getlfile(ui, hash):
> +    return util.chunkbuffer(openstore(ui=ui)._get(hash))

AttributeError would be raised if the default path isn't remote. Can you
send a follow up?

> --- a/tests/test-url-download.t
> +++ b/tests/test-url-download.t
> @@ -34,3 +34,21 @@ Check other kind of compatible url
>    $ hg debugdownload ./null.txt
>    1 0000000000000000000000000000000000000000
>  
> +Test largefile URL
> +------------------
> +
> +  $ cat << EOF >> $HGRCPATH
> +  > [extensions]
> +  > largefiles=
> +  > EOF
> +
> +  $ killdaemons.py
> +  $ rm -f error.log hg1.pid
> +  $ hg serve -R server -p $HGPORT -d --pid-file=hg1.pid -E error.log
> +  $ cat hg1.pid >> $DAEMON_PIDS
> +
> +  $ hg -R server debuglfput null.txt
> +  a57b57b39ee4dc3da1e03526596007f480ecdbe8
> +
> +  $ hg --traceback debugdownload "largefile://a57b57b39ee4dc3da1e03526596007f480ecdbe8" --config paths.default=http://localhost:$HGPORT/

debugdownload command could be optionalrepo=True so repo/.hg/hgrc are loaded.
Boris Feld - Jan. 18, 2018, 4:14 p.m.
`On Tue, 2018-01-09 at 21:08 +0900, Yuya Nishihara wrote:
> On Mon, 08 Jan 2018 22:16:47 +0100, Boris Feld wrote:
> > # HG changeset patch
> > # User Boris Feld <boris.feld@octobus.net>
> > # Date 1513861077 -3600
> > #      Thu Dec 21 13:57:57 2017 +0100
> > # Node ID 18db7b5d796d31fbced9d41dd50a61d0e62e6fcb
> > # Parent  a503a19221d6c6113ad1e3add9eb084be3177daf
> > # EXP-Topic largefile-url
> > # Available At https://bitbucket.org/octobus/mercurial-devel/
> > #              hg pull https://bitbucket.org/octobus/mercurial-deve
> > l/ -r 18db7b5d796d
> > largefiles: add support for 'largefiles://' url scheme
> > +_lfscheme = 'largefile://'
> > +def openlargefile(orig, ui, url_, data=''):
> 
> Nit: s/data=''/data=None/ according to the signature of url.open().
> 
> > +    if url_.startswith(_lfscheme):
> > +        if data:
> > +            msg = "cannot use data on a 'largefile://' url"
> > +            raise error.ProgrammingError(msg)
> 
> If the error can be triggered by user (by e.g. setting crafted repo
> path),
> it shouldn't be a ProgrammingError.

How could the user trigger that?

> 
> > +def getlfile(ui, hash):
> > +    return util.chunkbuffer(openstore(ui=ui)._get(hash))
> 
> AttributeError would be raised if the default path isn't remote. Can
> you
> send a follow up?

What attribute error? Could you be a bit more specific?

> > --- a/tests/test-url-download.t
> > +++ b/tests/test-url-download.t
> > @@ -34,3 +34,21 @@ Check other kind of compatible url
> >    $ hg debugdownload ./null.txt
> >    1 0000000000000000000000000000000000000000
> >  
> > +Test largefile URL
> > +------------------
> > +
> > +  $ cat << EOF >> $HGRCPATH
> > +  > [extensions]
> > +  > largefiles=
> > +  > EOF
> > +
> > +  $ killdaemons.py
> > +  $ rm -f error.log hg1.pid
> > +  $ hg serve -R server -p $HGPORT -d --pid-file=hg1.pid -E
> > error.log
> > +  $ cat hg1.pid >> $DAEMON_PIDS
> > +
> > +  $ hg -R server debuglfput null.txt
> > +  a57b57b39ee4dc3da1e03526596007f480ecdbe8
> > +
> > +  $ hg --traceback debugdownload "largefile://a57b57b39ee4dc3da1e0
> > 3526596007f480ecdbe8" --config paths.default=http://localhost:$HGPO
> > RT/
> 
> debugdownload command could be optionalrepo=True so repo/.hg/hgrc are
> loaded.

We just sent a followup for this one.

Patch

diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
--- a/hgext/largefiles/overrides.py
+++ b/hgext/largefiles/overrides.py
@@ -1479,3 +1479,14 @@  def upgraderequirements(orig, repo):
     if 'largefiles' in repo.requirements:
         reqs.add('largefiles')
     return reqs
+
+_lfscheme = 'largefile://'
+def openlargefile(orig, ui, url_, data=''):
+    if url_.startswith(_lfscheme):
+        if data:
+            msg = "cannot use data on a 'largefile://' url"
+            raise error.ProgrammingError(msg)
+        lfid = url_[len(_lfscheme):]
+        return storefactory.getlfile(ui, lfid)
+    else:
+        return orig(ui, url_, data=data)
diff --git a/hgext/largefiles/remotestore.py b/hgext/largefiles/remotestore.py
--- a/hgext/largefiles/remotestore.py
+++ b/hgext/largefiles/remotestore.py
@@ -27,7 +27,9 @@  class remotestore(basestore.basestore):
     '''a largefile store accessed over a network'''
     def __init__(self, ui, repo, url):
         super(remotestore, self).__init__(ui, repo, url)
-        self._lstore = localstore.localstore(self.ui, self.repo, self.repo)
+        self._lstore = None
+        if repo is not None:
+            self._lstore = localstore.localstore(self.ui, self.repo, self.repo)
 
     def put(self, source, hash):
         if self.sendfile(source, hash):
diff --git a/hgext/largefiles/storefactory.py b/hgext/largefiles/storefactory.py
--- a/hgext/largefiles/storefactory.py
+++ b/hgext/largefiles/storefactory.py
@@ -22,8 +22,9 @@  from . import (
 # During clone this function is passed the src's ui object
 # but it needs the dest's ui object so it can read out of
 # the config file. Use repo.ui instead.
-def openstore(repo, remote=None, put=False):
-    ui = repo.ui
+def openstore(repo=None, remote=None, put=False, ui=None):
+    if ui is None:
+        ui = repo.ui
 
     if not remote:
         lfpullsource = getattr(repo, 'lfpullsource', None)
@@ -37,12 +38,16 @@  def openstore(repo, remote=None, put=Fal
         # ui.expandpath() leaves 'default-push' and 'default' alone if
         # they cannot be expanded: fallback to the empty string,
         # meaning the current directory.
-        if path == 'default-push' or path == 'default':
+        if repo is None:
+            path = ui.expandpath('default')
+            path, _branches = hg.parseurl(path)
+            remote = hg.peer(repo or ui, {}, path)
+        elif (path == 'default-push' or path == 'default'):
             path = ''
             remote = repo
         else:
             path, _branches = hg.parseurl(path)
-            remote = hg.peer(repo, {}, path)
+            remote = hg.peer(repo or ui, {}, path)
 
     # The path could be a scheme so use Mercurial's normal functionality
     # to resolve the scheme to a repository and use its path
@@ -76,3 +81,6 @@  def openstore(repo, remote=None, put=Fal
     }
 
 _scheme_re = re.compile(r'^([a-zA-Z0-9+-.]+)://')
+
+def getlfile(ui, hash):
+    return util.chunkbuffer(openstore(ui=ui)._get(hash))
diff --git a/hgext/largefiles/uisetup.py b/hgext/largefiles/uisetup.py
--- a/hgext/largefiles/uisetup.py
+++ b/hgext/largefiles/uisetup.py
@@ -31,6 +31,7 @@  from mercurial import (
     sshpeer,
     subrepo,
     upgrade,
+    url,
     wireproto,
 )
 
@@ -160,6 +161,9 @@  def uisetup(ui):
     extensions.wrapfunction(scmutil, 'marktouched',
                             overrides.scmutilmarktouched)
 
+    extensions.wrapfunction(url, 'open',
+                            overrides.openlargefile)
+
     # create the new wireproto commands ...
     wireproto.commands['putlfile'] = (proto.putlfile, 'sha')
     wireproto.commands['getlfile'] = (proto.getlfile, 'sha')
diff --git a/tests/test-url-download.t b/tests/test-url-download.t
--- a/tests/test-url-download.t
+++ b/tests/test-url-download.t
@@ -34,3 +34,21 @@  Check other kind of compatible url
   $ hg debugdownload ./null.txt
   1 0000000000000000000000000000000000000000
 
+Test largefile URL
+------------------
+
+  $ cat << EOF >> $HGRCPATH
+  > [extensions]
+  > largefiles=
+  > EOF
+
+  $ killdaemons.py
+  $ rm -f error.log hg1.pid
+  $ hg serve -R server -p $HGPORT -d --pid-file=hg1.pid -E error.log
+  $ cat hg1.pid >> $DAEMON_PIDS
+
+  $ hg -R server debuglfput null.txt
+  a57b57b39ee4dc3da1e03526596007f480ecdbe8
+
+  $ hg --traceback debugdownload "largefile://a57b57b39ee4dc3da1e03526596007f480ecdbe8" --config paths.default=http://localhost:$HGPORT/
+  1 0000000000000000000000000000000000000000