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

Only activate direct mode by default for local #1973

Merged
merged 1 commit into from
Nov 23, 2023

Conversation

Alphare
Copy link
Contributor

@Alphare Alphare commented Nov 20, 2023

4538675 reverted my initial change.

This re-enables direct mode by default, making sure only the disk cache is affected. There may still be an issue with Windows, but details are sparse and no reproduction has been found yet.

@sylvestre

@glandium
Copy link
Collaborator

Rustfmt is not happy

@glandium
Copy link
Collaborator

For the record, I can confirm that this solves the issue of direct mode affecting non-local builds. This, however doesn't fix the "getting stuck on Windows" part.

I'm not sure, though, why SCCACHE_DIRECT=false wasn't fixing the issue... initialization order?

@Alphare Alphare force-pushed the direct-mode.bad-default branch from e56b338 to c3d3b44 Compare November 21, 2023 08:45
@Alphare
Copy link
Contributor Author

Alphare commented Nov 21, 2023

For the record, I can confirm that this solves the issue of direct mode affecting non-local builds. This, however doesn't fix the "getting stuck on Windows" part.

It will probably be a bit more involved of a bug hunt. I have not yet started to look at it, my end of November is going to be very full.

I'm not sure, though, why SCCACHE_DIRECT=false wasn't fixing the issue... initialization order?

Do you mean it wasn't disabling it for local config? Or for all configs? Because if the former, we may have another bug, but if the latter, this commit also fixed this behavior. Basically the config was set to true for all other Storage impls no matter the config... oops!

@glandium
Copy link
Collaborator

Do you mean it wasn't disabling it for local config? Or for all configs?

For non-local configs. I hadn't tried for local configs. But the builds that were hitting the "gets stuck" problem on Windows despite the non-local use weren't fixed by setting SCCACHE_DIRECT=false.

As for the "gets stuck" problem itself, I am able to reproduce it... but not locally, only on Firefox CI, which is not going to be super helpful to debug. I'll try a few things, though.

@glandium
Copy link
Collaborator

Note the tests are failing with this patch.

@Alphare Alphare force-pushed the direct-mode.bad-default branch 2 times, most recently from 6210ea2 to 53539b5 Compare November 22, 2023 08:57
4538675 reverted my initial change.

This re-enables direct mode by default, making sure only the disk
cache is affected. There may still be an issue with Windows, but
details are sparse and no reproduction has been found yet.
@Alphare
Copy link
Contributor Author

Alphare commented Nov 22, 2023

After fixing the previous failure, it looks like https://github.com/mozilla/sccache/actions/runs/6954981313/job/18922864423?pr=1973 fails, but I'm not convinced it's got anything to do with my patch.

@glandium
Copy link
Collaborator

glandium commented Nov 22, 2023

After fixing the previous failure, it looks like https://github.com/mozilla/sccache/actions/runs/6954981313/job/18922864423?pr=1973 fails, but I'm not convinced it's got anything to do with my patch.

It's an existing race condition: filed #1982.

@sylvestre sylvestre merged commit aa20391 into mozilla:main Nov 23, 2023
39 checks passed
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