--[Bugfix] - Storage of attribute source directory paths (#2541)

* --save filepath with trailing os separator * --Properly account for config directory ending in os sep ('/')
facebookresearch · Feb 3, 2025 · e6f0561 · e6f0561
1 parent acbe6f4
commit e6f0561
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 14 deletions.
diff --git a/src/esp/core/managedContainers/ManagedFileBasedContainer.h b/src/esp/core/managedContainers/ManagedFileBasedContainer.h
@@ -507,7 +507,7 @@ class ManagedFileBasedContainer : public ManagedContainer<T, Access> {
     std::string handleName = object->getHandle();
     auto loc = handleName.find_last_of('/');
     if (loc != std::string::npos) {
-      object->setFileDirectory(handleName.substr(0, loc));
+      object->setFileDirectory(handleName.substr(0, loc + 1));
     }
   }  // setFileDirectoryFromHandle
 

diff --git a/src/esp/metadata/managers/AbstractAttributesManager.h b/src/esp/metadata/managers/AbstractAttributesManager.h
@@ -491,7 +491,7 @@ void AbstractAttributesManager<T, Access>::buildAttrSrcPathsFromJSONAndLoad(
     const std::string& configDir,
     const std::string& extType,
     const io::JsonGenericValue& filePaths) {
-  std::size_t cfgLastDirLoc = configDir.find_last_of('/');
+  // configDir must end with an os sep (i.e. '/')
   for (rapidjson::SizeType i = 0; i < filePaths.Size(); ++i) {
     if (!filePaths[i].IsString()) {
       ESP_WARNING(Mn::Debug::Flag::NoSpace)
@@ -501,23 +501,16 @@ void AbstractAttributesManager<T, Access>::buildAttrSrcPathsFromJSONAndLoad(
       continue;
     }
     const char* fileString = filePaths[i].GetString();
-    // Only normalize paths for paths from config starting with ellipses.
-    // This is so that glob doesn't fail when the filepath from the config is
-    // trying to back-pedal across an OS link.
-    bool normalizePaths = (fileString[0] == '.') && (fileString[1] == '.') &&
-                          (fileString[2] == '/');
-
-    // TODO Eventually we should normalize all metadata paths in the system
-    std::string dsFilePath =
-        normalizePaths ? CrPath::join(configDir.substr(0, cfgLastDirLoc),
-                                      std::string(fileString).substr(3))
-                       : CrPath::join(configDir, fileString);
+    // The full filepath inferred by the given absolute config directory and the
+    // passed relative file paths
+    std::string dsFilePath = CrPath::join(configDir, fileString);
 
     ESP_VERY_VERBOSE(Mn::Debug::Flag::NoSpace)
         << "<" << this->objectType_ << "> : Config dir : " << configDir
-        << " : filePaths[" << i << "] : " << filePaths[i].GetString()
+        << " : filePaths[" << i << "] : " << fileString
         << " | Constructed File Path : " << dsFilePath;
 
+    // Search in derived full filepath
     std::vector<std::string> globPaths = io::globDirs(dsFilePath);
     if (globPaths.size() > 0) {
       for (const auto& globPath : globPaths) {