Submitter | phabricator |
---|---|
Date | Feb. 17, 2018, 10:03 a.m. |
Message ID | <differential-rev-PHID-DREV-cjqqjv4mwz2hlclf5lqk-req@phab.mercurial-scm.org> |
Download | mbox | patch |
Permalink | /patch/28020/ |
State | Superseded |
Headers | show |
Comments
Can you elaborate why you think it wasn't a good idea to replace open()? IMHO, it's as hackish as replacing '' with b''.
yuja added a comment. Can you elaborate why you think it wasn't a good idea to replace open()? IMHO, it's as hackish as replacing '' with b''. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D2297 To: pulkit, #hg-reviewers Cc: yuja, mercurial-devel
pulkit added a comment. In https://phab.mercurial-scm.org/D2297#37960, @yuja wrote: > Can you elaborate why you think it wasn't a good idea to replace open()? > > IMHO, it's as hackish as replacing '' with b''. Added couple of points. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D2297 To: pulkit, #hg-reviewers Cc: yuja, mercurial-devel
"There are around 100 occurences of open() call, it's better if we can prevent replacing all these calls "In future, when we will dropping all the compatibility code, we don't have to take care of changing pycompat.open back to open Yeah, that's the point why we have `from pycompat import open` to compensate the mess introduced by our code transformer. We want to keep 'rb' without b''. We could do that by the transformer, but I think it's better to keep the transformer less complicated. FWIW, we'll need to port many of these 100 open() calls to vfs. I have vague memory that Python 3.5 or 3.6 introduced another mess on Windows I/O around ANSI vs Unicode.
yuja added a comment. "There are around 100 occurences of open() call, it's better if we can prevent replacing all these calls "In future, when we will dropping all the compatibility code, we don't have to take care of changing pycompat.open back to open Yeah, that's the point why we have `from pycompat import open` to compensate the mess introduced by our code transformer. We want to keep 'rb' without b''. We could do that by the transformer, but I think it's better to keep the transformer less complicated. FWIW, we'll need to port many of these 100 open() calls to vfs. I have vague memory that Python 3.5 or 3.6 introduced another mess on Windows I/O around ANSI vs Unicode. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D2297 To: pulkit, #hg-reviewers Cc: yuja, mercurial-devel
pulkit abandoned this revision. pulkit added a comment. In https://phab.mercurial-scm.org/D2297#39249, @yuja wrote: > "There are around 100 occurences of open() call, it's better if we can > prevent replacing all these calls > "In future, when we will dropping all the compatibility code, we don't have > to take care of changing pycompat.open back to open > > Yeah, that's the point why we have `from pycompat import open` to compensate > the mess introduced by our code transformer. We want to keep 'rb' without b''. > We could do that by the transformer, but I think it's better to keep the > transformer less complicated. I agree with you that we should keep transformer less complicated as much possible. Abandoning this revision. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D2297 To: pulkit, #hg-reviewers Cc: yuja, mercurial-devel
Patch
diff --git a/mercurial/__init__.py b/mercurial/__init__.py --- a/mercurial/__init__.py +++ b/mercurial/__init__.py @@ -207,6 +207,13 @@ if argidx is not None: _ensureunicode(argidx) + # Bare open call (not an attribute on something else), the + # second argument (mode) must be a string, not bytes + elif fn == 'open' and not _isop(i - 1, '.'): + arg1idx = _findargnofcall(1) + if arg1idx is not None: + _ensureunicode(arg1idx) + # It changes iteritems/values to items/values as they are not # present in Python 3 world. elif fn in ('iteritems', 'itervalues'): @@ -220,7 +227,7 @@ # ``replacetoken`` or any mechanism that changes semantics of module # loading is changed. Otherwise cached bytecode may get loaded without # the new transformation mechanisms applied. - BYTECODEHEADER = b'HG\x00\x0a' + BYTECODEHEADER = b'HG\x00\x0b' class hgloader(importlib.machinery.SourceFileLoader): """Custom module loader that transforms source code.