Fix: Cross-Platform Path Handling Standardization

[yashksaini-coder]

What was wrong?

Issue #882 - The py-libp2p had inconsistent & platform-specific path handling that was causing failures for different operating systems. The previous work in the PR #792 was incomplete, leaving critical core modules with hardcoded paths and missing cross-platform utilities.

Specific Problems:

• Hardcoded temp directories in libp2p/utils/logging.py using platform-specific paths (/tmp for Unix, C:\Windows\Temp for Windows)
• Direct os.path.join() usage in examples and documentation instead of standardized utilities
• Missing virtual environment support for binary path resolution
• Inconsistent path handling across different modules
• Platform-specific failures when running on different operating systems

How was it fixed?

Comprehensive Cross-Platform Path Utilities Enhancement

Approach: Updated the existing libp2p/utils/paths.py module file with a new set of cross-platform utilities measures, for use by all the critical modules.

1. New Core Path Utilities

Added 5 new functions to libp2p/utils/paths.py:

• get_venv_path() - Virtual environment detection
• get_python_executable() - Python executable path resolution
• find_executable(name) - System PATH binary search
• get_script_binary_path() - Script binary directory
• get_binary_path(binary_name) - Virtual environment-aware binary resolution

2. Critical Core Module Migration

• libp2p/utils/logging.py: Replaced the hardcoded temp paths with create_temp_file() utility
• examples/kademlia/kademlia.py: Migrated from os.path.join() to join_paths()
• docs/conf.py: Updated to use get_project_root() and join_paths()

3. Comprehensive Testing & Quality Assurance

• Add 14 comprehensive test cases covering all new functionality
• Implemented cross-platform compatibility tests
• Checked backward compatibility with existing code
• Add proper type annotations & error handling

4. Audit + Review

• ✅ Created comprehensive audit script to identify remaining path issues
• ✅ Added detailed docstrings and usage examples
• ✅ Maintained pre-commit hook compliance

Technical Implementation Details

Before (Platform-Specific):

# libp2p/utils/logging.py
if os.name == "nt":  # Windows
    log_file = f"C:\\Windows\\Temp\\py-libp2p_{timestamp}.log"
else:  # Unix
    log_file = f"/tmp/py-libp2p_{timestamp}.log"

After (Cross-Platform):

# libp2p/utils/logging.py
from libp2p.utils.paths import create_temp_file
log_file = str(create_temp_file(prefix="py-libp2p_", suffix=".log"))

---

This PR successfully resolves #882 by completing the cross-platform path handling standardization, making py-libp2p more robust and maintainable across Windows, macOS, and Linux platforms while preserving all existing functionality.

cc: @seetadev @acul71 @pacrob

[acul71]

@yashksaini-coder

PR #886 Review: Cross-Platform Path Handling Standardization

Executive Summary

This PR successfully addresses issue #882 by implementing comprehensive cross-platform path handling utilities and migrating critical modules away from platform-specific hardcoded paths. However, there are three failing tests that need to be fixed before merging.

Test Failures Analysis

1. Address Validation Issue: tests/core/network/test_swarm.py::test_swarm_listen_multiple_addresses_connectivity

Root Cause: The get_available_interfaces() function returns duplicate IP addresses, causing the test to expect 3 listeners but only create 2.

Why This Happens:

• Pre-existing issue: This problem exists in BOTH the main branch and the PR branch
• Network interface duplication: On my Linux system, both Ethernet (enp44s0) and WiFi (wlo1) interfaces are on the same subnet (192.168.1.17/24)
• Function behavior: get_available_interfaces(0) returns:

  /ip4/192.168.1.17/tcp/0  # From enp44s0
  /ip4/192.168.1.17/tcp/0  # From wlo1 (DUPLICATE!)
  /ip4/127.0.0.1/tcp/0     # Loopback
  

• Swarm limitation: Can't create two listeners on the same IP:port combination

Important Discovery:

• Test file identical: Our local test_swarm.py is identical to the GitHub repository
• Address validation identical: The address_validation.py module is identical between branches
• CI vs Local difference: The test passes in CI but fails locally due to system-specific network configuration
• Not PR-related: This issue predates the current PR and exists in the main branch

Fix Required: Update get_available_interfaces() to deduplicate IP addresses:

def get_available_interfaces(port: int, protocol: str = "tcp") -> list[Multiaddr]:
    addrs: list[Multiaddr] = []
    seen_ips: set[str] = set()  # Track unique IPs
    
    for ip in _safe_get_network_addrs(4):
        if ip not in seen_ips:  # Only add unique IPs
            seen_ips.add(ip)
            addrs.append(Multiaddr(f"/ip4/{ip}/{protocol}/{port}"))
    
    # Ensure loopback is included
    if "127.0.0.1" not in seen_ips:
        addrs.append(Multiaddr(f"/ip4/127.0.0.1/{protocol}/{port}"))
    
    return addrs

2. Cross-Platform Path Test Issue: tests/utils/test_paths.py::TestCrossPlatformCompatibility::test_config_dir_platform_specific_windows

Root Cause: Test tries to simulate Windows behavior on a Linux system, but pathlib prevents cross-platform path creation.

Why This Fails on My Linux Box:

• Test monkeypatches os.name = "nt" to simulate Windows
• Test sets APPDATA = "C:\\Users\\Test\\AppData\\Roaming"
• get_config_dir() tries to create WindowsPath("C:/Users/Test/AppData/Roaming")
• pathlib throws UnsupportedOperation: cannot instantiate 'WindowsPath' on your system
• This is Python's design feature - prevents cross-platform path confusion

Fix Required: Make the test platform-aware so it only runs on Windows:

def test_config_dir_platform_specific_windows(self, monkeypatch):
    """Test config directory respects Windows conventions."""
    import platform
    
    # Only run this test on Windows systems
    if platform.system() != "Windows":
        pytest.skip("This test only runs on Windows systems")
    
    # Test Windows-specific behavior
    monkeypatch.setenv("APPDATA", "C:\\Users\\Test\\AppData\\Roaming")
    config_dir = get_config_dir()
    assert "AppData" in str(config_dir)
    assert "py-libp2p" in str(config_dir)

Alternative approaches:

• Conditional testing: Test different behavior on different platforms
• Separate test files: Create platform-specific test suites
• Parametrized tests: Test all platforms in one comprehensive test

3. Logging Test Issue: tests/utils/test_logging.py::test_default_log_file

Root Cause: Test was written for the old logging implementation that used datetime directly, but the PR removed this import and changed the temp file creation logic.

What Changed in the PR:

• Before: from datetime import datetime + hardcoded temp paths
• After: Uses create_temp_file() utility from libp2p.utils.paths
• Test failure: AttributeError: <module 'libp2p.utils.logging' from '...'> does not have the attribute 'datetime'

Why This Happened:

• yashksaini-coder modified the logging module to use cross-platform utilities
• But didn't update the corresponding test that was written by acul71
• Test still tries to patch datetime which no longer exists in the module

Fix Required: Update the test to work with the new implementation:

@pytest.mark.trio
async def test_default_log_file(clean_env):
    """Test logging to the default file path."""
    os.environ["LIBP2P_DEBUG"] = "INFO"
    
    # Mock the create_temp_file function instead of datetime
    with patch("libp2p.utils.paths.create_temp_file") as mock_create_temp:
        # Mock the temp file creation to return a predictable path
        mock_temp_file = Path("/tmp/test_py-libp2p_20240101_120000.log")
        mock_create_temp.return_value = mock_temp_file
        
        # Remove the log file if it exists
        mock_temp_file.unlink(missing_ok=True)
        
        setup_logging()
        
        # Rest of test remains the same...
        _listener_ready.wait(timeout=1)
        logger = logging.getLogger("libp2p")
        logger.info("Test message")
        
        await trio.sleep(0.1)
        
        if _current_listener is not None:
            _current_listener.stop()
        
        # Check the mocked temp file
        if mock_temp_file.exists():
            content = mock_temp_file.read_text()
            assert "Test message" in content

Recommendations

PR Status: APPROVE WITH MINOR FIXES - Only 2 test failures need attention:

1. ❌ Address validation duplication - NOT BLOCKING (pre-existing issue, not PR-related)
2. ✅ Fix cross-platform path test (test design issue)
3. ✅ Fix logging test (breaking change without test update)

Priority Order:

1. High: Fix logging test (breaks existing functionality)
2. Medium: Fix cross-platform path test (test design flaw)
3. Low: Address validation can be fixed in follow-up PR (system-specific edge case)

Code Quality Issues:

• Breaking changes made without updating corresponding tests
• Test assumptions that don't work across platforms
• Incomplete testing of the new implementation

Conclusion

The core cross-platform path handling implementation is excellent and the PR successfully addresses issue #882. The PR author (yashksaini-coder) should:

1. Fix the 2 PR-related test failures (logging and cross-platform path tests)
2. Address the pre-existing address validation issue in a separate follow-up PR
3. Ensure backward compatibility for the logging module changes

Updated Assessment:

• 2 test failures are PR-related and should be fixed before merging
• 1 test failure is pre-existing and not blocking this PR
• CI tests pass on most platforms, indicating good cross-platform compatibility
• The PR is close to merge-ready with minor test fixes

The PR demonstrates good software engineering practices and successfully achieves its main goal of cross-platform path handling standardization.