This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8099

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

Authored by mharbison72 on Feb 8 2020, 11:48 PM.

Details

Reviewers
pulkit
durin42
Group Reviewers
hg-reviewers
Commits
rHG234001d22ba6: lfs: use str for the open() mode when opening a blob for py3
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.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mharbison72 created this revision.Feb 8 2020, 11:48 PM
pulkit accepted this revision.Feb 10 2020, 11:30 AM
This revision is now accepted and ready to land.Feb 10 2020, 11:30 AM
pulkit requested changes to this revision.Feb 10 2020, 1:10 PM

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

This revision now requires changes to proceed.Feb 10 2020, 1:10 PM
mharbison72 requested review of this revision.Feb 10 2020, 1:57 PM
In 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

durin42 accepted this revision.Feb 10 2020, 4:39 PM
durin42 added a subscriber: durin42.

Looks fine to me for default, queued.

durin42 added a comment.Feb 10 2020, 4:39 PM

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

mharbison72 added a commit: rHG234001d22ba6: lfs: use str for the open() mode when opening a blob for py3.Feb 10 2020, 5:01 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG234001d22ba6: lfs: use str for the open() mode when opening a blob for py3 (authored by mharbison72). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mhgext/lfs/blobstore.py (2 lines)

Diff 20062

hgext/lfs/blobstore.py

else: else:
usercache = lfutil._usercachedir(repo.ui, b'lfs') usercache = lfutil._usercachedir(repo.ui, b'lfs')
self.cachevfs = lfsvfs(usercache) self.cachevfs = lfsvfs(usercache)
self.ui = repo.ui self.ui = repo.ui
def open(self, oid): def open(self, oid):
"""Open a read-only file descriptor to the named blob, in either the """Open a read-only file descriptor to the named blob, in either the
usercache or the local store.""" usercache or the local store."""
return open(self.path(oid), b'rb') return open(self.path(oid), 'rb')
def path(self, oid): def path(self, oid):
"""Build the path for the given blob ``oid``. """Build the path for the given blob ``oid``.
If the blob exists locally, the path may point to either the usercache If the blob exists locally, the path may point to either the usercache
or the local store. If it doesn't, it will point to the local store. or the local store. If it doesn't, it will point to the local store.
This is meant for situations where existing code that isn't LFS aware This is meant for situations where existing code that isn't LFS aware
needs to open a blob. Generally, prefer the ``open`` method on this needs to open a blob. Generally, prefer the ``open`` method on this