D5137: streamclone: new server config and some API changes for narrow stream clones

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

D5137: streamclone: new server config and some API changes for narrow stream clones

martinvonz (Martin von Zweigbergk)
pulkit created this revision.
Herald added a reviewer: durin42.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  This patch introduces a new server config `server.stream-narrow-clones` which if
  set to True will advertise that the server supports narrow stream clones.
 
  This patch also pass on the includes and excludes from getbundle command to
  streamclone generation code.
 
  There is a test added to show that the includepats and excludepats are correctly
  passed.
 
  Upcoming patches will implement storage layer filtering for streamclones and
  then we can remove the temporary error and plug in the whole logic together to
  make narrow stream clones working.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5137

AFFECTED FILES
  mercurial/bundle2.py
  mercurial/configitems.py
  mercurial/help/config.txt
  mercurial/streamclone.py
  tests/test-narrow-clone-stream.t

CHANGE DETAILS

diff --git a/tests/test-narrow-clone-stream.t b/tests/test-narrow-clone-stream.t
new file mode 100644
--- /dev/null
+++ b/tests/test-narrow-clone-stream.t
@@ -0,0 +1,39 @@
+Tests narrow stream clones
+
+  $ . "$TESTDIR/narrow-library.sh"
+
+Server setup
+
+  $ hg init master
+  $ cd master
+  $ mkdir dir
+  $ mkdir dir/src
+  $ cd dir/src
+  $ for x in `$TESTDIR/seq.py 20`; do echo $x > "f$x"; hg add "f$x"; hg commit -m "Commit src $x"; done
+
+  $ cd ..
+  $ mkdir tests
+  $ cd tests
+  $ for x in `$TESTDIR/seq.py 20`; do echo $x > "f$x"; hg add "f$x"; hg commit -m "Commit src $x"; done
+  $ cd ../../..
+
+Trying to stream clone when the server does not support it
+
+  $ hg clone --narrow ssh://user@dummy/master narrow --noupdate --include "dir/src/f10" --stream
+  streaming all changes
+  remote: abort: server does not support narrow stream clones
+  abort: pull failed on remote
+  [255]
+
+Enable stream clone on the server
+
+  $ echo "[server]" >> master/.hg/hgrc
+  $ echo "stream-narrow-clones=True" >> master/.hg/hgrc
+
+Cloning a specific file when stream clone is supported
+
+  $ hg clone --narrow ssh://user@dummy/master narrow --noupdate --include "dir/src/f10" --stream
+  streaming all changes
+  remote: abort: support for narrow stream clones is missing
+  abort: pull failed on remote
+  [255]
diff --git a/mercurial/streamclone.py b/mercurial/streamclone.py
--- a/mercurial/streamclone.py
+++ b/mercurial/streamclone.py
@@ -531,7 +531,7 @@
             finally:
                 fp.close()
 
-def generatev2(repo):
+def generatev2(repo, includes, excludes):
     """Emit content for version 2 of a streaming clone.
 
     the data stream consists the following entries:
@@ -544,6 +544,10 @@
     Returns a 3-tuple of (file count, file size, data iterator).
     """
 
+    # temporarily raise error until we add storage level logic
+    if includes or excludes:
+        raise error.Abort(_("support for narrow stream clones is missing"))
+
     with repo.lock():
 
         entries = []
diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
--- a/mercurial/help/config.txt
+++ b/mercurial/help/config.txt
@@ -1848,6 +1848,10 @@
     Older Mercurial clients only support zlib compression and this setting
     has no effect for legacy clients.
 
+``stream-narrow-clones``
+    Whether the server supports streaming narrow clones or not.
+    (default: False)
+
 ``uncompressed``
     Whether to allow clients to clone a repository using the
     uncompressed streaming protocol. This transfers about 40% more
diff --git a/mercurial/configitems.py b/mercurial/configitems.py
--- a/mercurial/configitems.py
+++ b/mercurial/configitems.py
@@ -1011,6 +1011,9 @@
 coreconfigitem('server', 'streamunbundle',
     default=False,
 )
+coreconfigitem('server', 'stream-narrow-clones',
+    default=False,
+)
 coreconfigitem('server', 'uncompressed',
     default=True,
 )
diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
--- a/mercurial/bundle2.py
+++ b/mercurial/bundle2.py
@@ -1687,7 +1687,17 @@
     # to avoid compression to consumers of the bundle.
     bundler.prefercompressed = False
 
-    filecount, bytecount, it = streamclone.generatev2(repo)
+    # get the inlcudes and excludes
+    includepats = kwargs[r'includepats']
+    excludepats = kwargs[r'excludepats']
+
+    narrowstream = repo.ui.configbool('server', 'stream-narrow-clones')
+
+    if (includepats or excludepats) and not narrowstream:
+        raise error.Abort(_('server does not support narrow stream clones'))
+
+    filecount, bytecount, it = streamclone.generatev2(repo, includepats,
+                                                      excludepats)
     requirements = _formatrequirementsspec(repo.requirements)
     part = bundler.newpart('stream2', data=it)
     part.addparam('bytecount', '%d' % bytecount, mandatory=True)



To: pulkit, durin42, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5137: streamclone: new server config and some API changes for narrow stream clones

martinvonz (Martin von Zweigbergk)
indygreg added a comment.


  I'll hold off formally accepting until the whole series is up. But this looks pretty good.
 
  We need to land this in 4.8 or less the client-side feature detection logic will require a new server capability (or a bump of the version of the capability string).

INLINE COMMENTS

> configitems.py:1014
>  )
> +coreconfigitem('server', 'stream-narrow-clones',
> +    default=False,

Perhaps we should mark this as experimental via an inline comment and forego documenting the config option for now?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5137

To: pulkit, durin42, #hg-reviewers
Cc: indygreg, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5137: streamclone: new server config and some API changes for narrow stream clones

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
pulkit added a comment.


  In https://phab.mercurial-scm.org/D5137#76870, @indygreg wrote:
 
  > I'll hold off formally accepting until the whole series is up. But this looks pretty good.
  >
  > We need to land this in 4.8 or less the client-side feature detection logic will require a new server capability (or a bump of the version of the capability string).
 
 
  The whole series is up now. Although this is important for us, I realize there is just 1 day left for the freeze and I won't push hard for getting this pushed in this cycle and will graft these csets if looks good. I know you are on a vacation, so thanks for taking time and reviewing.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5137

To: pulkit, durin42, #hg-reviewers
Cc: indygreg, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5137: streamclone: new server config and some API changes for narrow stream clones

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
indygreg added a comment.


  There are no children reviews of this one. Could you please set the parent-child relationships so the entire stack renders in the web UI?
 
  (I'm assuming there are commits that follow this one - an unimplemented narrow stream clone feature doesn't seem very useful!)

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5137

To: pulkit, durin42, #hg-reviewers
Cc: indygreg, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5137: streamclone: new server config and some API changes for narrow stream clones

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
pulkit added a comment.


  In https://phab.mercurial-scm.org/D5137#76876, @indygreg wrote:
 
  > There are no children reviews of this one. Could you please set the parent-child relationships so the entire stack renders in the web UI?
  >
  > (I'm assuming there are commits that follow this one - an unimplemented narrow stream clone feature doesn't seem very useful!)
 
 
  Should be good now.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5137

To: pulkit, durin42, #hg-reviewers
Cc: indygreg, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5137: streamclone: new server config and some API changes for narrow stream clones

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
martinvonz added inline comments.

INLINE COMMENTS

> indygreg wrote in configitems.py:1014
> Perhaps we should mark this as experimental via an inline comment and forego documenting the config option for now?

Makes sense, but I don't know how to do that, so I'll just rename it to `experimental.server.stream-narrow-clones` in flight (because we're close to the freeze)

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5137

To: pulkit, durin42, #hg-reviewers
Cc: martinvonz, indygreg, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5137: streamclone: new server config and some API changes for narrow stream clones

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
martinvonz added inline comments.

INLINE COMMENTS

> bundle2.py:1691-1692
> +    # get the inlcudes and excludes
> +    includepats = kwargs[r'includepats']
> +    excludepats = kwargs[r'excludepats']
> +

I had to change these to `kwargs.get()` to get `test-http.t` and many others to pass. (I also fixed the typo in the comment above)

> config.txt:1851-1854
> +``stream-narrow-clones``
> +    Whether the server supports streaming narrow clones or not.
> +    (default: False)
> +

I removed this per indygreg's comment

> streamclone.py:549
> +    if includes or excludes:
> +        raise error.Abort(_("support for narrow stream clones is missing"))
> +

I changed this to "server does not support narrow stream clones" to match the error from bundle2.

Btw, why is it raised in two places? Is the one in bundle2 that actually happens? Maybe this should be a ProgrammingError then?

> test-narrow-clone-stream.t:37
> +  streaming all changes
> +  remote: abort: support for narrow stream clones is missing
> +  abort: pull failed on remote

I changed this to "server does not support narrow stream clones" to match the actual output (test was failing)

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5137

To: pulkit, durin42, #hg-reviewers, martinvonz
Cc: martinvonz, indygreg, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5137: streamclone: new server config and some API changes for narrow stream clones

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
This revision was automatically updated to reflect the committed changes.
Closed by commit rHGaf62936c2508: streamclone: new server config and some API changes for narrow stream clones (authored by pulkit, committed by ).

CHANGED PRIOR TO COMMIT
  https://phab.mercurial-scm.org/D5137?vs=12208&id=12227#toc

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D5137?vs=12208&id=12227

REVISION DETAIL
  https://phab.mercurial-scm.org/D5137

AFFECTED FILES
  mercurial/bundle2.py
  mercurial/configitems.py
  mercurial/streamclone.py
  tests/test-narrow-clone-stream.t

CHANGE DETAILS

diff --git a/tests/test-narrow-clone-stream.t b/tests/test-narrow-clone-stream.t
new file mode 100644
--- /dev/null
+++ b/tests/test-narrow-clone-stream.t
@@ -0,0 +1,39 @@
+Tests narrow stream clones
+
+  $ . "$TESTDIR/narrow-library.sh"
+
+Server setup
+
+  $ hg init master
+  $ cd master
+  $ mkdir dir
+  $ mkdir dir/src
+  $ cd dir/src
+  $ for x in `$TESTDIR/seq.py 20`; do echo $x > "f$x"; hg add "f$x"; hg commit -m "Commit src $x"; done
+
+  $ cd ..
+  $ mkdir tests
+  $ cd tests
+  $ for x in `$TESTDIR/seq.py 20`; do echo $x > "f$x"; hg add "f$x"; hg commit -m "Commit src $x"; done
+  $ cd ../../..
+
+Trying to stream clone when the server does not support it
+
+  $ hg clone --narrow ssh://user@dummy/master narrow --noupdate --include "dir/src/f10" --stream
+  streaming all changes
+  remote: abort: server does not support narrow stream clones
+  abort: pull failed on remote
+  [255]
+
+Enable stream clone on the server
+
+  $ echo "[server]" >> master/.hg/hgrc
+  $ echo "stream-narrow-clones=True" >> master/.hg/hgrc
+
+Cloning a specific file when stream clone is supported
+
+  $ hg clone --narrow ssh://user@dummy/master narrow --noupdate --include "dir/src/f10" --stream
+  streaming all changes
+  remote: abort: server does not support narrow stream clones
+  abort: pull failed on remote
+  [255]
diff --git a/mercurial/streamclone.py b/mercurial/streamclone.py
--- a/mercurial/streamclone.py
+++ b/mercurial/streamclone.py
@@ -531,7 +531,7 @@
             finally:
                 fp.close()
 
-def generatev2(repo):
+def generatev2(repo, includes, excludes):
     """Emit content for version 2 of a streaming clone.
 
     the data stream consists the following entries:
@@ -544,6 +544,10 @@
     Returns a 3-tuple of (file count, file size, data iterator).
     """
 
+    # temporarily raise error until we add storage level logic
+    if includes or excludes:
+        raise error.Abort(_("server does not support narrow stream clones"))
+
     with repo.lock():
 
         entries = []
diff --git a/mercurial/configitems.py b/mercurial/configitems.py
--- a/mercurial/configitems.py
+++ b/mercurial/configitems.py
@@ -610,6 +610,9 @@
 coreconfigitem('experimental', 'server.manifestdata.recommended-batch-size',
     default=100000,
 )
+coreconfigitem('experimental.server', 'stream-narrow-clones',
+    default=False,
+)
 coreconfigitem('experimental', 'single-head-per-branch',
     default=False,
 )
diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
--- a/mercurial/bundle2.py
+++ b/mercurial/bundle2.py
@@ -1687,7 +1687,18 @@
     # to avoid compression to consumers of the bundle.
     bundler.prefercompressed = False
 
-    filecount, bytecount, it = streamclone.generatev2(repo)
+    # get the includes and excludes
+    includepats = kwargs.get(r'includepats')
+    excludepats = kwargs.get(r'excludepats')
+
+    narrowstream = repo.ui.configbool('experimental.server',
+                                      'stream-narrow-clones')
+
+    if (includepats or excludepats) and not narrowstream:
+        raise error.Abort(_('server does not support narrow stream clones'))
+
+    filecount, bytecount, it = streamclone.generatev2(repo, includepats,
+                                                      excludepats)
     requirements = _formatrequirementsspec(repo.requirements)
     part = bundler.newpart('stream2', data=it)
     part.addparam('bytecount', '%d' % bytecount, mandatory=True)



To: pulkit, durin42, #hg-reviewers, martinvonz
Cc: martinvonz, indygreg, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5137: streamclone: new server config and some API changes for narrow stream clones

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
pulkit added a comment.


  @martinvonz many thanks for changing things in flight. Any reason why https://phab.mercurial-scm.org/D5138 and https://phab.mercurial-scm.org/D5139 are not considered during review because with them, you won't have to change much things except the config knob to experimental.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5137

To: pulkit, durin42, #hg-reviewers, martinvonz
Cc: martinvonz, indygreg, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

Re: D5137: streamclone: new server config and some API changes for narrow stream clones

Martin von Zweigbergk via Mercurial-devel


On Thu, Oct 18, 2018, 02:22 pulkit (Pulkit Goyal) <[hidden email] wrote:
pulkit added a comment.


  @martinvonz many thanks for changing things in flight. Any reason why https://phab.mercurial-scm.org/D5138 and https://phab.mercurial-scm.org/D5139 are not considered during review because with them, you won't have to change much things except the config knob to experimental.

Each patch should pass tests in its own. Otherwise code archeology (including bisection) will get much harder.

I just got a new computer that's fast enough that i can run tests on each patch. I had been taking shortcuts before, so if you had sent knowingly broken patches before, that may have been why I didn't notice :)


REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5137

To: pulkit, durin42, #hg-reviewers, martinvonz
Cc: martinvonz, indygreg, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

Re: D5137: streamclone: new server config and some API changes for narrow stream clones

Pulkit Goyal


On Thu, Oct 18, 2018 at 4:10 PM Martin von Zweigbergk via Mercurial-devel <[hidden email]> wrote:


On Thu, Oct 18, 2018, 02:22 pulkit (Pulkit Goyal) <[hidden email] wrote:
pulkit added a comment.


  @martinvonz many thanks for changing things in flight. Any reason why https://phab.mercurial-scm.org/D5138 and https://phab.mercurial-scm.org/D5139 are not considered during review because with them, you won't have to change much things except the config knob to experimental.

Each patch should pass tests in its own. Otherwise code archeology (including bisection) will get much harder.

I just got a new computer that's fast enough that i can run tests on each patch. I had been taking shortcuts before, so if you had sent knowingly broken patches before, that may have been why I didn't notice :)

Hehe, I generally run test-suite on the full series but it's very rare that I break something in one patch and then fix in another. Nice to know about the new computer btw ;) 

_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel