Patchwork D6258: packaging: coerce paths to strings

login
register
mail settings
Submitter phabricator
Date April 16, 2019, 9:55 p.m.
Message ID <differential-rev-PHID-DREV-kpk7qghknqfmboyy7ph5-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/39669/
State Superseded
Headers show

Comments

phabricator - April 16, 2019, 9:55 p.m.
durin42 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Experimentally passing the pathlib.Path to zipfile.ZipFile fails on at
  least some Python versions. I've gotten frustrated enough I'd rather
  just force them to be strings and move on.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  contrib/packaging/hgpackaging/util.py

CHANGE DETAILS




To: durin42, #hg-reviewers
Cc: mercurial-devel
phabricator - April 16, 2019, 11:44 p.m.
durin42 added a comment.


  Argh. This change appears to be wrong (!) so I'm going to drop it.

REPOSITORY
  rHG Mercurial

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

To: durin42, #hg-reviewers, pulkit
Cc: mercurial-devel
phabricator - April 17, 2019, 9:49 a.m.
indygreg added a comment.


  In https://phab.mercurial-scm.org/D6258#91055, @durin42 wrote:
  
  > Argh. This change appears to be wrong (!) so I'm going to drop it.
  
  
  Eh? I don't see anything wrong with this. Casting a `pathlib.Path` to `str` should just work! In fact, it is necessary for older Python 3, as there was a long tail of stdlib functions that didn't accept `pathlib.*` when those types were first introduced. I believe 3.7 was the first release that fixed them all.

REPOSITORY
  rHG Mercurial

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

To: durin42, #hg-reviewers, pulkit
Cc: indygreg, mercurial-devel
phabricator - April 17, 2019, 6:44 p.m.
durin42 added a comment.


  In https://phab.mercurial-scm.org/D6258#91056, @indygreg wrote:
  
  > In https://phab.mercurial-scm.org/D6258#91055, @durin42 wrote:
  >
  > > Argh. This change appears to be wrong (!) so I'm going to drop it.
  >
  >
  > Eh? I don't see anything wrong with this. Casting a `pathlib.Path` to `str` should just work! In fact, it is necessary for older Python 3, as there was a long tail of stdlib functions that didn't accept `pathlib.*` when those types were first introduced. I believe 3.7 was the first release that fixed them all.
  
  
  At least one of the archive libraries can't cope with a destination being a Path but a source being a str, or something like that. zipfile can't (AFAICT) cope with Path objects on Python 3.5, but I worked around that by finding a Windows image at work that came with 3.7 already installed. :)

REPOSITORY
  rHG Mercurial

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

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

Patch

diff --git a/contrib/packaging/hgpackaging/util.py b/contrib/packaging/hgpackaging/util.py
--- a/contrib/packaging/hgpackaging/util.py
+++ b/contrib/packaging/hgpackaging/util.py
@@ -17,12 +17,12 @@ 
 
 
 def extract_tar_to_directory(source: pathlib.Path, dest: pathlib.Path):
-    with tarfile.open(source, 'r') as tf:
+    with tarfile.open(str(source), 'r') as tf:
         tf.extractall(dest)
 
 
 def extract_zip_to_directory(source: pathlib.Path, dest: pathlib.Path):
-    with zipfile.ZipFile(source, 'r') as zf:
+    with zipfile.ZipFile(str(source), 'r') as zf:
         zf.extractall(dest)