julia-actions / setup-julia

This action sets up a Julia environment for use in actions by downloading a specified version of Julia and adding it to PATH.
MIT License
93 stars 23 forks source link

[BUG] Using tar on Windows should call `$env:WINDIR/System32/tar` and not the Git bash provided `/usr/bin/tar` #205

Closed cderv closed 3 months ago

cderv commented 9 months ago

Describe the bug

Recent change introduced the use of tar on Windows

Currently, it is calling tar through PowerShell using the program name only, thus relying on PATH resolution.

It happens that /usr/bin/tar is used (probably from Git Bash) and not the Windows internal $env:WINDIR/System32/tar.exe. This is known to happen on github action (at least encountered in the past)

I encountered this because by modifying temp folder to be {{ runner.temp }} on Windows, it leads to having tar dealing with Windows path name with network drive letter. Only Windows's tar can handle that.

This is the issue I encountered using setup-julia action

$ Run julia-actions/setup-julia@v1.9.5
C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe -Command "tar xf D:\a\_temp\f585e5f8-9502-4dc6-9544-d8ebf784ca06 --strip-components=1 -C C:\hostedtoolcache\windows\julia\1.10.0\x64"
/usr/bin/tar: Cannot connect to D: resolve failed
Error: The process 'C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe' failed with exit code 1

To Reproduce

I can create a simple example reproducing the Can't connect error but this is not the main issue I would like to report here. I believe any action using 1.9.5 is calling the /usr/bin/tar instead of Window's tar.exe

If you would like a workflow file anyway, I can provide.

Expected behavior

Windows' own tar is used when using tar on Windows, which allows to correctly handle Windows paths.

This change ensures it is, and it solves issue I encountered

diff --git a/dist/index.js b/dist/index.js
index 74d0f94..8acd5f4 100644
--- a/dist/index.js
+++ b/dist/index.js
@@ -268,7 +268,7 @@ function installJulia(dest, versionInfo, version, arch) {
                 }
                 else {
                     // This is the more common path. Using .tar.gz is much faster
-                    yield exec.exec('powershell', ['-Command', `tar xf ${juliaDownloadPath} --strip-components=1 -C ${dest}`]);
+                    yield exec.exec('powershell', ['-Command', `& "$env:WINDIR/System32/tar" xf ${juliaDownloadPath} --strip-components=1 -C ${dest}`]);
                 }
                 return dest;
             case 'darwin':

(I modified in Fork in dist by convenience for testing.

Screenshots/Build logs

With current 1.9.5

image

With a fork using Windows' tar

image

Additional context

Nothing more I can think of.

Happy to provide anything more needed. Please do tell me.

Thanks

cderv commented 9 months ago

More detailed information about this issue as more tests show that this is not linked to a change of temporary directory. In fact, on github-hosted Windows runner, the default will always be on two drives and does not depend on TEMP env var

This means that currently, this action requires a tar that can handle Windows paths on two drives https://github.com/julia-actions/setup-julia/blob/a1561e938c17e7aaf8236334d6d533e774c71dcd/dist/index.js#L271

More detailed answer in #206 and #207

I would sum up this issue this way.

ViralBShah commented 8 months ago

206

wzhorton commented 5 months ago

Big thanks to @cderv for making a fork that implements fixes from PR #206 , which have not been merged yet. I ran into this issue when running package tests that depend on R via RCall.jl. For my fellow inexperienced GitHub Actions folks, I have a matrix of OS configs that includes os: [ubuntu-latest, windows-latest, macOS-latest]. Instead of a single setup line with - uses:julia-actions/setup-julia@v2, I replaced it with this which worked wonderfully:

    - name: Setup Julia (Default)
     uses: julia-actions/setup-julia@v2
     if: matrix.os != 'windows-latest'
     with:
       version: ${{ matrix.julia-version }}
       arch: ${{ matrix.julia-arch }}
    - name: Setup Julia (Windows w/ TAR Fix)
     uses: cderv/setup-julia@fix-tar
     if: matrix.os == 'windows-latest'
     with:
       version: ${{ matrix.julia-version }}
       arch: ${{ matrix.julia-arch }}

Of course, if and when the PR is merged none of this will be necessary, but for now I hope this is helpful.