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

depends cache is not invalidated when the compiler changes #17248

Closed
maflcko opened this issue Oct 25, 2019 · 9 comments
Closed

depends cache is not invalidated when the compiler changes #17248

maflcko opened this issue Oct 25, 2019 · 9 comments

Comments

@maflcko
Copy link
Member

maflcko commented Oct 25, 2019

The depends cache has no mechanism of being invalidated when the cpu architecture, the operating system, or even just the compiler changes from down under it.

I think long term we should be moving to proper compiler caches like ccache (#17103) or guix

@maflcko
Copy link
Member Author

maflcko commented Oct 25, 2019

For example, just one of the bugs I ran into due to this: #17233 (comment)

@maflcko
Copy link
Member Author

maflcko commented Oct 25, 2019

@dongcarl @theuni Any thoughts or objections?

@elichai
Copy link
Contributor

elichai commented Dec 5, 2019

How much time penalty we get by removing these two lines? https://github.com/bitcoin/bitcoin/blob/master/.travis.yml#L35:L36

We could also mount the ccache dir to the dockers and install+run ccache in the dockers.
that way we only cache the ccache dir

@laanwj
Copy link
Member

laanwj commented Dec 5, 2019

How much time penalty we get by removing these two lines? https://github.com/bitcoin/bitcoin/blob/master/.travis.yml#L35:L36

What would that do?
If it causes a depends rebuild on every travis run, especially with the runs with Qt included, you're looking at hour(s).

@elichai
Copy link
Contributor

elichai commented Dec 5, 2019

How much time penalty we get by removing these two lines? https://github.com/bitcoin/bitcoin/blob/master/.travis.yml#L35:L36

What would that do?
If it causes a depends rebuild on every travis run, especially with the runs with Qt included, you're looking at hour(s).

even with ccache?

@laanwj
Copy link
Member

laanwj commented Dec 5, 2019

I don't think ccache really helps that much with building qt. Mind that the qt build first builds a build system (qmake) for the host, then uses that to build the library for the target.
Boost has a similar weird build setup.
Even getting it to use ccache consistently will be a challenge, and after that you're still left with enough overhead.

maflcko pushed a commit that referenced this issue Apr 16, 2020
0c63187 ci: Limit cache size regardless of NO_DEPENDS (Hennadii Stepanov)

Pull request description:

  Close #18666.

ACKs for top commit:
  MarcoFalke:
    ACK 0c63187 . Depends has ccache disabled anyway and is cached regardless of whether ccache is there or not, see #17248

Tree-SHA512: b1bf98be0f844b4704abd177841b014f3900be8160496f0d12596310db607b4f544547e8c3cbfcf17c086a78afd251653363f3dd467b769ac0062bc19adc8144
sidhujag pushed a commit to syscoin/syscoin that referenced this issue Apr 17, 2020
0c63187 ci: Limit cache size regardless of NO_DEPENDS (Hennadii Stepanov)

Pull request description:

  Close bitcoin#18666.

ACKs for top commit:
  MarcoFalke:
    ACK 0c63187 . Depends has ccache disabled anyway and is cached regardless of whether ccache is there or not, see bitcoin#17248

Tree-SHA512: b1bf98be0f844b4704abd177841b014f3900be8160496f0d12596310db607b4f544547e8c3cbfcf17c086a78afd251653363f3dd467b769ac0062bc19adc8144
@maflcko
Copy link
Member Author

maflcko commented May 30, 2020

FYI. On the DrahtBot guix build, I am caching depends under a key that is equal to the hash of the whole guix tree. The same can probably not be done for depends and only to a lesser extent to gitian.

@dongcarl
Copy link
Contributor

Following up on this after the merge of #20629

For Guix builds

5200929 ensures that the GUIX_ENVIRONMENT env var is inclueded in the id string used for cache invalidation. This means that any change in the definition of our Guix packages will appropriately invalidate the depends cache

For non-Guix builds

4c7d418 adds a lot of toolchain information to the id string in order to trigger cache invalidation should any of it changes. Not as perfect as the situation with Guix, but I believe it's a good enough approximation for most use-cases.

@maflcko
Copy link
Member Author

maflcko commented Feb 17, 2021

kewl, thx. Closing for now

@maflcko maflcko closed this as completed Feb 17, 2021
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants