Submitter | phabricator |
---|---|
Date | Nov. 23, 2019, 5:15 a.m. |
Message ID | <differential-rev-PHID-DREV-ggynhzjcpyfaggsml5w5-req@mercurial-scm.org> |
Download | mbox | patch |
Permalink | /patch/43465/ |
State | Superseded |
Headers | show |
Comments
> """add a changegroup part to the requested bundle""" > - if not kwargs.get('cg', True): > + if not kwargs.get('cg', True) or not b2caps: > return Is it valid to call these functions with `b2caps=None`? I suspect it would be a bug or a data corruption.
yuja added a comment. > """add a changegroup part to the requested bundle""" > > - if not kwargs.get('cg', True): > > + if not kwargs.get('cg', True) or not b2caps: > > return Is it valid to call these functions with `b2caps=None`? I suspect it would be a bug or a data corruption. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7512/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7512 To: mharbison72, #hg-reviewers, dlax, indygreg Cc: yuja, mercurial-devel
mharbison72 added a comment. In D7512#110528 <https://phab.mercurial-scm.org/D7512#110528>, @yuja wrote: >> """add a changegroup part to the requested bundle""" >> >> - if not kwargs.get('cg', True): >> >> + if not kwargs.get('cg', True) or not b2caps: >> >> return > > Is it valid to call these functions with `b2caps=None`? I suspect it would > be a bug or a data corruption. The only caller I can find[1] will indeed pass something, even if it is `{}`. I can change these to asserts if you want. [1] https://www.mercurial-scm.org/repo/hg/file/tip/mercurial/exchange.py#l2448 REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7512/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7512 To: mharbison72, #hg-reviewers, dlax, indygreg Cc: yuja, mercurial-devel
> >> """add a changegroup part to the requested bundle""" > >> > >> - if not kwargs.get('cg', True): > >> > >> + if not kwargs.get('cg', True) or not b2caps: > >> > >> return > > > > Is it valid to call these functions with `b2caps=None`? I suspect it would > > be a bug or a data corruption. > > The only caller I can find[1] will indeed pass something, even if it is `{}`. I can change these to asserts if you want. > > [1] https://www.mercurial-scm.org/repo/hg/file/tip/mercurial/exchange.py#l2448 Well, I have no expertise around this module, so I have no idea which is better. I just don't know if it's valid to return successfully if b2caps is None.
yuja added a comment. > >> """add a changegroup part to the requested bundle""" > >> > >> - if not kwargs.get('cg', True): > >> > >> + if not kwargs.get('cg', True) or not b2caps: > >> > >> return > > > > Is it valid to call these functions with `b2caps=None`? I suspect it would > > be a bug or a data corruption. > The only caller I can find[1] will indeed pass something, even if it is `{}`. I can change these to asserts if you want. > [1] https://www.mercurial-scm.org/repo/hg/file/tip/mercurial/exchange.py#l2448 Well, I have no expertise around this module, so I have no idea which is better. I just don't know if it's valid to return successfully if b2caps is None. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7512/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7512 To: mharbison72, #hg-reviewers, dlax, indygreg Cc: yuja, mercurial-devel
mharbison72 added a comment. mharbison72 added a subscriber: marmoute. In D7512#110589 <https://phab.mercurial-scm.org/D7512#110589>, @yuja wrote: >> >> """add a changegroup part to the requested bundle""" >> >> >> >> - if not kwargs.get('cg', True): >> >> >> >> + if not kwargs.get('cg', True) or not b2caps: >> >> >> >> return >> > >> > Is it valid to call these functions with `b2caps=None`? I suspect it would >> > be a bug or a data corruption. >> The only caller I can find[1] will indeed pass something, even if it is `{}`. I can change these to asserts if you want. >> [1] https://www.mercurial-scm.org/repo/hg/file/tip/mercurial/exchange.py#l2448 > > Well, I have no expertise around this module, so I have no idea which is > better. I just don't know if it's valid to return successfully if b2caps > is None. @marmoute? REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7512/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7512 To: mharbison72, #hg-reviewers, dlax, indygreg Cc: marmoute, yuja, mercurial-devel
marmoute added a comment.
In D7512#110590 <https://phab.mercurial-scm.org/D7512#110590>, @mharbison72 wrote:
> @marmoute?
I will have a look.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST ACTION
https://phab.mercurial-scm.org/D7512/new/
REVISION DETAIL
https://phab.mercurial-scm.org/D7512
To: mharbison72, #hg-reviewers, dlax, indygreg
Cc: marmoute, yuja, mercurial-devel
Patch
diff --git a/mercurial/exchange.py b/mercurial/exchange.py --- a/mercurial/exchange.py +++ b/mercurial/exchange.py @@ -2474,7 +2474,7 @@ **kwargs ): """add a changegroup part to the requested bundle""" - if not kwargs.get('cg', True): + if not kwargs.get('cg', True) or not b2caps: return version = b'01' @@ -2536,7 +2536,7 @@ """add a bookmark part to the requested bundle""" if not kwargs.get('bookmarks', False): return - if b'bookmarks' not in b2caps: + if not b2caps or b'bookmarks' not in b2caps: raise error.Abort(_(b'no common bookmarks exchange method')) books = bookmod.listbinbookmarks(repo) data = bookmod.binaryencode(books) @@ -2577,7 +2577,7 @@ ): """add phase heads part to the requested bundle""" if kwargs.get('phases', False): - if not b'heads' in b2caps.get(b'phases'): + if not b2caps or not b'heads' in b2caps.get(b'phases'): raise error.Abort(_(b'no common phases exchange method')) if heads is None: heads = repo.heads() @@ -2641,7 +2641,7 @@ # Don't send unless: # - changeset are being exchanged, # - the client supports it. - if not (kwargs.get('cg', True) and b'hgtagsfnodes' in b2caps): + if not b2caps or not (kwargs.get('cg', True) and b'hgtagsfnodes' in b2caps): return outgoing = _computeoutgoing(repo, heads, common) @@ -2675,6 +2675,7 @@ # - narrow bundle isn't in play (not currently compatible). if ( not kwargs.get('cg', True) + or not b2caps or b'rev-branch-cache' not in b2caps or kwargs.get('narrow', False) or repo.ui.has_section(_NARROWACL_SECTION)