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

Fix cargo registry caching for builds #3613

Merged
merged 1 commit into from
Mar 17, 2020
Merged

Fix cargo registry caching for builds #3613

merged 1 commit into from
Mar 17, 2020

Conversation

bjorn3
Copy link
Member

@bjorn3 bjorn3 commented Mar 16, 2020

@bjorn3
Copy link
Member Author

bjorn3 commented Mar 16, 2020

Windows still has the caching issue. There is some work being done over at https://github.com/openethereum/openethereum/pull/11568 to fix this, but it doesn't work yet.

@matklad
Copy link
Member

matklad commented Mar 16, 2020

Hm, do we really need this?

Looking at https://github.com/rust-analyzer/rust-analyzer/runs/510919657?check_suite_focus=true#step:8:1, it seems like caching works as expected?

@bjorn3
Copy link
Member Author

bjorn3 commented Mar 16, 2020

@matklad
Copy link
Member

matklad commented Mar 16, 2020

Yeah... That probably means that we don't need to cache the registry at all? We cache compiled libraries anyway.

@bjorn3
Copy link
Member Author

bjorn3 commented Mar 16, 2020

Maybe

@bjorn3
Copy link
Member Author

bjorn3 commented Mar 16, 2020

It is useful when updating rustc, as in that case all crates need to be recompiled.

@matklad
Copy link
Member

matklad commented Mar 16, 2020

I think if we are recompiling the world, saving on network doesn't really make sense. I'd just remove this from caching then!

@matklad
Copy link
Member

matklad commented Mar 16, 2020

bors r+

Thanks!

Let's see how it works out...

@bjorn3
Copy link
Member Author

bjorn3 commented Mar 16, 2020

I think I should have kept the Cache cargo index step.

@matklad
Copy link
Member

matklad commented Mar 16, 2020

yeah, r+ doesn't mean anything anyway, this has to be merged manually...

@bjorn3
Copy link
Member Author

bjorn3 commented Mar 16, 2020

Forgot that gh apps can't merge workflow changes.

@bjorn3
Copy link
Member Author

bjorn3 commented Mar 16, 2020

Seems that the cargo registry cache is actually necessary: https://github.com/rust-analyzer/rust-analyzer/pull/3613/checks?check_run_id=511843473#step:7:12

@matklad matklad merged commit 96f19c3 into rust-lang:master Mar 17, 2020
@bjorn3 bjorn3 deleted the patch-1 branch March 17, 2020 10:15
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