Patchwork D5812: py3: pass str into ValueError to prevent b'' prefix in output

login
register
mail settings
Submitter phabricator
Date Feb. 3, 2019, 12:13 p.m.
Message ID <differential-rev-PHID-DREV-dpnlu7p5mtg3an5c7id6-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/38346/
State New
Headers show

Comments

phabricator - Feb. 3, 2019, 12:13 p.m.
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
  1. skip-blame because mostly just r'' prefixes

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  hgext/narrow/narrowbundle2.py
  mercurial/exchange.py

CHANGE DETAILS




To: pulkit, durin42, martinvonz, #hg-reviewers
Cc: mercurial-devel
phabricator - Feb. 3, 2019, 12:19 p.m.
pulkit added a comment.


  I am not whether this is correct. This should have fix failure of `test-lfs-serve.t#lfsremote-on` failure on py3 but it does not.

REPOSITORY
  rHG Mercurial

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

To: pulkit, durin42, martinvonz, #hg-reviewers
Cc: mercurial-devel
Yuya Nishihara - Feb. 3, 2019, 12:46 p.m.
>   I am not whether this is correct. This should have fix failure of `test-lfs-serve.t#lfsremote-on` failure on py3 but it does not.

As you guess, this isn't correct. For example, `_(unicode_sring)` will crash
if gettext is enabled.

I don't know why ValueError is used extensively in exchange.py and where it's
supposed to be caught, but the easiest workaround is to make a ValueError
subclass that implements __bytes__.

```
class ExchangeValueError(ValueError):
    __bytes__ = _tobytes
```
phabricator - Feb. 3, 2019, 12:48 p.m.
yuja added a comment.


  >   I am not whether this is correct. This should have fix failure of `test-lfs-serve.t#lfsremote-on` failure on py3 but it does not.
  
  As you guess, this isn't correct. For example, `_(unicode_sring)` will crash
  if gettext is enabled.
  
  I don't know why ValueError is used extensively in exchange.py and where it's
  supposed to be caught, but the easiest workaround is to make a ValueError
  subclass that implements __bytes__.
  
    class ExchangeValueError(ValueError):
        __bytes__ = _tobytes

REPOSITORY
  rHG Mercurial

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

To: pulkit, durin42, martinvonz, #hg-reviewers
Cc: yuja, mercurial-devel

Patch

diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -920,7 +920,7 @@ 
                       if v in changegroup.supportedoutgoingversions(
                           pushop.repo)]
         if not cgversions:
-            raise ValueError(_('no common changegroup version'))
+            raise ValueError(_(r'no common changegroup version'))
         version = max(cgversions)
     cgstream = changegroup.makestream(pushop.repo, pushop.outgoing, version,
                                       'push')
@@ -2130,22 +2130,22 @@ 
     Returns a 2-tuple of a dict with metadata about the generated bundle
     and an iterator over raw chunks (of varying sizes).
     """
-    kwargs = pycompat.byteskwargs(kwargs)
     info = {}
     usebundle2 = bundle2requested(bundlecaps)
     # bundle10 case
     if not usebundle2:
-        if bundlecaps and not kwargs.get('cg', True):
-            raise ValueError(_('request for bundle10 must include changegroup'))
+        if bundlecaps and not kwargs.get(r'cg', True):
+            raise ValueError(_(r'request for bundle10 must include changegroup'))
 
         if kwargs:
-            raise ValueError(_('unsupported getbundle arguments: %s')
-                             % ', '.join(sorted(kwargs.keys())))
+            raise ValueError(_(r'unsupported getbundle arguments: %s')
+                             % r', '.join(sorted(kwargs.keys())))
         outgoing = _computeoutgoing(repo, heads, common)
         info['bundleversion'] = 1
         return info, changegroup.makestream(repo, outgoing, '01', source,
                                             bundlecaps=bundlecaps)
 
+    kwargs = pycompat.byteskwargs(kwargs)
     # bundle20 case
     info['bundleversion'] = 2
     b2caps = {}
@@ -2184,7 +2184,7 @@ 
         cgversions = [v for v in cgversions
                       if v in changegroup.supportedoutgoingversions(repo)]
         if not cgversions:
-            raise ValueError(_('no common changegroup version'))
+            raise ValueError(_(r'no common changegroup version'))
         version = max(cgversions)
 
     outgoing = _computeoutgoing(repo, heads, common)
@@ -2228,7 +2228,7 @@ 
     if not kwargs.get(r'bookmarks', False):
         return
     if 'bookmarks' not in b2caps:
-        raise ValueError(_('no common bookmarks exchange method'))
+        raise ValueError(_(r'no common bookmarks exchange method'))
     books  = bookmod.listbinbookmarks(repo)
     data = bookmod.binaryencode(books)
     if data:
@@ -2263,7 +2263,7 @@ 
     """add phase heads part to the requested bundle"""
     if kwargs.get(r'phases', False):
         if not 'heads' in b2caps.get('phases'):
-            raise ValueError(_('no common phases exchange method'))
+            raise ValueError(_(r'no common phases exchange method'))
         if heads is None:
             heads = repo.heads()
 
diff --git a/hgext/narrow/narrowbundle2.py b/hgext/narrow/narrowbundle2.py
--- a/hgext/narrow/narrowbundle2.py
+++ b/hgext/narrow/narrowbundle2.py
@@ -54,11 +54,11 @@ 
         cgversions = [v for v in cgversions
                       if v in changegroup.supportedoutgoingversions(repo)]
         if not cgversions:
-            raise ValueError(_('no common changegroup version'))
+            raise ValueError(_(r'no common changegroup version'))
         version = max(cgversions)
     else:
-        raise ValueError(_("server does not advertise changegroup version,"
-                           " can't negotiate support for ellipsis nodes"))
+        raise ValueError(_(r"server does not advertise changegroup version,"
+                           r" can't negotiate support for ellipsis nodes"))
 
     include = sorted(filter(bool, kwargs.get(r'includepats', [])))
     exclude = sorted(filter(bool, kwargs.get(r'excludepats', [])))