Support shared-home installs by selecting Python/arch-compatible venv paths

[Copilot]

docs/install/mlcflow_unix_installer.sh assumed a single default ~/mlcflow environment, which breaks on shared homes used across mixed Python versions/architectures. This change makes default-env reuse compatibility-aware and falls back to a deterministic suffixed env name when needed.

• Installer behavior

  • Reuse ~/mlcflow only if its interpreter matches current python{major.minor} and platform.machine().
  • If incompatible, switch to ~/mlcflow_<arch>_py<major.minor> (e.g., ~/mlcflow_x86_64_py3.12).
  • Reuse existing suffixed env when it already matches.

• Implementation

  • Added is_venv_compatible() for venv interpreter validation.
  • Added resolve_default_venv_dir() and called it from setup_venv() when default path is used.
  • Added BASH_SOURCE main guard so installer functions can be sourced for tests.

• Docs and coverage

  • Updated docs/install/README.md with shared-home behavior note.
  • Added focused tests in tests/test_mlcflow_unix_installer.py for:
    • compatible default reuse,
    • incompatible default suffixing,
    • compatible suffixed-env reuse.

# New default-path behavior
if [ -d "$HOME/mlcflow" ] && is_venv_compatible "$HOME/mlcflow" "$PY_MAJOR_MINOR" "$PY_ARCH"; then
  VENV_DIR="$HOME/mlcflow"
else
  VENV_DIR="$HOME/mlcflow_${PY_ARCH}_py${PY_MAJOR_MINOR}"
fi

✅ PR Checklist

• Target branch is dev

📌 Note: PRs must be raised against dev. Do not commit directly to main.

✅ Testing & CI

• Have tested the changes in my local environment, else have properly conveyed in the PR description
• The change includes a GitHub Action to test the script(if it is possible to be added).
• No existing GitHub Actions are failing because of this change.

📚 Documentation

• README or help docs are updated for new features or changes.
• CLI help messages are meaningful and complete.

📁 File Hygiene & Output Handling

• No unintended files (e.g., logs, cache, temp files, pycache, output folders) are committed.

🛡️ Safety & Security

• No secrets or credentials are committed.
• Paths, shell commands, and environment handling are safe and portable.

🙌 Contribution Hygiene

• PR title and description are concise and clearly state the purpose of the change.
• Related issues (if any) are properly referenced using Fixes # or Closes #.
• All reviewer feedback has been addressed.

[arjunsuresh]

@copilot
Can you also include a github action to test this using multiple python versions?

[Copilot]

@copilot
Can you also include a github action to test this using multiple python versions?

Implemented in commit 386edce. Added a new CI job in .github/workflows/test-installer-curl.yml that runs a Python-version matrix (3.8/3.10/3.12) and validates the shared-home venv suffix selection/reuse logic.

[github-actions]

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

[github-actions]

🤖 AI PR Review Summary\n\nAdds a new feature to the Unix installer that manages virtual environment (venv) paths based on Python version and architecture compatibility. Introduces logic to detect incompatible default venvs and fallback to suffixed venv directories. Includes a new GitHub Actions test matrix to validate this behavior across Python 3.8, 3.10, and 3.12. Adds detailed inline documentation and unit tests for the new venv resolution logic. Risks include potential edge cases in venv compatibility detection and increased CI complexity. Design is clean and modular, but the fallback logic depends on accurate environment detection and may require maintenance as Python or platform details evolve.

[github-actions]

🤖 AI PR Review Summary\n\nAdds a new test matrix job in GitHub Actions to validate the shared-home virtual environment naming logic across multiple Python versions (3.8, 3.10, 3.12). Enhances the installer script to detect Python architecture and version to decide on virtual environment reuse or fallback to a suffixed venv path. Adds corresponding documentation and unit tests for the new venv resolution logic. Risks include potential complexity in venv path resolution and compatibility checks, but mitigated by comprehensive tests. Design is clean with clear separation of concerns and good test coverage.

[arjunsuresh]

@copilot fix GH action failures

[github-actions]

🤖 AI PR Review Summary\n\nAdds a new test matrix job in GitHub Actions to validate the shared-home virtual environment naming logic across Python versions 3.8, 3.10, and 3.12. The installer script is enhanced to detect Python architecture and major.minor version to decide whether to reuse the default venv or switch to a suffixed venv path. Documentation is updated to explain this behavior. A new Python test suite is added to verify the venv resolution logic. Risks include potential complexity in venv path resolution and ensuring compatibility checks are accurate. The design improves robustness of environment reuse but adds complexity to installer logic and CI.

[Copilot]

@copilot fix GH action failures

Fixed in the latest commit. There were two bugs causing cascading failures:

1. BASH_SOURCE[0] was unbound when piped via curl | bash (fixed in the previous commit 73ff660)
2. After unblocking main(), log_debug() was failing with exit code 1 under set -euo pipefail because $VERBOSE || return passes false's exit code (1) through bare return — fixed by changing it to return 0.

[github-actions]

🤖 AI PR Review Summary\n\nAdds a new test matrix job in GitHub Actions to validate the shared-home virtual environment naming logic across Python versions 3.8, 3.10, and 3.12. Enhances the installer script to detect Python version and architecture, and conditionally select or create a compatible virtual environment directory with a suffix if the default is incompatible. Adds detailed logging and compatibility checks for virtual environments. Includes documentation updates and new Python-based tests for the installer logic. Risks include potential complexity in environment detection and fallback logic, but mitigated by comprehensive tests. Design is modular and improves robustness of environment reuse.

[github-actions]

🤖 AI PR Review Summary\n\nAdds a new test matrix job to GitHub Actions to validate the shared-home virtual environment naming logic across multiple Python versions. Implements logic in the installer script to detect and reuse compatible virtual environments based on Python version and architecture, falling back to suffixed venv directories if incompatible. Adds documentation and unit tests for this behavior. Risks include potential subtle bugs in environment detection and compatibility checks, but mitigated by comprehensive tests. Design is clean and well-isolated, enhancing installer robustness and user experience.

[anandhu-eng]

Code review by Claude

Reviewed every commit on this branch (8ffb8e0 through e910102) — the shared-home venv suffixing feature, the follow-up fixes, and the new test/CI coverage.

🟡 Medium: stale/incompatible shared venv can be silently reused without revalidation

resolve_default_venv_dir() in docs/install/mlcflow_unix_installer.sh (lines ~352-372):

if [ -d "$DEFAULT_VENV_DIR" ]; then
    if is_venv_compatible "$DEFAULT_VENV_DIR" ...; then
        ...
    fi
    log_warn "Default virtual environment is incompatible..."
    VENV_DIR="$shared_venv_dir"      # <- reused with no compatibility check
    ...
    return
fi

if [ -d "$shared_venv_dir" ] && is_venv_compatible "$shared_venv_dir" ...; then   # <- checked here
    VENV_DIR="$shared_venv_dir"
    ...
fi

When the default venv exists but is incompatible, the code unconditionally points VENV_DIR at $shared_venv_dir — unlike the sibling branch a few lines below, it never verifies that path is actually a valid/compatible venv first. If a prior run left a broken or partial venv at that exact suffixed path (e.g. an interrupted python3 -m venv), setup_venv()'s later if [ -d "$VENV_DIR" ] just "reuses" it as-is, and the rest of the install (install_mlcflow, pull_repo) proceeds against a broken interpreter with no warning.

Suggestion: in that branch, also check is_venv_compatible "$shared_venv_dir" ... before deciding to reuse vs. recreate it — mirroring the logic in the branch below.

🟢 Minor: test coverage doesn't exercise the gap above

Neither the new unit tests (tests/test_mlcflow_unix_installer.py) nor the new CI job (test-installer-curl.yml → test-shared-home-python-matrix) cover "default incompatible and the shared/suffixed dir already exists but is itself incompatible/stale." Both only test: shared dir missing → suffix fallback, and shared dir present+compatible → reuse. This mirrors the code gap above, which is presumably why it wasn't caught.

🟢 Nit: implicit ordering dependency

resolve_default_venv_dir / is_venv_compatible depend on PY_ARCH / PY_MAJOR_MINOR being set by ensure_python() beforehand. main()'s current call order satisfies this, but nothing guards against a future reorder breaking it under set -u. Low priority given there's currently one call site.

✅ Already resolved within this branch's history

fff6445 added bare, unguarded log_debug calls; combined with log_debug/log_info using a bare return (no explicit exit code) under set -euo pipefail, this would have made the installer exit whenever log_debug ran with the default VERBOSE=false. Nice catch — this was self-corrected later in the same branch by 57b3abf and e910102 (explicit return 0).

✅ No issues found

73ff660's BASH_SOURCE[0]:-$0 fix is correct for the curl-pipe invocation case; the docs/install/README.md addition accurately describes the implemented behavior; is_venv_compatible's arch/major.minor check is a reasonable proxy for cross-OS incompatibility, since a mismatched interpreter binary would simply fail to execute rather than silently pass as "compatible."

---

Posted automatically by Claude Code at the requester's request.

[arjunsuresh]

@copilot can you see the above comment and do the changes?

[Copilot]

@copilot can you see the above comment and do the changes?

Fixed in the latest commit. In resolve_default_venv_dir(), when the default venv is incompatible, the code now checks whether the suffixed venv at $shared_venv_dir is itself valid/compatible before deciding to reuse it. If it exists but is stale or broken (e.g. from an interrupted install), it is removed so setup_venv() creates a fresh one rather than silently reusing it. Also added a test covering that scenario ("default incompatible + suffixed dir exists but is also incompatible → stale dir is removed").

[github-actions]

🤖 AI PR Review Summary\n\nAdds a new test matrix job to GitHub Actions to validate the shared-home virtual environment naming logic across Python versions 3.8, 3.10, and 3.12. Introduces logic in the installer script to detect and reuse compatible virtual environments based on Python major.minor version and architecture, falling back to suffixed venv paths if incompatible. Adds detailed logging and compatibility checks for venv reuse. Includes new Python-based tests to verify the venv resolution logic. Updates documentation to explain the shared-home venv naming behavior. Risks include potential edge cases in venv compatibility detection and increased CI time due to the new matrix job. Design is modular and well-tested, but the complexity of venv resolution might require ongoing maintenance.

[github-actions]

🤖 AI PR Review Summary\n\nAdds a new shared-home virtual environment naming scheme to the installer to handle Python version and architecture incompatibilities by appending a suffix to the default venv path. Introduces logic in the installer script to detect and reuse compatible venvs or fallback to suffixed venvs, removing stale incompatible ones. Adds a corresponding GitHub Actions matrix test for multiple Python versions to validate this behavior, updates documentation to explain the new naming scheme, and includes Python-based unit tests for the installer script's venv resolution logic. Risks include potential edge cases in venv compatibility detection and cleanup, but mitigated by thorough testing. Design is cleanly modular with clear separation of concerns.

[github-actions]

🤖 AI PR Review Summary\n\nAdds a new shared-home virtual environment naming scheme to the installer to handle Python version and architecture incompatibilities by appending a suffix to the default venv path. Introduces logic in the installer script to detect and reuse compatible venvs or fallback to suffixed venvs, removing stale incompatible ones. Adds comprehensive tests for this behavior and integrates the tests into the CI matrix. Updates documentation to explain the new behavior. Risks include potential edge cases in venv compatibility detection and cleanup, but mitigated by thorough testing. Design is clean and modular, enhancing installer robustness across Python versions and platforms.