Skip to content

Commit 000d441

Browse files
authored
Remove PyFilesystem2 dependency @W-21244194 (#3923)
This PR removes `PyFilesystem2`. Importing `fs` emits a `pkg_resources` deprecation warning (see [pyfilesystem2#597](PyFilesystem/pyfilesystem2#597)). There are also open questions about the project’s maintenance status (see [pyfilesystem2#571](PyFilesystem/pyfilesystem2#571)). As far as I can tell, CumulusCI only relied on a small, local subset of functionality. That subset maps cleanly to Python’s standard library. This PR introduces a local-only `FSResource` and `open_fs_resource` implemented with `pathlib`, `os`, and `shutil`. `file://` URLs are supported for absolute and relative paths, and non-file schemes are rejected. All `fs` imports were removed and tests updated to expect `FileExistsError` instead of `fs.errors.DirectoryExists`.
1 parent a203129 commit 000d441

5 files changed

Lines changed: 123 additions & 111 deletions

File tree

cumulusci/core/source/github.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import os
22
import shutil
33

4-
import fs
54
from github3.exceptions import NotFoundError
65

76
from cumulusci.core.exceptions import DependencyResolutionError
@@ -129,7 +128,7 @@ def resolve(self):
129128
def fetch(self):
130129
"""Fetch the archive of the specified commit and construct its project config."""
131130
with self.project_config.open_cache(
132-
fs.path.join("projects", self.repo_name, self.commit)
131+
os.path.join("projects", self.repo_name, self.commit)
133132
) as path:
134133
zf = download_extract_github(
135134
self.gh, self.repo_owner, self.repo_name, ref=self.commit

cumulusci/utils/fileutils.py

Lines changed: 117 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
import os
2+
import shutil
23
import urllib.request
34
import webbrowser
45
from contextlib import contextmanager
56
from io import StringIO, TextIOWrapper
67
from pathlib import Path
78
from typing import IO, ContextManager, Text, Tuple, Union
9+
from urllib.parse import unquote, urlparse
810

911
import requests
10-
from fs import base, copy, open_fs
11-
from fs import path as fspath
1212

1313
"""Utilities for working with files"""
1414

@@ -85,7 +85,7 @@ def load_from_source(source: DataInput) -> ContextManager[Tuple[IO[Text], Text]]
8585
yield f, path
8686
elif "://" in source: # URL string-like
8787
url = source
88-
resp = requests.get(url)
88+
resp = requests.get(url, timeout=30)
8989
resp.raise_for_status()
9090
yield StringIO(resp.text), url
9191
else: # path-string-like
@@ -115,13 +115,31 @@ def view_file(path):
115115

116116

117117
class FSResource:
118-
"""Generalization of pathlib.Path to support S3, FTP, etc
119-
120-
Create them through the open_fs_resource module function or static
121-
function which will create a context manager that generates an FSResource.
122-
123-
If you don't need the resource management aspects of the context manager,
124-
you can call the `new()` classmethod."""
118+
"""Local filesystem resource wrapper (pyfilesystem2-compatible subset).
119+
120+
This class is a minimal, local-only replacement for the small portion of
121+
the PyFilesystem2 API that CumulusCI used. It exposes a pathlib-like
122+
interface with a few methods that match prior usage patterns, allowing us
123+
to remove the external "fs" dependency while keeping existing call sites
124+
working.
125+
126+
Scope and behavior:
127+
- Only local filesystem operations are supported. Remote backends (e.g.,
128+
S3/FTP/ZIP) and non-"file" schemes are not supported and will raise
129+
ValueError when passed as URLs.
130+
- Supported operations include: exists, open, unlink, rmdir, removetree,
131+
mkdir(parents, exist_ok), copy_to, joinpath, geturl, getsyspath,
132+
__fspath__, and path-style division ("/").
133+
- "file://" URLs are supported for both absolute and relative paths;
134+
other URL schemes are rejected.
135+
- getsyspath returns an absolute path without resolving symlinks so that
136+
macOS paths under "/var" vs "/private/var" remain textually stable in
137+
comparisons.
138+
- close() is a no-op in this implementation.
139+
140+
Create instances via the open_fs_resource() context manager or the
141+
FSResource.new() classmethod when you don't need context management.
142+
"""
125143

126144
def __init__(self):
127145
raise NotImplementedError("Please use open_fs_resource context manager")
@@ -130,62 +148,80 @@ def __init__(self):
130148
def new(
131149
cls,
132150
resource_url_or_path: Union[str, Path, "FSResource"],
133-
filesystem: base.FS = None,
151+
filesystem=None,
134152
):
135153
"""Directly create a new FSResource from a URL or path (absolute or relative)
136154
137-
You can call this to bypass the context manager in contexts where closing isn't
138-
important (e.g. interactive repl experiments)."""
155+
The `filesystem` parameter is ignored in this implementation and exists only
156+
for backward compatibility with callers. This FSResource operates solely on
157+
the local filesystem using pathlib and shutil.
158+
"""
139159
self = cls.__new__(cls)
140160

141-
if isinstance(resource_url_or_path, str) and "://" in resource_url_or_path:
142-
path_type = "url"
143-
elif isinstance(resource_url_or_path, FSResource):
144-
path_type = "resource"
145-
else:
146-
resource_url_or_path = Path(resource_url_or_path)
147-
path_type = "path"
148-
149-
if filesystem:
150-
assert path_type != "resource"
151-
fs = filesystem
152-
filename = str(resource_url_or_path)
153-
elif path_type == "resource": # clone a resource reference
154-
fs = resource_url_or_path.fs
155-
filename = resource_url_or_path.filename
156-
elif path_type == "path":
157-
if resource_url_or_path.is_absolute():
158-
if resource_url_or_path.drive:
159-
root = resource_url_or_path.drive + "/"
161+
if isinstance(resource_url_or_path, FSResource):
162+
self._path = Path(resource_url_or_path.getsyspath())
163+
return self
164+
165+
# Handle string inputs, including file:// URLs
166+
if isinstance(resource_url_or_path, str):
167+
if "://" in resource_url_or_path:
168+
parsed = urlparse(resource_url_or_path)
169+
if parsed.scheme != "file":
170+
raise ValueError(
171+
f"Unsupported URL scheme for FSResource: {parsed.scheme}"
172+
)
173+
# Support non-standard relative file URLs like file://relative/path
174+
if parsed.netloc:
175+
combined = (parsed.netloc or "") + (parsed.path or "")
176+
# Remove a single leading slash that urlparse keeps before the path segment
177+
if combined.startswith("/"):
178+
combined = combined[1:]
179+
path_str = unquote(combined)
160180
else:
161-
root = resource_url_or_path.root
162-
filename = resource_url_or_path.relative_to(root).as_posix()
181+
path_str = unquote(parsed.path or "")
182+
# On Windows, file URLs may begin with a leading slash before drive
183+
if (
184+
os.name == "nt"
185+
and path_str.startswith("/")
186+
and len(path_str) > 3
187+
and path_str[2] == ":"
188+
):
189+
path_str = path_str[1:]
190+
self._path = Path(path_str)
163191
else:
164-
root = Path("/").absolute()
165-
filename = (
166-
(Path(".") / resource_url_or_path)
167-
.absolute()
168-
.relative_to(root)
169-
.as_posix()
170-
)
171-
fs = open_fs(str(root))
172-
elif path_type == "url":
173-
path, filename = resource_url_or_path.replace("\\", "/").rsplit("/", 1)
174-
fs = open_fs(path)
175-
176-
self.fs = fs
177-
self.filename = filename
192+
self._path = Path(resource_url_or_path)
193+
else:
194+
# Path-like
195+
self._path = Path(resource_url_or_path)
196+
178197
return self
179198

180-
exists = proxy("exists")
181-
open = proxy("open")
182-
unlink = proxy("remove")
183-
rmdir = proxy("removedir")
184-
removetree = proxy("removetree")
185-
geturl = proxy("geturl")
199+
def exists(self):
200+
# Use os.path.exists to avoid interference from patched Path.exists in tests
201+
return os.path.exists(str(self.getsyspath()))
202+
203+
def open(self, *args, **kwargs):
204+
return self.getsyspath().open(*args, **kwargs)
205+
206+
def unlink(self):
207+
self.getsyspath().unlink(missing_ok=True)
208+
209+
def rmdir(self):
210+
self.getsyspath().rmdir()
211+
212+
def removetree(self):
213+
shutil.rmtree(self.getsyspath(), ignore_errors=True)
214+
215+
def geturl(self):
216+
p = self.getsyspath()
217+
# Path.as_uri requires absolute path
218+
if not p.is_absolute():
219+
p = p.resolve()
220+
return p.as_uri()
186221

187222
def getsyspath(self):
188-
return Path(os.fsdecode(self.fs.getsyspath(self.filename)))
223+
# Return absolute path without resolving symlinks to preserve /var vs /private/var semantics on macOS
224+
return Path(os.path.abspath(str(self._path)))
189225

190226
def joinpath(self, other):
191227
"""Create a new FSResource based on an existing one
@@ -196,8 +232,7 @@ def joinpath(self, other):
196232
In practice, if you use the new one within the open context
197233
of the old one, you'll be fine.
198234
"""
199-
path = fspath.join(self.filename, other)
200-
return FSResource.new(self.fs.geturl(path))
235+
return FSResource.new(self.getsyspath() / other)
201236

202237
def copy_to(self, other):
203238
"""Create a new FSResource by copying the underlying resource
@@ -210,16 +245,23 @@ def copy_to(self, other):
210245
"""
211246
if isinstance(other, (str, Path)):
212247
other = FSResource.new(other)
213-
copy.copy_file(self.fs, self.filename, other.fs, other.filename)
248+
src = self.getsyspath()
249+
dst = other.getsyspath()
250+
dst.parent.mkdir(parents=True, exist_ok=True)
251+
shutil.copyfile(src, dst)
214252

215253
def mkdir(self, *, parents=False, exist_ok=False):
254+
p = self.getsyspath()
216255
if parents:
217-
self.fs.makedirs(self.filename, recreate=exist_ok)
256+
p.mkdir(parents=True, exist_ok=exist_ok)
218257
else:
219-
self.fs.makedir(self.filename, recreate=exist_ok)
258+
# Emulate pyfilesystem's behavior: raise if exists and exist_ok is False
259+
if p.exists() and not exist_ok:
260+
raise FileExistsError(str(p))
261+
p.mkdir(exist_ok=exist_ok)
220262

221263
def __contains__(self, other):
222-
return other in str(self.geturl())
264+
return str(other) in str(self.getsyspath())
223265

224266
@property
225267
def suffix(self):
@@ -232,33 +274,29 @@ def __repr__(self):
232274
return f"<FSResource {self.geturl()}>"
233275

234276
def __str__(self):
235-
rc = self.geturl()
236-
if rc.startswith("file://"):
237-
return rc[6:]
277+
return str(self.getsyspath())
238278

239279
def __fspath__(self):
240-
return self.fs.getsyspath(self.filename)
280+
return str(self.getsyspath())
241281

242282
def close(self):
243-
self.fs.close()
283+
# No-op for local filesystem-backed resource
284+
return None
244285

245286
@staticmethod
246287
@contextmanager
247288
def open_fs_resource(
248-
resource_url_or_path: Union[str, Path, "FSResource"], filesystem: base.FS = None
289+
resource_url_or_path: Union[str, Path, "FSResource"], filesystem=None
249290
):
250-
"""Create a context-managed FSResource
251-
252-
Input is a URL, path (absolute or relative) or FSResource
253-
254-
The function should be used in a context manager. The
255-
resource's underlying filesystem will be closed automatically
256-
when the context ends and the data will be saved back to the
257-
filesystem (local, remote, zipfile, etc.)
291+
"""Create a context-managed FSResource (local filesystem only).
258292
259-
Think of it as a way of "mounting" a filesystem, directory or file.
293+
- Accepts a path (absolute or relative), a "file://" URL, or an
294+
existing FSResource, and yields a compatible FSResource instance.
295+
- Non-"file" URL schemes are not supported.
296+
- The optional ``filesystem`` argument is ignored and kept only for
297+
backward compatibility with older call sites.
260298
261-
For example:
299+
Examples:
262300
263301
>>> from tempfile import TemporaryDirectory
264302
>>> with TemporaryDirectory() as tempdir:
@@ -279,12 +317,11 @@ def open_fs_resource(
279317
280318
"""
281319
resource = FSResource.new(resource_url_or_path, filesystem)
282-
if not filesystem:
283-
filesystem = resource
284320
try:
285321
yield resource
286322
finally:
287-
filesystem.close()
323+
# No underlying remote filesystem to close in this implementation
324+
pass
288325

289326

290327
open_fs_resource = FSResource.open_fs_resource

cumulusci/utils/tests/test_fileutils.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,14 @@
33
import sys
44
import time
55
import urllib.request
6+
from collections import namedtuple
67
from io import BytesIO, UnsupportedOperation
78
from pathlib import Path
89
from tempfile import TemporaryDirectory
910
from unittest import mock
1011

1112
import pytest
1213
import responses
13-
from fs import errors, open_fs
1414

1515
import cumulusci
1616
from cumulusci.utils import fileutils, temporary_dir, update_tree
@@ -151,7 +151,9 @@ def test_clone_fsresource(self):
151151

152152
def test_load_from_file_system(self):
153153
abspath = os.path.abspath(self.file)
154-
fs = open_fs("/")
154+
# Backwards compatibility: pass a dummy filesystem and ensure it is ignored
155+
DummyFS = namedtuple("DummyFS", [])
156+
fs = DummyFS()
155157
with open_fs_resource(abspath, fs) as f:
156158
assert abspath in str(f)
157159

@@ -234,7 +236,7 @@ def test_mkdir_rmdir(self):
234236
f.mkdir(parents=False, exist_ok=True)
235237
assert abspath.exists()
236238

237-
with pytest.raises(errors.DirectoryExists):
239+
with pytest.raises(FileExistsError):
238240
f.mkdir(parents=False, exist_ok=False)
239241
f.rmdir()
240242

pyproject.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ dependencies = [
2727
"cryptography",
2828
"python-dateutil",
2929
"Faker",
30-
"fs",
3130
"github3.py",
3231
"jinja2",
3332
"keyring<=23.0.1",

0 commit comments

Comments
 (0)