Apply suggestions from code review

seanh · seanh · commit f1295e0fb85a · 2022-07-30T16:51:47.000+01:00
This commit applies suggestions from the code review in #6
diff --git a/.cookiecutter/includes/HACKING.md b/.cookiecutter/includes/HACKING.md
@@ -0,0 +1,49 @@
+
+Testing Manually
+----------------
+
+Normally if you wanted to test a command manually in dev you'd do so through
+tox, for example:
+
+```terminal
+$ tox -qe dev --run-command 'pip-sync-faster --help'
+usage: pip-sync-faster [-h] [-v]
+
+options:
+  -h, --help     show this help message and exit
+  -v, --version
+```
+
+But there's a problem with running `pip-sync-faster` commands in this way: a
+command like `tox -e dev --run-command 'pip-sync-faster requirements.txt'` will
+run `pip-sync requirements.txt` and `pip-sync` will sync the
+current virtualenv (`.tox/dev/`) with the `requirements.txt` file. Everything
+in `requirements.txt` will get installed into `.tox/dev/`, which you probably
+don't want. Even worse everything _not_ in `requirements.txt` will get
+_removed_ from `.tox/dev/` including `pip-sync-faster` itself!
+
+To avoid this problem run `pip-sync-faster` in a temporary virtualenv instead.
+This installs the contents of `requirements.txt` into the temporary venv so
+your `.tox/dev/` env doesn't get messed up. And it does not install
+`pip-sync-faster` into the temporary venv so there's no issue with `pip-sync`
+uninstalling `pip-sync-faster`:
+
+```terminal
+# Make a temporary directory.
+tempdir=$(mktemp -d)
+
+# Create a virtualenv in the temporary directory.
+python3 -m venv $tempdir
+
+# Activate the virtualenv.
+source $tempdir/bin/activate
+
+# Install pip-tools in the virtualenv (pip-sync-faster needs pip-tools).
+pip install pip-tools
+
+# Call pip-sync-faster to install a requirements file into the temporary virtualenv.
+PYTHONPATH=src python3 -m pip_sync_faster /path/to/requirements.txt
+
+# When you're done testing deactivate the temporary virtualenv.
+deactivate
+```
diff --git a/.cookiecutter/includes/README.md b/.cookiecutter/includes/README.md
@@ -27,6 +27,27 @@ If any of the given requirements files doesn't have a matching cached hash then
 pip-sync-faster calls pip-sync forwarding all command line arguments and
 options.
 
+## You need to add `pip-sync-faster` to your requirements file
+
+A command like `pip-sync-faster requirements.txt` will call
+`pip-sync requirements.txt` which will uninstall anything not in
+`requirements.txt` from the active venv, including `pip-sync-faster` itself!
+
+You can add `pip-sync-faster` to `requirements.txt` so that it doesn't get
+uninstalled.
+
+### Running `pip-sync-faster` directly instead
+
+Alternatively as long as `pip-tools` is installed in the active venv you can
+run `pip-sync-faster` directly with a command like:
+
+```bash
+PYTHONPATH=/path/to/pip-sync-faster/src python3 -m pip_sync_faster requirements.txt
+```
+
+This doesn't rely on `pip-sync-faster` being installed so there's no issue with
+`pip-sync` uninstalling it.
+
 ## pip-sync-faster doesn't sync modified virtualenvs
 
 If you modify your requirements files pip-sync-faster will notice the change
diff --git a/.cookiecutter/includes/tox/deps b/.cookiecutter/includes/tox/deps
@@ -0,0 +1 @@
+lint,tests: pytest-mock
diff --git a/HACKING.md b/HACKING.md
@@ -100,3 +100,52 @@ To change the project's formatting, linting and test dependencies:
    ```
 
 3. Commit everything to git and send a pull request
+
+Testing Manually
+----------------
+
+Normally if you wanted to test a command manually in dev you'd do so through
+tox, for example:
+
+```terminal
+$ tox -qe dev --run-command 'pip-sync-faster --help'
+usage: pip-sync-faster [-h] [-v]
+
+options:
+  -h, --help     show this help message and exit
+  -v, --version
+```
+
+But there's a problem with running `pip-sync-faster` commands in this way: a
+command like `tox -e dev --run-command 'pip-sync-faster requirements.txt'` will
+run `pip-sync requirements.txt` as a subprocess and `pip-sync` will sync the
+current virtualenv (`.tox/dev/`) with the `requirements.txt` file. Everything
+in `requirements.txt` will get installed into `.tox/dev/`, which you probably
+don't want. Even worse everything _not_ in `requirements.txt` will get
+_removed_ from `.tox/dev/` including `pip-sync-faster` itself!
+
+To avoid this problem run `pip-sync-faster` in a temporary virtualenv instead.
+This installs the contents of `requirements.txt` into the temporary venv so
+your `.tox/dev/` env doesn't get messed up. And it does not install
+`pip-sync-faster` into the temporary venv so there's no issue with `pip-sync`
+uninstalling `pip-sync-faster`:
+
+```terminal
+# Make a temporary directory.
+tempdir=$(mktemp -d)
+
+# Create a virtualenv in the temporary directory.
+python3 -m venv $tempdir
+
+# Activate the virtualenv.
+source $tempdir/bin/activate
+
+# Install pip-tools in the virtualenv (pip-sync-faster needs pip-tools).
+pip install pip-tools
+
+# Call pip-sync-faster to install a requirements file into the temporary virtualenv.
+PYTHONPATH=src python3 -m pip_sync_faster /path/to/requirements.txt
+
+# When you're done testing deactivate the temporary virtualenv.
+deactivate
+```
diff --git a/README.md b/README.md
@@ -35,13 +35,34 @@ user    0m0.029s
 sys     0m0.008s
 ```
 
-pip-sync-faster does this by saving hashes of the given requirements files in a
+`pip-sync-faster` does this by saving hashes of the given requirements files in a
 JSON file within the virtualenv and not calling pip-sync if the hashes haven't
 changed.
 If any of the given requirements files doesn't have a matching cached hash then
 pip-sync-faster calls pip-sync forwarding all command line arguments and
 options.
 
+## You need to add `pip-sync-faster` to your requirements file
+
+A command like `pip-sync-faster requirements.txt` will call
+`pip-sync requirements.txt` which will uninstall anything not in
+`requirements.txt` from the active venv, including `pip-sync-faster` itself!
+
+You can add `pip-sync-faster` to `requirements.txt` so that it doesn't get
+uninstalled.
+
+### Running `pip-sync-faster` directly instead
+
+Alternatively as long as `pip-tools` is installed in the active venv you can
+run `pip-sync-faster` directly with a command like:
+
+```bash
+PYTHONPATH=/path/to/pip-sync-faster/src python3 -m pip_sync_faster requirements.txt
+```
+
+This doesn't rely on `pip-sync-faster` being installed so there's no issue with
+`pip-sync` uninstalling it.
+
 ## pip-sync-faster doesn't sync modified virtualenvs
 
 If you modify your requirements files pip-sync-faster will notice the change
@@ -54,4 +75,4 @@ Calling pip-sync directly in this case would re-sync your virtualenv with your
 requirements files, but calling pip-sync-faster won't.
 
 If you can live with this limitation then you can use pip-sync-faster and save
-yourself a few hundred milliseconds.  If not you should just use pip-sync.
+yourself a few hundred milliseconds. If not you should just use pip-sync.
diff --git a/pyproject.toml b/pyproject.toml
@@ -43,6 +43,9 @@ ignore = [
 branch = true
 parallel = true
 source = ["pip_sync_faster", "tests/unit"]
+omit = [
+    "*/pip_sync_faster/__main__.py",
+]
 
 [tool.coverage.paths]
 source = ["src", ".tox/*tests/lib/python*/site-packages"]
diff --git a/setup.cfg b/setup.cfg
@@ -25,7 +25,7 @@ where = src
 
 [options.entry_points]
 console_scripts =
-    pip-sync-faster = pip_sync_faster.main:entry_point
+    pip-sync-faster = pip_sync_faster.cli:cli
 
 [pycodestyle]
 ignore =
diff --git a/src/pip_sync_faster/__main__.py b/src/pip_sync_faster/__main__.py
@@ -0,0 +1,5 @@
+import sys
+
+from pip_sync_faster.cli import cli
+
+sys.exit(cli())
diff --git a/src/pip_sync_faster/cli.py b/src/pip_sync_faster/cli.py
@@ -0,0 +1,28 @@
+from argparse import ArgumentParser
+from importlib.metadata import version
+from subprocess import CalledProcessError
+
+from pip_sync_faster.sync import sync
+
+
+def cli(_argv=None):  # pylint:disable=inconsistent-return-statements
+    parser = ArgumentParser(
+        description="Synchronize the active venv with requirements.txt files."
+    )
+    parser.add_argument(
+        "--version", action="store_true", help="show the version and exit"
+    )
+    parser.add_argument(
+        "src_files", nargs="*", help="the requirements.txt files to synchronize"
+    )
+
+    args = parser.parse_known_args(_argv)
+
+    if args[0].version:
+        print(f"pip-sync-faster, version {version('pip-sync-faster')}")
+        return
+
+    try:
+        sync(args[0].src_files)
+    except CalledProcessError as err:
+        return err.returncode
diff --git a/src/pip_sync_faster/main.py b/src/pip_sync_faster/main.py
diff --git a/src/pip_sync_faster/sync.py b/src/pip_sync_faster/sync.py
@@ -0,0 +1,50 @@
+import hashlib
+import json
+import sys
+from os import environ
+from os.path import abspath
+from pathlib import Path
+from subprocess import run
+
+
+def get_hash(path):
+    """Return the hash of the given file."""
+    hashobj = hashlib.sha512()
+
+    with open(path, "rb") as file:
+        hashobj.update(file.read())
+
+    return hashobj.hexdigest()
+
+
+def get_hashes(paths):
+    """Return a dict mapping the given files to their hashes."""
+    return {abspath(path): get_hash(abspath(path)) for path in paths}
+
+
+def sync(src_files):
+    cached_hashes_path = Path(environ["VIRTUAL_ENV"]) / "pip_sync_faster.json"
+
+    try:
+        with open(cached_hashes_path, "r", encoding="utf-8") as handle:
+            cached_hashes = json.load(handle)
+    except FileNotFoundError:
+        cached_hashes = {}
+
+    hashes = get_hashes(src_files)
+
+    if hashes == cached_hashes:
+        return
+
+    # The hashes did not match the cached ones. This can happen if:
+    #
+    # * This is the first time that pip-sync-faster has been called for this venv
+    # * One or more of the requirements files has changed
+    # * pip-sync-faster was called with a different set of requirements files
+
+    run(["pip-sync", *sys.argv[1:]], check=True)
+
+    # Replace the cached hashes file with one containing the correct hashes for
+    # the requirements files that pip-sync-faster was called with this time.
+    with open(cached_hashes_path, "w", encoding="utf-8") as handle:
+        json.dump(hashes, handle)
diff --git a/tests/functional/pip_sync_faster_test.py b/tests/functional/pip_sync_faster_test.py
diff --git a/tests/unit/pip_sync_faster/cli_test.py b/tests/unit/pip_sync_faster/cli_test.py
@@ -0,0 +1,37 @@
+from importlib.metadata import version
+from subprocess import CalledProcessError
+
+import pytest
+
+from pip_sync_faster.cli import cli
+
+
+def test_cli(sync):
+    exit_code = cli(["requirements/dev.txt", "--foo", "bar"])
+
+    sync.assert_called_once_with(["requirements/dev.txt"])
+    assert not exit_code
+
+
+def test_version(capsys):
+    exit_code = cli(["--version"])
+
+    assert (
+        capsys.readouterr().out.strip()
+        == f"pip-sync-faster, version {version('pip-sync-faster')}"
+    )
+    assert not exit_code
+
+
+def test_if_pip_sync_fails(sync):
+    sync.side_effect = CalledProcessError(23, ["pip-sync"])
+
+    exit_code = cli(["requirements/dev.txt"])
+
+    # It echoes pip-sync's exit code.
+    assert exit_code == 23
+
+
+@pytest.fixture(autouse=True)
+def sync(mocker):
+    return mocker.patch("pip_sync_faster.cli.sync", autospec=True)
diff --git a/tests/unit/pip_sync_faster/main_test.py b/tests/unit/pip_sync_faster/main_test.py
diff --git a/tests/unit/pip_sync_faster/sync_test.py b/tests/unit/pip_sync_faster/sync_test.py
diff --git a/tox.ini b/tox.ini
