Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test changing the windows temp dir #207

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Conversation

IanButterworth
Copy link
Member

Adds test for #205

Should fail on windows on master

@IanButterworth IanButterworth requested a review from a team as a code owner January 15, 2024 23:45
@IanButterworth
Copy link
Member Author

@cderv would you be able to help get this reproducing the issue.

Also see question here #206 (comment)

@cderv
Copy link

cderv commented Jan 16, 2024

yes, I'll try to diagnose further the issue of why /usr/bin/tar is used in my workflow. Maybe it is not the temp dir at all.

I'll look at #206 also.

@cderv
Copy link

cderv commented Jan 16, 2024

So I spent my morning trying to understand deeper what was happening, and it was in fact an unexpected configuration and conflict between two actions:

So if I tweak/fix the PATH before Julia setup action, I can make it work.

Workflow file if you want to try it yourself

name: test-julia-install
on: [workflow_dispatch]

jobs:
  install:
    runs-on: windows-latest
    name: Test Julia install on windows
    steps:
      - uses: actions/checkout@v4
      - run: $env:PATH -Split ";"
        shell: powershell
      - run: gcm tar
        shell: powershell
      - uses: r-lib/actions/setup-r@v2
        # uncomment the following line to fix julia setup 
        # but it will have side effect probably on R setup
        # with:
        #  windows-path-include-rtools: false
      - run: $env:PATH -Split ";"
        shell: powershell
      - run: gcm tar
        shell: powershell
      - uses: julia-actions/setup-julia@v1
        with:
          version: '1.10'

However, I believe it is still not ideal for julia-actions/setup-julia to use a tar that could be incompatible with the expected command

yield exec.exec('powershell', ['-Command', `tar xf ${juliaDownloadPath} --strip-components=1 -C ${dest}`]);

  • juliaDownloadPath will be in D: as it is based on RUNNER_TEMP by default through tc.downloadTool()

    setup-julia/dist/index.js

    Lines 234 to 236 in a1561e9

    const juliaDownloadPath = yield retry((bail) => __awaiter(this, void 0, void 0, function* () {
    return yield tc.downloadTool(downloadURL);
    }), {

  • dest will be a folder in the hostedtoolcache folder on C:

    setup-julia/dist/index.js

    Lines 429 to 439 in a1561e9

    core.debug(`could not find Julia ${arch}/${version} in cache`);
    // https://github.com/julia-actions/setup-julia/pull/196
    // we want julia to be installed with unmodified file mtimes
    // but `tc.cacheDir` uses `cp` internally which destroys mtime
    // and `tc` provides no API to get the tool directory alone
    // so hack it by installing a empty directory then use the path it returns
    // and extract the archives directly to that location
    const emptyDir = fs.mkdtempSync('empty');
    juliaPath = yield tc.cacheDir(emptyDir, 'julia', version, arch);
    yield installer.installJulia(juliaPath, versionInfo, version, arch);
    core.debug(`added Julia to cache: ${juliaPath}`);

So either ensuring the right tar is used or the same drive is used could be safer.

Sorry for the trouble in the first place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants