Skip to content

[WIP] WIP address feedback on finalize yadm stow setup PR#148

Closed
Ven0m0 with Copilot wants to merge 10 commits into
mainfrom
copilot/sub-pr-147
Closed

[WIP] WIP address feedback on finalize yadm stow setup PR#148
Ven0m0 with Copilot wants to merge 10 commits into
mainfrom
copilot/sub-pr-147

Conversation

Copilot AI commented Dec 29, 2025

Copy link
Copy Markdown
Contributor

Thanks for the feedback on #147. I've created this new PR, which merges into #147, to address your comment. I will work on the changes and keep this PR's description up to date as I make progress.

Original PR: #147
Triggering review: #147 (comment)

@copilot open a new pull request to apply changes based on this feedback


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

claude and others added 9 commits December 29, 2025 00:47
Add missing usr/ directory structure to complete the dotfiles
management system. The repository now has a complete production-ready
setup for managing both user and system configurations.

Changes:
- Created usr/ directory with documentation
- Added usr/README.md explaining system-wide program deployment
- Added usr/.gitkeep to track the directory

Setup now complete:
- YADM manages Home/ → ~/ (user dotfiles)
- Tuckr manages etc/ → /etc (system configs)
- Tuckr manages usr/ → /usr (system programs)

All scripts validated:
- Bootstrap, yadm-sync, and setup scripts have valid syntax
- All scripts are executable and follow bash best practices
- Documentation is comprehensive and production-ready

Result: Complete dotfiles management system ∴ YADM + Tuckr properly configured
Enhance portability by adding automatic fallback to stow when tuckr
is unavailable. This ensures the dotfiles work on systems where tuckr
cannot be installed (e.g., Debian, older distros).

Changes to bootstrap (Home/.config/yadm/bootstrap):
- deploy_system_configs() now tries tuckr first, falls back to stow
- install_base_deps() tries installing tuckr, falls back to stow
- Clear messaging about which tool is being used

Changes to setup.sh:
- install_packages() tries tuckr first, falls back to stow
- link_system_configs() uses tuckr or stow automatically
- final_steps() shows correct command based on available tool

New script (Home/.local/bin/deploy-system-configs.sh):
- Unified wrapper for deploying system configs
- Auto-detects tuckr/stow and uses whichever is available
- Supports --unlink for removing symlinks
- Can deploy individual packages (etc, usr) or all at once

Documentation updates (YADM.md):
- Updated all references to mention tuckr/stow support
- Added examples for both tools in all sections
- Troubleshooting section includes stow installation
- Clarified fallback behavior throughout

Benefits:
- Works on Debian/Ubuntu without tuckr
- Graceful degradation when tuckr unavailable
- Maintains hook support when tuckr is present
- Single deployment script for user convenience

Result: Complete portability ∴ stow fallback ensures universal compatibility
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This work-in-progress PR addresses feedback from #147 by adding stow as a fallback option when tuckr is unavailable for deploying system-level configuration files. The changes enable the dotfiles setup to work on systems where tuckr cannot be installed.

Key changes:

  • Added fallback logic to automatically use stow when tuckr is unavailable during package installation and system config deployment
  • Created a new helper script deploy-system-configs.sh to provide a unified interface for deploying system configs with auto-detection of available tools
  • Updated documentation throughout to reflect the dual tuckr/stow support with usage examples for both tools

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 11 comments.

Show a summary per file
File Description
setup.sh Added stow fallback logic in package installation and system config linking functions
Home/.config/yadm/bootstrap Modified install_base_deps and deploy_system_configs functions to support stow as fallback
Home/.local/bin/deploy-system-configs.sh New helper script providing unified interface for deploying system configs with tuckr/stow auto-detection
YADM.md Updated documentation with examples for both tuckr and stow deployment methods
usr/README.md New documentation explaining the usr/ directory structure and deployment process
usr/.gitkeep Added placeholder file to track the usr/ directory in git
.github/dependabot.yml Removed unused configuration properties and added allow directives for pip/uv ecosystems

info "Using tuckr for deployment"
local hooks_file="${REPO_DIR}/hooks.toml"
for pkg in "${PACKAGES[@]}"; do
local src="${REPO_DIR}/${pkg}"

Copilot AI Dec 29, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The local keyword is being used outside of a function scope. In bash, local is only valid within functions. This should be a regular variable declaration without the local keyword.

Copilot uses AI. Check for mistakes.
if ! has tuckr; then
warn "tuckr not installed; skipping etc/usr."
warn "Install tuckr then run: sudo tuckr link -d $REPO_DIR -t / etc usr"
local pkg use_tuckr=true

Copilot AI Dec 29, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable use_tuckr=true is declared but never used in the function. This unused variable should be removed to keep the code clean.

Suggested change
local pkg use_tuckr=true
local pkg

Copilot uses AI. Check for mistakes.
Comment thread YADM.md
cd $(yadm rev-parse --show-toplevel) && sudo stow -t / etc usr

# Option 3: Using helper script (auto-detects tuckr/stow)
sudo deploy-system-configs

Copilot AI Dec 29, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The command examples show inconsistent usage. Line 189 shows etc usr as separate arguments, line 192 also shows etc usr as separate arguments, but line 195 shows sudo deploy-system-configs without arguments (which would default to both). For consistency and clarity, all examples should explicitly show the arguments when demonstrating multi-package deployment: sudo deploy-system-configs etc usr.

Suggested change
sudo deploy-system-configs
sudo deploy-system-configs etc usr

Copilot uses AI. Check for mistakes.
Comment thread YADM.md
sudo tuckr link -d $(yadm rev-parse --show-toplevel) -t / etc usr

# Option 2: Using stow (fallback)
cd $(yadm rev-parse --show-toplevel) && sudo stow -t / etc usr

Copilot AI Dec 29, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The command sudo stow -t / etc usr at line 192 is incorrect. The stow syntax requires the -d option to specify the source directory when not already in that directory. The correct command would be: cd $(yadm rev-parse --show-toplevel) && sudo stow -t / -d . etc usr or simply cd $(yadm rev-parse --show-toplevel) && sudo stow -t / etc usr (since -d . is the default when in the directory). This matches the pattern used elsewhere in the code.

Suggested change
cd $(yadm rev-parse --show-toplevel) && sudo stow -t / etc usr
cd $(yadm rev-parse --show-toplevel) && sudo stow -t / -d . etc usr

Copilot uses AI. Check for mistakes.
Comment thread YADM.md
sudo tuckr link -d "$PWD" -t / etc usr

# OR using stow:
sudo stow -t / etc usr

Copilot AI Dec 29, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The command sudo stow -t / etc usr at line 400 is incorrect. The stow syntax requires being in the repository directory or specifying it with -d. The correct command would be: sudo stow -t / -d . etc usr (when already in the repo directory) or the cd command should be shown first like in line 395: cd "$PWD" && sudo stow -t / etc usr.

Suggested change
sudo stow -t / etc usr
sudo stow -t / -d "$PWD" etc usr

Copilot uses AI. Check for mistakes.
Comment thread YADM.md
cd $(yadm rev-parse --show-toplevel) && sudo stow -t / etc usr

# Option 3: Using helper script (auto-detects tuckr/stow)
sudo deploy-system-configs

Copilot AI Dec 29, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation refers to the script as deploy-system-configs (lines 195, 280, 403) but the actual filename is deploy-system-configs.sh. Users would need to type deploy-system-configs.sh to execute it unless a symlink or alias without the .sh extension is created. Consider either: (1) renaming the file to remove the .sh extension to match the documentation, or (2) updating the documentation to include the .sh extension in all command examples.

Suggested change
sudo deploy-system-configs
sudo deploy-system-configs.sh

Copilot uses AI. Check for mistakes.
# Deploy using tuckr or stow
if has tuckr; then
info "Using tuckr for deployment"
local hooks_file="${REPO_DIR}/hooks.toml"

Copilot AI Dec 29, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The local keyword is being used outside of a function scope. In bash, local is only valid within functions. At the top level of a script, these should be regular variable declarations without the local keyword. This will cause the script to fail when executed.

Copilot uses AI. Check for mistakes.
tuckr unlink -d "$REPO_DIR" -t / "$pkg" || warn "Failed to unlink $pkg"
else
info "Linking ${pkg}/ → / (tuckr)"
local cmd=(tuckr link -d "$REPO_DIR" -t / "$pkg")

Copilot AI Dec 29, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The local keyword is being used outside of a function scope. In bash, local is only valid within functions. This should be a regular variable declaration without the local keyword.

Copilot uses AI. Check for mistakes.
elif has stow; then
info "Using stow for deployment (tuckr not available)"
for pkg in "${PACKAGES[@]}"; do
local src="${REPO_DIR}/${pkg}"

Copilot AI Dec 29, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The local keyword is being used outside of a function scope. In bash, local is only valid within functions. This should be a regular variable declaration without the local keyword.

Copilot uses AI. Check for mistakes.
Comment thread setup.sh
Comment on lines 89 to 115
link_system_configs(){
if ! has tuckr; then
warn "tuckr not found; skipping etc/usr deploy"
local pkg
if has tuckr; then
info "Using tuckr for system config deployment"
local hooks_file="${WORKTREE}/hooks.toml"
for pkg in etc usr; do
local src="${WORKTREE}/${pkg}"
[[ -d $src ]] || { warn "Directory not found: $src"; continue; }
info "Linking ${pkg}/ → / (tuckr)"
local cmd=(sudo tuckr link -d "$WORKTREE" -t / "$pkg")
[[ -f $hooks_file ]] && cmd+=(-H "$hooks_file")
"${cmd[@]}" || warn "Failed to link ${pkg}"
done
elif has stow; then
info "tuckr not found, using stow as fallback"
for pkg in etc usr; do
local src="${WORKTREE}/${pkg}"
[[ -d $src ]] || { warn "Directory not found: $src"; continue; }
info "Linking ${pkg}/ → / (stow)"
(cd "$WORKTREE" && sudo stow -t / -d . "$pkg") || warn "Failed to stow ${pkg}"
done
else
warn "Neither tuckr nor stow found; skipping etc/usr deploy"
warn "Install with: paru -S tuckr OR paru -S stow"
return 0
fi
local hooks_file="${WORKTREE}/hooks.toml" pkg
for pkg in etc usr; do
local src="${WORKTREE}/${pkg}"
[[ -d $src ]] || { warn "Directory not found: $src"; continue; }
info "Linking ${pkg}/ → /"
local cmd=(sudo tuckr link -d "$WORKTREE" -t / "$pkg")
[[ -f $hooks_file ]] && cmd+=(-H "$hooks_file")
"${cmd[@]}" || warn "Failed to link ${pkg}"
done
}

Copilot AI Dec 29, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The system config deployment logic is duplicated across three files: setup.sh (lines 89-115), Home/.config/yadm/bootstrap (lines 70-99), and Home/.local/bin/deploy-system-configs.sh (lines 77-112). This duplication makes maintenance harder and increases the risk of inconsistencies. Consider extracting this logic into a shared function or having the other scripts call the deploy-system-configs.sh helper script instead of reimplementing the same logic.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants