Skip to content

Commit 53ac208

Browse files
committed
Remove PyFilesystem2 dependency
This commit 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 commit 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 b2ddbe8 commit 53ac208

5 files changed

Lines changed: 124 additions & 107 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: 116 additions & 76 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

@@ -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,13 +245,20 @@ 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):
222264
return other in str(self.geturl())
@@ -234,31 +276,30 @@ def __repr__(self):
234276
def __str__(self):
235277
rc = self.geturl()
236278
if rc.startswith("file://"):
237-
return rc[6:]
279+
return rc[7:] if rc.startswith("file:///") else rc[6:]
280+
return rc
238281

239282
def __fspath__(self):
240-
return self.fs.getsyspath(self.filename)
283+
return str(self.getsyspath())
241284

242285
def close(self):
243-
self.fs.close()
286+
# No-op for local filesystem-backed resource
287+
return None
244288

245289
@staticmethod
246290
@contextmanager
247291
def open_fs_resource(
248-
resource_url_or_path: Union[str, Path, "FSResource"], filesystem: base.FS = None
292+
resource_url_or_path: Union[str, Path, "FSResource"], filesystem=None
249293
):
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.)
294+
"""Create a context-managed FSResource (local filesystem only).
258295
259-
Think of it as a way of "mounting" a filesystem, directory or file.
296+
- Accepts a path (absolute or relative), a "file://" URL, or an
297+
existing FSResource, and yields a compatible FSResource instance.
298+
- Non-"file" URL schemes are not supported.
299+
- The optional ``filesystem`` argument is ignored and kept only for
300+
backward compatibility with older call sites.
260301
261-
For example:
302+
Examples:
262303
263304
>>> from tempfile import TemporaryDirectory
264305
>>> with TemporaryDirectory() as tempdir:
@@ -279,12 +320,11 @@ def open_fs_resource(
279320
280321
"""
281322
resource = FSResource.new(resource_url_or_path, filesystem)
282-
if not filesystem:
283-
filesystem = resource
284323
try:
285324
yield resource
286325
finally:
287-
filesystem.close()
326+
# No underlying remote filesystem to close in this implementation
327+
pass
288328

289329

290330
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)