DeLaGuardo / setup-clojure

GitHub Action to provision clojure's most popular build tools for Linux, Mac OS X and Windows.
MIT License
183 stars 27 forks source link

Use lein.bat on Windows rather than Powershell #78

Closed borkdude closed 1 year ago

borkdude commented 1 year ago

When shelling out to lein, it's easier with lein.bat than lein powershell. Would you consider using lein.bat instead?

https://github.com/DeLaGuardo/setup-clojure/blob/a3c51198398090cd894cac6b6b71d8f5c2a45406/src/leiningen.ts#L30

I was running into this issue here: https://github.com/borkdude/timbre/actions/runs/3499442197/jobs/5861008418

cc @lread @DeLaGuardo

lread commented 1 year ago

Ya, I think lein.bat is a better choice too. Works with all shells.

Are there any advantages to lein.ps1?

ikappaki commented 1 year ago

Hi,

I have just also stumbled upon this independently and was about to raise an issue 😅

this should be considered a bug IMHO . lein cannot be found when trying to invoke it on the cmd shell

'lein' is not recognized as an internal or external command,
operable program or batch file.

The expecation here is I think that the action should work with any shell.

To reproduce

  1. Setup GH action for MS-Windows that installs lein with setup-clojure and calls lein version on the powershell, pwsh and cmd shells

    test-lein-cmd:
    runs-on: windows-latest
    
    steps:
      - name: Checkout
        uses: actions/checkout@v3
    
      - name: Prepare java
        uses: actions/setup-java@v3
        with:
          distribution: 'zulu'
          java-version: '8'
    
      - name: Install lein
        uses: DeLaGuardo/setup-clojure@9.5
        with:
          lein: '2.9.10'
    
      - name: lein powershell
        shell: powershell
        run: |
          lein version
    
      - name: lein pwsh
        shell: pwsh
        run: |
          lein version
    
      - name: lein cmd
        shell: cmd
        run: |
          lein version
  2. Run the GH action, you should see tha the powershell and pwsh succeed but cmd fails (only showing the latter below)
    
    2022-11-20T17:22:49.2775007Z ##[group]Run lein version
    2022-11-20T17:22:49.2775419Z lein version
    2022-11-20T17:22:49.2810382Z shell: C:\Windows\system32\cmd.EXE /D /E:ON /V:OFF /S /C "CALL "{0}""
    2022-11-20T17:22:49.2810728Z env:
    2022-11-20T17:22:49.2811669Z   JAVA_HOME: C:\hostedtoolcache\windows\Java_Zulu_jdk\8.0.352-8\x64
    2022-11-20T17:22:49.2812093Z   JAVA_HOME_8_X64: C:\hostedtoolcache\windows\Java_Zulu_jdk\8.0.352-8\x64
    2022-11-20T17:22:49.2812531Z   LEIN_HOME: C:\hostedtoolcache\windows\Leiningen\2.9.10-9-5\x64
    2022-11-20T17:22:49.2812865Z ##[endgroup]
    2022-11-20T17:22:49.3016487Z 'lein' is not recognized as an internal or external command,
    2022-11-20T17:22:49.3017128Z operable program or batch file.
    2022-11-20T17:22:49.3044548Z ##[error]Process completed with exit code 1.


The solution here is either to replace the `.ps1` script with `https://raw.githubusercontent.com/technomancy/leiningen/stable/bin/lein.bat` as pointed out by @borkdude, or install the `.bat` script alongside the `.ps1` file. I suppose tha latter option, to answer @lread's question,  has the advantage that when running a job with PowerShell (the default shell on GH actions) it will still use the ps1 script which will give slightly better perfofrmance than spawning a new conhost to execute the batch script.

Thanks
DeLaGuardo commented 1 year ago

Instead of choosing one from lein.ps1 and lein.bat, I decided to use both :) This should fix the issue. Feel free to reopen in case something isn't working

borkdude commented 1 year ago

Thanks! Will try soon