From 2b52fb40f79ee3f634b14ff166fefeb7f15472ae Mon Sep 17 00:00:00 2001 From: Yoriyasu Yano <430092+yorinasub17@users.noreply.github.com> Date: Thu, 20 Dec 2018 18:03:36 -0800 Subject: [PATCH 01/10] Change windows separator with unix separator in regex --- pex/third_party/__init__.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pex/third_party/__init__.py b/pex/third_party/__init__.py index e845b13a8..3d12889cc 100644 --- a/pex/third_party/__init__.py +++ b/pex/third_party/__init__.py @@ -116,8 +116,11 @@ def iter_root_packages(self, relpath): yield package def _filter_names(self, relpath, pattern, group): + prefix = self.prefix + ((relpath + os.sep) if relpath else '') + if os.sep == '\\': + prefix = prefix.replace('\\', '/') pat = re.compile(r'^{prefix}{pattern}$' - .format(prefix=self.prefix + ((relpath + os.sep) if relpath else ''), + .format(prefix=prefix, pattern=pattern)) with contextlib.closing(zipfile.ZipFile(self.zipfile_path)) as zf: From 8567355cca2a82a50fd1dc9ac44a708259dd0254 Mon Sep 17 00:00:00 2001 From: Yoriyasu Yano <430092+yorinasub17@users.noreply.github.com> Date: Thu, 20 Dec 2018 18:36:17 -0800 Subject: [PATCH 02/10] Fix indentation --- pex/third_party/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pex/third_party/__init__.py b/pex/third_party/__init__.py index 3d12889cc..77161f97f 100644 --- a/pex/third_party/__init__.py +++ b/pex/third_party/__init__.py @@ -118,7 +118,7 @@ def iter_root_packages(self, relpath): def _filter_names(self, relpath, pattern, group): prefix = self.prefix + ((relpath + os.sep) if relpath else '') if os.sep == '\\': - prefix = prefix.replace('\\', '/') + prefix = prefix.replace('\\', '/') pat = re.compile(r'^{prefix}{pattern}$' .format(prefix=prefix, pattern=pattern)) From f17e3a1ad6b2bdfd26b8251879679b4f8775dc0d Mon Sep 17 00:00:00 2001 From: Yoriyasu Yano <430092+yorinasub17@users.noreply.github.com> Date: Tue, 9 Apr 2019 14:11:53 -0700 Subject: [PATCH 03/10] Simplify to force / instead of using os.sep --- pex/third_party/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pex/third_party/__init__.py b/pex/third_party/__init__.py index 77161f97f..bf0dcf542 100644 --- a/pex/third_party/__init__.py +++ b/pex/third_party/__init__.py @@ -116,9 +116,9 @@ def iter_root_packages(self, relpath): yield package def _filter_names(self, relpath, pattern, group): - prefix = self.prefix + ((relpath + os.sep) if relpath else '') - if os.sep == '\\': - prefix = prefix.replace('\\', '/') + # We use '/' here instead of os.sep because the zip file format spec specifies that paths must use forward slashes. + # See section 4.4.17 of https://support.pkware.com/display/PKZIP/APPNOTE + prefix = self.prefix + ((relpath + '/') if relpath else '') pat = re.compile(r'^{prefix}{pattern}$' .format(prefix=prefix, pattern=pattern)) From 081fd1c18e1e5c65de53f9ea805531491414bacb Mon Sep 17 00:00:00 2001 From: Yoriyasu Yano <430092+yorinasub17@users.noreply.github.com> Date: Tue, 9 Apr 2019 18:53:59 -0700 Subject: [PATCH 04/10] Update link to zip spec and style update for pattern --- pex/third_party/__init__.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/pex/third_party/__init__.py b/pex/third_party/__init__.py index bf0dcf542..23b6da2a7 100644 --- a/pex/third_party/__init__.py +++ b/pex/third_party/__init__.py @@ -117,12 +117,9 @@ def iter_root_packages(self, relpath): def _filter_names(self, relpath, pattern, group): # We use '/' here instead of os.sep because the zip file format spec specifies that paths must use forward slashes. - # See section 4.4.17 of https://support.pkware.com/display/PKZIP/APPNOTE - prefix = self.prefix + ((relpath + '/') if relpath else '') - pat = re.compile(r'^{prefix}{pattern}$' - .format(prefix=prefix, - pattern=pattern)) - + # See section 4.4.17 of https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT + relpath_pat = '{}/'.format(relpath) if relpath else '' + pat = re.compile(r'^{}{}{}$'.format(self.prefix, relpath_pat, pattern)) with contextlib.closing(zipfile.ZipFile(self.zipfile_path)) as zf: for name in zf.namelist(): match = pat.match(name) From 08db4b30bd2c34e49340f104dbbc36e6d4202722 Mon Sep 17 00:00:00 2001 From: Yoriyasu Yano <430092+yorinasub17@users.noreply.github.com> Date: Tue, 9 Apr 2019 20:05:57 -0700 Subject: [PATCH 05/10] Fix style error --- pex/third_party/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pex/third_party/__init__.py b/pex/third_party/__init__.py index 23b6da2a7..da9663a44 100644 --- a/pex/third_party/__init__.py +++ b/pex/third_party/__init__.py @@ -116,7 +116,8 @@ def iter_root_packages(self, relpath): yield package def _filter_names(self, relpath, pattern, group): - # We use '/' here instead of os.sep because the zip file format spec specifies that paths must use forward slashes. + # We use '/' here instead of os.sep because the zip file format spec specifies that paths must + # use forward slashes. # See section 4.4.17 of https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT relpath_pat = '{}/'.format(relpath) if relpath else '' pat = re.compile(r'^{}{}{}$'.format(self.prefix, relpath_pat, pattern)) From 539e67136c32166bdb3ed9afbbf2d7118fa736c0 Mon Sep 17 00:00:00 2001 From: Yoriyasu Yano <430092+yorinasub17@users.noreply.github.com> Date: Tue, 9 Apr 2019 21:07:15 -0700 Subject: [PATCH 06/10] Fix prefix to use / instead os.sep as well --- pex/third_party/__init__.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/pex/third_party/__init__.py b/pex/third_party/__init__.py index da9663a44..7acf4872c 100644 --- a/pex/third_party/__init__.py +++ b/pex/third_party/__init__.py @@ -100,9 +100,13 @@ def containing(cls, root): prefix = '' path = root while path: + # We use '/' here instead of os.sep because the zip file format spec specifies that paths + # must use forward slashes. See section 4.4.17 of + # https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT if zipfile.is_zipfile(path): - return cls(zipfile_path=path, prefix=prefix + os.sep if prefix else '') - prefix = os.path.join(prefix, os.path.basename(path)) + return cls(zipfile_path=path, prefix='{}/'.format(prefix) if prefix else '') + path_basename = os.path.basename(path) + prefix = '{}/{}'.format(prefix, path_basename) if prefix else path_basename path = os.path.dirname(path) raise ValueError('Could not find the zip file housing {}'.format(root)) @@ -117,8 +121,8 @@ def iter_root_packages(self, relpath): def _filter_names(self, relpath, pattern, group): # We use '/' here instead of os.sep because the zip file format spec specifies that paths must - # use forward slashes. - # See section 4.4.17 of https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT + # use forward slashes. See section 4.4.17 of + # https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT relpath_pat = '{}/'.format(relpath) if relpath else '' pat = re.compile(r'^{}{}{}$'.format(self.prefix, relpath_pat, pattern)) with contextlib.closing(zipfile.ZipFile(self.zipfile_path)) as zf: From 3241d07686173db18ac92361f693fb221f40541f Mon Sep 17 00:00:00 2001 From: Yoriyasu Yano <430092+yorinasub17@users.noreply.github.com> Date: Tue, 9 Apr 2019 21:09:24 -0700 Subject: [PATCH 07/10] ensure relpath uses / --- pex/third_party/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pex/third_party/__init__.py b/pex/third_party/__init__.py index 7acf4872c..3e0e87b7b 100644 --- a/pex/third_party/__init__.py +++ b/pex/third_party/__init__.py @@ -123,7 +123,7 @@ def _filter_names(self, relpath, pattern, group): # We use '/' here instead of os.sep because the zip file format spec specifies that paths must # use forward slashes. See section 4.4.17 of # https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT - relpath_pat = '{}/'.format(relpath) if relpath else '' + relpath_pat = '{}/'.format(relpath.replace(os.sep, '/')) if relpath else '' pat = re.compile(r'^{}{}{}$'.format(self.prefix, relpath_pat, pattern)) with contextlib.closing(zipfile.ZipFile(self.zipfile_path)) as zf: for name in zf.namelist(): From 07e88b310afcbcb9144708f28cceeb00b3bfd251 Mon Sep 17 00:00:00 2001 From: Yoriyasu Yano <430092+yorinasub17@users.noreply.github.com> Date: Tue, 9 Apr 2019 23:42:50 -0700 Subject: [PATCH 08/10] Clean up some more by using replace --- pex/third_party/__init__.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pex/third_party/__init__.py b/pex/third_party/__init__.py index 3e0e87b7b..4e1803548 100644 --- a/pex/third_party/__init__.py +++ b/pex/third_party/__init__.py @@ -102,11 +102,10 @@ def containing(cls, root): while path: # We use '/' here instead of os.sep because the zip file format spec specifies that paths # must use forward slashes. See section 4.4.17 of - # https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT + # https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT. if zipfile.is_zipfile(path): - return cls(zipfile_path=path, prefix='{}/'.format(prefix) if prefix else '') - path_basename = os.path.basename(path) - prefix = '{}/{}'.format(prefix, path_basename) if prefix else path_basename + return cls(zipfile_path=path, prefix='{}{}'.format(prefix, os.sep).replace(os.sep, '/')) + prefix = os.path.join(prefix, os.path.basename(path)).replace(os.sep, '/') path = os.path.dirname(path) raise ValueError('Could not find the zip file housing {}'.format(root)) @@ -122,8 +121,9 @@ def iter_root_packages(self, relpath): def _filter_names(self, relpath, pattern, group): # We use '/' here instead of os.sep because the zip file format spec specifies that paths must # use forward slashes. See section 4.4.17 of - # https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT - relpath_pat = '{}/'.format(relpath.replace(os.sep, '/')) if relpath else '' + # https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT. Note that relpath is constructed + # using os.sep by the time it is passed in here, so we need to replace it with '/'. + relpath_pat = '' if not relpath else '{}{}'.format(relpath, os.sep).replace(os.sep, '/') pat = re.compile(r'^{}{}{}$'.format(self.prefix, relpath_pat, pattern)) with contextlib.closing(zipfile.ZipFile(self.zipfile_path)) as zf: for name in zf.namelist(): From 83ae15837e77b5fe78b6b29acf0cc387415288ed Mon Sep 17 00:00:00 2001 From: Yoriyasu Yano <430092+yorinasub17@users.noreply.github.com> Date: Tue, 9 Apr 2019 23:47:28 -0700 Subject: [PATCH 09/10] Make comments better --- pex/third_party/__init__.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/pex/third_party/__init__.py b/pex/third_party/__init__.py index 4e1803548..4f9f57080 100644 --- a/pex/third_party/__init__.py +++ b/pex/third_party/__init__.py @@ -100,8 +100,8 @@ def containing(cls, root): prefix = '' path = root while path: - # We use '/' here instead of os.sep because the zip file format spec specifies that paths - # must use forward slashes. See section 4.4.17 of + # We replace os.sep with '/' here because the zip file format spec specifies that paths must + # use forward slashes. See section 4.4.17 of # https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT. if zipfile.is_zipfile(path): return cls(zipfile_path=path, prefix='{}{}'.format(prefix, os.sep).replace(os.sep, '/')) @@ -119,10 +119,9 @@ def iter_root_packages(self, relpath): yield package def _filter_names(self, relpath, pattern, group): - # We use '/' here instead of os.sep because the zip file format spec specifies that paths must + # We replace os.sep with '/' here because the zip file format spec specifies that paths must # use forward slashes. See section 4.4.17 of - # https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT. Note that relpath is constructed - # using os.sep by the time it is passed in here, so we need to replace it with '/'. + # https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT. relpath_pat = '' if not relpath else '{}{}'.format(relpath, os.sep).replace(os.sep, '/') pat = re.compile(r'^{}{}{}$'.format(self.prefix, relpath_pat, pattern)) with contextlib.closing(zipfile.ZipFile(self.zipfile_path)) as zf: From 2546df0e40600d9810777a4803a3d06e6f722d2d Mon Sep 17 00:00:00 2001 From: Yoriyasu Yano <430092+yorinasub17@users.noreply.github.com> Date: Sat, 20 Apr 2019 11:36:55 -0700 Subject: [PATCH 10/10] Clean up code based on PR comments --- pex/third_party/__init__.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/pex/third_party/__init__.py b/pex/third_party/__init__.py index 4f9f57080..9ce6894a0 100644 --- a/pex/third_party/__init__.py +++ b/pex/third_party/__init__.py @@ -100,12 +100,13 @@ def containing(cls, root): prefix = '' path = root while path: - # We replace os.sep with '/' here because the zip file format spec specifies that paths must - # use forward slashes. See section 4.4.17 of + # We use '/' here because the zip file format spec specifies that paths must use + # forward slashes. See section 4.4.17 of # https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT. if zipfile.is_zipfile(path): - return cls(zipfile_path=path, prefix='{}{}'.format(prefix, os.sep).replace(os.sep, '/')) - prefix = os.path.join(prefix, os.path.basename(path)).replace(os.sep, '/') + return cls(zipfile_path=path, prefix='{}/'.format(prefix) if prefix else '') + path_base = os.path.basename(path) + prefix = '{}/{}'.format(path_base, prefix) if prefix else path_base path = os.path.dirname(path) raise ValueError('Could not find the zip file housing {}'.format(root)) @@ -119,10 +120,10 @@ def iter_root_packages(self, relpath): yield package def _filter_names(self, relpath, pattern, group): - # We replace os.sep with '/' here because the zip file format spec specifies that paths must - # use forward slashes. See section 4.4.17 of + # We use '/' here because the zip file format spec specifies that paths must use + # forward slashes. See section 4.4.17 of # https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT. - relpath_pat = '' if not relpath else '{}{}'.format(relpath, os.sep).replace(os.sep, '/') + relpath_pat = '' if not relpath else '{}/'.format(relpath.replace(os.sep, '/')) pat = re.compile(r'^{}{}{}$'.format(self.prefix, relpath_pat, pattern)) with contextlib.closing(zipfile.ZipFile(self.zipfile_path)) as zf: for name in zf.namelist():