HC/ Conditioning the use of potential paths for git bash file #4

Merged
allspice-hermes merged 2 commits from hc into main 2025-05-22 16:05:32 +00:00
No description provided.
allspice-hermes added 2 commits 2025-05-21 19:33:35 +00:00
allspice-hermes requested review from daniel-allspice 2025-05-21 19:33:43 +00:00
allspice-hermes changed title from HC/ Condioning the use of potential paths for git bash file to HC/ Conditioning the use of potential paths for git bash file 2025-05-21 19:34:03 +00:00
AllSpiceAlice reviewed 2025-05-21 20:19:08 +00:00
AllSpiceAlice left a comment
Owner

Your batch script is overall well-structured and performs its intended function—attempting to locate Git Bash in typical locations and using it to run Git commands (svn rebase, pull, and push). Here’s a detailed review:


What’s Working Well

1. Environment Variable Fallbacks

  • You're checking two common installation paths for Git Bash, which is good for accommodating different install scenarios.

2. Graceful Failure Handling

  • The else block provides a clear error message and uses exit /b 1, which is a correct way to exit a batch file with a non-zero code.

3. Consistent Execution

  • All Git commands are executed using the detected bash.exe, which ensures they run in the expected shell environment.

🔍 Suggestions for Improvement

1. Quoting Paths

You’re doing this correctly in most places, but to prevent subtle issues with spaces in paths, always quote all uses of variables, including in the command executions:

"%BASH_EXE%" -c "git svn rebase"

Already handled properly.

2. Use setlocal / endlocal

Encapsulate environment changes to avoid leaking variables:

@echo off
setlocal

:: Your script here...

endlocal

This ensures %BASH_EXE%, %GIT_BASH1%, etc., don’t persist after the script ends.

3. Check for Git Executable (optional)

You might want to confirm that git is accessible within bash by running:

"%BASH_EXE%" -c "command -v git > /dev/null || { echo 'Git not found in Bash.'; exit 1; }"

This could be helpful in misconfigured environments where bash.exe exists but git isn’t in the PATH.

4. Use Labels (Optional)

For more structured flow, especially if the script grows, consider using goto and labels.


💡 Optional Enhancements

  • Add logging for which path was selected:
echo Using Git Bash at "%BASH_EXE%"
  • Add an option to prompt the user before executing push, if this is risky:
set /p USERCONFIRM="Ready to push changes? (Y/N): "
if /I not "%USERCONFIRM%"=="Y" exit /b

Final Version with Minor Tweaks

@echo off
setlocal

:: Attempt to locate Git Bash in common locations
set "GIT_BASH1=C:\Program Files\Git\bin\bash.exe"
set "GIT_BASH2=%USERPROFILE%\AppData\Local\Programs\Git\bin\bash.exe"

if exist "%GIT_BASH1%" (
    set "BASH_EXE=%GIT_BASH1%"
) else if exist "%GIT_BASH2%" (
    set "BASH_EXE=%GIT_BASH2%"
) else (
    echo Git Bash not found. Please install Git for Windows and try again.
    exit /b 1
)

echo Using Git Bash at "%BASH_EXE%"

"%BASH_EXE%" -c "git svn rebase"
"%BASH_EXE%" -c "git pull"
"%BASH_EXE%" -c "git push"

pause
endlocal

Let me know if you'd like this script adapted for use in other shells, like PowerShell or WSL, or to add options like dry-run or logging to a file.

Your batch script is overall well-structured and performs its intended function—attempting to locate `Git Bash` in typical locations and using it to run Git commands (`svn rebase`, `pull`, and `push`). Here’s a detailed review: --- ## ✅ **What’s Working Well** ### 1. **Environment Variable Fallbacks** * You're checking two common installation paths for `Git Bash`, which is good for accommodating different install scenarios. ### 2. **Graceful Failure Handling** * The `else` block provides a clear error message and uses `exit /b 1`, which is a correct way to exit a batch file with a non-zero code. ### 3. **Consistent Execution** * All Git commands are executed using the detected `bash.exe`, which ensures they run in the expected shell environment. --- ## 🔍 **Suggestions for Improvement** ### 1. **Quoting Paths** You’re doing this correctly in most places, but to prevent subtle issues with spaces in paths, always quote all uses of variables, including in the command executions: ```batch "%BASH_EXE%" -c "git svn rebase" ``` ✅ Already handled properly. ### 2. **Use `setlocal` / `endlocal`** Encapsulate environment changes to avoid leaking variables: ```batch @echo off setlocal :: Your script here... endlocal ``` This ensures `%BASH_EXE%`, `%GIT_BASH1%`, etc., don’t persist after the script ends. ### 3. **Check for Git Executable (optional)** You might want to confirm that `git` is accessible within `bash` by running: ```batch "%BASH_EXE%" -c "command -v git > /dev/null || { echo 'Git not found in Bash.'; exit 1; }" ``` This could be helpful in misconfigured environments where `bash.exe` exists but `git` isn’t in the `PATH`. ### 4. **Use Labels (Optional)** For more structured flow, especially if the script grows, consider using `goto` and labels. --- ## 💡 **Optional Enhancements** * Add logging for which path was selected: ```batch echo Using Git Bash at "%BASH_EXE%" ``` * Add an option to prompt the user before executing `push`, if this is risky: ```batch set /p USERCONFIRM="Ready to push changes? (Y/N): " if /I not "%USERCONFIRM%"=="Y" exit /b ``` --- ## ✅ Final Version with Minor Tweaks ```batch @echo off setlocal :: Attempt to locate Git Bash in common locations set "GIT_BASH1=C:\Program Files\Git\bin\bash.exe" set "GIT_BASH2=%USERPROFILE%\AppData\Local\Programs\Git\bin\bash.exe" if exist "%GIT_BASH1%" ( set "BASH_EXE=%GIT_BASH1%" ) else if exist "%GIT_BASH2%" ( set "BASH_EXE=%GIT_BASH2%" ) else ( echo Git Bash not found. Please install Git for Windows and try again. exit /b 1 ) echo Using Git Bash at "%BASH_EXE%" "%BASH_EXE%" -c "git svn rebase" "%BASH_EXE%" -c "git pull" "%BASH_EXE%" -c "git push" pause endlocal ``` Let me know if you'd like this script adapted for use in other shells, like PowerShell or WSL, or to add options like dry-run or logging to a file.
AllSpiceAlice reviewed 2025-05-21 20:25:10 +00:00
AllSpiceAlice left a comment
Owner

Direct from Chad:

This batch script is a well-thought-out automation for setting up an AllSpice-integrated SVN monorepo using Git Bash and a remote shell script. Here's a detailed breakdown of its structure, strengths, and a few areas that could use improvement.


Strengths

1. Professional Initialization

  • Clear use of @echo off, color, and title improves UX.
  • cls ensures a clean interface.
  • Nice branding and informational output using echo blocks.

2. Timestamp Logging

  • Effective use of WMIC to generate a timestamp (yyyy-mm-dd_hh:mm:ss).
  • Useful for logging and tracking runs.

3. Safe Workspace Management

  • Checks for AllSpice directory and creates it if missing.
  • Copies the script itself to the workspace — good for reproducibility.

4. Safe Download and Execution

  • Uses curl -L to ensure the script is downloaded, with fallback on failure.
  • Runs the downloaded Bash script via Git Bash using tee to pipe output and remove ANSI codes.

5. Dynamic Git Bash Discovery

  • Checks common Git Bash install paths — makes the script portable across systems.

🛠️ Suggestions for Improvement

1. ANSI Escape Cleanup in Bash Redirection

This line:

"%BASH_EXE%" -c "$USERPROFILE/allspice/svn-to-allspice.sh 2>&1 | tee >(sed 's/\x1B\[[0-9;]*[mK]//g' >> %LOGFILE%)"
  • Problem: This Bash expression uses advanced process substitution (>) which may not be available or behave as expected on Windows Git Bash.
  • Solution: Simplify or validate that Git Bash supports process substitution. Alternatively, you can use a Bash wrapper script that handles this with script, sed, or simpler redirection.

2. Missing Quote Escaping / Path Expansion

Paths with spaces can break the script, especially here:

cd /d %RESULT%
  • Suggestion:

    cd /d "%RESULT%"
    

Also, when referencing paths like $USERPROFILE, ensure it’s properly expanded in Bash. This may not work correctly from a batch-initiated Bash session. Instead:

"$HOME/allspice/svn-to-allspice.sh"

Or pass it in explicitly:

"%BASH_EXE%" -c "\"$HOME/allspice/svn-to-allspice.sh\" 2>&1 | tee -a allspice.log"

3. Hardcoded File References

  • latest-repo.txt is used without verification:

    set /p RESULT=<latest-repo.txt
    

    If this file doesn’t exist or is malformed, the script could break.

  • Recommendation:

    if not exist latest-repo.txt (
        echo [%LOGTIME%] Error: latest-repo.txt not found.
        exit /b 1
    )
    set /p RESULT=<latest-repo.txt
    

4. Avoid Repeated setlocal

You have two setlocal calls — one around directory setup and one before the download. While this technically works, be aware that they nest. Consider merging into a single setlocal enabledelayedexpansion at the top for simplicity unless nesting is intentional.


Optional Enhancements

  • Logging Enhancements: Append all batch output to the log file:

    >> %LOGFILE% 2>&1
    
  • Run-Time Prompts: Add a confirmation before proceeding to clone or run remote scripts.

  • Versioning: Consider tagging the svn-to-allspice.sh download with a version query param for cache-busting.


Final Thoughts

You’ve done a great job creating a robust, user-friendly batch script for setting up an AllSpice + SVN integration workflow. With just a few small changes around quoting, and error checking, it will be even more reliable and portable.

@echo off
setlocal enabledelayedexpansion

:: Set color and title
color 09
title AllSpice Loves SVN Monorepos
cls

:: Get timestamp (yyyy-mm-dd_hh:mm:ss)
for /f "tokens=2 delims==" %%I in ('wmic os get localdatetime /value') do set datetime=%%I
set "LOGTIME=%datetime:~0,4%-%datetime:~4,2%-%datetime:~6,2%_%datetime:~8,2%:%datetime:~10,2%:%datetime:~12,2%"
set "LOGFILE=allspice.log"

:: Display intro
echo [%LOGTIME%] Current Timestamp: %LOGTIME%
echo ----------------------------------------------------------------
echo.
echo     AllSpice Loves SVN Monorepos v0.9.1
echo.
echo     1. Cloning a fresh AllSpice Repo...
echo     2. Deep Cloning an existing SVN Monorepo subfolder...
echo        - Monorepo is many projects in one SVN repo
echo        - SVN is cloned INTO the AllSpice Git repo
echo     3. Adding hooks and fetch-svn.bat
echo.
echo ----------------------------------------------------------------
echo.

:: Define workspace
set "ALLSPICE_DIR=%USERPROFILE%\allspice"

:: Create directory if it doesn't exist
if not exist "!ALLSPICE_DIR!" (
    mkdir "!ALLSPICE_DIR!"
)

:: Copy this script to the AllSpice directory
set "SCRIPT_NAME=%~nx0"
set "SCRIPT_PATH=!ALLSPICE_DIR!\!SCRIPT_NAME!"
copy /Y "%~f0" "!SCRIPT_PATH!" >nul 2>&1

:: Change to AllSpice directory
cd /d "!ALLSPICE_DIR!"

:: Download svn-to-allspice.sh
set "URL=https://hub.allspice.io/AllSpice-Demos/AllSpice-Loves-SVN-Monorepos/raw/branch/main/.allspice/svn-to-allspice.sh"
set "OUTPUT=svn-to-allspice.sh"

echo [%LOGTIME%] - [00] Downloading install script...
curl -L -o "%OUTPUT%" "%URL%"
if %errorlevel% neq 0 (
    echo [%LOGTIME%] ERROR: Download failed! >> "%LOGFILE%"
    echo [%LOGTIME%] ERROR: Download failed!
    exit /b 1
)
echo [%LOGTIME%] Download successful: %OUTPUT% >> "%LOGFILE%"
echo.

:: Attempt to locate Git Bash
set "GIT_BASH1=C:\Program Files\Git\bin\bash.exe"
set "GIT_BASH2=%USERPROFILE%\AppData\Local\Programs\Git\bin\bash.exe"

if exist "%GIT_BASH1%" (
    set "BASH_EXE=%GIT_BASH1%"
) else if exist "%GIT_BASH2%" (
    set "BASH_EXE=%GIT_BASH2%"
) else (
    echo [%LOGTIME%] ERROR: Git Bash not found. Please install Git for Windows and try again.
    exit /b 1
)

echo [%LOGTIME%] Found Git Bash: %BASH_EXE%
echo [%LOGTIME%] Running install script...
echo.

:: Run the install script and pipe output to log (cleaned of ANSI codes)
"%BASH_EXE%" -c "\"$HOME/allspice/svn-to-allspice.sh\" 2>&1 | tee >(sed 's/\x1B\[[0-9;]*[mK]//g' >> allspice.log)"

:: Check for latest-repo.txt
if not exist latest-repo.txt (
    echo [%LOGTIME%] ERROR: latest-repo.txt not found.
    exit /b 1
)

set /p RESULT=<latest-repo.txt
if not exist "%RESULT%" (
    echo [%LOGTIME%] ERROR: Cloned directory '%RESULT%' not found.
    exit /b 1
)

cd /d "%RESULT%"
echo [%LOGTIME%] Entering repo: %RESULT%
echo Launching interactive Git Bash...

"%BASH_EXE%" --login -i
Direct from Chad: This batch script is a well-thought-out automation for setting up an AllSpice-integrated SVN monorepo using Git Bash and a remote shell script. Here's a detailed breakdown of its structure, strengths, and a few areas that could use improvement. --- ## ✅ **Strengths** ### 1. **Professional Initialization** * Clear use of `@echo off`, `color`, and `title` improves UX. * `cls` ensures a clean interface. * Nice branding and informational output using `echo` blocks. ### 2. **Timestamp Logging** * Effective use of WMIC to generate a timestamp (`yyyy-mm-dd_hh:mm:ss`). * Useful for logging and tracking runs. ### 3. **Safe Workspace Management** * Checks for `AllSpice` directory and creates it if missing. * Copies the script itself to the workspace — good for reproducibility. ### 4. **Safe Download and Execution** * Uses `curl -L` to ensure the script is downloaded, with fallback on failure. * Runs the downloaded Bash script via Git Bash using tee to pipe output and remove ANSI codes. ### 5. **Dynamic Git Bash Discovery** * Checks common Git Bash install paths — makes the script portable across systems. --- ## 🛠️ **Suggestions for Improvement** ### 1. **ANSI Escape Cleanup in Bash Redirection** This line: ```batch "%BASH_EXE%" -c "$USERPROFILE/allspice/svn-to-allspice.sh 2>&1 | tee >(sed 's/\x1B\[[0-9;]*[mK]//g' >> %LOGFILE%)" ``` * **Problem**: This Bash expression uses advanced process substitution (`>`) which may not be available or behave as expected on Windows Git Bash. * **Solution**: Simplify or validate that Git Bash supports process substitution. Alternatively, you can use a Bash wrapper script that handles this with `script`, `sed`, or simpler redirection. ### 2. **Missing Quote Escaping / Path Expansion** Paths with spaces can break the script, especially here: ```batch cd /d %RESULT% ``` * **Suggestion**: ```batch cd /d "%RESULT%" ``` Also, when referencing paths like `$USERPROFILE`, ensure it’s properly expanded in Bash. This may not work correctly from a batch-initiated Bash session. Instead: ```bash "$HOME/allspice/svn-to-allspice.sh" ``` Or pass it in explicitly: ```batch "%BASH_EXE%" -c "\"$HOME/allspice/svn-to-allspice.sh\" 2>&1 | tee -a allspice.log" ``` ### 3. **Hardcoded File References** * `latest-repo.txt` is used without verification: ```batch set /p RESULT=<latest-repo.txt ``` If this file doesn’t exist or is malformed, the script could break. * **Recommendation**: ```batch if not exist latest-repo.txt ( echo [%LOGTIME%] Error: latest-repo.txt not found. exit /b 1 ) set /p RESULT=<latest-repo.txt ``` ### 4. **Avoid Repeated `setlocal`** You have two `setlocal` calls — one around directory setup and one before the download. While this technically works, be aware that they nest. Consider merging into a single `setlocal enabledelayedexpansion` at the top for simplicity unless nesting is intentional. --- ## ✨ **Optional Enhancements** * **Logging Enhancements**: Append all batch output to the log file: ```batch >> %LOGFILE% 2>&1 ``` * **Run-Time Prompts**: Add a confirmation before proceeding to clone or run remote scripts. * **Versioning**: Consider tagging the `svn-to-allspice.sh` download with a version query param for cache-busting. --- ## ✅ Final Thoughts You’ve done a great job creating a robust, user-friendly batch script for setting up an AllSpice + SVN integration workflow. With just a few small changes around quoting, and error checking, it will be even more reliable and portable. ``` @echo off setlocal enabledelayedexpansion :: Set color and title color 09 title AllSpice Loves SVN Monorepos cls :: Get timestamp (yyyy-mm-dd_hh:mm:ss) for /f "tokens=2 delims==" %%I in ('wmic os get localdatetime /value') do set datetime=%%I set "LOGTIME=%datetime:~0,4%-%datetime:~4,2%-%datetime:~6,2%_%datetime:~8,2%:%datetime:~10,2%:%datetime:~12,2%" set "LOGFILE=allspice.log" :: Display intro echo [%LOGTIME%] Current Timestamp: %LOGTIME% echo ---------------------------------------------------------------- echo. echo AllSpice Loves SVN Monorepos v0.9.1 echo. echo 1. Cloning a fresh AllSpice Repo... echo 2. Deep Cloning an existing SVN Monorepo subfolder... echo - Monorepo is many projects in one SVN repo echo - SVN is cloned INTO the AllSpice Git repo echo 3. Adding hooks and fetch-svn.bat echo. echo ---------------------------------------------------------------- echo. :: Define workspace set "ALLSPICE_DIR=%USERPROFILE%\allspice" :: Create directory if it doesn't exist if not exist "!ALLSPICE_DIR!" ( mkdir "!ALLSPICE_DIR!" ) :: Copy this script to the AllSpice directory set "SCRIPT_NAME=%~nx0" set "SCRIPT_PATH=!ALLSPICE_DIR!\!SCRIPT_NAME!" copy /Y "%~f0" "!SCRIPT_PATH!" >nul 2>&1 :: Change to AllSpice directory cd /d "!ALLSPICE_DIR!" :: Download svn-to-allspice.sh set "URL=https://hub.allspice.io/AllSpice-Demos/AllSpice-Loves-SVN-Monorepos/raw/branch/main/.allspice/svn-to-allspice.sh" set "OUTPUT=svn-to-allspice.sh" echo [%LOGTIME%] - [00] Downloading install script... curl -L -o "%OUTPUT%" "%URL%" if %errorlevel% neq 0 ( echo [%LOGTIME%] ERROR: Download failed! >> "%LOGFILE%" echo [%LOGTIME%] ERROR: Download failed! exit /b 1 ) echo [%LOGTIME%] Download successful: %OUTPUT% >> "%LOGFILE%" echo. :: Attempt to locate Git Bash set "GIT_BASH1=C:\Program Files\Git\bin\bash.exe" set "GIT_BASH2=%USERPROFILE%\AppData\Local\Programs\Git\bin\bash.exe" if exist "%GIT_BASH1%" ( set "BASH_EXE=%GIT_BASH1%" ) else if exist "%GIT_BASH2%" ( set "BASH_EXE=%GIT_BASH2%" ) else ( echo [%LOGTIME%] ERROR: Git Bash not found. Please install Git for Windows and try again. exit /b 1 ) echo [%LOGTIME%] Found Git Bash: %BASH_EXE% echo [%LOGTIME%] Running install script... echo. :: Run the install script and pipe output to log (cleaned of ANSI codes) "%BASH_EXE%" -c "\"$HOME/allspice/svn-to-allspice.sh\" 2>&1 | tee >(sed 's/\x1B\[[0-9;]*[mK]//g' >> allspice.log)" :: Check for latest-repo.txt if not exist latest-repo.txt ( echo [%LOGTIME%] ERROR: latest-repo.txt not found. exit /b 1 ) set /p RESULT=<latest-repo.txt if not exist "%RESULT%" ( echo [%LOGTIME%] ERROR: Cloned directory '%RESULT%' not found. exit /b 1 ) cd /d "%RESULT%" echo [%LOGTIME%] Entering repo: %RESULT% echo Launching interactive Git Bash... "%BASH_EXE%" --login -i ```
allspice-hermes merged commit 4cf82adcf3 into main 2025-05-22 16:05:32 +00:00
Sign in to join this conversation.
No Label
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: AllSpice-Resources/AllSpice-SVN-Monorepos#4
No description provided.