Skip to content

Output msysRootDir from action.#59

Closed
jeremyd2019 wants to merge 1 commit intomsys2:masterfrom
jeremyd2019:output_msysRootDir
Closed

Output msysRootDir from action.#59
jeremyd2019 wants to merge 1 commit intomsys2:masterfrom
jeremyd2019:output_msysRootDir

Conversation

@jeremyd2019
Copy link
Member

It's reasonable that another step might want to know this.

Calling autorebase.bat seemed a good way to test this, even though it shouldn't be needed on x86_64

@lazka
Copy link
Member

lazka commented Aug 1, 2020

Can you give an example for what it's needed? Do you know you can call cygpath -m /?

@jeremyd2019
Copy link
Member Author

I've been playing around with a fork of this action that installs the last i686 msys release, and a workflow to build the packages I've missed out on since i686 stopped getting updated (it's actually been going surprisingly well). I wanted to call autorebase.bat after updating the packages I'd already built, so needed to know the path from a cmd run step. It didn't occur to me to call out to cygpath, but I had seen the docs that steps could output variables and thought that made enough sense to offer that change upstream.

@lazka
Copy link
Member

lazka commented Aug 1, 2020

ok, thanks. That seems like a very uncommon use case, though I'm not against it per se, except maybe the naming.

@eine any thoughts?

@jeremyd2019
Copy link
Member Author

For reference, using cygpath instead of an output variable:

    - name: test output
      shell: cmd
      run: |
        set MSYSTEM=MSYS
        FOR /F "tokens=* USEBACKQ" %%g IN (`CALL msys2 -c 'cygpath -w /'`) DO (SET "MSYSROOT=%%g")
        echo %MSYSROOT%
        call "%MSYSROOT%\autorebase.bat"

@lazka
Copy link
Member

lazka commented Aug 1, 2020

Untested, but something like this should also work in powershell:

Invoke-Expression (msys2 -c 'cygpath -m /autorebase.bat')

@eine
Copy link
Collaborator

eine commented Aug 1, 2020

Honestly, I don't like the feature to set outputs from Actions. It feels like a not very well thought ad-hoc solution. Instead, I'd use some more natural solution, such as setting an environment variable: https://github.com/actions/toolkit/tree/main/packages/core#exporting-variables.

However, if the last comment by @lazka works, maybe it's better to just document that in the README. On the other hand, I believe that msys2.cmd was always located in MSYS2's /, prior to latest cache changes. Hence the root could be extracted from the location of the command file. An alternative to setting an envvar would be to ensure that this holds.

@jeremyd2019
Copy link
Member Author

To better match what I was doing, I tested the following:

& (Join-Path (msys2 -c 'cygpath -w /') 'autorebase.bat')

and confirmed that that works. I'm sure giving autorebase.bat to cygpath instead of using Join-Path would also work.

@jeremyd2019
Copy link
Member Author

And if you really want an output for some reason, you can set one from a run step:

    - name: test cygpath
      id: cygpath
      run: |
        $env:MSYSTEM = 'MSYS'
        Write-Output ('::set-output name=msysRootDir::{0}' -f (msys2 -c 'cygpath -w /'))
    - name: autorebase
      run: |
        $env:MSYSTEM = 'MSYS'
        & (Join-Path '${{ steps.cygpath.outputs.msysRootDir }}' 'autorebase.bat')

@jeremyd2019 jeremyd2019 closed this Aug 3, 2020
jeremyd2019 added a commit to jeremyd2019/setup-msys2 that referenced this pull request Aug 3, 2020
After discussion in msys2#59 it was discovered this could be
done without an output, and that the upstream author is not partial to
the output mechanism, so remove in favor of external solution.

This reverts commit 35ff6c7.
@jeremyd2019 jeremyd2019 deleted the output_msysRootDir branch November 14, 2020 01:26
@msink
Copy link

msink commented May 21, 2021

for anyone like me wanted it in environment variable:

      - name: Export MSYS2_ROOT
        run: echo "MSYS2_ROOT=$(msys2 -c 'cygpath -m /')" >> $env:GITHUB_ENV

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.

4 participants