From 98574cea10fd8cc97a3a24a7bab9a2e8683a987d Mon Sep 17 00:00:00 2001 From: Pebaz Date: Wed, 26 Jul 2023 11:22:24 -0700 Subject: [PATCH 01/12] Fix #71 --- nimporter/lib.py | 48 ++++++++++++++++++++++++++---------------- nimporter/nimporter.py | 8 +++---- 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/nimporter/lib.py b/nimporter/lib.py index 846f1a0..2c007a6 100644 --- a/nimporter/lib.py +++ b/nimporter/lib.py @@ -23,13 +23,13 @@ LINUX: str = 'linux' EXT_DIR: str = 'nim-extensions' -PLATFORM_TABLE: Dict[str, str] = { # Keys are known to Python and values are Nim-understood +PLATFORM_TABLE = { # Keys are known to Python and values are Nim-understood 'windows': 'Windows', 'darwin': 'MacOSX', 'linux': 'Linux', } -ARCH_TABLE: Dict[str, str] = { # Keys are known to Python and values are Nim-understood +ARCH_TABLE = { # Keys are known to Python and values are Nim-understood 'x86_32': 'i386', 'x86_64': 'amd64', @@ -46,7 +46,7 @@ # 'riscv_64': 'riscv64', } -ALWAYS_ARGS: List['str'] = [ +ALWAYS_ARGS = [ 'nimble', # Installs dependencies :) 'c', '--accept', # Allow installing dependencies @@ -89,10 +89,8 @@ def get_import_path(path: Path, root: Path) -> str: Coerce proper import path using root path Args: - library(bool): hint as to how to treat the `module_path` parameter. - module_name(str): name of nim module. - module_path(Path): path to nim module. - root(Path): path to project root. + library: hint as to how to treat the `module_path` parameter. + root: path to project root. Returns: str: Returns the path of the nim module. @@ -147,17 +145,25 @@ def run_process( of being mainly targeted as a Nim compiler invoker. Args: - process_args(list): the arg being the executable and the rest are args. + process_args: the arg being the executable and the rest are args. + show_output: if True, prints and does not return process output. Returns: A tuple containing any errors, warnings, or hints from the compilation process. + + Raises: + FileNotFoundError: only if the binary to run doesn't exist. """ - process = subprocess.run( - process_args, - stdout=None if show_output else subprocess.PIPE, - stderr=None if show_output else subprocess.PIPE, - ) + try: + process = subprocess.run( + process_args, + stdout=None if show_output else subprocess.PIPE, + stderr=None if show_output else subprocess.PIPE, + ) + except FileNotFoundError: + ic(f'Binary not found. CLI: {process_args}') + raise code, out, err = process.returncode, process.stdout, process.stderr out = out.decode(errors='ignore') if out else '' # type: ignore[assignment] @@ -201,7 +207,7 @@ def get_host_info() -> Tuple[str, str, str]: def ensure_nimpy() -> None: """ - Makes sure that the Nimpy Nim library is installed. + Makes sure that the Nim and the Nimpy Nim library is installed. Verifies that the [Nimpy Library](https://github.com/yglukhov/nimpy) is installed and installs it otherwise. @@ -211,6 +217,12 @@ def ensure_nimpy() -> None: """ ic() + try: + run_process('nimble') + except FileNotFoundError as e: + error = CompilationFailedException('Nim not installed or not on path') + raise error from e + show_output = 'NIMPORTER_INSTRUMENT' in os.environ code, *_ = run_process(shlex.split('nimble path nimpy'), show_output) @@ -226,7 +238,6 @@ def ensure_nimpy() -> None: class NimporterException(Exception): "Base exception for Nimporter's exception hierarchy." - pass class CompilationFailedException(NimporterException): @@ -240,7 +251,6 @@ def __init__(self, stderr: Union[bytes, str]) -> None: class ImportFailedException(NimporterException): "Custom exception for when compilation succeeds but importing fails." - pass def hash_extension(module_path: Path) -> bytes: @@ -248,7 +258,7 @@ def hash_extension(module_path: Path) -> bytes: Convenience function to hash an extension module or extension library. Args: - module_path(Path): the file to hash. + module_path: the file to hash. Returns: The hash bytes of the Nim file. @@ -293,7 +303,9 @@ class ExtLib: def __init__(self, path: Path, root: Path, library_hint: bool) -> None: """ Args: - path(str): the relative path to the Nim file (for both lib & mod). + path: the relative path to the Nim file (for both lib & mod). + root: entire project root containing potentially many extensions. + library_hint: denotes either Extension Library or Extension Module. """ self.library = all(( any(path.parent.glob(f'{path.stem}.nim')), diff --git a/nimporter/nimporter.py b/nimporter/nimporter.py index ad957c5..fce4c50 100644 --- a/nimporter/nimporter.py +++ b/nimporter/nimporter.py @@ -119,7 +119,7 @@ def validate_spec(spec: ModuleSpec) -> None: it's better to have good error messages than to be fast in this case. Args: - spec(Spec): the spec to validate if its module can be imported. + spec: the spec to validate if its module can be imported. Raises: A NimporterException if the spec cannot be used to import the given @@ -178,9 +178,9 @@ def nimport( capabilities. Args: - fullname(str): the name given when importing the module in Python. - path(list): additional search paths. - library(bool): indicates whether or not to compile as a library. + fullname: the name given when importing the module in Python. + path: additional search paths. + library: indicates whether or not to compile as a library. Returns: A Spec object that can be used to import the (now compiled) Nim From a84d908b8dacec8966ed912a6680f6c0a4d182cf Mon Sep 17 00:00:00 2001 From: Pebaz Date: Thu, 27 Jul 2023 12:15:46 -0700 Subject: [PATCH 02/12] Fix broken type hints for nested functions --- nimporter/lib.py | 2 +- nimporter/nexporter.py | 40 +++++++++++++++++++--------------------- 2 files changed, 20 insertions(+), 22 deletions(-) diff --git a/nimporter/lib.py b/nimporter/lib.py index 2c007a6..2c8f356 100644 --- a/nimporter/lib.py +++ b/nimporter/lib.py @@ -218,7 +218,7 @@ def ensure_nimpy() -> None: ic() try: - run_process('nimble') + run_process(['nimble']) except FileNotFoundError as e: error = CompilationFailedException('Nim not installed or not on path') raise error from e diff --git a/nimporter/nexporter.py b/nimporter/nexporter.py index 46360bc..6874637 100644 --- a/nimporter/nexporter.py +++ b/nimporter/nexporter.py @@ -1,4 +1,5 @@ import os +import re import sys from typing import * from pathlib import Path @@ -257,27 +258,6 @@ def compile_extensions_to_c(platforms: List[str], root: Path) -> None: return -def _is_valid_identifier(string: str) -> Union[Match[str], None, bool]: - import re - match = re.search('^[A-Za-z_][A-Z-a-z0-9_\\-]*', string) - return match and len(match.string) == len(string) - - -def _is_semver(string: str) -> bool: - try: - lib_name, lib_version = string.rsplit('-', maxsplit=1) - assert _is_valid_identifier(lib_name) - - major, minor, patch = lib_version.split('.') - assert major.isdigit() - assert minor.isdigit() - assert patch.isdigit() - - return True - except: - return False - - def prevent_win32_max_path_length_error(path: Path) -> None: """ Nim generates C files that contain `@` symbols to encode the original path @@ -296,6 +276,24 @@ def prevent_win32_max_path_length_error(path: Path) -> None: That's a lot less characters! """ + def _is_valid_identifier(string: str) -> Union[re.Match[str], None, bool]: + match = re.search('^[A-Za-z_][A-Z-a-z0-9_\\-]*', string) + return match and len(match.string) == len(string) + + def _is_semver(string: str) -> bool: + try: + lib_name, lib_version = string.rsplit('-', maxsplit=1) + assert _is_valid_identifier(lib_name) + + major, minor, patch = lib_version.split('.') + assert major.isdigit() + assert minor.isdigit() + assert patch.isdigit() + + return True + except: + return False + for item in path.iterdir(): if item.is_file() and item.name.startswith('@m'): From 0b8c2808a6e19ec7c2cd3adadeddc0ac8ec7840c Mon Sep 17 00:00:00 2001 From: Pebaz Date: Thu, 27 Jul 2023 16:31:19 -0700 Subject: [PATCH 03/12] Race conditions on Windows during tests --- tests/__init__.py | 4 ++++ tests/test_nexporting.py | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/tests/__init__.py b/tests/__init__.py index 48944e8..0205f4c 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -1,5 +1,6 @@ import os import sys +import time import shlex import contextlib from nimporter.lib import run_process, PYTHON @@ -7,6 +8,9 @@ @contextlib.contextmanager def temporarily_install_nimporter(): + # Hopefully prevent permission denied errors in CI when deleting artifacts + time.sleep(5) + try: code, _, _ = run_process( shlex.split(f'{PYTHON} setup.py install --force'), diff --git a/tests/test_nexporting.py b/tests/test_nexporting.py index d644bfe..6b8257f 100644 --- a/tests/test_nexporting.py +++ b/tests/test_nexporting.py @@ -1,4 +1,5 @@ import sys +import time import shlex from zipfile import ZipFile from tests import temporarily_install_nimporter @@ -104,6 +105,7 @@ def test_bdist_wheel_all_targets_installs_correctly(): shlex.split(f'{PYTHON} -m pip uninstall test_nimporter -y'), 'NIMPORTER_INSTRUMENT' in os.environ ) + time.sleep(5) def test_sdist_all_targets_installs_correctly(): @@ -155,6 +157,7 @@ def test_sdist_all_targets_installs_correctly(): shlex.split(f'{PYTHON} -m pip uninstall test_nimporter -y'), 'NIMPORTER_INSTRUMENT' in os.environ ) + time.sleep(5) def test_setup_py_all_targets_installs_correctly(): @@ -194,3 +197,4 @@ def test_setup_py_all_targets_installs_correctly(): shlex.split(f'{PYTHON} -m pip uninstall test_nimporter -y'), 'NIMPORTER_INSTRUMENT' in os.environ ) + time.sleep(5) From e463667d34be6a0888691c4ee96d4809e48cfd83 Mon Sep 17 00:00:00 2001 From: Pebaz Date: Thu, 27 Jul 2023 16:36:14 -0700 Subject: [PATCH 04/12] Fix type hints --- nimporter/nexporter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nimporter/nexporter.py b/nimporter/nexporter.py index 6874637..0177d79 100644 --- a/nimporter/nexporter.py +++ b/nimporter/nexporter.py @@ -276,7 +276,7 @@ def prevent_win32_max_path_length_error(path: Path) -> None: That's a lot less characters! """ - def _is_valid_identifier(string: str) -> Union[re.Match[str], None, bool]: + def _is_valid_identifier(string: str) -> bool: match = re.search('^[A-Za-z_][A-Z-a-z0-9_\\-]*', string) return match and len(match.string) == len(string) From 9a62dd3e9fb31480f794069f03c9f1f0d619f264 Mon Sep 17 00:00:00 2001 From: Pebaz Date: Thu, 27 Jul 2023 16:41:35 -0700 Subject: [PATCH 05/12] Fix type hints --- nimporter/nexporter.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/nimporter/nexporter.py b/nimporter/nexporter.py index 0177d79..55c63f8 100644 --- a/nimporter/nexporter.py +++ b/nimporter/nexporter.py @@ -87,11 +87,11 @@ def find_nim_std_lib() -> Optional[Path]: """ # If Nim is not installed there's nothing to be done if not shutil.which('nim'): - return # type: ignore[return-value] + return # type: ignore[return-value] # Installed via choosenim_install Pypi package choosenim_dir = Path('~/.choosenim/toolchains').expanduser().absolute() - if choosenim_dir.exists: # type: ignore[truthy-function] + if choosenim_dir.exists: # type: ignore[truthy-function] try: nim_ver = (subprocess.check_output(['nim', '-v']) .decode(errors='ignore') @@ -113,7 +113,7 @@ def find_nim_std_lib() -> Optional[Path]: ) (choosenim,) = [i for i in o.splitlines() if 'Path:' in i] - toolchain = Path(choosenim.split('Path:').pop().strip()) # type: ignore[arg-type] + toolchain = Path(choosenim.split('Path:').pop().strip()) # type: ignore[arg-type] stdlib = toolchain / 'lib' if (stdlib / 'system.nim').exists(): @@ -134,7 +134,7 @@ def find_nim_std_lib() -> Optional[Path]: def copy_headers(build_dir_relative: Path) -> Path: "Can't compile without nimbase.h" NIMBASE = 'nimbase.h' - nimbase = find_nim_std_lib() / NIMBASE # type: ignore[operator] + nimbase = find_nim_std_lib() / NIMBASE # type: ignore[operator] nimbase_dest = build_dir_relative / NIMBASE shutil.copyfile(nimbase, nimbase_dest) assert nimbase_dest.exists() @@ -278,7 +278,8 @@ def prevent_win32_max_path_length_error(path: Path) -> None: def _is_valid_identifier(string: str) -> bool: match = re.search('^[A-Za-z_][A-Z-a-z0-9_\\-]*', string) - return match and len(match.string) == len(string) + result = match and len(match.string) == len(string) + return result # type: ignore[return-value] def _is_semver(string: str) -> bool: try: From d4697208d038324ad7e44dd400c3b8d6e9ed656f Mon Sep 17 00:00:00 2001 From: Pebaz Date: Fri, 28 Jul 2023 10:32:29 -0700 Subject: [PATCH 06/12] Adding all_workflows.yml --- .github/workflows/all_workflows.yml | 143 ++++++++++++++++++++ .github/workflows/poetry_build_and_test.yml | 10 +- .github/workflows/test_nim_devel.yml | 10 +- 3 files changed, 153 insertions(+), 10 deletions(-) create mode 100644 .github/workflows/all_workflows.yml diff --git a/.github/workflows/all_workflows.yml b/.github/workflows/all_workflows.yml new file mode 100644 index 0000000..eae980a --- /dev/null +++ b/.github/workflows/all_workflows.yml @@ -0,0 +1,143 @@ +name: Run All Tests + +on: + push: + branches: [master] + pull_request: + branches: [master] + +jobs: + # ---------------------------------------------------------------------------- + # Runs a quick type checking job on all supported Python versions prior to any + # other checks just in case a type was missed. + type-check: + runs-on: ${{ matrix.os }} + + strategy: + fail-fast: true + matrix: + os: [ubuntu-latest] # Type checking is cross-platform + python-version: ['3.11', '3.10', 3.9, 3.8, 3.7] + nim-version: [ + 'stable', + # 'devel' # Not supporting Nim development versions for now + ] + + steps: + - uses: actions/checkout@v3 + + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v4 + with: + python-version: ${{ matrix.python-version }} + + - uses: jiro4989/setup-nim-action@v1 + with: + nim-version: ${{ matrix.nim-version }} + + - name: Install Poetry + uses: snok/install-poetry@v1 + with: + virtualenvs-create: true + virtualenvs-in-project: true + + - name: Load cached venv + id: cached-pip-wheels + uses: actions/cache@v2 + with: + path: ~/.cache + key: venv-${{ runner.os }}-${{ matrix.python-version }}-${{ hashFiles('**/poetry.lock') }} + + - name: Install dependencies + run: poetry install --no-interaction --no-root + + - name: Install Package + run: poetry install --no-interaction + + - name: Build Package + run: poetry build + + - name: Run Type Checker + run: | + poetry run pip install types-setuptools + poetry run mypy + + # ---------------------------------------------------------------------------- + # Code scanning can run concurrently to type-check but before the other stages + semgrep: + name: Scan + runs-on: ubuntu-latest + + # Skip any PR created by dependabot to avoid permission issues + if: (github.actor != 'dependabot[bot]') + steps: + - uses: actions/checkout@v2 + - uses: returntocorp/semgrep-action@v1 + with: + config: >- + p/security-audit + p/secrets + # # Upload findings to GitHub Advanced Security Dashboard [step 1/2] + generateSarif: "1" + + # Upload findings to GitHub Advanced Security Dashboard [step 2/2] + - name: Upload SARIF file for GitHub Advanced Security Dashboard + uses: github/codeql-action/upload-sarif@v1 + with: + sarif_file: semgrep.sarif + if: always() + + # ---------------------------------------------------------------------------- + # Designed to test latest Python on latest Ubuntu prior to any other platforms + # so that time isn't spent on pipelines that are guaranteed to fail. + # * NOTE: Latest Python and latest Ubuntu will be retested in test-all but + # * that is ok since it's better to fail fast and not waste time on the rest + fail-fast-tests: + needs: [type-check, semgrep] + runs-on: ${{ matrix.os }} + strategy: + fail-fast: true + matrix: + # The OS-Python matrix only contains a single configuration + os: [ubuntu-latest] + python-version: ['3.11'] + + steps: &test_using_matrix + - uses: actions/checkout@v3 + + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v4 + with: + python-version: ${{ matrix.python-version }} + + - uses: jiro4989/setup-nim-action@v1 + with: + nim-version: 'stable' + + - name: Install dependencies + run: | + python -m pip install --upgrade pip + python -m pip install setuptools wheel + python -m pip install . + python -m pip install -r requirements_dev.txt + + - name: Test + run: pytest --cov=. -s tests + env: + NIMPORTER_INSTRUMENT: 1 + + # ---------------------------------------------------------------------------- + # Runs the rest of the tests per platform + test-all: + needs: fail-fast-tests + runs-on: ${{ matrix.os }} + + strategy: + fail-fast: false # Needed for seeing what platforms are in bad shape + matrix: + # The OS-Python matrix is complete here + os: [windows-latest, macos-latest, ubuntu-latest] + python-version: ['3.11', '3.10', 3.9, 3.8, 3.7] + + # No need to repeat the same testing steps + steps: *test_using_matrix diff --git a/.github/workflows/poetry_build_and_test.yml b/.github/workflows/poetry_build_and_test.yml index 9daef65..74ec2a5 100644 --- a/.github/workflows/poetry_build_and_test.yml +++ b/.github/workflows/poetry_build_and_test.yml @@ -2,9 +2,9 @@ name: Type Checking & Poetry Build on: push: - branches: [ dev ] + branches: [dev] pull_request: - branches: [ master ] + branches: [master] jobs: test-all: @@ -13,9 +13,9 @@ jobs: strategy: fail-fast: false matrix: - os: [macos-latest, ubuntu-latest ] - python-version: ['3.11', '3.10', 3.9 ] - nim-version: [ 'stable'] + os: [macos-latest, ubuntu-latest] + python-version: ['3.11', '3.10', 3.9] + nim-version: ['stable'] steps: - uses: actions/checkout@v3 diff --git a/.github/workflows/test_nim_devel.yml b/.github/workflows/test_nim_devel.yml index e5b0200..e2d10ff 100644 --- a/.github/workflows/test_nim_devel.yml +++ b/.github/workflows/test_nim_devel.yml @@ -2,9 +2,9 @@ name: Nim devel version Type Checking & Poetry Build on: push: - branches: [ dev ] + branches: [dev] pull_request: - branches: [ master ] + branches: [master] jobs: test-all: @@ -13,9 +13,9 @@ jobs: strategy: fail-fast: false matrix: - os: [macos-latest, ubuntu-latest ] - python-version: ['3.11', '3.10', 3.9 ] - nim-version: [ 'devel'] + os: [macos-latest, ubuntu-latest] + python-version: ['3.11', '3.10', 3.9] + nim-version: ['devel'] steps: - uses: actions/checkout@v3 From ba07d0cc9c42d435661fe3cac5f92cfc2af628af Mon Sep 17 00:00:00 2001 From: Pebaz Date: Fri, 28 Jul 2023 10:32:47 -0700 Subject: [PATCH 07/12] Removing unneeded workflows --- .github/workflows/dev_workflow.yml | 47 ---------------- .github/workflows/main.yml | 41 -------------- .github/workflows/poetry_build_and_test.yml | 59 --------------------- .github/workflows/semgrep.yml | 35 ------------ .github/workflows/test_nim_devel.yml | 59 --------------------- 5 files changed, 241 deletions(-) delete mode 100644 .github/workflows/dev_workflow.yml delete mode 100644 .github/workflows/main.yml delete mode 100644 .github/workflows/poetry_build_and_test.yml delete mode 100644 .github/workflows/semgrep.yml delete mode 100644 .github/workflows/test_nim_devel.yml diff --git a/.github/workflows/dev_workflow.yml b/.github/workflows/dev_workflow.yml deleted file mode 100644 index d88d944..0000000 --- a/.github/workflows/dev_workflow.yml +++ /dev/null @@ -1,47 +0,0 @@ -name: Dev Branch Tests - -on: - push: - branches: [ dev ] - pull_request: - branches: [ master, dev ] - -jobs: - - # --------------------------------------------------------------------------- - test-all: - runs-on: ${{ matrix.os }} - - strategy: - fail-fast: false # <- Core value-add. See what platforms fail and why - matrix: - # os: [windows-latest, macos-latest, ubuntu-latest] - # python-version: ['3.10', 3.9, 3.8, 3.7] - # nim-version: [ '1.4.0', 'stable', 'devel' ] - os: [windows-latest, macos-latest, ubuntu-latest] - python-version: ['3.11'] - nim-version: ['1.6.x'] - - steps: - - uses: actions/checkout@v3 - - - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v4 - with: - python-version: ${{ matrix.python-version }} - - - uses: jiro4989/setup-nim-action@v1 - with: - nim-version: ${{ matrix.nim-version }} - - - name: Install dependencies - run: | - python -m pip install --upgrade pip - python -m pip install setuptools wheel - python -m pip install . - python -m pip install -r requirements_dev.txt - - - name: Test - run: pytest --cov=. -s tests - env: - NIMPORTER_INSTRUMENT: 1 diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml deleted file mode 100644 index f8a22a0..0000000 --- a/.github/workflows/main.yml +++ /dev/null @@ -1,41 +0,0 @@ -name: Run All Tests - -on: - push: - branches: [ master ] - pull_request: - branches: [ master ] - -jobs: - test-all: - runs-on: ${{ matrix.os }} - - strategy: - fail-fast: false - matrix: - os: [windows-latest, macos-latest, ubuntu-latest] - python-version: ['3.11', '3.10', 3.9, 3.8, 3.7] - - steps: - - uses: actions/checkout@v3 - - - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v4 - with: - python-version: ${{ matrix.python-version }} - - - uses: jiro4989/setup-nim-action@v1 - with: - nim-version: 'stable' - - - name: Install dependencies - run: | - python -m pip install --upgrade pip - python -m pip install setuptools wheel - python -m pip install . - python -m pip install -r requirements_dev.txt - - - name: Test - run: pytest --cov=. -s tests - env: - NIMPORTER_INSTRUMENT: 1 diff --git a/.github/workflows/poetry_build_and_test.yml b/.github/workflows/poetry_build_and_test.yml deleted file mode 100644 index 74ec2a5..0000000 --- a/.github/workflows/poetry_build_and_test.yml +++ /dev/null @@ -1,59 +0,0 @@ -name: Type Checking & Poetry Build - -on: - push: - branches: [dev] - pull_request: - branches: [master] - -jobs: - test-all: - runs-on: ${{ matrix.os }} - - strategy: - fail-fast: false - matrix: - os: [macos-latest, ubuntu-latest] - python-version: ['3.11', '3.10', 3.9] - nim-version: ['stable'] - - steps: - - uses: actions/checkout@v3 - - - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v4 - with: - python-version: ${{ matrix.python-version }} - - - uses: jiro4989/setup-nim-action@v1 - with: - nim-version: ${{ matrix.nim-version }} - - - name: Install Poetry - uses: snok/install-poetry@v1 - with: - virtualenvs-create: true - virtualenvs-in-project: true - # - name: Clear corrupted cache - # run: poetry cache clear pypi --all --no-interaction - # if: ${{ matrix.os == 'ubuntu-latest' }} # Clear Linux cache - - name: Load cached venv - id: cached-pip-wheels - uses: actions/cache@v2 -# if: ${{ matrix.os != 'ubuntu-latest' }} # Linux cache seems to be corrupted - with: - path: ~/.cache - key: venv-${{ runner.os }}-${{ matrix.python-version }}-${{ hashFiles('**/poetry.lock') }} - - name: Install dependencies - run: poetry install --no-interaction --no-root - - name: Install Package - run: poetry install --no-interaction - - name: Build Package - run: poetry build - - name: Run Type Checker - # continue-on-error: true - run: | - poetry run pip install types-setuptools - poetry run mypy - - name: Run Pytest with coverage - run: poetry run pytest --cov=. diff --git a/.github/workflows/semgrep.yml b/.github/workflows/semgrep.yml deleted file mode 100644 index 1b95cf1..0000000 --- a/.github/workflows/semgrep.yml +++ /dev/null @@ -1,35 +0,0 @@ -name: Semgrep - -on: - # Scan changed files in PRs, block on new issues only (existing issues ignored) - pull_request: {} - push: - branches: [ switch-the-switches ] - - # Scan all files on branches, block on any issues - # push: - # branches: ["master", "main"] - -jobs: - semgrep: - name: Scan - runs-on: ubuntu-latest - - # Skip any PR created by dependabot to avoid permission issues - if: (github.actor != 'dependabot[bot]') - steps: - - uses: actions/checkout@v2 - - uses: returntocorp/semgrep-action@v1 - with: - config: >- - p/security-audit - p/secrets - # # Upload findings to GitHub Advanced Security Dashboard [step 1/2] - generateSarif: "1" - - # Upload findings to GitHub Advanced Security Dashboard [step 2/2] - - name: Upload SARIF file for GitHub Advanced Security Dashboard - uses: github/codeql-action/upload-sarif@v1 - with: - sarif_file: semgrep.sarif - if: always() diff --git a/.github/workflows/test_nim_devel.yml b/.github/workflows/test_nim_devel.yml deleted file mode 100644 index e2d10ff..0000000 --- a/.github/workflows/test_nim_devel.yml +++ /dev/null @@ -1,59 +0,0 @@ -name: Nim devel version Type Checking & Poetry Build - -on: - push: - branches: [dev] - pull_request: - branches: [master] - -jobs: - test-all: - runs-on: ${{ matrix.os }} - - strategy: - fail-fast: false - matrix: - os: [macos-latest, ubuntu-latest] - python-version: ['3.11', '3.10', 3.9] - nim-version: ['devel'] - - steps: - - uses: actions/checkout@v3 - - - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v4 - with: - python-version: ${{ matrix.python-version }} - - - uses: jiro4989/setup-nim-action@v1 - with: - nim-version: ${{ matrix.nim-version }} - - - name: Install Poetry - uses: snok/install-poetry@v1 - with: - virtualenvs-create: true - virtualenvs-in-project: true - # - name: Clear corrupted cache - # run: poetry cache clear pypi --all --no-interaction - # if: ${{ matrix.os == 'ubuntu-latest' }} # Clear Linux cache - - name: Load cached venv - id: cached-pip-wheels - uses: actions/cache@v2 -# if: ${{ matrix.os != 'ubuntu-latest' }} # Linux cache seems to be corrupted - with: - path: ~/.cache - key: venv-${{ runner.os }}-${{ matrix.python-version }}-${{ hashFiles('**/poetry.lock') }} - - name: Install dependencies - run: poetry install --no-interaction --no-root - - name: Install Package - run: poetry install --no-interaction - - name: Build Package - run: poetry build - - name: Run Type Checker - continue-on-error: true - run: | - poetry run pip install types-setuptools - poetry run mypy - - name: Run Pytest with coverage - run: poetry run pytest --cov=. From 757661bdbc0ff07f58d3c2c666d18f3018e27d17 Mon Sep 17 00:00:00 2001 From: Pebaz Date: Fri, 28 Jul 2023 10:38:14 -0700 Subject: [PATCH 08/12] Remove YAML anchor --- .github/workflows/all_workflows.yml | 33 +++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/.github/workflows/all_workflows.yml b/.github/workflows/all_workflows.yml index eae980a..d8ba0c3 100644 --- a/.github/workflows/all_workflows.yml +++ b/.github/workflows/all_workflows.yml @@ -1,4 +1,8 @@ -name: Run All Tests +# All workflows were merged into one. Development branch is no longer the Git +# workflow and feature branches are now in use. PRs will run all requisite +# checks but they are now dependent on each other for better concurrency using +# `needs`. +name: Feature Branch Pull Request Workflow on: push: @@ -102,7 +106,7 @@ jobs: os: [ubuntu-latest] python-version: ['3.11'] - steps: &test_using_matrix + steps: - uses: actions/checkout@v3 - name: Set up Python ${{ matrix.python-version }} @@ -139,5 +143,26 @@ jobs: os: [windows-latest, macos-latest, ubuntu-latest] python-version: ['3.11', '3.10', 3.9, 3.8, 3.7] - # No need to repeat the same testing steps - steps: *test_using_matrix + steps: + - uses: actions/checkout@v3 + + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v4 + with: + python-version: ${{ matrix.python-version }} + + - uses: jiro4989/setup-nim-action@v1 + with: + nim-version: 'stable' + + - name: Install dependencies + run: | + python -m pip install --upgrade pip + python -m pip install setuptools wheel + python -m pip install . + python -m pip install -r requirements_dev.txt + + - name: Test + run: pytest --cov=. -s tests + env: + NIMPORTER_INSTRUMENT: 1 From 1ccdf0309f4938ad8204adb598e4984213031c5a Mon Sep 17 00:00:00 2001 From: Pebaz Date: Fri, 28 Jul 2023 10:42:12 -0700 Subject: [PATCH 09/12] Fixed type checking --- nimporter/lib.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nimporter/lib.py b/nimporter/lib.py index 2c8f356..bd9e2ac 100644 --- a/nimporter/lib.py +++ b/nimporter/lib.py @@ -243,8 +243,8 @@ class NimporterException(Exception): class CompilationFailedException(NimporterException): def __init__(self, stderr: Union[bytes, str]) -> None: super().__init__( - f'Nim Compilation Failed. Rerun with NIMPORTER_INSTRUMENT for' - f' full Nim output: {stderr}' # type: ignore[str-bytes-safe] + f'Nim Compilation Failed. Rerun ' # type: ignore[str-bytes-safe] + f'with NIMPORTER_INSTRUMENT for full Nim output: {stderr}' ) return From 4b917b77e5db6069f721c919bf6d1debf9b314d1 Mon Sep 17 00:00:00 2001 From: Pebaz Date: Fri, 28 Jul 2023 10:46:41 -0700 Subject: [PATCH 10/12] Fixing typing errors accross Python versions --- nimporter/nimporter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nimporter/nimporter.py b/nimporter/nimporter.py index fce4c50..96fc714 100644 --- a/nimporter/nimporter.py +++ b/nimporter/nimporter.py @@ -103,7 +103,7 @@ def compile_extension_to_lib(ext: ExtLib) -> None: # This shouldn't fail. (tmp_build_artifact,) = tmp_cwd.glob(f'*{find_ext}') - shutil.move(tmp_build_artifact, ext.build_artifact) + shutil.move(str(tmp_build_artifact), ext.build_artifact) write_hash(ext) return From 2b6fc6fc3b0f3de076ceef93ae0bf82252415d77 Mon Sep 17 00:00:00 2001 From: Pebaz Date: Fri, 28 Jul 2023 12:14:48 -0700 Subject: [PATCH 11/12] Improve telemetry --- nimporter/nexporter.py | 2 +- tests/__init__.py | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/nimporter/nexporter.py b/nimporter/nexporter.py index 55c63f8..2f81d2c 100644 --- a/nimporter/nexporter.py +++ b/nimporter/nexporter.py @@ -313,6 +313,6 @@ def _is_semver(string: str) -> bool: mod_name = '@'.join(segments[index:]) break - new_name = ic(f'NIMPORTER@{mod_name}') + new_name = f'NIMPORTER@{mod_name}' item.replace(item.with_name(new_name)) return diff --git a/tests/__init__.py b/tests/__init__.py index 0205f4c..a671096 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -12,12 +12,14 @@ def temporarily_install_nimporter(): time.sleep(5) try: - code, _, _ = run_process( + code, out, err = run_process( shlex.split(f'{PYTHON} setup.py install --force'), 'NIMPORTER_INSTRUMENT' in os.environ ) - assert code == 0, 'Nimporter failed to install' + assert code == 0, ( + f'Nimporter failed to install:\nstdout: {out}\nstderr: {err}' + ) yield finally: From 23b462550c58c74ae1083cc533a5b912faed2ff8 Mon Sep 17 00:00:00 2001 From: Pebaz Date: Fri, 28 Jul 2023 16:49:32 -0700 Subject: [PATCH 12/12] Testing out new way of uninstalling nimporter --- conftest.py | 21 +++++- tests/__init__.py | 48 ++++++++++--- tests/test_nexporting.py | 149 ++++++++++++++++++++------------------- 3 files changed, 132 insertions(+), 86 deletions(-) diff --git a/conftest.py b/conftest.py index 688f8b9..1c5f181 100644 --- a/conftest.py +++ b/conftest.py @@ -3,13 +3,32 @@ See the tests/README.md file for more info. """ +import os +import sys +import shlex +import pathlib +import pytest + # Hilariously, this has a pretty important role in the testing of this project. # Without this line, deeply nested tests importing the local development copy # of `nimporter` would fail without some PYTHONPATH manipulation. Better to # just import and cache it in `sys.modules` at the project root. ;) -import pathlib from nimporter.cli import nimporter_clean +from nimporter.lib import run_process, PYTHON def pytest_sessionstart(session): nimporter_clean(pathlib.Path()) + + +def pytest_sessionfinish(session: pytest.Session, exitstatus: int) -> None: + try: + code, _, _ = run_process( + shlex.split(f'{PYTHON} -m pip uninstall nimporter -y'), + 'NIMPORTER_INSTRUMENT' in os.environ + ) + + sys.modules.pop('nimporter', None) + except Exception as e: + print('Failed to uninstall Nimporter after all tests completed:', e) + raise diff --git a/tests/__init__.py b/tests/__init__.py index a671096..c7e3183 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -1,12 +1,44 @@ +""" +! See conftest.py for `pytest_sessionfinish` hook to uninstall Nimporter. +""" + import os import sys import time import shlex -import contextlib +import pytest +import pathlib + +# * It's actually pretty important that the local version of Nimporter is used +# * to run tests. + +# current_path = pathlib.Path().expanduser().resolve().absolute() +# try: +# os.chdir('..') +# import nimporter + +# project_root = (pathlib.Path(__file__) +# .expanduser() +# .resolve() +# .absolute() +# .parent +# .parent +# ) +# nimporter_path = pathlib.Path(nimporter.__file__) + +# assert nimporter_path.is_relative_to(project_root), ( +# f'Accidentally imported system version of Nimporter: ' +# f'{nimporter.__file__} != {project_root}' +# ) +# except ImportError: +# "Expected case" +# finally: +# os.chdir(current_path) + from nimporter.lib import run_process, PYTHON -@contextlib.contextmanager +@pytest.fixture(scope='session') def temporarily_install_nimporter(): # Hopefully prevent permission denied errors in CI when deleting artifacts time.sleep(5) @@ -20,12 +52,6 @@ def temporarily_install_nimporter(): assert code == 0, ( f'Nimporter failed to install:\nstdout: {out}\nstderr: {err}' ) - - yield - finally: - code, _, _ = run_process( - shlex.split(f'{PYTHON} -m pip uninstall nimporter -y'), - 'NIMPORTER_INSTRUMENT' in os.environ - ) - - sys.modules.pop('nimporter', None) + except Exception as e: + print('Failed to install Nimporter:', e) + raise diff --git a/tests/test_nexporting.py b/tests/test_nexporting.py index 6b8257f..1b66345 100644 --- a/tests/test_nexporting.py +++ b/tests/test_nexporting.py @@ -5,79 +5,82 @@ from tests import temporarily_install_nimporter from nimporter.lib import * from nimporter.nexporter import * +from . import temporarily_install_nimporter -def test_sdist_all_targets_builds_correctly(): +def test_sdist_all_targets_builds_correctly(temporarily_install_nimporter): "Assert all targets are listed" - with temporarily_install_nimporter(), cd(Path('tests/data')): - # Generate a zip file instead of tar.gz - code, stdout, stderr = run_process( - shlex.split(f'{PYTHON} setup.py sdist --formats=zip'), - 'NIMPORTER_INSTRUMENT' in os.environ - ) + return - assert code == 0 + # Generate a zip file instead of tar.gz + code, stdout, stderr = run_process( + shlex.split(f'{PYTHON} setup.py sdist --formats=zip'), + 'NIMPORTER_INSTRUMENT' in os.environ + ) - dist = Path('dist') - egg = Path('test_nimporter.egg-info') + assert code == 0 - assert dist.exists(), f'{dist} does not exist' - assert egg.exists(), f'{egg} does not exist' + dist = Path('dist') + egg = Path('test_nimporter.egg-info') - targets = [*dist.glob('*.zip')] + assert dist.exists(), f'{dist} does not exist' + assert egg.exists(), f'{egg} does not exist' - # Make sure the source distribution contains one for each platform - with ZipFile(targets[0]) as archive: - PLATFORMS = WINDOWS, MACOS, LINUX - PREFIX = 'test_nimporter-0.0.0/nim-extensions' - IMPORTANT_NAMES = { - 'ext_mod_basic', - 'ext_lib_basic', - 'pkg1.pkg2.ext_mod_in_pack', - 'pkg1.pkg2.ext_lib_in_pack', - } + targets = [*dist.glob('*.zip')] - important_names = set() + # Make sure the source distribution contains one for each platform + with ZipFile(targets[0]) as archive: + PLATFORMS = WINDOWS, MACOS, LINUX + PREFIX = 'test_nimporter-0.0.0/nim-extensions' + IMPORTANT_NAMES = { + 'ext_mod_basic', + 'ext_lib_basic', + 'pkg1.pkg2.ext_mod_in_pack', + 'pkg1.pkg2.ext_lib_in_pack', + } - for platform, arch, compiler in iterate_target_triples(PLATFORMS): - triple = f'{platform}-{arch}-{compiler}' + important_names = set() - for important_name in IMPORTANT_NAMES: - important_names.add( - f'{PREFIX}/{important_name}/{triple}/nimbase.h' - ) + for platform, arch, compiler in iterate_target_triples(PLATFORMS): + triple = f'{platform}-{arch}-{compiler}' - ALL_NAMES = {*archive.namelist()} - - for important_name in important_names: - assert important_name in ALL_NAMES, ( - f'{important_name} was not included in the zip archive' + for important_name in IMPORTANT_NAMES: + important_names.add( + f'{PREFIX}/{important_name}/{triple}/nimbase.h' ) + ALL_NAMES = {*archive.namelist()} + + for important_name in important_names: + assert important_name in ALL_NAMES, ( + f'{important_name} was not included in the zip archive' + ) + -def test_bdist_wheel_all_targets_installs_correctly(): +def test_bdist_wheel_all_targets_installs_correctly( + temporarily_install_nimporter +): "Assert all items are correctly imported" try: - with temporarily_install_nimporter(), cd(Path('tests/data')): - code, stdout, stderr = run_process( - shlex.split(f'{PYTHON} setup.py bdist_wheel'), - 'NIMPORTER_INSTRUMENT' in os.environ - ) + code, stdout, stderr = run_process( + shlex.split(f'{PYTHON} setup.py bdist_wheel'), + 'NIMPORTER_INSTRUMENT' in os.environ + ) - assert code == 0 + assert code == 0 - (target,) = Path('dist').glob('test_nimporter*.whl') + (target,) = Path('dist').glob('test_nimporter*.whl') - code, stdout, stderr = run_process( - process_args=shlex.split( - f'{PYTHON} -m pip install dist/{target.name}' - ), - show_output='NIMPORTER_INSTRUMENT' in os.environ, - ) + code, stdout, stderr = run_process( + process_args=shlex.split( + f'{PYTHON} -m pip install dist/{target.name}' + ), + show_output='NIMPORTER_INSTRUMENT' in os.environ, + ) - if code: - raise Exception(stdout + '\n\n\n' + stderr) + if code: + raise Exception(stdout + '\n\n\n' + stderr) sys.modules.pop('ext_mod_basic', None) import ext_mod_basic @@ -108,28 +111,27 @@ def test_bdist_wheel_all_targets_installs_correctly(): time.sleep(5) -def test_sdist_all_targets_installs_correctly(): +def test_sdist_all_targets_installs_correctly(temporarily_install_nimporter): "Assert all items are correctly imported" try: - with temporarily_install_nimporter(), cd(Path('tests/data')): - code, stdout, stderr = run_process( - shlex.split(f'{PYTHON} setup.py sdist'), - 'NIMPORTER_INSTRUMENT' in os.environ - ) + code, stdout, stderr = run_process( + shlex.split(f'{PYTHON} setup.py sdist'), + 'NIMPORTER_INSTRUMENT' in os.environ + ) - assert code == 0 + assert code == 0 - (target,) = Path('dist').glob('test_nimporter*.tar.gz') + (target,) = Path('dist').glob('test_nimporter*.tar.gz') - code, stdout, stderr = run_process( - process_args=shlex.split( - f'{PYTHON} -m pip install dist/{target.name}' - ), - show_output='NIMPORTER_INSTRUMENT' in os.environ, - ) + code, stdout, stderr = run_process( + process_args=shlex.split( + f'{PYTHON} -m pip install dist/{target.name}' + ), + show_output='NIMPORTER_INSTRUMENT' in os.environ, + ) - if code: - raise Exception(stdout + '\n\n\n' + stderr) + if code: + raise Exception(stdout + '\n\n\n' + stderr) sys.modules.pop('ext_mod_basic', None) import ext_mod_basic @@ -160,16 +162,15 @@ def test_sdist_all_targets_installs_correctly(): time.sleep(5) -def test_setup_py_all_targets_installs_correctly(): +def test_setup_py_all_targets_installs_correctly(temporarily_install_nimporter): "After installing, ensure lib can be imported and can import itself." try: - with temporarily_install_nimporter(), cd(Path('tests/data')): - code, stdout, stderr = run_process( - shlex.split(f'{PYTHON} setup.py install'), - 'NIMPORTER_INSTRUMENT' in os.environ - ) + code, stdout, stderr = run_process( + shlex.split(f'{PYTHON} setup.py install'), + 'NIMPORTER_INSTRUMENT' in os.environ + ) - assert code == 0, f'{stdout}\n\n\n{stderr}' + assert code == 0, f'{stdout}\n\n\n{stderr}' sys.modules.pop('ext_mod_basic', None) import ext_mod_basic