[feature] Make cygwin-packages dir configurable

[sebthom]

Currently it is hardcoded to C:\cygwin-packages

[jon-turney]

Can you explain briefly the uses for such a feature?

[sebthom]

I want to do:

jobs:
  build:
    runs-on: windows-latest
    steps:     
    - name: "Cache: Cywgin Repository"
      uses: actions/cache@v3
      with:
        path: "${{ runner.tool_cache }}/cygwin-packages"
        key: ${{ runner.os }}-cygwin-${{ github.run_id }}
        restore-keys: |
          ${{ runner.os }}-cygwin-

    - uses: cygwin/cygwin-install-action@master
       with:
         packages-dir: ${{ runner.tool_cache }}/cygwin-packages

Also I don't see why the install dir can be customized but the packages cache dir is hardcoded.

[me-and]

What's the need to have the cygwin-packages directory under $runner.tool_cache?

If you just want to use caching of the package directory, that's something I'm working on having built into this action over at #6; that's hit some snags with recent versions of the caching code (see actions/cache#1073) but I'm hoping to have a PR I'm happy with and that works around all the issues within the next few days.

If you don't want to wait for that, I've been caching the package directory as part of my builds with the action storing the package directory in its current location. You can see how I've done that at https://github.com/cygporter/workflows/blob/58618f37c070847efafc8ead9712c9294012d5f8/.github/workflows/prep-release.yml#L51-L96.

[sebthom]

The path C:\cygwin-packages is implementation internal knowledge and theoretically can change any moment. So I rather prefer to be specific and define the package location.

Even if you don't decide to make it configurable. You should not install cygwin or cygwin-packages directly under C: but put it in the runner.tool_cache because that is the designated caching place for tools.

[me-and]

The path C:\cygwin-packages is implementation internal knowledge and theoretically can change any moment. So I rather prefer to be specific and define the package location.

Yes, this makes sense to me. Of course, I can't imagine it would change without changing the version of the action, but I understand the argument. @jon-turney definitely gets to make the calls here, but you could always submit a PR to add this function.

Even if you don't decide to make it configurable. You should not install cygwin or cygwin-packages directly under C: but put it in the runner.tool_cache because that is the designated caching place for tools.

I disagree with this. The GitHub contexts documentation says $runner.tool_cache is "The path to the directory containing preinstalled tools for GitHub-hosted runners". Cygwin isn't a preinstalled tool – the whole point of this action existing is that you have to install it! – so I really don't think it's appropriate for Cygwin to put itself there.

[sebthom]

Yes, this makes sense to me. Of course, I can't imagine it would change without changing the version of the action, but I understand the argument. @jon-turney definitely gets to make the calls here, but you could always submit a PR to add this function.

Yes I can do that but would want to wait for @jon-turney final decision.

Even if you don't decide to make it configurable. You should not install cygwin or cygwin-packages directly under C: but put it in the runner.tool_cache because that is the designated caching place for tools.

I disagree with this. The GitHub contexts documentation says $runner.tool_cache is "The path to the directory containing preinstalled tools for GitHub-hosted runners". Cygwin isn't a preinstalled tool – the whole point of this action existing is that you have to install it! – so I really don't think it's appropriate for Cygwin to put itself there.

When looking at setup-python/setup-java/setup-node actions they all install to $runner.tool_cache. So maybe yes, $runner.tool_cache is prepopulated but certainly not used as a readonly location. So it would make perfect sense for that cygwin also installs itself there. But anyways, that is not my current issue :-)

[me-and]

Even if you don't decide to make it configurable. You should not install cygwin or cygwin-packages directly under C: but put it in the runner.tool_cache because that is the designated caching place for tools.

I disagree with this. The GitHub contexts documentation says $runner.tool_cache is "The path to the directory containing preinstalled tools for GitHub-hosted runners". Cygwin isn't a preinstalled tool – the whole point of this action existing is that you have to install it! – so I really don't think it's appropriate for Cygwin to put itself there.

When looking at setup-python/setup-java/setup-node actions they all install to $runner.tool_cache. So maybe yes, $runner.tool_cache is prepopulated but certainly not used as a readonly location. So it would make perfect sense for that cygwin also installs itself there. But anyways, that is not my current issue :-)

Huh. So they do! I think the GitHub documentation could benefit from being improved in that case, and might go submit a PR to that end myself…

[me-and]

Raised for now at github/docs#23338; I've raised an issue rather than a PR for now, as I'm not quite sure what the intent of the value is, so I don't know what the correct solution to document is…

[jon-turney]

The path C:\cygwin-packages is implementation internal knowledge and theoretically can change any moment. So I rather prefer to be specific and define the package location.

Yes, it's an internal detail.

It's obviously much more beneficial if we can do caching within this action itself, rather than making every user of this action cobble together their own way of doing it.

I'm not aware of any other reasons for exposing this detail, so right now it would seem better to keep it private, rather than making it configurable.

[mgood7123]

should this be closed ?

[sebthom]

@mgood7123 no, still not solved.