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

[CI] enable cache for Unix jobs #5903

Closed

Conversation

Alizter
Copy link
Collaborator

@Alizter Alizter commented Jun 20, 2022

No description provided.

@Alizter Alizter force-pushed the ps/rr/_ci__enable_cache_for_unix_jobs branch 2 times, most recently from 9d089df to d0c632d Compare June 22, 2022 08:47
@Alizter Alizter closed this Jun 23, 2022
@Alizter Alizter reopened this Jul 13, 2022
@Alizter Alizter force-pushed the ps/rr/_ci__enable_cache_for_unix_jobs branch 4 times, most recently from 5f8e2ac to 5bfe33b Compare July 14, 2022 19:33
@emillon emillon force-pushed the ps/rr/_ci__enable_cache_for_unix_jobs branch 2 times, most recently from c1d2a2b to da16764 Compare July 27, 2022 09:47
@emillon
Copy link
Collaborator

emillon commented Jul 27, 2022

I tried to set DUNE_CACHE=disabled for the rest of the run but it was still causing problems. I think there's an issue with dune's test suite itself when the cache is enabled.

@Alizter Alizter force-pushed the ps/rr/_ci__enable_cache_for_unix_jobs branch from da16764 to f218c38 Compare September 6, 2022 20:11
@emillon emillon force-pushed the ps/rr/_ci__enable_cache_for_unix_jobs branch 2 times, most recently from 47e57c8 to e711513 Compare September 7, 2022 14:04
@emillon
Copy link
Collaborator

emillon commented Sep 7, 2022

@Alizter disabling the dune cache for tests through (env) seems to work (I'll submit that in a separate PR)` but coq is still not cached. The opam file for coq 8.16.0 still overrides COQ_USE_DUNE and so it continues to use the make-based build. Do you know why that's the case?

@Alizter
Copy link
Collaborator Author

Alizter commented Sep 7, 2022

@emillon The 8.16 makefile just calls dune under the hood so it shouldn't be an issue. We are generating the build rules ourselves since Dune is unable to generate and execute at the same time. AFAIK if DUNE_CACHE=enabled then it should work. Have a look at Makefile.dune.

@emillon
Copy link
Collaborator

emillon commented Sep 7, 2022

@emillon The 8.16 makefile just calls dune under the hood so it shouldn't be an issue. We are generating the build rules ourselves since Dune is unable to generate and execute at the same time. AFAIK if DUNE_CACHE=enabled then it should work. Have a look at Makefile.dune.

I understand that it's the intent, but this does not correspond to what is in the tarball that's being used by opam:

What confuses me is that in master, the Makefile just calls out to dune. But the version in the 8.16.0 release tarball does not have this so we're never going into Makefile.dune through opam.

Do you see what I mean? I don't know how the coq release process works in terms of branches and tags but the changes in the Makefile have not been propagated to the release tag.

@Alizter
Copy link
Collaborator Author

Alizter commented Sep 7, 2022

@emillon Sorry you are correct! It has been some time since I thought about it. I guess we have to wait one more version to take advantage of the Dune caching via opam.

The changes Emilio and I made switching everything to Dune is in coq/coq#15560 and that was tagged 8.17 rather than 8.16. The reason 8.16 released now is because the Summer got in the way.

@emillon emillon force-pushed the ps/rr/_ci__enable_cache_for_unix_jobs branch 2 times, most recently from 51804b1 to f83e618 Compare September 8, 2022 09:41
@emillon
Copy link
Collaborator

emillon commented Sep 12, 2022

Makes sense! so 8.17 should use the dune cache out of the box.

@emillon emillon force-pushed the ps/rr/_ci__enable_cache_for_unix_jobs branch 4 times, most recently from 6db69c4 to 9bd398b Compare September 12, 2022 14:55
Signed-off-by: Ali Caglayan <alizter@gmail.com>

ps-id: a9c15650-7c22-444a-a6cf-d8a6a56d830b
Signed-off-by: Ali Caglayan <alizter@gmail.com>
@Alizter Alizter force-pushed the ps/rr/_ci__enable_cache_for_unix_jobs branch from 9bd398b to 5c62118 Compare October 9, 2022 22:35
@Alizter Alizter closed this Oct 12, 2022
@Alizter Alizter deleted the ps/rr/_ci__enable_cache_for_unix_jobs branch October 12, 2022 22:45
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.

2 participants