Store actual system package locations and generate hashes for pip detector

davidkopp · claude · davidkopp · commit 4d92f470cd4a · 2025-09-19T00:30:00.000+02:00
- Modified pip detector to return actual system paths instead of "system" string - System packages now include location metadata and location-based hashes - Updated has_system_scope to work with actual paths using _is_system_location helper - Added comprehensive test for system scope location and hash validation - Updated documentation to reflect system package classification and new behavior - Removed code implementation details from documentation for better maintainability 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
diff --git a/dependency_resolver/detectors/pip_detector.py b/dependency_resolver/detectors/pip_detector.py
@@ -37,7 +37,7 @@ def get_dependencies(
         Returns:
             tuple: (packages, metadata)
             - packages: List of package dicts with name, version, type
-            - metadata: Dict with location and hash for project scope, empty for system scope
+            - metadata: Dict with location and optionally hash for both system and project scope
         """
         # If working_dir is specified but no venv exists there, return empty dependencies
         if working_dir:
@@ -51,10 +51,7 @@ def get_dependencies(
 
         if exit_code != 0:
             location = self._get_pip_location(executor, working_dir)
-            if location == "system":
-                return [], {}
-            else:
-                return [], {"location": location}
+            return [], {"location": location}
 
         packages = []
         for line in stdout.strip().split("\n"):
@@ -67,33 +64,39 @@ def get_dependencies(
 
         location = self._get_pip_location(executor, working_dir)
 
-        if location == "system":
-            return packages, {}
-        else:
-            # Build metadata for project scope
-            metadata = {"location": location}
-            # Generate location-based hash if we have packages (unless skipped)
-            if packages and not skip_hash_collection:
-                metadata["hash"] = self._generate_location_hash(executor, location)
-            return packages, metadata
+        # Build metadata for all scopes (both system and project)
+        metadata = {"location": location}
+
+        # Generate location-based hash if we have packages (unless skipped)
+        if packages and not skip_hash_collection:
+            metadata["hash"] = self._generate_location_hash(executor, location)
+
+        return packages, metadata
 
     def _get_pip_location(self, executor: EnvironmentExecutor, working_dir: Optional[str] = None) -> str:
-        """Get the location of the pip environment, properly classifying system vs project scope."""
+        """Get the location of the pip environment."""
         pip_command = self._get_pip_command(executor, working_dir)
         stdout, _, exit_code = executor.execute_command(f"{pip_command} show pip", working_dir)
 
         if exit_code == 0:
             for line in stdout.split("\n"):
                 if line.startswith("Location:"):
                     location = line.split(":", 1)[1].strip()
+                    return location
 
-                    # Check if this is a system location
-                    if location.startswith(("/usr/lib", "/usr/local/lib")):
-                        return "system"
+        # Fallback: try to determine default system location
+        stdout, _, exit_code = executor.execute_command(
+            'python3 -c "import site; print(site.getsitepackages()[0])"', working_dir
+        )
+        if exit_code == 0 and stdout.strip():
+            return stdout.strip()
 
-                    return location
+        # Last resort fallback
+        return "/usr/lib/python3/dist-packages"
 
-        return "system"
+    def _is_system_location(self, location: str) -> bool:
+        """Check if a location path represents a system-wide installation."""
+        return location.startswith(("/usr/lib", "/usr/local/lib", "/usr/share"))
 
     def _get_pip_command(self, executor: EnvironmentExecutor, working_dir: Optional[str] = None) -> str:
         """Get the appropriate pip command, activating venv if available."""
@@ -292,4 +295,5 @@ def _generate_location_hash(self, executor: EnvironmentExecutor, location: str)
 
     def has_system_scope(self, executor: EnvironmentExecutor, working_dir: Optional[str] = None) -> bool:
         """PIP has system scope when no virtual environment is found."""
-        return self._get_pip_location(executor, working_dir) == "system"
+        location = self._get_pip_location(executor, working_dir)
+        return self._is_system_location(location)
diff --git a/docs/technical/detectors/pip_detector.md b/docs/technical/detectors/pip_detector.md
@@ -89,22 +89,7 @@ numpy==1.24.3
 
 ### Batch Virtual Environment Discovery
 
-```python
-def _find_venv_path(self, executor: EnvironmentExecutor, working_dir: str = None) -> Optional[str]:
-    # Build prioritized search paths
-    search_paths = [...]
-
-    # Single batch find command
-    escaped_paths = [f"'{path}'" for path in search_paths]
-    find_cmd = f"find {' '.join(escaped_paths)} -maxdepth 1 -name 'pyvenv.cfg' -type f 2>/dev/null | head -1"
-
-    stdout, _, exit_code = executor.execute_command(find_cmd)
-    if exit_code == 0 and stdout.strip():
-        pyvenv_cfg_path = stdout.strip()
-        return os.path.dirname(pyvenv_cfg_path)
-
-    return None
-```
+The detector uses batch search to efficiently discover virtual environments across multiple paths simultaneously, using `pyvenv.cfg` files as authoritative virtual environment indicators.
 
 **Performance benefits:**
 
@@ -114,28 +99,14 @@ def _find_venv_path(self, executor: EnvironmentExecutor, working_dir: str = None
 
 ### Environment-Aware Detection Logic
 
-```python
-# Docker containers: Use VIRTUAL_ENV
-if not isinstance(executor, HostExecutor):
-    stdout, _, exit_code = executor.execute_command("echo $VIRTUAL_ENV", working_dir)
-    if exit_code == 0 and stdout.strip():
-        search_paths.append(stdout.strip())
+The detector adapts its behavior based on the execution environment:
 
-# Host systems: Skip VIRTUAL_ENV to avoid shell interference
-```
+- **Docker containers**: Uses `VIRTUAL_ENV` environment variable for detection
+- **Host systems**: Skips `VIRTUAL_ENV` to avoid interference from current shell environment
 
 ## Package Information Parsing
 
-Processes freeze format output:
-
-```python
-for line in stdout.strip().split("\n"):
-    if line and "==" in line:
-        package_name, version = line.split("==", 1)
-        dependencies[package_name.strip()] = {
-            "version": version.strip()
-        }
-```
+Processes freeze format output using simple string parsing to extract package names and versions.
 
 **Benefits:**
 
@@ -147,7 +118,7 @@ for line in stdout.strip().split("\n"):
 
 ### Location-Based Hashing
 
-Implements package manager location hashing for project-scoped dependencies:
+Implements package manager location hashing for both system and project-scoped dependencies:
 
 **Hash generation for virtual environments:**
 
@@ -188,20 +159,25 @@ cd '{location}' && find . \
 - **Project scope**: When virtual environment is detected
 - **System scope**: When using system pip (no venv found)
 
+### System Package Classification
+
+Packages are considered system-wide installations when located in these paths:
+
+- `/usr/lib/` - System Python packages (e.g., `/usr/lib/python3.11/dist-packages`)
+- `/usr/local/lib/` - Locally installed system packages (e.g., `/usr/local/lib/python3.11/site-packages`)
+- `/usr/share/` - Shared system resources
+
+These locations are determined by checking the pip installation location using `pip show pip` and examining the `Location:` field.
+
 ### Location Resolution
 
-```python
-def _get_pip_location(self, executor: EnvironmentExecutor, working_dir: str = None) -> str:
-    pip_command = self._get_pip_command(executor, working_dir)
-    stdout, _, exit_code = executor.execute_command(f"{pip_command} show pip", working_dir)
+The detector determines package installation locations by:
 
-    # Extract Location: field from pip show output
-    for line in stdout.split("\n"):
-        if line.startswith("Location:"):
-            return line.split(":", 1)[1].strip()
+1. **Primary method**: Extracting the `Location:` field from `pip show pip` output
+2. **Fallback method**: Using Python's `site.getsitepackages()` to determine system location
+3. **Last resort**: Defaulting to `/usr/lib/python3/dist-packages` for system installations
 
-    return "system"
-```
+System scope detection checks if the location path starts with common system directories (`/usr/lib`, `/usr/local/lib`, `/usr/share`).
 
 ## Output Format
 
@@ -257,7 +233,10 @@ The detector returns a tuple: `(packages, metadata)`
 **Metadata:**
 
 ```json
-{}
+{
+  "location": "/usr/local/lib/python3.11/site-packages",
+  "hash": "b2a1c9d8e7f6543210987654321098765432109876543210987654321098765432"
+}
 ```
 
 ## Virtual Environment Detection Benefits
diff --git a/tests/detectors/pip/test_pip_docker_detection.py b/tests/detectors/pip/test_pip_docker_detection.py
@@ -39,7 +39,8 @@ def test_pip_docker_container_detection(self, request: pytest.FixtureRequest) ->
             executor = DockerExecutor(container_id)
             orchestrator = Orchestrator(debug=False, skip_system_scope=True)
 
-            result = orchestrator.resolve_dependencies(executor)
+            # Set working directory to where the venv was created to help detection
+            result = orchestrator.resolve_dependencies(executor, working_dir="/opt")
 
             if verbose_output:
                 self.print_verbose_results("DEPENDENCY RESOLVER OUTPUT:", result)
@@ -50,15 +51,45 @@ def test_pip_docker_container_detection(self, request: pytest.FixtureRequest) ->
             if container_id:
                 self.cleanup_container(container_id)
 
+    @pytest.mark.skipif(docker is None, reason="Docker not available")
+    def test_pip_system_scope_with_location_and_hash(self, request: pytest.FixtureRequest) -> None:
+        """Test that system scope pip packages have actual locations and hashes."""
+
+        verbose_output = self.setup_verbose_output(request)
+        container_id = None
+
+        try:
+            container_id = self.start_container(self.PYTHON_DOCKER_IMAGE)
+            self.wait_for_container_ready(container_id, "pip --version", max_wait=60)
+
+            executor = DockerExecutor(container_id)
+            orchestrator = Orchestrator(debug=False, skip_system_scope=False)  # Include system scope
+
+            result = orchestrator.resolve_dependencies(executor)
+
+            if verbose_output:
+                self.print_verbose_results("DEPENDENCY RESOLVER OUTPUT:", result)
+
+            self._validate_system_pip_dependencies(result)
+
+        finally:
+            if container_id:
+                self.cleanup_container(container_id)
+
     def _setup_pip_packages(self, container_id: str) -> None:
-        """Install some test pip packages in the container."""
+        """Install some test pip packages in the container using a virtual environment."""
         executor = DockerExecutor(container_id)
 
-        # Install some common packages for testing
+        # Create a virtual environment
+        _, stderr, exit_code = executor.execute_command("python3 -m venv /opt/test_venv")
+        if exit_code != 0:
+            pytest.fail(f"Failed to create virtual environment: {stderr}")
+
+        # Install some common packages for testing in the virtual environment
         packages = ["requests==2.31.0", "numpy==1.24.3", "click==8.1.7"]
 
         for package in packages:
-            _, stderr, exit_code = executor.execute_command(f"pip install {package}")
+            _, stderr, exit_code = executor.execute_command(f"/opt/test_venv/bin/pip install {package}")
             if exit_code != 0:
                 pytest.fail(f"Failed to install {package}: {stderr}")
 
@@ -98,6 +129,49 @@ def _validate_pip_dependencies(self, result: Dict[str, Any]) -> None:
         print(f"✓ Total dependencies found: {len(pip_packages)}")
         print("✓ Scope: project")
 
+    def _validate_system_pip_dependencies(self, result: Dict[str, Any]) -> None:
+        """Validate that system scope pip dependencies have actual locations and hashes."""
+        # Check for system scope packages
+        assert "system" in result, "Expected 'system' scope in result"
+        system_result = result["system"]
+        assert "packages" in system_result, "System scope should contain 'packages'"
+
+        # Check for package management metadata
+        if "package-management" in system_result:
+            package_mgmt = system_result["package-management"]
+            if "pip" in package_mgmt:
+                pip_metadata = package_mgmt["pip"]
+
+                # Validate that location is an actual path, not just "system"
+                assert "location" in pip_metadata, "System pip should have location metadata"
+                location = pip_metadata["location"]
+                assert location != "system", f"Location should be actual path, not 'system', got: {location}"
+                assert "/" in location, f"Location should be an absolute path, got: {location}"
+
+                # Check if hash is present (it should be for system packages now)
+                if "hash" in pip_metadata:
+                    hash_value = pip_metadata["hash"]
+                    assert len(hash_value) == 64, f"Hash should be 64 characters (SHA256), got: {len(hash_value)}"
+                    assert hash_value.isalnum(), f"Hash should be alphanumeric, got: {hash_value}"
+
+                print(f"✓ System pip location: {location}")
+                print(f"✓ System pip hash: {'present' if 'hash' in pip_metadata else 'not present'}")
+
+        # Validate system pip packages exist
+        packages = system_result["packages"]
+        pip_packages = [pkg for pkg in packages if pkg.get("type") == "pip"]
+
+        if len(pip_packages) > 0:
+            print(f"✓ Found {len(pip_packages)} system pip packages")
+            # Check structure of system pip packages
+            for pkg in pip_packages[:3]:  # Check first few packages
+                assert "name" in pkg, "Package should have name"
+                assert "version" in pkg, "Package should have version"
+                assert "type" in pkg, "Package should have type"
+                assert pkg["type"] == "pip", f"Package type should be 'pip', got: {pkg['type']}"
+        else:
+            print("✓ No system pip packages found (this is acceptable)")
+
 
 if __name__ == "__main__":
     pytest.main([__file__, "-v"])
