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

Containerized Alpine support #131

Closed
mvorisek opened this issue Feb 3, 2023 · 8 comments · Fixed by #145
Closed

Containerized Alpine support #131

mvorisek opened this issue Feb 3, 2023 · 8 comments · Fixed by #145

Comments

@mvorisek
Copy link
Contributor

mvorisek commented Feb 3, 2023

Please add support for Alpine, currently, apt is tried to be used to install ccache but Alpine needs ccache.

CI repro: https://github.com/php/php-src/actions/runs/4083551771/jobs/7039218638#step:6:18

Example GH workflow:

    runs-on: ubuntu-20.04
    container:
      image: alpine:3.15
    steps:
      - name: git checkout
        uses: actions/checkout@v3
      - name: ccache
        uses: hendrikmuhs/ccache-action@v1.2
@vadz
Copy link
Contributor

vadz commented Mar 9, 2023

FWIW I just install ccache myself before using this action as a workaround for now: if ccache is already installed, it's not going to try installing it and so works even in non-Debian environments.

@mvorisek
Copy link
Contributor Author

Thank you for your comment. The workaround is working, however the issue should be fixed officially.

@hendrikmuhs
Copy link
Owner

however the issue should be fixed officially.

This is a small opensource side-project, the best way to approach this is a PR.

@vadz
Copy link
Contributor

vadz commented Mar 10, 2023

however the issue should be fixed officially.

This is a small opensource side-project, the best way to approach this is a PR.

I thought about doing it (which is why I actually searched for container-related issues and found this one), but then decided that it probably wasn't a good idea to try installing ccache under all platforms, there are just too many of them, so installing ccache before using the action if you don't use a supported one looks like a reasonable solution.

However the action could at least check if it's not running under a Debian system and give a clear error in this case instead of just failing trying to run apt-get. Would you like to have a PR doing this?

@mvorisek
Copy link
Contributor Author

Debian/Ubuntu apt and Alpine apk package managers are very common, thus both should be supported out of the box.

@vadz
Copy link
Contributor

vadz commented Mar 10, 2023

It's not up to me to decide this, but supporting apt is natural because this is what is used in GitHub Actions environment. Starting to support anything else opens the door to supporting all package managers (are you sure apk is really more popular than dnf or pacman or ...) and so seems unwise.

@hendrikmuhs
Copy link
Owner

The check sounds like a good idea. It is preferable to fail fast with a clear error message.

Regarding support for other os's: I agree that 100% coverage isn't possible, but I am also not against it. I happily merge a PR that adds support.

vadz added a commit to vadz/ccache-action that referenced this issue Mar 11, 2023
This is not as nice as installing it ourselves, but this isn't that easy
to do in general, while giving a message explaining what needs to be
done is pretty simple, so do at least this.

See hendrikmuhs#131.
hendrikmuhs pushed a commit that referenced this issue Apr 14, 2023
This is not as nice as installing it ourselves, but this isn't that easy
to do in general, while giving a message explaining what needs to be
done is pretty simple, so do at least this.

See #131.
@lukasz-mitka
Copy link

I'd suggest skipping package managers altogether and using pre-build binaries from releases https://github.com/ccache/ccache/releases and https://github.com/mozilla/sccache/releases where possible.

This way it would also enable selecting particular version of the package.

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 a pull request may close this issue.

4 participants