Patchwork D8099: lfs: use str for the open() mode when opening a blob for py3

login
register
mail settings
Submitter phabricator
Date Feb. 9, 2020, 4:48 a.m.
Message ID <differential-rev-PHID-DREV-gppllg2rh5y7jzmcqmfk-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/45055/
State Superseded
Headers show

Comments

phabricator - Feb. 9, 2020, 4:48 a.m.
mharbison72 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  The other fix for this was to leave the mode as bytes, and import
  `pycompat.open()` like a bunch of other modules do.  But I think it's confusing
  to still use bytes at the python boundary, and obviously error prone.  Grepping
  for ` open\(.+, ['"][a-z]+['"]\)` and ` open\(.+, b['"][a-z]+['"]\)` outside of
  `tests`, there are 51 and 87 uses respectively, so it's not like this is a rare
  direct usage.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  hgext/lfs/blobstore.py

CHANGE DETAILS




To: mharbison72, #hg-reviewers
Cc: mercurial-devel
phabricator - Feb. 10, 2020, 6:10 p.m.
This revision now requires changes to proceed.
pulkit added a comment.
pulkit requested changes to this revision.


  This one fails to apply on tip of stable branch. Can you rebase and resend?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8099/new/

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

To: mharbison72, #hg-reviewers, pulkit
Cc: mercurial-devel
phabricator - Feb. 10, 2020, 6:57 p.m.
mharbison72 added a comment.
mharbison72 requested review of this revision.


  In D8099#120114 <https://phab.mercurial-scm.org/D8099#120114>, @pulkit wrote:
  
  > This one fails to apply on tip of stable branch. Can you rebase and resend?
  
  I think the thing this is fixing is on default

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8099/new/

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

To: mharbison72, #hg-reviewers, pulkit
Cc: mercurial-devel
phabricator - Feb. 10, 2020, 9:39 p.m.
durin42 added a comment.
durin42 accepted this revision.


  Looks fine to me for default, queued.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8099/new/

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

To: mharbison72, #hg-reviewers, pulkit, durin42
Cc: durin42, mercurial-devel
phabricator - Feb. 10, 2020, 9:40 p.m.
durin42 added a comment.


  Forgot to say: I'd be a big fan of unwinding pycompat.open() now that we're done with the module transformer.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8099/new/

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

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

Patch

diff --git a/hgext/lfs/blobstore.py b/hgext/lfs/blobstore.py
--- a/hgext/lfs/blobstore.py
+++ b/hgext/lfs/blobstore.py
@@ -128,7 +128,7 @@ 
     def open(self, oid):
         """Open a read-only file descriptor to the named blob, in either the
         usercache or the local store."""
-        return open(self.path(oid), b'rb')
+        return open(self.path(oid), 'rb')
 
     def path(self, oid):
         """Build the path for the given blob ``oid``.