diff --git a/lib/fish/deployer.fish b/lib/fish/deployer.fish index f47b2b1..4474fb1 100644 --- a/lib/fish/deployer.fish +++ b/lib/fish/deployer.fish @@ -45,17 +45,26 @@ function deployer-deploy-module fedpunk-config-init end - # Normalize git URLs to module names for consistent config storage - # git@gitlab.com:org/thinkpad-fans.git -> thinkpad-fans - set -l module_name "$module_ref" - if module-ref-is-url "$module_ref" - # Extract repo name from URL (same logic as external-module-get-storage-path) - set module_name (string replace -r '\.git$' '' "$module_ref") - set module_name (string replace -r '^.*[/:]' '' "$module_name") + # Normalize module reference before storing + # - URLs: Keep as-is (fixes duplicate entry bug) + # - Paths: Convert to absolute (fixes relative path resolution) + # - Names: Keep as-is + set -l config_ref "$module_ref" + + # Check if it's a path (contains / but not a URL) + if string match -q '*/*' "$module_ref"; and not module-ref-is-url "$module_ref" + # It's a path - convert to absolute + set -l expanded_path (string replace -r '^~' "$HOME" "$module_ref") + + # If not already absolute, make it absolute relative to PWD + if not string match -q '/*' "$expanded_path" + set expanded_path (realpath "$expanded_path" 2>/dev/null; or echo "$PWD/$expanded_path") + end + + set config_ref "$expanded_path" end - # Add normalized module name to config - fedpunk-config-add-module "$module_name" + fedpunk-config-add-module "$config_ref" # Use existing fedpunk-module deploy (already handles local + git) if fedpunk-module deploy "$module_ref" diff --git a/test/ci/test-env-login-vs-nonlogin-shells.sh b/test/ci/test-env-login-vs-nonlogin-shells.sh new file mode 100755 index 0000000..17f591e --- /dev/null +++ b/test/ci/test-env-login-vs-nonlogin-shells.sh @@ -0,0 +1,192 @@ +#!/bin/bash +# Regression test for env vars only available in login shells +# +# This test reproduces the issue where: +# 1. User deploys a module with environment variables +# 2. Env vars are NOT available in current shell +# 3. Env vars are NOT available in new non-login shell (bash from bash) +# 4. Env vars ARE available in new login shell (ssh, new terminal) +# +# Expected behavior: Env vars should be available everywhere +# Current behavior: Only in login shells (bug) + +set -e + +echo "" +echo "=========================================" +echo "Login vs Non-Login Shell Env Test" +echo "=========================================" +echo "Reproducing the 'had to run bash again' bug" +echo "" + +# Setup test environment +TEST_DIR=$(mktemp -d -t fedpunk-shell-test-XXXXXX) +trap "rm -rf $TEST_DIR" EXIT + +echo "Test environment: $TEST_DIR" +echo "" + +# Override HOME for isolated testing +export HOME="$TEST_DIR/home" +mkdir -p "$HOME/.config/fedpunk/profile.d" +mkdir -p "$TEST_DIR/etc/profile.d" + +# Simulate module deployment generating env config +# Use unique var name to avoid pollution from current shell +UNIQUE_VAR="FEDPUNK_TEST_$$_$(date +%s)" +cat > "$HOME/.config/fedpunk/profile.d/fedpunk-env.sh" < "$TEST_DIR/etc/profile.d/fedpunk.sh" <<'EOF' +#!/bin/sh +# Fedpunk environment variables +export FEDPUNK_SYSTEM=/usr/share/fedpunk +export FEDPUNK_USER=$HOME/.local/share/fedpunk + +# Auto-load user module environment variables +if [ -f "$HOME/.config/fedpunk/profile.d/fedpunk-env.sh" ]; then + . "$HOME/.config/fedpunk/profile.d/fedpunk-env.sh" +fi +EOF + +echo "Created simulated /etc/profile.d/fedpunk.sh" +echo "" + +# +# Test 1: Current shell does NOT have env vars (expected) +# +echo "=== Test 1: Current shell after deployment ===" +echo "This simulates: user just ran 'fedpunk module deploy'" +echo "" + +if [ -z "$FEDPUNK_TEST_API" ]; then + echo " ✓ EXPECTED: FEDPUNK_TEST_API not set in current shell" + echo " (User would need to manually source the config)" +else + echo " ✗ UNEXPECTED: FEDPUNK_TEST_API is set: $FEDPUNK_TEST_API" +fi + +echo "" + +# +# Test 2: Non-login shell does NOT have env vars (BUG) +# +echo "=== Test 2: New non-login shell (bash from bash) ===" +echo "This simulates: user runs 'bash' from their current bash shell" +echo "" + +# Non-login interactive shell (like running 'bash' from bash) +NONLOGIN_VALUE=$(bash -c " +export HOME='$HOME' +echo \$FEDPUNK_TEST_API +") + +if [ -z "$NONLOGIN_VALUE" ]; then + echo " ❌ BUG REPRODUCED: FEDPUNK_TEST_API not set in non-login shell" + echo " /etc/profile.d/ is NOT sourced for non-login shells" + echo " This is the bug the user experienced!" + BUG_REPRODUCED=1 +else + echo " ✓ UNEXPECTED: FEDPUNK_TEST_API is set: $NONLOGIN_VALUE" + echo " (Test environment might be different from real world)" +fi + +echo "" + +# +# Test 3: Login shell HAS env vars (works but requires new login) +# +echo "=== Test 3: New login shell (ssh, new terminal) ===" +echo "This simulates: user SSH'ing into container or opening new terminal" +echo "" + +# Login shell (sources /etc/profile which sources /etc/profile.d/*) +LOGIN_VALUE=$(bash --login -c " +export HOME='$HOME' +# Manually source profile.d to simulate login shell behavior +. '$TEST_DIR/etc/profile.d/fedpunk.sh' +echo \$FEDPUNK_TEST_API +") + +if [ "$LOGIN_VALUE" = "https://api.test.com" ]; then + echo " ✓ WORKS: FEDPUNK_TEST_API set in login shell" + echo " Value: $LOGIN_VALUE" + echo " But user had to start a NEW login shell to get it!" +else + echo " ✗ FAIL: FEDPUNK_TEST_API not set even in login shell" + echo " Expected: 1" + echo " Got: $LOGIN_VALUE" + exit 1 +fi + +echo "" + +# +# Test 4: User's workaround - running 'bash' might create login shell +# +echo "=== Test 4: User's workaround behavior ===" +echo "When user ran 'bash' again, they might have gotten lucky with:" +echo " - Container configuration that forces login shells" +echo " - .bashrc that sources /etc/profile" +echo " - bash --login being their default" +echo "" + +# Try various bash invocations +echo "Testing different bash invocations:" + +# Standard non-login +STANDARD=$(bash -c "export HOME='$HOME'; echo \$FEDPUNK_TEST_API") +echo " bash -c: '$STANDARD' (empty = not set)" + +# Interactive non-login +INTERACTIVE=$(bash --init-file /dev/null -i -c "export HOME='$HOME'; echo \$FEDPUNK_TEST_API" 2>/dev/null || echo "") +echo " bash -i: '$INTERACTIVE' (empty = not set)" + +# Login shell +LOGIN=$(bash --login -c "export HOME='$HOME'; . '$TEST_DIR/etc/profile.d/fedpunk.sh'; echo \$FEDPUNK_TEST_API") +echo " bash --login: '$LOGIN' (should be 'https://api.test.com')" + +echo "" + +# +# Summary +# +echo "=========================================" +if [ -n "$BUG_REPRODUCED" ]; then + echo "BUG REPRODUCED!" +else + echo "Bug not reproduced (environment differs)" +fi +echo "=========================================" +echo "" +echo "Issue Summary:" +echo " ❌ Current shell: Env vars NOT available after deployment" +echo " ❌ Non-login shell (bash from bash): Env vars NOT available" +echo " ✅ Login shell (new terminal/ssh): Env vars available" +echo "" +echo "Real-world impact:" +echo " - User deploys module" +echo " - Tries to use env vars → NOT AVAILABLE" +echo " - Runs 'bash' → STILL NOT AVAILABLE (unless login shell)" +echo " - Has to start new terminal/ssh → Finally available" +echo "" +echo "Root cause:" +echo " /etc/profile.d/ only sourced for LOGIN shells, not:" +echo " - Current shell (where deployment happens)" +echo " - Non-login interactive shells (bash from bash)" +echo " - Subshells" +echo "" +echo "Solutions needed:" +echo " 1. Source config in current shell during deployment" +echo " 2. Add to /etc/bash.bashrc for non-login interactive shells" +echo " 3. Add to /etc/zshrc for zsh" +echo "" diff --git a/test/ci/test-external-module-params-no-duplicate.sh b/test/ci/test-external-module-params-no-duplicate.sh index 67ca74f..47de3cc 100755 --- a/test/ci/test-external-module-params-no-duplicate.sh +++ b/test/ci/test-external-module-params-no-duplicate.sh @@ -112,16 +112,14 @@ echo "" # # Test 2: Simulate the bug scenario - deployer-deploy-module with params # -echo "=== Test 2: Simulate deployer-deploy-module bug scenario ===" +echo "=== Test 2: Deploy module with parameters (testing fix) ===" # Initialize config run_fish "fedpunk-config-init" 2>&1 || true -# Simulate the buggy behavior: -# Step 1: deployer-deploy-module adds NORMALIZED NAME (line 58) -NORMALIZED_NAME="test-params-ext-module" -run_fish "fedpunk-config-add-module '$NORMALIZED_NAME'" 2>&1 || true -echo " Step 1: Added normalized name to config: $NORMALIZED_NAME" +# Step 1: deployer-deploy-module adds module URL (FIXED behavior) +run_fish "fedpunk-config-add-module '$TEST_MODULE_URL'" 2>&1 || true +echo " Step 1: Added module URL to config: $TEST_MODULE_URL" # Step 2: param-save-to-config tries to find using URL (not name) # This is what happens when fedpunk-module deploy calls param-prompt-required @@ -131,7 +129,7 @@ param-save-to-config '$TEST_MODULE_URL' 'auth_mode' 'enabled' " 2>&1 || true echo " Step 2: Saved params using URL: $TEST_MODULE_URL" -echo " (This is where the duplicate gets created if bug exists)" +echo " (Should update existing entry, not create duplicate)" echo "" # diff --git a/test/ci/test-local-path-modules.sh b/test/ci/test-local-path-modules.sh new file mode 100755 index 0000000..e80748e --- /dev/null +++ b/test/ci/test-local-path-modules.sh @@ -0,0 +1,162 @@ +#!/bin/bash +# Test local path module deployment +# +# Tests: +# 1. Relative path (./module) should be converted to absolute path +# 2. Absolute path (/path/to/module) should work +# 3. Path with ~ should be expanded + +set -e + +echo "" +echo "=========================================" +echo "Local Path Module Test" +echo "=========================================" +echo "" + +# Setup test environment +TEST_DIR=$(mktemp -d -t fedpunk-local-path-test-XXXXXX) +trap "rm -rf $TEST_DIR" EXIT + +echo "Test environment: $TEST_DIR" +echo "" + +# Override HOME and XDG for isolated testing +export HOME="$TEST_DIR/home" +export XDG_CONFIG_HOME="$HOME/.config" +export XDG_DATA_HOME="$HOME/.local/share" +mkdir -p "$HOME" +mkdir -p "$HOME/.config/fish/conf.d" + +# Use LOCAL git repository (not system installation) +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +export FEDPUNK_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)" +export FEDPUNK_SYSTEM="$FEDPUNK_ROOT" +export FEDPUNK_USER="$HOME/.local/share/fedpunk" + +# Helper function to run fish with our local libs +run_fish() { + fish -c " +set -gx FEDPUNK_ROOT '$FEDPUNK_ROOT' +set -gx FEDPUNK_SYSTEM '$FEDPUNK_SYSTEM' +set -gx FEDPUNK_USER '$FEDPUNK_USER' +set -gx HOME '$HOME' +source \$FEDPUNK_SYSTEM/lib/fish/paths.fish +$1 +" +} + +# +# Test 1: Create test module with relative path +# +echo "=== Test 1: Deploy module with relative path (./module) ===" + +TEST_MODULE_DIR="$TEST_DIR/my-test-module" +mkdir -p "$TEST_MODULE_DIR/config" + +cat > "$TEST_MODULE_DIR/module.yaml" <<'EOF' +module: + name: my-test-module + description: Test module for local path + +packages: + dnf: [] + +stow: + target: $HOME + conflicts: warn +EOF + +echo "test config" > "$TEST_MODULE_DIR/config/test.txt" + +# Change to test dir and deploy with relative path +cd "$TEST_DIR" + +run_fish " +source \$FEDPUNK_SYSTEM/lib/fish/config.fish +source \$FEDPUNK_SYSTEM/lib/fish/deployer.fish +fedpunk-config-init +deployer-deploy-module './my-test-module' +" 2>&1 | grep -v "^Deploying\|^==>" || true + +CONFIG_FILE="$HOME/.config/fedpunk/fedpunk.yaml" + +echo "" +echo "Config file contents:" +cat "$CONFIG_FILE" +echo "" + +# Check what was stored +MODULE_REF=$(yq '.modules.enabled[0]' "$CONFIG_FILE") + +echo "Stored module reference: $MODULE_REF" +echo "" + +if echo "$MODULE_REF" | grep -q "^/"; then + echo "✅ SUCCESS: Relative path converted to absolute: $MODULE_REF" +elif echo "$MODULE_REF" | grep -q "^\."; then + echo "❌ FAIL: Stored relative path: $MODULE_REF" + echo " This will break when resolved from different directory!" + exit 1 +else + echo "⚠ WARNING: Unexpected format: $MODULE_REF" +fi + +echo "" + +# +# Test 2: Verify module can be resolved from different directory +# +echo "=== Test 2: Resolve module from different directory ===" + +# Change to a different directory +cd "$HOME" + +RESOLVED_PATH=$(run_fish " +source \$FEDPUNK_SYSTEM/lib/fish/module-resolver.fish +module-resolve-path '$MODULE_REF' +" 2>/dev/null) + +if [ -d "$RESOLVED_PATH" ]; then + echo "✅ SUCCESS: Module resolved from different directory" + echo " Stored: $MODULE_REF" + echo " Resolved: $RESOLVED_PATH" +else + echo "❌ FAIL: Module could not be resolved from different directory" + echo " Stored: $MODULE_REF" + echo " Tried to resolve: $RESOLVED_PATH" + exit 1 +fi + +echo "" + +# +# Test 3: Test absolute path +# +echo "=== Test 3: Deploy with absolute path ===" + +run_fish " +source \$FEDPUNK_SYSTEM/lib/fish/config.fish +yq -i '.modules.enabled = []' '$CONFIG_FILE' +source \$FEDPUNK_SYSTEM/lib/fish/deployer.fish +deployer-deploy-module '$TEST_MODULE_DIR' +" 2>&1 | grep -v "^Deploying\|^==>" || true + +ABS_MODULE_REF=$(yq '.modules.enabled[0]' "$CONFIG_FILE") + +echo "Stored absolute path: $ABS_MODULE_REF" + +if [ "$ABS_MODULE_REF" = "$TEST_MODULE_DIR" ]; then + echo "✅ SUCCESS: Absolute path stored correctly" +else + echo "❌ FAIL: Absolute path changed" + echo " Expected: $TEST_MODULE_DIR" + echo " Got: $ABS_MODULE_REF" + exit 1 +fi + +echo "" +echo "=========================================" +echo "All local path tests passed!" +echo "=========================================" +echo ""