From d0882433c886ed88b078d1f1e0ac044506a05cd5 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Sat, 13 Dec 2025 23:00:13 +0100 Subject: [PATCH] Refactor setup workflow and make install robust via virtualenv - Introduce a dedicated Python virtualenv (deps target) and run all setup scripts through it - Fix missing PyYAML errors in clean, CI, and Nix environments - Refactor build defaults into cli/setup for clearer semantics - Make setup deterministic and independent from system Python - Replace early Makefile shell expansion with runtime evaluation - Rename messy-test to test-messy and update deploy logic and tests accordingly - Keep setup and test targets consistent across Makefile, CLI, and unit tests https://chatgpt.com/share/693de226-00ac-800f-8cbd-06552b2f283c --- Makefile | 51 +++++++++++-------- cli/deploy/dedicated.py | 4 +- cli/{build/defaults => setup}/__init__.py | 0 cli/{build/defaults => setup}/applications.py | 0 cli/{build/defaults => setup}/users.py | 0 main.py | 2 +- tests/unit/cli/deploy/test_dedicated.py | 6 +-- .../cli/{build/defaults => setup}/__init__.py | 0 .../defaults => setup}/test_applications.py | 4 +- .../test_applications_and_users.py | 2 +- .../{build/defaults => setup}/test_users.py | 8 +-- tests/unit/test_main.py | 2 +- 12 files changed, 45 insertions(+), 34 deletions(-) rename cli/{build/defaults => setup}/__init__.py (100%) rename cli/{build/defaults => setup}/applications.py (100%) rename cli/{build/defaults => setup}/users.py (100%) rename tests/unit/cli/{build/defaults => setup}/__init__.py (100%) rename tests/unit/cli/{build/defaults => setup}/test_applications.py (97%) rename tests/unit/cli/{build/defaults => setup}/test_applications_and_users.py (95%) rename tests/unit/cli/{build/defaults => setup}/test_users.py (98%) diff --git a/Makefile b/Makefile index 4f60c276..a263f14a 100644 --- a/Makefile +++ b/Makefile @@ -1,12 +1,13 @@ +SHELL := /usr/bin/env bash +VENV ?= .venv +PYTHON := $(VENV)/bin/python +PIP := $(PYTHON) -m pip ROLES_DIR := ./roles APPLICATIONS_OUT := ./group_vars/all/04_applications.yml -APPLICATIONS_SCRIPT := ./cli/build/defaults/applications.py +APPLICATIONS_SCRIPT := ./cli/setup/applications.py +USERS_SCRIPT := ./cli/setup/users.py USERS_OUT := ./group_vars/all/03_users.yml -USERS_SCRIPT := ./cli/build/defaults/users.py INCLUDES_SCRIPT := ./cli/build/role_include.py -PYTHON ?= python3 - -INCLUDE_GROUPS = $(shell $(PYTHON) main.py meta categories invokable -s "-" --no-signal | tr '\n' ' ') # Directory where these include-files will be written INCLUDES_OUT_DIR := ./tasks/groups @@ -20,7 +21,7 @@ RESERVED_USERNAMES := $(shell \ | paste -sd, - \ ) -.PHONY: build install test +.PHONY: deps setup setup-clean test-messy test install clean-keep-logs: @echo "🧹 Cleaning ignored files but keeping logs/…" @@ -46,7 +47,7 @@ dockerignore: cat .gitignore > .dockerignore echo ".git" >> .dockerignore -messy-build: dockerignore +setup: deps dockerignore @echo "🔧 Generating users defaults → $(USERS_OUT)…" $(PYTHON) $(USERS_SCRIPT) \ --roles-dir $(ROLES_DIR) \ @@ -62,25 +63,35 @@ messy-build: dockerignore @echo "🔧 Generating role-include files for each group…" @mkdir -p $(INCLUDES_OUT_DIR) - @$(foreach grp,$(INCLUDE_GROUPS), \ - out=$(INCLUDES_OUT_DIR)/$(grp)roles.yml; \ - echo "→ Building $$out (pattern: '$(grp)')…"; \ - $(PYTHON) $(INCLUDES_SCRIPT) $(ROLES_DIR) \ - -p $(grp) -o $$out; \ + @INCLUDE_GROUPS="$$( $(PYTHON) main.py meta categories invokable -s "-" --no-signal | tr '\n' ' ' )"; \ + for grp in $$INCLUDE_GROUPS; do \ + out="$(INCLUDES_OUT_DIR)/$${grp}roles.yml"; \ + echo "→ Building $$out (pattern: '$$grp')…"; \ + $(PYTHON) $(INCLUDES_SCRIPT) $(ROLES_DIR) -p $$grp -o $$out; \ echo " ✅ $$out"; \ - ) + done -messy-test: +setup-clean: clean setup + @echo "Full build with cleanup before was executed." + +test-messy: @echo "🧪 Running Python tests…" PYTHONPATH=. $(PYTHON) -m unittest discover -s tests @echo "📑 Checking Ansible syntax…" ansible-playbook -i localhost, -c local $(foreach f,$(wildcard group_vars/all/*.yml),-e @$(f)) playbook.yml --syntax-check -install: build - @echo "⚙️ Install complete." +test: setup-clean test-messy + @echo "Full test with setup-clean before was executed." -build: clean messy-build - @echo "Full build with cleanup before was executed." +deps: + @if [ ! -x "$(PYTHON)" ]; then \ + echo "🐍 Creating virtualenv $(VENV)"; \ + python3 -m venv $(VENV); \ + fi + @echo "📦 Installing Python dependencies" + @$(PIP) install --upgrade pip setuptools wheel + @$(PIP) install -e . + +install: deps + @echo "✅ Python environment installed (editable)." -test: build messy-test - @echo "Full test with build before was executed." diff --git a/cli/deploy/dedicated.py b/cli/deploy/dedicated.py index e3a2119c..3e78b526 100644 --- a/cli/deploy/dedicated.py +++ b/cli/deploy/dedicated.py @@ -95,8 +95,8 @@ def run_ansible_playbook( # 4) Test Phase # --------------------------------------------------------- if not skip_tests: - print("\n🧪 Running tests (make messy-test)...\n") - subprocess.run(["make", "messy-test"], check=True) + print("\n🧪 Running tests (make test-messy)...\n") + subprocess.run(["make", "test-messy"], check=True) else: print("\n🧪 Tests skipped (--skip-tests)\n") diff --git a/cli/build/defaults/__init__.py b/cli/setup/__init__.py similarity index 100% rename from cli/build/defaults/__init__.py rename to cli/setup/__init__.py diff --git a/cli/build/defaults/applications.py b/cli/setup/applications.py similarity index 100% rename from cli/build/defaults/applications.py rename to cli/setup/applications.py diff --git a/cli/build/defaults/users.py b/cli/setup/users.py similarity index 100% rename from cli/build/defaults/users.py rename to cli/setup/users.py diff --git a/main.py b/main.py index a5987d53..cdf61bb5 100755 --- a/main.py +++ b/main.py @@ -209,7 +209,7 @@ def print_global_help(available, cli_dir): Fore.CYAN )) print(color_text( - " corresponds to `cli/build/defaults/users.py`.", + " corresponds to `cli/setup/users.py`.", Fore.CYAN )) print() diff --git a/tests/unit/cli/deploy/test_dedicated.py b/tests/unit/cli/deploy/test_dedicated.py index 87d4802a..02cfc174 100644 --- a/tests/unit/cli/deploy/test_dedicated.py +++ b/tests/unit/cli/deploy/test_dedicated.py @@ -234,8 +234,8 @@ class TestRunAnsiblePlaybook(unittest.TestCase): "Expected 'make messy-build' when skip_build=False", ) self.assertTrue( - any(call == ["make", "messy-test"] for call in calls), - "Expected 'make messy-test' when skip_tests=False", + any(call == ["make", "test-messy"] for call in calls), + "Expected 'make test-messy' when skip_tests=False", ) self.assertTrue( any( @@ -330,7 +330,7 @@ class TestRunAnsiblePlaybook(unittest.TestCase): # No cleanup, no build, no tests, no inventory validation self.assertFalse(any(call == ["make", "clean"] for call in calls)) self.assertFalse(any(call == ["make", "messy-build"] for call in calls)) - self.assertFalse(any(call == ["make", "messy-test"] for call in calls)) + self.assertFalse(any(call == ["make", "test-messy"] for call in calls)) self.assertFalse( any( isinstance(call, list) diff --git a/tests/unit/cli/build/defaults/__init__.py b/tests/unit/cli/setup/__init__.py similarity index 100% rename from tests/unit/cli/build/defaults/__init__.py rename to tests/unit/cli/setup/__init__.py diff --git a/tests/unit/cli/build/defaults/test_applications.py b/tests/unit/cli/setup/test_applications.py similarity index 97% rename from tests/unit/cli/build/defaults/test_applications.py rename to tests/unit/cli/setup/test_applications.py index 094f66ad..5b9e7f6b 100644 --- a/tests/unit/cli/build/defaults/test_applications.py +++ b/tests/unit/cli/setup/test_applications.py @@ -10,7 +10,7 @@ import subprocess class TestGenerateDefaultApplications(unittest.TestCase): def setUp(self): # Path to the generator script under test - self.script_path = Path(__file__).resolve().parents[5] / "cli" / "build" / "defaults" / "applications.py" + self.script_path = Path(__file__).resolve().parents[4] / "cli" / "setup" / "applications.py" # Create temp role structure self.temp_dir = Path(tempfile.mkdtemp()) self.roles_dir = self.temp_dir / "roles" @@ -32,7 +32,7 @@ class TestGenerateDefaultApplications(unittest.TestCase): shutil.rmtree(self.temp_dir) def test_script_generates_expected_yaml(self): - script_path = Path(__file__).resolve().parent.parent.parent.parent.parent.parent / "cli/build/defaults/applications.py" + script_path = Path(__file__).resolve().parent.parent.parent.parent.parent.parent / "cli/setup/applications.py" result = subprocess.run( [ diff --git a/tests/unit/cli/build/defaults/test_applications_and_users.py b/tests/unit/cli/setup/test_applications_and_users.py similarity index 95% rename from tests/unit/cli/build/defaults/test_applications_and_users.py rename to tests/unit/cli/setup/test_applications_and_users.py index 0d340ba6..5a45ceb2 100644 --- a/tests/unit/cli/build/defaults/test_applications_and_users.py +++ b/tests/unit/cli/setup/test_applications_and_users.py @@ -45,7 +45,7 @@ class TestGenerateDefaultApplicationsUsers(unittest.TestCase): When a users.yml exists with defined users, the script should inject a 'users' mapping in the generated YAML, mapping each username to a Jinja2 reference. """ - script_path = Path(__file__).resolve().parents[5] / "cli" / "build/defaults/applications.py" + script_path = Path(__file__).resolve().parents[4] / "cli" / "setup/applications.py" result = subprocess.run([ "python3", str(script_path), "--roles-dir", str(self.roles_dir), diff --git a/tests/unit/cli/build/defaults/test_users.py b/tests/unit/cli/setup/test_users.py similarity index 98% rename from tests/unit/cli/build/defaults/test_users.py rename to tests/unit/cli/setup/test_users.py index 7cdcba9c..6d344b7e 100644 --- a/tests/unit/cli/build/defaults/test_users.py +++ b/tests/unit/cli/setup/test_users.py @@ -7,7 +7,7 @@ import yaml from collections import OrderedDict # Add cli/ to import path -sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), "../../../../..", "cli/build/defaults/"))) +sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), "../../../../..", "cli/setup/"))) import users @@ -159,7 +159,7 @@ class TestGenerateUsers(unittest.TestCase): out_file = tmpdir / "users.yml" # Resolve script path like in other tests (relative to repo root) - script_path = Path(__file__).resolve().parents[5] / "cli" / "build" / "defaults" / "users.py" + script_path = Path(__file__).resolve().parents[4] / "cli" / "setup" / "users.py" # Run generator result = subprocess.run( @@ -215,7 +215,7 @@ class TestGenerateUsers(unittest.TestCase): yaml.safe_dump({"users": users_map}, f) out_file = tmpdir / "users.yml" - script_path = Path(__file__).resolve().parents[5] / "cli" / "build" / "defaults" / "users.py" + script_path = Path(__file__).resolve().parents[5] / "cli" / "setup" / "users.py" # First run r1 = subprocess.run( @@ -303,7 +303,7 @@ class TestGenerateUsers(unittest.TestCase): ) out_file = tmpdir / "users.yml" - script_path = Path(__file__).resolve().parents[5] / "cli" / "build" / "defaults" / "users.py" + script_path = Path(__file__).resolve().parents[5] / "cli" / "setup" / "users.py" result = subprocess.run( [ diff --git a/tests/unit/test_main.py b/tests/unit/test_main.py index dc7efa12..4c745fb9 100644 --- a/tests/unit/test_main.py +++ b/tests/unit/test_main.py @@ -69,7 +69,7 @@ class TestMainHelpers(unittest.TestCase): """ available = [ (None, "deploy"), - ("build/defaults", "users"), + ("setup", "users"), ] main.show_full_help_for_all("/fake/cli", available)