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

Disable sccache when building ICU for Android #756

Closed

Conversation

kendalharland
Copy link
Contributor

@kendalharland kendalharland commented Jul 1, 2024

These jobs are hitting a known fatal error in sccache where files aren't removed from the cache:

sccache: encountered fatal error
sccache: error: thread 'tokio-runtime-worker' panicked at src\lru_disk_cache\mod.rs:204:17: Error removing file from cache: `"C:\\a\\swift-build\\swift-build/.sccache\\preprocessor\\8\\f\\2\\8f2e7d1b9af7ff2cc23c6829fc3cae3044c631d88bf597901be5e04795779aad"`: failed to remove file `C:\a\swift-build\swift-build/.sccache\preprocessor\8\f\2\8f2e7d1b9af7ff2cc23c6829fc3cae3044c631d88bf597901be5e04795779aad`
sccache: caused by: thread 'tokio-runtime-worker' panicked at src\lru_disk_cache\mod.rs:204:17: Error removing file from cache: `"C:\\a\\swift-build\\swift-build/.sccache\\preprocessor\\8\\f\\2\\8f2e7d1b9af7ff2cc23c6829fc3cae3044c631d88bf597901be5e04795779aad"`: failed to remove file `C:\a\swift-build\swift-build/.sccache\preprocessor\8\f\2\8f2e7d1b9af7ff2cc23c6829fc3cae3044c631d88bf597901be5e04795779aad`

There's an open issue about this at mozilla/sccache#2092.

Test

https://github.com/thebrowsercompany/swift-build/actions/runs/9749014927

@kendalharland kendalharland force-pushed the kendal/disable-icu-sccache branch from 00341c0 to e80892f Compare July 1, 2024 18:11
Copy link
Owner

@compnerd compnerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please inline the error message in the commit message so that we can follow what is going on, the logs are inaccessible due to the corporate security and broken Arc.

@kendalharland
Copy link
Contributor Author

Please inline the error message in the commit message so that we can follow what is going on, the logs are inaccessible due to the corporate security and broken Arc.

Done

@kendalharland kendalharland requested a review from compnerd July 1, 2024 20:32
@compnerd
Copy link
Owner

compnerd commented Jul 1, 2024

Hmm, its unclear if this is really the right approach - it just seems like you have a corrupted cache? What happens if you clear out the cache?

@kendalharland
Copy link
Contributor Author

kendalharland commented Jul 1, 2024

Hmm, its unclear if this is really the right approach - it just seems like you have a corrupted cache? What happens if you clear out the cache?

I don't think clearing the cache (before the CMake configure or build step) will fix things, given that this is flaking on both GitHub and Azure images where each Android ICU job executes in a fresh VM instance. But I could be missing something. It seems like a bug in sccache that surfaces with higher probably when building for Android, so disabling it just unbreaks the build for the time being.

That being said, how does one clear the cache used by sccache? Where in the workflow could we do this if we wanted to workaround the error when building ICU?

@compnerd
Copy link
Owner

compnerd commented Jul 1, 2024

The way that sccache works is that the content is cached, even on a fresh VM image. You should look at the caches for the job, clearing out all the old runs should hopefully fix the issue.

@kendalharland
Copy link
Contributor Author

Ah, I see what you mean. I just deleted each of the android ICU caches and will see if that works.

@kendalharland
Copy link
Contributor Author

That did it, thanks for the tip!

@kendalharland
Copy link
Contributor Author

Reopening because the sccache corruption comes back each time we attempt to target Android when building ICU, and it's not immediately clear what is causing this. We'll need to disable this while we debug.

@kendalharland kendalharland force-pushed the kendal/disable-icu-sccache branch from e80892f to 005ec27 Compare July 8, 2024 16:38
@kendalharland
Copy link
Contributor Author

I'll need @compnerd to merge since I don't have write access

Copy link
Owner

@compnerd compnerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do think that taking a closer look to the change is valuable - I'm concerned that this will remain disabled indefinitely.

@@ -590,10 +593,10 @@ jobs:
-D BUILD_DATA=${{ matrix.BUILD_DATA }} `
-D CMAKE_BUILD_TYPE=Release `
-D CMAKE_C_COMPILER=${{ matrix.cc }} `
-D CMAKE_C_COMPILER_LAUNCHER=sccache `
-D CMAKE_C_COMPILER_LAUNCHER=${{ matrix.os == 'Windows' && 'sccache' || '' }} `
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this change needed with the condition above?

@kendalharland
Copy link
Contributor Author

kendalharland commented Jul 9, 2024

This is not needed after #757. The reason for the cache misses is that the cache size was too small to begin with, so the first cache we create when building Android for ICU does not contain all of the expected entries causing sccache to subsequently fail when purging existing entries to make space. It's unclear why sccache fails to purge files with 'failed to remove file from cache' errors, but this is no longer blocking.

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.

3 participants