[CASFS] Fix FileWithFixedStatus after CASFS refactoring #11235

[cachemeifyoucan]

Add CASFileWithFixedStatus to keep CAS references when fetching VFS overlayed file from CASBackedFileSystem.

Fixes: #11616

[cachemeifyoucan]

@swift-ci please test llvm

[jansvoboda11]

I think this should also fix FileManager::getObjectRefForFileContent().

It returns llvm::ErrorOr<std::optional<cas::ObjectRef>> and in IncludeTreeBuilder::addToFileList() we correctly handle the ErrorOr part, but then assert on std::optional. I'd argue that if the file exists but has no CAS object associated with it, we should return an error instead of std::nullopt. There's another user of that function that just blindly dereferences the optional, which is another crash waiting to happen.

Maybe the function should just return llvm::ErrorOr<cas::ObjectRef>? That's a semantic change, but I think it fits our existing use-cases.

[cachemeifyoucan]

Maybe the function should just return llvm::ErrorOr<cas::ObjectRef>? That's a semantic change, but I think it fits our existing use-cases.

I think that is a good idea. I am generally confident that it is an error in configuration/bug in code if we ask for ObjectRef but underlying file system is not configured correctly.

[cachemeifyoucan]

Yikes, there is a layering violation when building dynamic. Support->CAS->Support. Let me think about to fix that.

[cachemeifyoucan]

@swift-ci please test llvm

[cachemeifyoucan]

@swift-ci please test llvm

[jansvoboda11]

LGTM.

But I wonder how robust the dyn_cast<llvm::cas::CASBackedFile> in FileManager really is. What if the RedirectingFileSystem gets refactored and starts wrapping the underlying file in its own custom type? I don't have a good suggestion how to improve this, so let's not block the fix on that.

[cachemeifyoucan]

LGTM.

But I wonder how robust the dyn_cast<llvm::cas::CASBackedFile> in FileManager really is. What if the RedirectingFileSystem gets refactored and starts wrapping the underlying file in its own custom type? I don't have a good suggestion how to improve this, so let's not block the fix on that.

We need to make sure the configuration of file system when using CAS based compilation is configured correctly regardless. I will think if we have ways to make this better when we decide to upstream. My concern is mostly the current dependency that CAS -> Support so VFS can't have CAS in base implementation.

If you think the reverse problem we kind of wrapped all File under a CASBackedFile to make it have ObjectRef, we just don't want to wrap it multiple times because an intermediate layer lost the reference so we wasted time to insert into CAS again.