Skip to content

Commit cb284f6

Browse files
[CAS] Improve CAS ObjectStore initialization
Make `getDefaultOnDiskCASPath` return error types as the function can fail (current fatal errors) on certain OS configurations. Also deprecate the API that uses DefaultOnDiskStableID.
1 parent 6a343d8 commit cb284f6

File tree

7 files changed

+34
-52
lines changed

7 files changed

+34
-52
lines changed

clang/tools/driver/cc1depscan_main.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -833,7 +833,12 @@ void ScanServer::start(bool Exclusive, ArrayRef<const char *> CASArgs) {
833833
return;
834834
SmallString<64> LLVMCasStorage;
835835
SmallString<64> CASPath;
836-
CASOpts.getResolvedCASPath(CASPath);
836+
if (auto Err = CASOpts.getResolvedCASPath(CASPath)) {
837+
// Failure to resolve a cas path for validation is ignored, because any
838+
// error will be reported when attempting to open the cas.
839+
llvm::consumeError(std::move(Err));
840+
return;
841+
}
837842
ExitOnErr(llvm::cas::validateOnDiskUnifiedCASDatabasesIfNeeded(
838843
CASPath, /*CheckHash=*/true,
839844
/*AllowRecovery=*/true,

llvm/include/llvm/CAS/CASConfiguration.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,6 @@ class CASConfiguration {
4949
return !(LHS == RHS);
5050
}
5151

52-
// Get resolved CASPath.
53-
void getResolvedCASPath(llvm::SmallVectorImpl<char> &Result) const;
54-
5552
// Create CASDatabase from the CASConfiguration.
5653
llvm::Expected<std::pair<std::shared_ptr<llvm::cas::ObjectStore>,
5754
std::shared_ptr<llvm::cas::ActionCache>>>
@@ -72,6 +69,9 @@ class CASConfiguration {
7269
createFromSearchConfigFile(
7370
StringRef Path,
7471
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS = nullptr);
72+
73+
/// Get resolved CASPath.
74+
Error getResolvedCASPath(llvm::SmallVectorImpl<char> &Result) const;
7575
};
7676

7777
} // namespace llvm::cas

llvm/include/llvm/CAS/ObjectStore.h

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -358,27 +358,15 @@ std::unique_ptr<ObjectStore> createInMemoryCAS();
358358
bool isOnDiskCASEnabled();
359359

360360
/// Gets or creates a persistent on-disk path at \p Path.
361-
///
362-
/// Deprecated: if \p Path resolves to \a getDefaultOnDiskCASStableID(),
363-
/// automatically opens \a getDefaultOnDiskCASPath() instead.
364-
///
365-
/// FIXME: Remove the special behaviour for getDefaultOnDiskCASStableID(). The
366-
/// client should handle this logic, if/when desired.
367361
Expected<std::unique_ptr<ObjectStore>> createOnDiskCAS(const Twine &Path);
368362

369363
/// Set \p Path to a reasonable default on-disk path for a persistent CAS for
370364
/// the current user.
371-
void getDefaultOnDiskCASPath(SmallVectorImpl<char> &Path);
365+
Error getDefaultOnDiskCASPath(SmallVectorImpl<char> &Path);
372366

373367
/// Get a reasonable default on-disk path for a persistent CAS for the current
374368
/// user.
375-
std::string getDefaultOnDiskCASPath();
376-
377-
/// FIXME: Remove.
378-
void getDefaultOnDiskCASStableID(SmallVectorImpl<char> &Path);
379-
380-
/// FIXME: Remove.
381-
std::string getDefaultOnDiskCASStableID();
369+
llvm::Expected<std::string> getDefaultOnDiskCASPath();
382370

383371
/// Create ObjectStore from a string identifier.
384372
/// Currently the string identifier is using URL scheme with following supported

llvm/lib/CAS/CASConfiguration.cpp

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,13 @@
1515
using namespace llvm;
1616
using namespace llvm::cas;
1717

18-
void CASConfiguration::getResolvedCASPath(
18+
Error CASConfiguration::getResolvedCASPath(
1919
llvm::SmallVectorImpl<char> &Result) const {
20-
if (CASPath == "auto") {
21-
getDefaultOnDiskCASPath(Result);
22-
} else {
23-
Result.assign(CASPath.begin(), CASPath.end());
24-
}
20+
if (CASPath == "auto")
21+
return getDefaultOnDiskCASPath(Result);
22+
23+
Result.assign(CASPath.begin(), CASPath.end());
24+
return Error::success();
2525
}
2626

2727
Expected<std::pair<std::shared_ptr<ObjectStore>, std::shared_ptr<ActionCache>>>
@@ -34,7 +34,8 @@ CASConfiguration::createDatabases() const {
3434
}
3535

3636
SmallString<128> PathBuf;
37-
getResolvedCASPath(PathBuf);
37+
if (auto E = getResolvedCASPath(PathBuf))
38+
return std::move(E);
3839

3940
std::pair<std::unique_ptr<ObjectStore>, std::unique_ptr<ActionCache>> DBs;
4041
return createOnDiskUnifiedCASDatabases(PathBuf);

llvm/lib/CAS/ObjectStore.cpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -336,8 +336,12 @@ createPluginCASImpl(const Twine &URL) {
336336
}
337337
}
338338

339-
if (OnDiskPath.empty())
340-
OnDiskPath = getDefaultOnDiskCASPath();
339+
if (OnDiskPath.empty()) {
340+
auto Path = getDefaultOnDiskCASPath();
341+
if (!Path)
342+
return Path.takeError();
343+
OnDiskPath = *Path;
344+
}
341345

342346
std::pair<std::shared_ptr<ObjectStore>, std::shared_ptr<ActionCache>> CASDBs;
343347
if (Error E = createPluginCASDatabases(PluginPath, OnDiskPath, PluginArgs)
@@ -372,7 +376,8 @@ cas::createCASFromIdentifier(StringRef Path) {
372376
// FIXME: some current default behavior.
373377
SmallString<256> PathBuf;
374378
if (Path == "auto") {
375-
getDefaultOnDiskCASPath(PathBuf);
379+
if (auto E = getDefaultOnDiskCASPath(PathBuf))
380+
return std::move(E);
376381
Path = PathBuf;
377382
}
378383

llvm/lib/CAS/OnDiskCAS.cpp

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -206,10 +206,6 @@ Expected<std::unique_ptr<ObjectStore>> cas::createOnDiskCAS(const Twine &Path) {
206206
Path.toVector(AbsPath);
207207
sys::fs::make_absolute(AbsPath);
208208

209-
// FIXME: Remove this and update clients to do this logic.
210-
if (AbsPath == getDefaultOnDiskCASStableID())
211-
AbsPath = StringRef(getDefaultOnDiskCASPath());
212-
213209
return OnDiskCAS::open(AbsPath);
214210
#else
215211
return createStringError(inconvertibleErrorCode(), "OnDiskCAS is disabled");
@@ -224,26 +220,16 @@ cas::builtin::createObjectStoreFromUnifiedOnDiskCache(
224220

225221
static constexpr StringLiteral DefaultName = "cas";
226222

227-
void cas::getDefaultOnDiskCASStableID(SmallVectorImpl<char> &Path) {
228-
Path.assign(DefaultDirProxy.begin(), DefaultDirProxy.end());
229-
llvm::sys::path::append(Path, DefaultDir, DefaultName);
230-
}
231-
232-
std::string cas::getDefaultOnDiskCASStableID() {
233-
SmallString<128> Path;
234-
getDefaultOnDiskCASStableID(Path);
235-
return Path.str().str();
236-
}
237-
238-
void cas::getDefaultOnDiskCASPath(SmallVectorImpl<char> &Path) {
239-
// FIXME: Should this return 'Error' instead of hard-failing?
223+
Error cas::getDefaultOnDiskCASPath(SmallVectorImpl<char> &Path) {
240224
if (!llvm::sys::path::cache_directory(Path))
241-
report_fatal_error("cannot get default cache directory");
225+
return createStringError("cache directory is not available");
242226
llvm::sys::path::append(Path, DefaultDir, DefaultName);
227+
return Error::success();
243228
}
244229

245-
std::string cas::getDefaultOnDiskCASPath() {
230+
Expected<std::string> cas::getDefaultOnDiskCASPath() {
246231
SmallString<128> Path;
247-
getDefaultOnDiskCASPath(Path);
232+
if (auto E = getDefaultOnDiskCASPath(Path))
233+
return std::move(E);
248234
return Path.str().str();
249235
}

llvm/tools/llc/llc.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -346,10 +346,7 @@ static std::unique_ptr<ToolOutputFile> GetOutputStream(const char *TargetName,
346346
static std::shared_ptr<cas::ObjectStore> getCAS() {
347347
if (CASPath.empty())
348348
return cas::createInMemoryCAS();
349-
auto MaybeCAS =
350-
CASPath == "auto"
351-
? cas::createCASFromIdentifier(cas::getDefaultOnDiskCASPath())
352-
: cas::createCASFromIdentifier(CASPath);
349+
auto MaybeCAS = cas::createCASFromIdentifier(CASPath);
353350
if (MaybeCAS)
354351
return std::move(*MaybeCAS);
355352
reportError(toString(MaybeCAS.takeError()));

0 commit comments

Comments
 (0)