Fall back to direct cache write if tempfile creation on the same fs fails

[myzhang1029]

Please see #2288 for the discussion.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 0% with 171 lines in your changes missing coverage. Please review.
✅ Project coverage is 27.84%. Comparing base (624ef6d) to head (0f0ed67).

Files with missing lines	Patch %	Lines
tests/system.rs	0.00%	98 Missing ⚠️
src/cache/cache.rs	0.00%	73 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (624ef6d) and HEAD (0f0ed67). Click for more details.

HEAD has 10 uploads less than BASE

Flag	BASE (624ef6d)	HEAD (0f0ed67)
	11	1

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2369       +/-   ##
===========================================
- Coverage   71.22%   27.84%   -43.38%     
===========================================
  Files          64       54       -10     
  Lines       35214    33666     -1548     
===========================================
- Hits        25080     9374    -15706     
- Misses      10134    24292    +14158     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[sylvestre]

would it be possible to add a test to make sure we don't regress in the future?
thanks

[myzhang1029]

@sylvestre
I am not familiar with a similar concept under Windows, so is it fine if I hardcode a test for something under /dev and make this test case cfg(unix)?

[myzhang1029]

Rebased on main and added two tests. Both tests succeed on the branch and fail before the patch.

I am not entirely sure if the use of /dev/fd is portable enough to all the unices that Rust and sccache intend to support. Please feel free to skip that one if not.

[sylvestre]

could you please add high level tests in https://github.com/mozilla/sccache/blob/main/tests/system.rs too ? thanks

like

sccache/tests/system.rs

Line 1825 in 7025295

fn test_sccache_command(preprocessor_cache_mode: bool) {

[myzhang1029]

@sylvestre Sorry for the delayed response. I am planning to test this by adding two functions that are similar to test_basic_compile in system.rs.

One of them should test /dev/null and the other for /dev/stdout (AssertCmd::assert seems to capture stdout already --- better approach maybe?) They would:

1. Compiles a file
2. Repeats the compilation command
3. Check that both commands succeed and that #miss and #hit are both 1.

[myzhang1029]

On a second thought, I have a question:

I think I named the PR a bit too narrow. Maybe like

Fall back to direct cache write if parent directory is not writable

I was think of reframing the PR this way, because this way, we don't have to hardcode /dev anymore. We can instead:

1. run the first compilation command
2. chmod u-w the destination dir
3. run the second (cached) pass

[myzhang1029]

@AJIOB I have pushed the change. Sorry for the delay.

[myzhang1029]

There is an issue with the tests: we use the same INPUT, so the cache stats depend on the previous tests. I feel like it would become a maintenance disaster to hardcode the expected stats into the tests in this case. Would it be acceptable to put a pair of start_local_daemon stop_local_daemon in each test?

[myzhang1029]

CI should work now.

I have relaxed one more error handling: in the case that same-fs file creation fails, we also allow chmod to fail.