feat: Replace hardcoded Path.home() with configurable BASE_DIR#780
Open
itdir wants to merge 12 commits intodw-0:developfrom
Open
feat: Replace hardcoded Path.home() with configurable BASE_DIR#780itdir wants to merge 12 commits intodw-0:developfrom
itdir wants to merge 12 commits intodw-0:developfrom
Conversation
Update backup root initialization to use the path of the Kiauh root directory instead of only using home directory.
…ASE_DIR env var Introduces a centralized BASE_DIR constant in core/constants.py that reads from the KIAUH_BASE_DIR environment variable, defaulting to Path.home(). All component and extension directory constants now use BASE_DIR instead of hardcoded Path.home() calls, enabling system-wide installs to /opt/*, /srv/*, or any custom location. Co-authored-by: itdir <41797199+itdir@users.noreply.github.com>
…rity Co-authored-by: itdir <41797199+itdir@users.noreply.github.com>
…tory fallbacks - Validate KIAUH_BASE_DIR: empty, whitespace-only, or relative paths safely fall back to Path.home() instead of producing unexpected behavior - set_nginx_permissions(): always check home directory permissions (upstream behavior) plus BASE_DIR when it differs, since NGINX needs traversal access - Backup fallback search: search both Path.home() and BASE_DIR when they differ, ensuring existing home-directory installs are always found Co-authored-by: itdir <41797199+itdir@users.noreply.github.com>
Co-authored-by: itdir <41797199+itdir@users.noreply.github.com>
…ctory with full proposal - Enhance BASE_DIR resolution: env var > kiauh.cfg > Path.home() - Add base_dir to AppSettings and KiauhSettings for persistence - Add base_dir option to default.kiauh.cfg (commented, self-documenting) - Add startup logging when non-default base dir is active - Create docs/proposal-configurable-base-dir.md with full analysis, SWOT, pros/cons, migration guide, and justification for upstream maintainers Co-authored-by: itdir <41797199+itdir@users.noreply.github.com>
…oject_root validation, config example Co-authored-by: itdir <41797199+itdir@users.noreply.github.com>
Configurable base directory with settings integration and upstream proposal
…E_DIR env var Squashed commits: - b9d030f Initial plan - 8336ecd Replace hardcoded Path.home() with configurable BASE_DIR from KIAUH_BASE_DIR env var - 7e3efd5 Address code review: fix fd leak in tempfile, rename variable for clarity - c2aac00 Ensure backward compatibility: validate BASE_DIR, preserve home directory fallbacks - 817406a Address code review: use shlex.quote for shell safety, use set for dedup - a5bb03f Comprehensive alternative: settings-integrated configurable base directory with full proposal - 2eabc25 Address code review: module-level imports, dual separator support, project_root validation, config example
dw-0
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Analysis, Evaluation, and Justification for Configurable Base Directory in KIAUH
Introduction
The proposal outlined in docs/proposal-configurable-base-dir.md introduces a configurable base directory (BASE_DIR) for all KIAUH (Klipper Installation And Update Helper) component and extension installations. This change centralizes the installation root path, allowing users to override the default user home directory (Path.home()) via an environment variable (KIAUH_BASE_DIR) or a configuration file option (base_dir in kiauh.cfg). The default behavior remains unchanged for backward compatibility, ensuring that existing single-user, home-directory-based setups continue functioning without modification.
In contrast, the existing pull request (PR) #779, titled "Derive backup directory from KIAUH install root in BackupService," focuses narrowly on updating the backup service to derive its directory from the install root. While this is a targeted improvement, it addresses only one aspect of a broader systemic issue: KIAUH's hard-coded reliance on the user's home directory for all installations. This analysis evaluates the configurable base directory proposal as a superior, more comprehensive solution, particularly for system-wide installations (e.g., multi-printer farms, shared workstations, containerized environments, or CI/CD pipelines). It justifies why this approach is better than PR #779, emphasizing scalability, consistency, and long-term maintainability.
Analysis of the Configurable Base Directory Proposal
The proposal fundamentally refactors KIAUH's path resolution by introducing a single, centrally managed BASE_DIR constant in core/constants.py. This constant is resolved at import time with the following priority order:
Key implementation details include:
Scope: Replaces ~25 hard-coded Path.home() references across components (e.g., Klipper, Moonraker) and extensions, ensuring all paths (e.g., ~/klipper, ~/moonraker) now derive from BASE_DIR.
Backward Compatibility: No changes for users who don't specify overrides—everything installs to ~/component_name as before.
Fallback Mechanisms: Backup and NGINX permission checks search both ~ and BASE_DIR to handle mixed or legacy setups.
Documentation and Safety: Includes self-documenting config options, startup logging for non-default paths, and security fixes (e.g., using tempfile.mkstemp() for temp files).
For system-wide installations, this enables scenarios previously impossible or cumbersome:
Multi-printer farms: Install separate Klipper stacks under /srv/printer1/, /srv/printer2/, etc., without user-specific paths.
Shared or system-wide deployments: Use /opt/klipper-stack/ for enterprise or multi-user environments.
Container/native deployments: Override to ephemeral or non-home paths in Docker, Podman, or systemd-nspawn, avoiding reliance on $HOME.
CI/CD and testing: Set deterministic paths for automated pipelines, preventing pollution of test user directories.
Backup portability: Backups no longer reference user-specific ~, making them portable across systems.
The proposal is minimal and surgical: it touches ~25 files with identical mechanical changes (Path.home() → BASE_DIR), adding no new dependencies, logic, or classes beyond the resolver function and config support.
Evaluation of Benefits and Trade-offs
Strengths:
Zero-Impact Default: As noted, existing users experience no disruption—default behavior matches upstream KIAUH exactly.
Broad Applicability: Solves path-related issues holistically, not piecemeal. For example, it addresses not just backups (as in PR #779) but also installation, data directories, service configurations, and permissions across all components.
Integration with Existing Systems: Leverages KIAUH's settings framework (kiauh.cfg) and follows established patterns (e.g., module-level constants like SYSTEMD).
Security and Robustness Improvements: Fixes predictable temp file paths and adds dual-search fallbacks for compatibility.
Future-Proofing: Opens doors for advanced features like profile management or fleet administration, which a narrow backup-only change (PR #779) cannot support.
Minimal Overhead: The diff is large but mechanical, with low maintenance risk—new components simply use BASE_DIR instead of Path.home().
Weaknesses:
Large Diff: Spans ~25 files, which may appear intimidating during review, though each change is trivial and consistent.
Runtime Limitations: As a module-level constant, changes require restarting KIAUH (similar to other constants like NGINX_SITES_AVAILABLE).
Potential Misconfiguration: Invalid paths could lead to unexpected installs, mitigated by absolute-path validation and clear documentation.
Dual Parse Overhead: kiauh.cfg is read twice (once for BASE_DIR, once by settings), but this is intentional to avoid circular imports and has negligible performance impact.
Comparison to Upstream (dw-0/kiauh) and PR #779:
Upstream Status Quo: Hard-codes Path.home() everywhere, limiting KIAUH to single-user, home-directory setups. This blocks system-wide use cases, forcing workarounds like manual symlinks or post-install moves.
PR #779 (Backup Directory Only): A positive but limited fix—it updates BackupService to use the install root for backups, improving portability for that specific feature. However, it leaves other paths (e.g., Klipper, Moonraker dirs) tied to ~, creating inconsistency. For system-wide installs, users still can't relocate the core components without additional hacks, and the change doesn't integrate with KIAUH's broader settings system.
The proposal builds on PR #779 by encompassing it (backup directories are now derived from BASE_DIR) while extending the fix universally. PR #779 is a band-aid; this is a root-cause solution.
Justification: Why This is Better for System-Wide Installations Than PR #779
System-wide installations demand flexibility beyond individual features like backups. PR #779 improves backup portability but ignores the core issue: KIAUH's assumption that everything lives under a single user's home directory. This creates friction in professional or scaled environments, where paths must be deterministic, shareable, and non-user-specific. The configurable base directory proposal directly addresses this by:
Enabling True System-Wide Flexibility: Unlike PR #779, which only adjusts backups, this allows relocating all components (Klipper, Moonraker, extensions) to paths like /opt/klipper-farm/. In a multi-printer farm, this means clean separation without relying on user accounts or manual interventions—critical for ops teams or shared servers.
Ensuring Consistency and Maintainability: PR #779 introduces inconsistency by fixing backups while leaving other paths hard-coded, potentially leading to bugs (e.g., backups pointing to /opt/ while Klipper still installs to ~). The proposal enforces a single source of truth (BASE_DIR), reducing maintenance overhead and future conflicts. New components automatically inherit the configurable behavior, avoiding the need for repeated PRs like #779.
Supporting Advanced Deployment Scenarios: System-wide setups often involve containers, CI, or multi-user systems where $HOME is unreliable or unavailable. PR #779 doesn't help here—backups might relocate, but core installs fail. The proposal's env-var override (KIAUH_BASE_DIR) makes it container-native and CI-friendly, enabling seamless integration without code changes.
Backward Compatibility Without Compromise: PR #779 is safe but narrow; this proposal maintains that safety while delivering broader value. It doesn't force changes on users but empowers those who need them, aligning with KIAUH's user-friendly ethos.
Long-Term Value and Community Impact: KIAUH serves a growing community, including hobbyists scaling to farms and professionals in manufacturing. PR #779 is a tactical win; this is strategic, paving the way for features like per-printer configs or automated fleet management. Rejecting it in favor of piecemeal fixes (like PR #779) would stall KIAUH's evolution, limiting its adoption in enterprise or scaled environments.
In summary, while PR #779 is a worthwhile incremental improvement, the configurable base directory proposal offers a more robust, scalable, and forward-looking solution. It transforms KIAUH from a home-directory-centric tool into a versatile installer suitable for diverse deployment models, without sacrificing simplicity for existing users. For system-wide installations, this isn't just better—it's essential. The kiauh maintainers should prioritize this change to unlock more of KIAUH's potential.