D4931: narrow: only send the narrowspecs back if ACL in play

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

D4931: narrow: only send the narrowspecs back if ACL in play

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

REVISION SUMMARY
  I am unable to think why we need to send narrowspecs back from the server. The
  current state adds a 'narrow:spec' part to each changegroup which is generated
  when narrow extension is enabled. So we are sending narrowspecs on pull also.
 
  There is a problem with sending the narrowspecs the way we are doing it right
  now. We add include and exclude as parameter of the 'narrow:spec' bundle2 part.
  The the len of include or exclude string increase 255 which is obvious while
  working on large repos, bundle2 generation code breaks. For more on that refer
  issue5952 on bugzilla.
 
  I was thinking why we need to send the narrowspecs back, and deleted the
  'narrow:spec' bundle2 part generation code and found that only narrow-acl test
  has some failure.
 
  With this patch, we will only send the 'narrow:spec' bundle2 part if ACL is
  enabled because the original narrowspecs in those cases can be a subset of
  narrowspecs user requested.
 
  There are phase related output change in couple of tests. The output change
  shows that we are now dealing in public phases completely. So maybe sending the
  narrow:spec bundle2 part was preventing phases being exchanged or phase bundle2
  data being applied.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  hgext/narrow/narrowbundle2.py
  mercurial/exchange.py
  tests/test-narrow-rebase.t
  tests/test-narrow-widen-no-ellipsis.t

CHANGE DETAILS

diff --git a/tests/test-narrow-widen-no-ellipsis.t b/tests/test-narrow-widen-no-ellipsis.t
--- a/tests/test-narrow-widen-no-ellipsis.t
+++ b/tests/test-narrow-widen-no-ellipsis.t
@@ -137,7 +137,6 @@
   adding file changes
   added 5 changesets with 4 changes to 2 files
   new changesets *:* (glob)
-  3 local changesets published
   (run 'hg update' to get a working copy)
   $ hg update -r 'desc("add wider")'
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
diff --git a/tests/test-narrow-rebase.t b/tests/test-narrow-rebase.t
--- a/tests/test-narrow-rebase.t
+++ b/tests/test-narrow-rebase.t
@@ -86,7 +86,6 @@
 
   $ hg update -q 'desc("conflicting outside/f1")'
   $ hg phase -f -d .
-  no phases changed
   $ hg rebase -d 'desc("modify outside/f1")'
   rebasing 4:707c035aadb6 "conflicting outside/f1"
   abort: conflict in file 'outside/f1' is outside narrow clone
diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -2164,7 +2164,8 @@
     if 'treemanifest' in repo.requirements:
         part.addparam('treemanifest', '1')
 
-    if kwargs.get(r'narrow', False) and (include or exclude):
+    if (kwargs.get(r'narrow', False) and kwargs.get('narrow_acl', False)
+        and (include or exclude)):
         narrowspecpart = bundler.newpart('narrow:spec')
         if include:
             narrowspecpart.addparam(
diff --git a/hgext/narrow/narrowbundle2.py b/hgext/narrow/narrowbundle2.py
--- a/hgext/narrow/narrowbundle2.py
+++ b/hgext/narrow/narrowbundle2.py
@@ -270,6 +270,7 @@
         repo = args[1]
         if repo.ui.has_section(_NARROWACL_SECTION):
             kwargs = exchange.applynarrowacl(repo, kwargs)
+            kwargs['narrow_acl'] = True
 
         if (kwargs.get(r'narrow', False) and
             repo.ui.configbool('experimental', 'narrowservebrokenellipses')):



To: pulkit, durin42, martinvonz, #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
|

D4931: narrow: only send the narrowspecs back if ACL in play

martinvonz (Martin von Zweigbergk)
pulkit added a comment.


  @martinvonz Looks like this got missed for discussing in sprint, can you have a look at this please? This is important because once we have narrowspecs greater than 255 length and and don't use ellipses, we hit https://bz.mercurial-scm.org/show_bug.cgi?id=5952

REPOSITORY
  rHG Mercurial

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

To: pulkit, durin42, martinvonz, #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
|

D4931: narrow: only send the narrowspecs back if ACL in play

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


  In https://phab.mercurial-scm.org/D4931#76476, @pulkit wrote:
 
  > @martinvonz Looks like this got missed for discussing in sprint, can you have a look at this please? This is important because once we have narrowspecs greater than 255 length and and don't use ellipses, we hit https://bz.mercurial-scm.org/show_bug.cgi?id=5952
 
 
  Maybe we can instead require `experimental.httppostargs` to be on when using narrow?

REPOSITORY
  rHG Mercurial

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

To: pulkit, durin42, martinvonz, #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
|

D4931: narrow: only send the narrowspecs back if ACL in play

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


  In https://phab.mercurial-scm.org/D4931#76568, @martinvonz wrote:
 
  > In https://phab.mercurial-scm.org/D4931#76476, @pulkit wrote:
  >
  > > @martinvonz Looks like this got missed for discussing in sprint, can you have a look at this please? This is important because once we have narrowspecs greater than 255 length and and don't use ellipses, we hit https://bz.mercurial-scm.org/show_bug.cgi?id=5952
  >
  >
  > Maybe we can instead require `experimental.httppostargs` to be on when using narrow?
 
 
  The problem is the narrow specs are being transported as a bundle2 part parameter and those have a limit of 255 bytes. The narrow spec needs to be transported as a bundle2 part payload, which can be infinite length. (Or not transferred at all.)

REPOSITORY
  rHG Mercurial

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

To: pulkit, durin42, martinvonz, #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
|

D4931: narrow: only send the narrowspecs back if ACL in play

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


  In https://phab.mercurial-scm.org/D4931#76571, @indygreg wrote:
 
  > In https://phab.mercurial-scm.org/D4931#76568, @martinvonz wrote:
  >
  > > In https://phab.mercurial-scm.org/D4931#76476, @pulkit wrote:
  > >
  > > > @martinvonz Looks like this got missed for discussing in sprint, can you have a look at this please? This is important because once we have narrowspecs greater than 255 length and and don't use ellipses, we hit https://bz.mercurial-scm.org/show_bug.cgi?id=5952
  > >
  > >
  > > Maybe we can instead require `experimental.httppostargs` to be on when using narrow?
  >
  >
  > The problem is the narrow specs are being transported as a bundle2 part parameter and those have a limit of 255 bytes. The narrow spec needs to be transported as a bundle2 part payload, which can be infinite length. (Or not transferred at all.)
 
 
  Oh, I had completely forgotten that we sometimes send that back. This patch looks fine then, I just have one little suggestion.

INLINE COMMENTS

> narrowbundle2.py:273
>              kwargs = exchange.applynarrowacl(repo, kwargs)
> +            kwargs['narrow_acl'] = True
>  

maybe this line belongs on exchange.py:1903?

REPOSITORY
  rHG Mercurial

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

To: pulkit, durin42, martinvonz, #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
|

D4931: narrow: only send the narrowspecs back if ACL in play

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
pulkit updated this revision to Diff 12192.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D4931?vs=11774&id=12192

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

AFFECTED FILES
  mercurial/exchange.py
  tests/test-narrow-rebase.t
  tests/test-narrow-widen-no-ellipsis.t

CHANGE DETAILS

diff --git a/tests/test-narrow-widen-no-ellipsis.t b/tests/test-narrow-widen-no-ellipsis.t
--- a/tests/test-narrow-widen-no-ellipsis.t
+++ b/tests/test-narrow-widen-no-ellipsis.t
@@ -137,7 +137,6 @@
   adding file changes
   added 5 changesets with 4 changes to 2 files
   new changesets *:* (glob)
-  3 local changesets published
   (run 'hg update' to get a working copy)
   $ hg update -r 'desc("add wider")'
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
diff --git a/tests/test-narrow-rebase.t b/tests/test-narrow-rebase.t
--- a/tests/test-narrow-rebase.t
+++ b/tests/test-narrow-rebase.t
@@ -86,7 +86,6 @@
 
   $ hg update -q 'desc("conflicting outside/f1")'
   $ hg phase -f -d .
-  no phases changed
   $ hg rebase -d 'desc("modify outside/f1")'
   rebasing 4:707c035aadb6 "conflicting outside/f1"
   abort: conflict in file 'outside/f1' is outside narrow clone
diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -1900,6 +1900,7 @@
     new_args = {}
     new_args.update(kwargs)
     new_args[r'narrow'] = True
+    new_args[r'narrow_acl'] = True
     new_args[r'includepats'] = req_includes
     if req_excludes:
         new_args[r'excludepats'] = req_excludes
@@ -2164,7 +2165,8 @@
     if 'treemanifest' in repo.requirements:
         part.addparam('treemanifest', '1')
 
-    if kwargs.get(r'narrow', False) and (include or exclude):
+    if (kwargs.get(r'narrow', False) and kwargs.get('narrow_acl', False)
+        and (include or exclude)):
         narrowspecpart = bundler.newpart('narrow:spec')
         if include:
             narrowspecpart.addparam(



To: pulkit, durin42, martinvonz, #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
|

D4931: narrow: only send the narrowspecs back if ACL in play

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 rHGcb516a854bc7: narrow: only send the narrowspecs back if ACL in play (authored by pulkit, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D4931?vs=12192&id=12226

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

AFFECTED FILES
  mercurial/exchange.py
  tests/test-narrow-rebase.t
  tests/test-narrow-widen-no-ellipsis.t

CHANGE DETAILS

diff --git a/tests/test-narrow-widen-no-ellipsis.t b/tests/test-narrow-widen-no-ellipsis.t
--- a/tests/test-narrow-widen-no-ellipsis.t
+++ b/tests/test-narrow-widen-no-ellipsis.t
@@ -137,7 +137,6 @@
   adding file changes
   added 5 changesets with 4 changes to 2 files
   new changesets *:* (glob)
-  3 local changesets published
   (run 'hg update' to get a working copy)
   $ hg update -r 'desc("add wider")'
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
diff --git a/tests/test-narrow-rebase.t b/tests/test-narrow-rebase.t
--- a/tests/test-narrow-rebase.t
+++ b/tests/test-narrow-rebase.t
@@ -86,7 +86,6 @@
 
   $ hg update -q 'desc("conflicting outside/f1")'
   $ hg phase -f -d .
-  no phases changed
   $ hg rebase -d 'desc("modify outside/f1")'
   rebasing 4:707c035aadb6 "conflicting outside/f1"
   abort: conflict in file 'outside/f1' is outside narrow clone
diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -1907,6 +1907,7 @@
     new_args = {}
     new_args.update(kwargs)
     new_args[r'narrow'] = True
+    new_args[r'narrow_acl'] = True
     new_args[r'includepats'] = req_includes
     if req_excludes:
         new_args[r'excludepats'] = req_excludes
@@ -2171,7 +2172,8 @@
     if 'treemanifest' in repo.requirements:
         part.addparam('treemanifest', '1')
 
-    if kwargs.get(r'narrow', False) and (include or exclude):
+    if (kwargs.get(r'narrow', False) and kwargs.get('narrow_acl', False)
+        and (include or exclude)):
         narrowspecpart = bundler.newpart('narrow:spec')
         if include:
             narrowspecpart.addparam(



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