setup-julia icon indicating copy to clipboard operation
setup-julia copied to clipboard

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

Open cderv opened this issue 1 year ago • 2 comments

Describe the bug

Recent change introduced the use of tar on Windows

  • https://github.com/julia-actions/setup-julia/pull/171

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)

  • https://github.com/actions/runner-images/issues/480

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/[email protected]
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
  • This shows that /usr/bin/tar is the own throwing the error
  • The error is linked to network drive letter. It happens because in my context, Temp folder is modified to use {{ runner.temp }} which is on D: and it tries to untar on C:.

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 avatar Jan 15 '24 14:01 cderv

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

  • RUNNER_TEMP will be in D:
  • hostedtoolcache will be in C:

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.

  • Using a tar.exe that handles Windows paths with Drive later is required.
  • This action will use the first tar on PATH
  • Users need to insure the right tar is on PATH (it happens other actions can modify $GITHUB_PATH and mess this up)
  • If not, a workaround is to add the tar.exe that works first on PATH before running setup-julia

cderv avatar Jan 16 '24 11:01 cderv

#206

ViralBShah avatar Jan 19 '24 17:01 ViralBShah

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.

wzhorton avatar May 03 '24 03:05 wzhorton