Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MacOS and Windows support #397

Merged
merged 34 commits into from
Jan 8, 2024
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
36e957a
Allow to use pip on Windows
mknorps Dec 21, 2023
8c4ecb9
Allow to use virtual environment site-packages on Windows
mknorps Dec 21, 2023
9915676
Do not delete temporary files automatically for NamedTemporaryFile
mknorps Dec 21, 2023
d689171
Use system-agnostic command for running Python and devnull
mknorps Dec 21, 2023
442db96
Test local env for all systems
mknorps Dec 21, 2023
294a473
Add more information on the real projects run of integration tests.
mknorps Dec 21, 2023
03fcd44
Skip real project tests if run on Windows
mknorps Dec 21, 2023
5843d1e
Make paths system-agnostic
mknorps Dec 21, 2023
73e1819
Script for creating virtual environment for the experiment dependent …
mknorps Dec 21, 2023
4f1e2f8
Set limits to maximum number of examples and length of test_cmdline_o…
mknorps Dec 21, 2023
3b5e3b6
Skip test on Symlinks for Windows
mknorps Dec 21, 2023
4b65302
Remove a deadline for resolver tests where install_deps option is used
mknorps Dec 21, 2023
c77b8b7
Adjust settings test to how Windows displays text in the stdout
mknorps Dec 21, 2023
6bf2fec
Use a single quote notation of additional conditions of requirements
mknorps Dec 21, 2023
e091d77
Remove backslash at the and of source code rendering
mknorps Dec 21, 2023
df57258
Check first two array entries for logging to stdin
mknorps Dec 21, 2023
8c55c30
Add MacOS and Windows to the CI matrix
mknorps Dec 21, 2023
91e10ca
Apply suggestions from code review: os.path.join -> str(Path(...))
mknorps Jan 2, 2024
7b31225
Apply suggestions from code review;
mknorps Jan 2, 2024
142f697
Apply @jherland's code review suggestions to test_cmdline
mknorps Jan 2, 2024
70f23fd
Move site_packages helper test function to utils
mknorps Jan 2, 2024
8dabf9a
Drop usage of platform library
mknorps Jan 2, 2024
0ba84f9
Apply @jherland's suggestion on Real Projects tests info wording
mknorps Jan 2, 2024
7357a88
Fix linter
mknorps Jan 3, 2024
d0fb833
Add a test case for a path with disc name on Windows
mknorps Jan 3, 2024
5363e33
Fix duplicate
mknorps Jan 3, 2024
5932b06
Fix test types. Use string as expected values and transform / -> \ if…
mknorps Jan 3, 2024
14ce40c
Apply suggestions from code review
mknorps Jan 3, 2024
f4b2a0a
Use unlink to remove temporary file. Trick needed for Windows support
invalid-email-address Jan 4, 2024
a5fb067
Move site_packages to fawtydeps utils.
mknorps Jan 4, 2024
a5af8be
Skip traverse project tests on Windows
mknorps Jan 8, 2024
a5a6ad0
Add condition for a virtual environment on Windows to contain a Pytho…
mknorps Jan 8, 2024
e16b180
Apply Johan's code review suggestions
mknorps Jan 8, 2024
976911f
Paths on Windows are equal regardless of capital/lower case
invalid-email-address Jan 8, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/format.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ jobs:
fail-fast: false
matrix:
python-version: ["3.11"]
os: [ubuntu-22.04]
os: [macos-latest, windows-latest, ubuntu-latest]
runs-on: ${{ matrix.os }}

steps:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/lint.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ jobs:
fail-fast: false
matrix:
python-version: ["3.7", "3.8", "3.9", "3.10", "3.11", "3.12"]
os: [ubuntu-22.04]
os: [ubuntu-latest]
runs-on: ${{ matrix.os }}

steps:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/self_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ jobs:
fail-fast: false
matrix:
python-version: ["3.7", "3.8", "3.9", "3.10", "3.11"]
os: [ubuntu-22.04]
os: [ubuntu-latest]
runs-on: ${{ matrix.os }}

steps:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ jobs:
fail-fast: false
matrix:
python-version: ["3.7", "3.8", "3.9", "3.10", "3.11", "3.12"]
os: [ubuntu-22.04]
os: [macos-latest, windows-latest, ubuntu-latest]
runs-on: ${{ matrix.os }}

steps:
Expand Down
2 changes: 1 addition & 1 deletion fawltydeps/extract_declared_dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ def parse_value(value: str) -> Iterator[DeclaredDependency]:
# TODO: try leveraging RequirementsFile.from_string once
# pip-requirements-parser updates.
# See: https://github.com/nexB/pip-requirements-parser/pull/17
with NamedTemporaryFile(mode="wt") as tmp:
with NamedTemporaryFile(mode="wt", delete=False) as tmp:
jherland marked this conversation as resolved.
Show resolved Hide resolved
tmp.write(value)
tmp.flush()
for dep in parse_requirements_txt(Path(tmp.name)):
Expand Down
17 changes: 16 additions & 1 deletion fawltydeps/packages.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""Encapsulate the lookup of packages and their provided import names."""

import logging
import os
import subprocess
import sys
import tempfile
Expand Down Expand Up @@ -345,6 +346,15 @@ def find_package_dirs(cls, path: Path) -> Iterator[Path]:
if found:
return

# Check for packages on Windows
if sys.platform.startswith("win"):
for site_packages in path.glob(os.path.join("Lib", "site-packages")):
if site_packages.is_dir():
yield site_packages
found = True
if found:
return

jherland marked this conversation as resolved.
Show resolved Hide resolved
# Given path is not a python environment, but it might be _inside_ one.
# Try again with parent directory
if path.parent != path:
Expand Down Expand Up @@ -422,8 +432,13 @@ def installed_requirements(
text=True,
check=False,
)
if sys.platform.startswith("win"): # Windows
pip_path = venv_dir / "Scripts" / "pip.exe"
else: # Assume POSIX
pip_path = venv_dir / "bin" / "pip"

argv = [
f"{venv_dir}/bin/pip",
f"{pip_path}",
"install",
"--no-deps",
"--quiet",
Expand Down
9 changes: 8 additions & 1 deletion fawltydeps/types.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Common types used across FawltyDeps."""

import os
import sys
from abc import ABC, abstractmethod
from dataclasses import asdict, dataclass, field, replace
Expand Down Expand Up @@ -98,7 +99,7 @@ def __post_init__(self) -> None:

def render(self, detailed: bool) -> str:
if detailed and self.base_dir is not None:
return f"{self.path} (using {self.base_dir}/ as base for 1st-party imports)"
return f"{self.path} (using {self.base_dir} as base for 1st-party imports)"
return f"{self.path}"


Expand Down Expand Up @@ -155,6 +156,12 @@ def __post_init__(self) -> None:
elif self.path.match("__pypackages__/?.*/lib"):
return # also ok

# Support Windows projects
if sys.platform.startswith("win") and self.path.match(
os.path.join("Lib", "site-packages")
):
return # also ok

mknorps marked this conversation as resolved.
Show resolved Hide resolved
raise ValueError(f"{self.path} is not a valid dir for Python packages!")

def render(self, detailed: bool) -> str:
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ profile = "black"
main.jobs = 4
main.py-version = "3.7"
reports.output-format = "colorized"
"messages control".disable = "fixme,logging-fstring-interpolation,unspecified-encoding,too-few-public-methods,consider-using-in,duplicate-code"
"messages control".disable = "fixme,logging-fstring-interpolation,unspecified-encoding,too-few-public-methods,consider-using-in,duplicate-code,too-many-locals,too-many-branches"

[tool.mypy]
files = ['*.py', 'fawltydeps/*.py', 'tests/*.py']
Expand Down
5 changes: 2 additions & 3 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from fawltydeps.types import TomlData

from .project_helpers import TarballPackage
from .utils import site_packages


@pytest.fixture
Expand Down Expand Up @@ -52,9 +53,7 @@ def create_one_fake_venv(
venv_dir.parent.mkdir(parents=True, exist_ok=True)
venv.create(venv_dir, with_pip=False)

# Create fake packages
major, minor = py_version
site_dir = venv_dir / f"lib/python{major}.{minor}/site-packages"
site_dir = site_packages(venv_dir)
assert site_dir.is_dir()
for package_name, import_names in fake_packages.items():
# Create just enough files under site_dir to fool importlib_metadata
Expand Down
38 changes: 30 additions & 8 deletions tests/project_helpers.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""Common helpers shared between test_real_project and test_sample_projects."""
import hashlib
import logging
import os
import shlex
import subprocess
import sys
Expand Down Expand Up @@ -100,7 +101,7 @@ def get(self, cache: pytest.Cache) -> Path:

@property
def cache_key(self) -> str:
return f"fawltydeps/{self.tarball_name()}"
return str(Path("fawltydeps", self.tarball_name()))

def tarball_path(self, cache: pytest.Cache) -> Path:
return self.cache_dir(cache) / self.tarball_name()
Expand All @@ -126,18 +127,37 @@ class CachedExperimentVenv:
requirements: List[str] # PEP 508 requirements, passed to 'pip install'

def venv_script_lines(self, venv_path: Path) -> List[str]:
if sys.platform.startswith("win"):
pip_path = venv_path / "Scripts" / "pip.exe"
python_path = venv_path / "Scripts" / "python.exe"
return (
[
f"rd /s /q {venv_path}",
f"{sys.executable} -m venv {venv_path}",
f"{python_path} -m pip install --upgrade pip",
]
+ [
f'{python_path} -m pip install --no-deps "{req}"'
for req in self.requirements
]
+ [
f"type nul > {venv_path / '.installed'}",
]
)

pip_path = venv_path / "bin" / "pip"
return (
[
f"rm -rf {venv_path}",
f"python3 -m venv {venv_path}",
f"{venv_path}/bin/pip install --upgrade pip",
f"{sys.executable} -m venv {venv_path}",
f"{pip_path} install --upgrade pip",
]
+ [
f"{venv_path}/bin/pip install --no-deps {shlex.quote(req)}"
f"{pip_path} install --no-deps {shlex.quote(req)}"
for req in self.requirements
]
+ [
f"touch {venv_path}/.installed",
f"touch {venv_path / '.installed'}",
]
)

Expand All @@ -150,7 +170,7 @@ def venv_hash(self) -> str:
The Python version currently used to run the tests is used to compute
the hash and create the venv.
"""
dummy_script = self.venv_script_lines(Path("/dev/null"))
dummy_script = self.venv_script_lines(Path(os.devnull))
jherland marked this conversation as resolved.
Show resolved Hide resolved
py_version = f"{sys.version_info.major},{sys.version_info.minor}"
script_and_version_bytes = ("".join(dummy_script) + py_version).encode()
return hashlib.sha256(script_and_version_bytes).hexdigest()
Expand All @@ -165,22 +185,23 @@ def __call__(self, cache: pytest.Cache) -> Path:
script or the requirements to create that venv change.
"""
# We cache venv dirs using the hash from create_venv_hash
cached_str = cache.get(f"fawltydeps/{self.venv_hash()}", None)
cached_str = cache.get(str(Path("fawltydeps", self.venv_hash())), None)
if cached_str is not None and Path(cached_str, ".installed").is_file():
return Path(cached_str) # already cached

# Must run the script to set up the venv
venv_dir = Path(cache.mkdir(f"fawltydeps_venv_{self.venv_hash()}"))
logger.info(f"Creating venv at {venv_dir}...")
venv_script = self.venv_script_lines(venv_dir)

jherland marked this conversation as resolved.
Show resolved Hide resolved
subprocess.run(
" && ".join(venv_script),
check=True, # fail if any of the commands fail
shell=True, # pass multiple shell commands to the subprocess
)
# Make sure the venv has been installed
assert (venv_dir / ".installed").is_file()
cache.set(f"fawltydeps/{self.venv_hash()}", str(venv_dir))
cache.set(str(Path("fawltydeps", self.venv_hash())), str(venv_dir))
return venv_dir


Expand Down Expand Up @@ -275,6 +296,7 @@ def from_toml(cls, name: str, data: TomlData) -> "BaseExperiment":

def get_venv_dir(self, cache: pytest.Cache) -> Path:
"""Get this venv's dir and create it if necessary."""
print(f"EXPERIMENT REQUIREMENTS: {self.requirements}")
mknorps marked this conversation as resolved.
Show resolved Hide resolved
return CachedExperimentVenv(self.requirements)(cache)


Expand Down
2 changes: 1 addition & 1 deletion tests/real_projects/python-algorithms.toml
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ requirements = [
"scikit-learn",
"statsmodels",
"sympy",
"tensorflow; python_version < \"3.11\"",
"tensorflow; python_version < '3.11'",
"texttable",
"tweepy",
"xgboost",
Expand Down
Loading
Loading