Enable ccache for Alpine builds on Travis CI

[PeterDaveHello]

Reimplement #918, only inject ccache related instructions on Travis CI build lifecycle.

[tianon]

I'm off for the holiday, just happened to take a look and see the oddity and figured I'd point it out. I can take a look Monday, but the security release is Tuesday isn't it? That's not a lot of time to get fixes here and in official images before it drops, so I think the revert would be simpler and safer so that can be revisited following the release.

[chorrell]

Unfortunately I think he's right. We should revert the two alpine/ccache commits and revisit in a new PR.

I'm not sure we have enough time to try and get #850 fixed and merged for next week, but that's probably more doable and something we want to happen before the security release. We also need to make the upstream PR for #921.

I'm not sure we want to have a security release include a switch from jessie to stretch, but at this point we have no choice I guess.

I'm not going to be available much today or tomorrow, still sick 🤧

[LaurentGoderre]

@tianon@chorrell@PeterDaveHello do you think this is sufficient for now?

#929

[chorrell]

I think it should be a full revert, not just commenting out the code in the Dockerfiles

[chorrell]

And it will have to be a brand new PR. I was not able to use the revert --> PR option in Github for both commits

[LaurentGoderre]

@chorrell full revert of just the Dockerfile? I think we can keep the Travis CI part no?

[chorrell]

I guess if it's the easier thing to do and it doesn't affect the images or build times, sure.

[PeterDaveHello]

@tianon@chorrell@LaurentGoderre @nodejs/docker please help review this PR, thanks.

[yosifkit]

I feel that this travis-only injection means that you won't even be testing the same thing that is submitted to official-images.

[PeterDaveHello]

@yosifkit you are right, I hope if those are only for ccache, would be okay?

[tianon]

I think what he's getting at is essentially, what's the point then? What problem is it solving if only done for Travis here?

[PeterDaveHello]

It's helping speed up the build time on Travis CI ;)

[tianon]

Sure, but to what end? For what purpose? What problem does that solve? Isn't the point of Travis to pre-verify changes here before submitting them? If that's not the point, then IMO you might as well just disable Travis because I don't see any other reason to use it in this way?

[PeterDaveHello]

For a long time and still now, we spend so much time on Travis CI, especially when we have multiple PRs or branches triggered the CI pipeline, which slows down how fast we can deliver the updates, that's a frequently complained issue from users. Travis CI still very every changes, just with additional ccache to help speed it up. It saves time, reduces builds pending, and helps release new versions and verify changes more quickly.

The ccache discussion started in #693, I didn't see any down vote there, #703 is also trying to use ccache another way, so I'm being confused here, is the problem here about ccache or the injection implementation?

[LaurentGoderre]

@tianon I think we ideally would want to solve it in your builds as well but that would be far more complicated. Reducing the time it takes to build those image is very worthwhile IMHO.

[PeterDaveHello]

@chorrell@SimenB would you like to leave some comments?

[LaurentGoderre]

This should not be needed anymore with the new unofficial build, in theory.

[chorrell]

The ccache or sccache optimization could be used in the build repo though https://github.com/nodejs/unofficial-builds

[chorrell]

Oh! Nevermind, it already is :)

https://github.com/nodejs/unofficial-builds/blob/master/recipes/musl/run.sh#L19

[LaurentGoderre]

I think we can close this one since we have the alpine build now