From cbc113650240d83c84e5f832b4d088e52baae658 Mon Sep 17 00:00:00 2001 From: Google DeepMind Date: Wed, 14 Jan 2026 10:37:31 -0800 Subject: [PATCH] Roll back VFS refactor due internal breakages PiperOrigin-RevId: 856273160 Change-Id: I4097e05ed4e48ebee05b4347d0f227d4f39c1b09 --- doc/APIreference/functions.rst | 19 - doc/changelog.rst | 2 - doc/includes/references.h | 5 - include/mujoco/mjplugin.h | 9 - include/mujoco/mujoco.h | 7 - python/mujoco/introspect/functions.py | 46 -- src/user/user_resource.cc | 222 +++++++-- src/user/user_vfs.cc | 638 ++++++++------------------ src/user/user_vfs.h | 121 +---- test/user/user_resource_test.cc | 2 +- test/user/user_vfs_test.cc | 162 ------- test/xml/xml_api_test.cc | 2 +- 12 files changed, 412 insertions(+), 823 deletions(-) diff --git a/doc/APIreference/functions.rst b/doc/APIreference/functions.rst index 62db7ea4..8ac117b3 100644 --- a/doc/APIreference/functions.rst +++ b/doc/APIreference/functions.rst @@ -1357,25 +1357,6 @@ The VFS must first be allocated using :ref:`mj_defaultVFS` and must be freed wit Initialize an empty VFS, :ref:`mj_deleteVFS` must be called to deallocate the VFS. -.. _mj_mountVFS: - -`mj_mountVFS <#mj_mountVFS>`__ -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -.. mujoco-include:: mj_mountVFS - -Mount a ResourceProvider to handle file operations under the given path; return 0: success, -2: repeated name, -1: invalid resource provider. - -.. _mj_unmountVFS: - -`mj_unmountVFS <#mj_unmountVFS>`__ -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -.. mujoco-include:: mj_unmountVFS - -Unmount a previously mounted ResourceProvider; return 0: success, -1: not found in VFS. - .. _mj_addFileVFS: `mj_addFileVFS <#mj_addFileVFS>`__ diff --git a/doc/changelog.rst b/doc/changelog.rst index 8baf9867..e809f83e 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -45,8 +45,6 @@ General See :ref:`rangefinder` for details. - Cameras now have an :ref:`output` attribute, parsed into the ``mjModel.cam_output`` bitfield. Unused by the renderer, it serves as a convenient location to store a camera's supported output types. -- Added :ref:`mj_mountVFS` and :ref:`mj_unmountVFS` functions for mounting a custom VFS provider. Mounting Allows - providers to be used to open/read/close resources dynamically at arbitrary paths. - Non-breaking ABI changes: - The type of the ``sig`` (signature) argument of :ref:`mj_stateSize` and related functions has been changed from diff --git a/doc/includes/references.h b/doc/includes/references.h index c58a42d9..98bed114 100644 --- a/doc/includes/references.h +++ b/doc/includes/references.h @@ -1619,7 +1619,6 @@ typedef struct mjModel_ mjModel; struct mjResource_ { char* name; // name of resource (filename, etc) void* data; // opaque data pointer - mjVFS* vfs; // pointer to the VFS char timestamp[512]; // timestamp of the resource const struct mjpResourceProvider* provider; // pointer to the provider }; @@ -1629,8 +1628,6 @@ struct mjpResourceProvider { mjfOpenResource open; // opening callback mjfReadResource read; // reading callback mjfCloseResource close; // closing callback - mjfMountResource mount; // mounting callback (optional) - mjfUnmountResource unmount; // unmounting callback (optional) mjfResourceModified modified; // resource modified callback (optional) void* data; // opaque data pointer (resource invariant) }; @@ -3087,8 +3084,6 @@ typedef struct mjvFigure_ mjvFigure; //----------------------------- MJAPI FUNCTIONS -------------------------------- void mj_defaultVFS(mjVFS* vfs); -int mj_mountVFS(mjVFS* vfs, const char* filepath, const mjpResourceProvider* provider); -int mj_unmountVFS(mjVFS* vfs, const char* filename); int mj_addFileVFS(mjVFS* vfs, const char* directory, const char* filename); int mj_addBufferVFS(mjVFS* vfs, const char* name, const void* buffer, int nbuffer); int mj_deleteFileVFS(mjVFS* vfs, const char* filename); diff --git a/include/mujoco/mjplugin.h b/include/mujoco/mjplugin.h index 93268618..880da990 100644 --- a/include/mujoco/mjplugin.h +++ b/include/mujoco/mjplugin.h @@ -27,7 +27,6 @@ struct mjResource_ { char* name; // name of resource (filename, etc) void* data; // opaque data pointer - mjVFS* vfs; // pointer to the VFS char timestamp[512]; // timestamp of the resource const struct mjpResourceProvider* provider; // pointer to the provider }; @@ -43,12 +42,6 @@ typedef int (*mjfReadResource)(mjResource* resource, const void** buffer); // callback for closing a resource (responsible for freeing any allocated memory) typedef void (*mjfCloseResource)(mjResource* resource); -// callback for mounting a resource (provider), returns zero on failure -typedef int (*mjfMountResource)(mjResource* resource); - -// callback for unmounting a resource (provider), returns zero on failure -typedef int (*mjfUnmountResource)(mjResource* resource); - // callback for checking if the current resource was modified from the time // specified by the timestamp // returns 0 if the resource's timestamp matches the provided timestamp @@ -62,8 +55,6 @@ struct mjpResourceProvider { mjfOpenResource open; // opening callback mjfReadResource read; // reading callback mjfCloseResource close; // closing callback - mjfMountResource mount; // mounting callback (optional) - mjfUnmountResource unmount; // unmounting callback (optional) mjfResourceModified modified; // resource modified callback (optional) void* data; // opaque data pointer (resource invariant) }; diff --git a/include/mujoco/mujoco.h b/include/mujoco/mujoco.h index 197d42f1..b42d7891 100644 --- a/include/mujoco/mujoco.h +++ b/include/mujoco/mujoco.h @@ -78,13 +78,6 @@ MJAPI extern const char* mjRNDSTRING[mjNRNDFLAG][3]; // Initialize an empty VFS, mj_deleteVFS must be called to deallocate the VFS. MJAPI void mj_defaultVFS(mjVFS* vfs); -// Mount a ResourceProvider to handle file operations under the given path; return 0: success, -// 2: repeated name, -1: invalid resource provider. -MJAPI int mj_mountVFS(mjVFS* vfs, const char* filepath, const mjpResourceProvider* provider); - -// Unmount a previously mounted ResourceProvider; return 0: success, -1: not found in VFS. -MJAPI int mj_unmountVFS(mjVFS* vfs, const char* filename); - // Add file to VFS; return 0: success, 2: repeated name, -1: failed to load. MJAPI int mj_addFileVFS(mjVFS* vfs, const char* directory, const char* filename); diff --git a/python/mujoco/introspect/functions.py b/python/mujoco/introspect/functions.py index 33f48e46..072a63e3 100644 --- a/python/mujoco/introspect/functions.py +++ b/python/mujoco/introspect/functions.py @@ -40,52 +40,6 @@ FUNCTIONS: Mapping[str, FunctionDecl] = dict([ ), doc='Initialize an empty VFS, mj_deleteVFS must be called to deallocate the VFS.', # pylint: disable=line-too-long )), - ('mj_mountVFS', - FunctionDecl( - name='mj_mountVFS', - return_type=ValueType(name='int'), - parameters=( - FunctionParameterDecl( - name='vfs', - type=PointerType( - inner_type=ValueType(name='mjVFS'), - ), - ), - FunctionParameterDecl( - name='filepath', - type=PointerType( - inner_type=ValueType(name='char', is_const=True), - ), - ), - FunctionParameterDecl( - name='provider', - type=PointerType( - inner_type=ValueType(name='mjpResourceProvider', is_const=True), # pylint: disable=line-too-long - ), - ), - ), - doc='Mount a ResourceProvider to handle file operations under the given path; return 0: success, 2: repeated name, -1: invalid resource provider.', # pylint: disable=line-too-long - )), - ('mj_unmountVFS', - FunctionDecl( - name='mj_unmountVFS', - return_type=ValueType(name='int'), - parameters=( - FunctionParameterDecl( - name='vfs', - type=PointerType( - inner_type=ValueType(name='mjVFS'), - ), - ), - FunctionParameterDecl( - name='filename', - type=PointerType( - inner_type=ValueType(name='char', is_const=True), - ), - ), - ), - doc='Unmount a previously mounted ResourceProvider; return 0: success, -1: not found in VFS.', # pylint: disable=line-too-long - )), ('mj_addFileVFS', FunctionDecl( name='mj_addFileVFS', diff --git a/src/user/user_resource.cc b/src/user/user_resource.cc index 378daa0f..a1482b44 100644 --- a/src/user/user_resource.cc +++ b/src/user/user_resource.cc @@ -14,71 +14,216 @@ #include "user/user_resource.h" +#include #include #include #include +#include #include #include #include #include #include +#include #include +#if defined (__unix__) || (defined (__APPLE__) && defined (__MACH__)) + #include +#endif + +#ifdef _WIN32 + #define stat _stat +#endif + #include #include "engine/engine_plugin.h" +#include "engine/engine_util_misc.h" #include "user/user_util.h" #include "user/user_vfs.h" -mjResource* mju_openResource(const char* dir, const char* name, - const mjVFS* vfs, char* error, size_t nerror) { - // TODO: Update API to use non-const pointer. Unfortunately, while this is - // ABI stable, it will cause compiler errors in user code that is const - // correct. - mjVFS* non_const_vfs = const_cast(vfs); +namespace { - // TODO: Eventually, we should make `vfs` a required argument. In the - // meantime, when passing in a nullptr VFS, we will create a VFS dynamically - // that self-destructs when the resource is closed (or if the resource could - // not be opened). - if (non_const_vfs == nullptr) { - mjVFS* local_vfs = (mjVFS*)mju_malloc(sizeof(mjVFS)); - mj_defaultVFS(local_vfs); - mujoco::user::VFS::Upcast(local_vfs)->SetToSelfDestruct([](mjVFS* ptr) { - mj_deleteVFS(ptr); - mju_free(ptr); - }); +using mujoco::user::FileToMemory; - non_const_vfs = local_vfs; +// file buffer used internally for the OS filesystem +struct FileSpec { + bool is_read; // set to nonzero if buffer was read into + std::vector buffer; // raw bytes from file + time_t mtime; // last modified time +}; + +// callback for opening a resource, returns zero on failure +int FileOpen(mjResource* resource, char* error, size_t nerror) { + resource->provider = nullptr; + resource->data = new FileSpec; + const char* name = resource->name; + FileSpec* spec = (FileSpec*) resource->data; + spec->is_read = false; + struct stat file_stat; + if (stat(name, &file_stat) == 0) { + memcpy(&spec->mtime, &file_stat.st_mtime, sizeof(time_t)); + } else { + if (error) { + snprintf(error, nerror, "Error opening file '%s': %s", name, + strerror(errno)); + } + return 0; + } + mju_encodeBase64(resource->timestamp, (uint8_t*) &spec->mtime, + sizeof(time_t)); + return 1; +} + +// OS filesystem read callback +int FileRead(mjResource* resource, const void** buffer) { + FileSpec* spec = (FileSpec*) resource->data; + + // only read once from file + if (!spec->is_read) { + spec->buffer = FileToMemory(resource->name); + spec->is_read = true; + } + *buffer = spec->buffer.data(); + return spec->buffer.size(); +} + +// OS filesystem close callback +void FileClose(mjResource* resource) { + FileSpec* spec = (FileSpec*) resource->data; + if (spec) delete spec; +} + +// OS filesystem modified callback +int FileModified(const mjResource* resource, const char*timestamp) { + if (mju_isValidBase64(timestamp) != sizeof(time_t)) { + return 1; // error (assume modified) } - mjResource* resource = - mujoco::user::VFS::Upcast(non_const_vfs)->Open(dir ? dir : "", name); + time_t time1, time2; + mju_decodeBase64((uint8_t*) &time1, timestamp); + time2 = ((FileSpec*) resource->data)->mtime; + double diff = difftime(time2, time1); + if (diff < 0) return -1; + if (diff > 0) return 1; + return 0; +} +} // namespace + +// open the given resource; if the name doesn't have a prefix matching with a +// resource provider, then the OS filesystem is used +mjResource* mju_openResource(const char* dir, const char* name, + const mjVFS* vfs, char* error, size_t nerror) { + // no error so far if (error) { - if (resource) { - error[0] = '\0'; - } else { - std::snprintf(error, nerror, "Error opening file '%s'", name); + error[0] = '\0'; + } + + mjResource* resource = (mjResource*) mju_malloc(sizeof(mjResource)); + if (resource == nullptr) { + if (error) { + strncpy(error, "could not allocate memory", nerror); + error[nerror - 1] = '\0'; + } + return nullptr; + } + + // clear out resource + memset(resource, 0, sizeof(mjResource)); + + // make space for filename + std::string fullname = mjuu_combinePaths(dir, name); + std::size_t n = fullname.size(); + resource->name = (char*) mju_malloc(sizeof(char) * (n + 1)); + if (resource->name == nullptr) { + if (error) { + strncpy(error, "could not allocate memory", nerror); + error[nerror - 1] = '\0'; + } + mju_closeResource(resource); + return nullptr; + } + + // first priority is to check the VFS + if (vfs != nullptr) { + memcpy(resource->name, name, + sizeof(char) * (std::strlen(name) + 1)); + const mjpResourceProvider* provider = GetVfsResourceProvider(); + resource->data = (void*) vfs; + resource->provider = provider; + if (provider->open(resource)) { + return resource; } } - return resource; + // copy full path over + memcpy(resource->name, fullname.c_str(), sizeof(char) * (n + 1)); + + // find provider based off prefix of name + const mjpResourceProvider* provider = mjp_getResourceProvider(resource->name); + if (provider != nullptr) { + resource->provider = provider; + resource->data = nullptr; + if (provider->open(resource)) { + return resource; + } + + if (error) { + snprintf(error, nerror, "could not open '%s'" + "using a resource provider matching prefix '%s'", + resource->name, provider->prefix); + } + + mju_closeResource(resource); + return nullptr; + } + + // lastly fallback to OS filesystem + if (FileOpen(resource, error, nerror)) { + return resource; + } + mju_closeResource(resource); + return nullptr; } + + +// close the given resource; no-op if resource is NULL void mju_closeResource(mjResource* resource) { - if (resource && resource->vfs) { - mujoco::user::VFS::Upcast(resource->vfs)->Close(resource); + if (resource == nullptr) { + return; } + + // use the resource provider close callback + const mjpResourceProvider* provider = resource->provider; + if (provider) { + if (provider->close) provider->close(resource); + } else { + FileClose(resource); // clear OS filesystem if present + } + + // free resource + if (resource->name) mju_free(resource->name); + mju_free(resource); } + + +// set buffer to bytes read from the resource and return number of bytes in +// buffer; return negative value if error int mju_readResource(mjResource* resource, const void** buffer) { - if (resource && resource->vfs) { - return mujoco::user::VFS::Upcast(resource->vfs)->Read(resource, buffer); + if (resource->provider) { + return resource->provider->read(resource, buffer); } - return -1; // default (error reading bytes) + + // if provider is NULL, then OS filesystem is used + return FileRead(resource, buffer); } + + +// get directory path of resource void mju_getResourceDir(mjResource* resource, const char** dir, int* ndir) { *dir = nullptr; *ndir = 0; @@ -102,11 +247,22 @@ void mju_getResourceDir(mjResource* resource, const char** dir, int* ndir) { } } + + +// return 0 if the resource's timestamp matches the provided timestamp +// return > 0 if the resource is younger than the given timestamp +// return < 0 if the resource is older than the given timestamp int mju_isModifiedResource(const mjResource* resource, const char* timestamp) { - if (resource && resource->provider && resource->provider->modified) { - return resource->provider->modified(resource, timestamp); + // provider is not OS filesystem + if (resource->provider) { + if (resource->provider->modified) { + return resource->provider->modified(resource, timestamp); + } + return 1; // default (modified) } - return 1; // default (assume modified) + + // fallback to OS filesystem + return FileModified(resource, timestamp); } mjSpec* mju_decodeResource(mjResource* resource, const char* content_type, const mjVFS* vfs) { diff --git a/src/user/user_vfs.cc b/src/user/user_vfs.cc index 4c3c4868..5e8cfedb 100644 --- a/src/user/user_vfs.cc +++ b/src/user/user_vfs.cc @@ -14,505 +14,265 @@ #include "user/user_vfs.h" -#include -#ifdef _WIN32 -#define stat _stat -#endif - -#include -#include #include -#include #include -#include -#include -#include -#include +#include +#include +#include #include -#include #include #include #include -#include #include "engine/engine_util_misc.h" #include "user/user_util.h" namespace { -// struct for holding the contents of a file -struct ResourceFileData { - std::vector contents; - time_t modified_time = 0; - bool is_read = false; +using mujoco::user::FilePath; +using mujoco::user::FileToMemory; + +// internal struct for VFS files +struct VFSFile { + FilePath filename; + std::vector filedata; + std::size_t filesize; + uint64_t filestamp; }; -time_t GetModifiedTime(const char* path) { - struct stat file_stat; - if (stat(path, &file_stat) == 0) { - return file_stat.st_mtime; - } - return 0; +// internal container class for VFS +class VFS { + public: + // returns true if the file exists in the VFS + bool HasFile(const FilePath& filename) const; + + // returns inserted mjuuVFSFile if the file was added successfully. This class + // assumes ownership of the buffer. + VFSFile* AddFile(const FilePath& filename, std::vector&& buffer, + uint64_t filestamp); + + // returns the internal file struct for the given filename + const VFSFile* GetFile(const FilePath& filename) const; + + // deletes file from VFS, return 0: success, -1: not found + int DeleteFile(const FilePath& filename); + + private: + std::unordered_map files_; +}; + +// returns the internal VFS class pointer from the VFS C struct +inline VFS* GetVFSImpl(const mjVFS* vfs) { + return vfs->impl_ ? static_cast(vfs->impl_) : nullptr; } -int OpenFile(const char* filename, mjResource* resource) { - const time_t mtime = GetModifiedTime(filename); - if (mtime != 0) { - ResourceFileData* data = new ResourceFileData(); - resource->data = data; - - data->modified_time = mtime; - mju_encodeBase64(resource->timestamp, (uint8_t*)&mtime, sizeof(time_t)); - return 1; - } - return 0; +// strip path prefix from filename and make lowercase +FilePath StripPath(const char* filename) { + return FilePath(filename).StripPath().Lower(); } -int ReadFile(const char* filename, mjResource* resource, const void** buffer) { - ResourceFileData* data = (ResourceFileData*)resource->data; - if (!data->is_read) { - data->contents = mujoco::user::FileToMemory(filename); - data->is_read = true; +// copies data into a buffer and produces a hash of the data +uint64_t vfs_memcpy(std::vector& dest, const void* src, size_t n) { + uint64_t hash = 0xcbf29ce484222325; // magic number + uint64_t prime = 0x100000001b3; // magic prime + const uint8_t* bytes = (uint8_t*) src; + for (size_t i = 0; i < n; i++) { + dest.push_back(bytes[i]); + + // do FNV-1 hash + hash |= bytes[i]; + hash *= prime; } - *buffer = data->contents.data(); - return static_cast(data->contents.size()); + return hash; } -void CloseFile(mjResource* resource) { - delete (ResourceFileData*)resource->data; - resource->data = nullptr; +// VFS hash function implemented using the FNV-1 hash +uint64_t vfs_hash(const std::vector& buffer) { + uint64_t hash = 0xcbf29ce484222325; // magic number + uint64_t prime = 0x100000001b3; // magic prime + const uint8_t* bytes = (uint8_t*) buffer.data(); + std::size_t n = buffer.size(); + for (std::size_t i = 0; i < n; i++) { + hash |= bytes[i]; + hash *= prime; + } + return hash; } -int FileModified(const mjResource* resource, const char* timestamp) { - if (mju_isValidBase64(timestamp) != sizeof(time_t)) { - return 1; - } - time_t time; - mju_decodeBase64((uint8_t*)&time, timestamp); - - ResourceFileData* data = (ResourceFileData*)resource->data; - const double diff = difftime(data->modified_time, time); - - if (diff < 0) return -1; - if (diff > 0) return 1; - return 0; +bool VFS::HasFile(const FilePath& filename) const { + return files_.find(filename.Str()) != files_.end(); } -std::string StripPathAndLower(std::string path) { - std::size_t n = path.find_last_of("/\\"); - if (n != std::string::npos) { - path = path.substr(n + 1); +VFSFile* VFS::AddFile(const FilePath& filename, std::vector&& buffer, + uint64_t filestamp) { + auto [it, inserted] = files_.insert({filename.Str(), VFSFile()}); + if (!inserted) { + return nullptr; // repeated name } - std::transform(path.begin(), path.end(), path.begin(), - [](unsigned char c) { return std::tolower(c); }); - return path; + it->second.filename = filename; + it->second.filedata = buffer; + it->second.filestamp = filestamp; + return &(it->second); } -} // namespace - -namespace mujoco::user { - -VFS::VFS(mjVFS* vfs) : self_(vfs) { - mjp_defaultResourceProvider(&default_provider_); - default_provider_.open = [](mjResource* res) { - return OpenFile(res->name, res); - }; - default_provider_.read = [](mjResource* res, const void** buffer) { - return ReadFile(res->name, res, buffer); - }; - default_provider_.close = [](mjResource* res) { - CloseFile(res); - }; - default_provider_.modified = [](const mjResource* res, const char* time) { - return FileModified(res, time); - }; - - default_provider_.prefix = nullptr; - default_mount_.vfs = self_; - default_mount_.provider = &default_provider_; - default_mount_.data = nullptr; - default_mount_.name = nullptr; -} - -VFS::~VFS() { - if (!open_resources_.empty()) { - mju_warning( - "VFS destroyed with %zu open resources. Resources will be invalidated.", - open_resources_.size()); - } - for (auto& [ptr, res] : open_resources_) { - if (res->provider->close) { - res->provider->close(res.get()); - } - } - open_resources_.clear(); - - for (auto& [path, res] : mounts_) { - if (res->provider->unmount) { - res->provider->unmount(res.get()); - } - } - mounts_.clear(); -} - -mjResource* VFS::Open(const char* dir, const char* name) { - const std::string path = FilePath(dir, name).Str(); - - const mjResource* mount = FindMount(path); - if (!mount) { - MaybeSelfDestruct(); +const VFSFile* VFS::GetFile(const FilePath& filename) const { + auto it = files_.find(filename.Str()); + if (it == files_.end()) { return nullptr; } - - ResourcePtr res = CreateResource(path.c_str(), mount->provider); - - // Smuggle the mounted resource provider's mount-specific data pointer in the - // requested resource's data pointer. This allows the provider to access its - // own per-mount data without any intrusive changes to the provider interface. - res->data = mount->data; - const int result = mount->provider->open(res.get()); - // If the data pointer was not modified, then that means the resource did not - // set its own data pointer. So, we need to set it back to nullptr. - if (res->data == mount->data) { - res->data = nullptr; - } - - if (result == 0) { - res.reset(); - MaybeSelfDestruct(); - return nullptr; - } - - std::lock_guard lock(mutex_); - mjResource* res_ptr = res.get(); - open_resources_.emplace(res_ptr, std::move(res)); - return res_ptr; + return &it->second; } -VFS::Status VFS::Mount(const FilePath& path, - const mjpResourceProvider* provider) { - if (!provider) { - return kInvalidResourceProvider; +int VFS::DeleteFile(const FilePath& filename) { + auto it = files_.find(filename.Str()); + if (it == files_.end()) { + return -1; } - { - std::lock_guard lock(mutex_); - if (mounts_.contains(path.Str())) { - return kRepeatedName; + files_.erase(it); + return 0; +} + +// open callback for the VFS resource provider +int Open(mjResource* resource) { + if (!resource || !resource->name || !resource->data) { + return 0; + } + + const mjVFS* vfs = (const mjVFS*) resource->data; + const VFS* cvfs = GetVFSImpl(vfs); + const VFSFile* file = cvfs->GetFile(StripPath(resource->name)); + if (file == nullptr) { + file = cvfs->GetFile(FilePath(resource->name)); + if (file == nullptr) { + return 0; } } - ResourcePtr res = CreateResource(path.c_str(), provider); - provider->mount(res.get()); - - std::lock_guard lock(mutex_); - mounts_.emplace(path.Str(), std::move(res)); - return kSuccess; + resource->data = (void*) file; + resource->timestamp[0] = '\0'; + if (file->filestamp) { + mju_encodeBase64(resource->timestamp, (uint8_t*) &file->filestamp, + sizeof(uint64_t)); + } + return 1; } -VFS::Status VFS::Close(mjResource* res) { - VFS::Status status = kInvalidResource; - bool last_resource = false; - { - std::lock_guard lock(mutex_); - if (auto it = open_resources_.find(res); it != open_resources_.end()) { - if (res->provider->close) { - res->provider->close(res); - } - open_resources_.erase(it); - last_resource = open_resources_.empty(); - status = kSuccess; - } - } - - if (status == kSuccess && last_resource) { - MaybeSelfDestruct(); - } - return status; -} - -VFS::Status VFS::Unmount(const FilePath& path) { - std::lock_guard lock(mutex_); - if (auto it = mounts_.find(path.Str()); it != mounts_.end()) { - if (it->second->provider->unmount) { - it->second->provider->unmount(it->second.get()); - } - mounts_.erase(it); - return kSuccess; - } - return kInvalidResourceProvider; -} - -int VFS::Read(mjResource* resource, const void** buffer) { - if (resource && resource->provider && resource->provider->read) { - return resource->provider->read(resource, buffer); - } - return kFailedToRead; -} - -VFS::ResourcePtr VFS::CreateResource(std::string_view name, - const mjpResourceProvider* provider) { - mjResource* res = new mjResource(); - res->vfs = self_; - res->provider = provider; - res->data = nullptr; - res->name = new char[name.size() + 1]; - std::strncpy(res->name, name.data(), name.size()); - res->name[name.size()] = 0; - res->timestamp[0] = 0; - - return ResourcePtr(res, [](mjResource* ptr) { - if (ptr->data) { - // TODO: Make this an error eventually. For now, we continue to allow - // users to free their data pointers without resetting them to nullptr. - mju_warning( - "mjResource::data is not null; did you forget to close/unmount it?"); - } - delete[] ptr->name; - delete ptr; - }); -} - -mjResource* VFS::FindMount(const std::string& fullpath) { - std::lock_guard lock(mutex_); - - std::string str = fullpath; - while (!str.empty()) { - auto it = mounts_.find(str); - if (it != mounts_.end()) { - return it->second.get(); - } - - std::size_t n = str.find_last_of("/\\"); - if (n == std::string::npos) { - str = ""; - } else { - str = str.substr(0, n); - } - } - - const mjpResourceProvider* provider = - mjp_getResourceProvider(fullpath.c_str()); - if (provider) { - if (auto it = mounts_.find(provider->prefix); it != mounts_.end()) { - return it->second.get(); - } - - ResourcePtr res = CreateResource(provider->prefix, provider); - mjResource* res_ptr = res.get(); - mounts_.emplace(provider->prefix, std::move(res)); - if (provider->mount) { - provider->mount(res_ptr); - } - return res_ptr; - } - - // Legacy use-case: match on just the case-insensitive filename. - const std::string filename = StripPathAndLower(fullpath); - for (auto& [path, res] : mounts_) { - if (StripPathAndLower(path) == filename) { - return res.get(); - } - } - - return &default_mount_; -} - -void VFS::MaybeSelfDestruct() { - if (destructor_) { - destructor_(self_); - } -} - -void VFS::SetToSelfDestruct(std::function destructor) { - destructor_ = std::move(destructor); -} - -VFS* VFS::Upcast(mjVFS* vfs) { - return vfs ? static_cast(vfs->impl_) : nullptr; -} - -const VFS* VFS::Upcast(const mjVFS* vfs) { - return vfs ? static_cast(vfs->impl_) : nullptr; -} - -} // namespace mujoco::user - -void mj_defaultVFS(mjVFS* vfs) { - if (vfs == nullptr) { - mju_error("mjVFS is null."); - } else { - vfs->impl_ = new mujoco::user::VFS(vfs); - } -} - -void mj_deleteVFS(mjVFS* vfs) { - if (vfs) { - delete mujoco::user::VFS::Upcast(vfs); - vfs->impl_ = nullptr; - } -} - -int mj_mountVFS(mjVFS* vfs, const char* filepath, - const mjpResourceProvider* provider) { - mujoco::user::VFS* impl = mujoco::user::VFS::Upcast(vfs); - if (impl == nullptr) { - mju_error("mjVFS is null."); - return mujoco::user::VFS::kInvalidVfs; - } - - if (filepath == nullptr) { - return mujoco::user::VFS::kNotFound; - } - const mujoco::user::FilePath path(filepath); - const mujoco::user::VFS::Status status = impl->Mount(path, provider); - return static_cast(status); -} - -int mj_unmountVFS(mjVFS* vfs, const char* filename) { - mujoco::user::VFS* impl = mujoco::user::VFS::Upcast(vfs); - if (impl == nullptr) { - mju_error("mjVFS is null."); - return mujoco::user::VFS::kInvalidVfs; - } - - if (filename == nullptr) { - return mujoco::user::VFS::kNotFound; - } - const mujoco::user::FilePath path(filename); - const mujoco::user::VFS::Status status = impl->Unmount(path); - return static_cast(status); -} - -namespace { - -// In order to support older APIs (namely mj_addFileVFS and mj_addBufferVFS), -// we create custom providers dynamically that are mounted to the VFS. -template -static int MountCustomProvider(mjVFS* vfs, Args&&... args) { - mujoco::user::VFS* impl = mujoco::user::VFS::Upcast(vfs); - if (impl == nullptr) { - mju_error("mjVFS is null."); +// read callback for the VFS resource provider +int Read(mjResource* resource, const void** buffer) { + if (!resource || !resource->name || !resource->data) { + *buffer = nullptr; return -1; } - Provider* provider = new Provider(std::forward(args)...); - provider->mount = [](mjResource* res) { - return static_cast(mujoco::user::VFS::kSuccess); - }; - provider->unmount = [](mjResource* res) { - delete (Provider*)res->provider; - return static_cast(mujoco::user::VFS::kSuccess); - }; - - const mujoco::user::VFS::Status status = - impl->Mount(provider->MountPath(), provider); - if (status != mujoco::user::VFS::kSuccess) { - delete provider; + const VFSFile* file = static_cast(resource->data); + if (file == nullptr) { + *buffer = nullptr; + return -1; } - return static_cast(status); + + *buffer = file->filedata.data(); + return file->filedata.size(); } -// Custom provider for mj_addFileVFS. -class FileProvider : public mjpResourceProvider { - public: - FileProvider(const char* dir, const char* filename) { - mjp_defaultResourceProvider(this); +// close callback for the VFS resource provider +void Close(mjResource* resource) { +} - file_path = mujoco::user::FilePath(dir ? dir : "", filename); - mount_path = file_path.StripPath().Lower(); - - open = [](mjResource* res) { - FileProvider* self = (FileProvider*)res->provider; - return OpenFile(self->file_path.c_str(), res); - }; - read = [](mjResource* res, const void** buffer) { - FileProvider* self = (FileProvider*)res->provider; - return ReadFile(self->file_path.c_str(), res, buffer); - }; - close = [](mjResource* res) { CloseFile(res); }; - modified = [](const mjResource* res, const char* time) { - return FileModified(res, time); - }; +// modified callback for the VFS resource provider +// return > 0 if modified and 0 if unmodified +int Modified(const mjResource* resource, const char* timestamp) { + uint64_t filestamp; + if (mju_isValidBase64(timestamp) > sizeof(uint64_t)) { + return 2; // error (assume modified) } - const mujoco::user::FilePath& MountPath() const { return mount_path; } + mju_decodeBase64((uint8_t*) &filestamp, timestamp); + if (!filestamp) return 3; // no hash (assume modified) - mujoco::user::FilePath file_path; - mujoco::user::FilePath mount_path; -}; + if (resource) { + const VFSFile* file = static_cast(resource->data); + if (file == nullptr) return 4; // missing file (assume modified) + if (!file->filestamp) return 5; // missing filestamp (assume modified) -// Custom provider for mj_addBufferVFS. -class BufferProvider : public mjpResourceProvider { - public: - BufferProvider(const char* name, const void* src, size_t n) { - mjp_defaultResourceProvider(this); - - path = mujoco::user::FilePath(name); - - static constexpr std::uint64_t prime = 0x100000001b3; - hash = n ? 0xcbf29ce484222325 : 0; - - const std::uint8_t* src_bytes = static_cast(src); - contents.reserve(n); - for (size_t i = 0; i < n; i++) { - contents.push_back(src_bytes[i]); - hash |= src_bytes[i]; - hash *= prime; + if (file->filestamp == filestamp) { + return 0; // unmodified } - - open = [](mjResource* res) { - BufferProvider* self = (BufferProvider*)res->provider; - mju_encodeBase64(res->timestamp, (std::uint8_t*)&self->hash, - sizeof(self->hash)); - return 1; - }; - read = [](mjResource* res, const void** out) { - BufferProvider* self = (BufferProvider*)res->provider; - *out = reinterpret_cast(self->contents.data()); - return static_cast(self->contents.size()); - }; - close = [](mjResource* res) { - // no-op - }; - modified = [](const mjResource* res, const char* timestamp) { - const BufferProvider* self = (const BufferProvider*)res->provider; - if (mju_isValidBase64(timestamp) > sizeof(std::uint64_t)) { - return 1; - } - std::uint64_t test = 0; - mju_decodeBase64((std::uint8_t*)&test, timestamp); - if (self->hash != test) { - return 1; - } - return 0; - }; } + return 1; // modified +} - const mujoco::user::FilePath& MountPath() const { return path; } - - mujoco::user::FilePath path; - std::vector contents; - std::uint64_t hash = 0; -}; } // namespace -int mj_addFileVFS(mjVFS* vfs, const char* directory, const char* filename) { - return MountCustomProvider(vfs, directory, filename); +// initialize to empty (no deallocation) +void mj_defaultVFS(mjVFS* vfs) { + vfs->impl_ = new VFS(); } +// add file to VFS, return 0: success, 2: repeated name, -1: failed to load +int mj_addFileVFS(mjVFS* vfs, const char* directory, const char* filename) { + VFS* cvfs = GetVFSImpl(vfs); + + // make full name + const char* dir = directory != nullptr ? directory : ""; + FilePath fullname = FilePath(dir, filename); + + // strip path + FilePath newname = StripPath(filename); + + // check beforehand for repeated name, to avoid reading file into memory + if (cvfs->HasFile(newname)) { + return 2; + } + + // allocate and read + std::vector buffer = FileToMemory(fullname.c_str()); + if (buffer.empty()) { + return -1; + } + + if (!cvfs->AddFile(newname, std::move(buffer), vfs_hash(buffer))) { + return 2; // AddFile failed, SHOULD NOT OCCUR + } + return 0; +} + +// add file from buffer into VFS int mj_addBufferVFS(mjVFS* vfs, const char* name, const void* buffer, int nbuffer) { - return MountCustomProvider(vfs, name, buffer, nbuffer); + std::vector inbuffer; + VFS* cvfs = GetVFSImpl(vfs); + VFSFile* file; + if (!(file = cvfs->AddFile(FilePath(name), std::move(inbuffer), 0))) { + return 2; // AddFile failed, repeated name + } + try { + file->filedata.reserve(nbuffer); + } catch (...) { + cvfs->DeleteFile(file->filename); // delete file on error + return -1; + } + file->filestamp = vfs_memcpy(file->filedata, buffer, nbuffer); + return 0; } +// delete file from VFS, return 0: success, -1: not found in VFS int mj_deleteFileVFS(mjVFS* vfs, const char* filename) { - if (filename == nullptr) { - return mujoco::user::VFS::kNotFound; + VFS* cvfs = GetVFSImpl(vfs); + if (cvfs->DeleteFile(StripPath(filename))) { + return cvfs->DeleteFile(FilePath(filename)); } - - if (mj_unmountVFS(vfs, filename) != 0) { - mujoco::user::FilePath path(filename); - return mj_unmountVFS(vfs, path.StripPath().Lower().c_str()); - } - return mujoco::user::VFS::kSuccess; + return 0; +} + +// delete all files from VFS +void mj_deleteVFS(mjVFS* vfs) { + if (vfs) { + delete GetVFSImpl(vfs); + } +} + +const mjpResourceProvider* GetVfsResourceProvider() { + static mjpResourceProvider provider + = { nullptr, &Open, &Read, &Close, &Modified, nullptr }; + return &provider; } diff --git a/src/user/user_vfs.h b/src/user/user_vfs.h index b8d8013f..416de6f7 100644 --- a/src/user/user_vfs.h +++ b/src/user/user_vfs.h @@ -17,115 +17,38 @@ #ifndef MUJOCO_SRC_USER_USER_VFS_H_ #define MUJOCO_SRC_USER_USER_VFS_H_ -#include -#include -#include -#include -#include -#include +#include +#include #include -#include -#include "user/user_util.h" +#include -namespace mujoco::user { +#ifdef __cplusplus +extern "C" { +#endif -// Underlying Virtual File System implementation for opaque mjVFS struct. -// -// This class owns and manages all the mjResource instances that are created -// using the mju_openResource. Its main job is to find the correct -// mjpResourceProvider to handle the mju open/read/close operations. It does -// this by "mounting" mjpResourceProviders at specific paths such that any -// operation within that path will be handled by the mjpResourceProvider. -// -// Mounting can be done explicitly (using mj_mountVFS) or implicitly (using -// mjp_registerResourceProvider). If no provider is found for a given path, -// then a "default" provider is used that uses normal C file operations to -// open/read/close files. -// -// To support legacy use-cases (where the VFS is an optional argument), a -// "self-destruct" mode can be configured so that a temporary VFS instance can -// be created with a lifetime tied to the opened mjResource instance. -// -// This class itself is thread-safe, but it makes no guarantees about the -// thread-safety of the underlying mjResourceProviders. -class VFS { - public: - explicit VFS(mjVFS* vfs); - ~VFS(); +// Initialize an empty VFS, mj_deleteVFS must be called to deallocate the VFS +MJAPI void mj_defaultVFS(mjVFS* vfs); - VFS(const VFS&) = delete; - VFS& operator=(const VFS&) = delete; +// add file to VFS, return 0: success, 2: repeated name, -1: not found on disk +MJAPI int mj_addFileVFS(mjVFS* vfs, const char* directory, const char* filename); - // Status codes for VFS operations. Values are based on the mj_VFS APIs. - enum Status { - kSuccess = 0, - kFailedToLoad = -1, - kFailedToRead = -1, - kNotFound = -1, - kRepeatedName = 2, - kInvalidVfs = -1, - kInvalidResource = -1, - kInvalidResourceProvider = -1, - }; +// add file from buffer into VFS, return 0: success, 2: repeated name, -1: failed to load +MJAPI int mj_addBufferVFS(mjVFS* vfs, const char* filename, const void* buffer, int nbuffer); - // Opens a mjResource for the given path, or nullptr on error. If successful, - // will invoke the 'open' callback for the mjpResourceProvider associated with - // the path/dir. - mjResource* Open(const char* dir, const char* name); +// return file index in VFS, or -1 if not found in VFS +MJAPI int mj_findFileVFS(const mjVFS* vfs, const char* filename); - // Sets `buffer` to the contents of the resource and returns the number of - // bytes of the content. This is done by invoking the 'read' callback for the - // mjpResourceProvider associated with the resource. Returns -1 on error. - int Read(mjResource* resource, const void** buffer); +// delete file from VFS, return 0: success, -1: not found in VFS +MJAPI int mj_deleteFileVFS(mjVFS* vfs, const char* filename); - // Closes the resource by invoking the 'close' callback for the - // mjpResourceProvider associated with the resource. - Status Close(mjResource* resource); +// delete all files from VFS +MJAPI void mj_deleteVFS(mjVFS* vfs); - // Mounts a ResourceProvider at the given path. All subsequent operations - // under `path` will be delegated to the `provider` until it is unmounted. - Status Mount(const FilePath& path, const mjpResourceProvider* provider); +#ifdef __cplusplus +} +#endif - // Unmounts the ResourceProvider from the given path. - Status Unmount(const FilePath& path); - - // Sets a destructor to be called when the VFS has no more open resources. - // Assumes that `destructor` will delete `this`. - // - // This is useful for when you want to create a temporary VFS instance with - // a lifetime tied to a single mjResource to be opened. The `destructor` - // should be set to `delete this` and any other cleanup that needs to happen. - void SetToSelfDestruct(std::function destructor); - - // Converts the public C-API pointer to the internal C++ class. - static VFS* Upcast(mjVFS* vfs); - static const VFS* Upcast(const mjVFS* vfs); - - private: - using ResourcePtr = std::unique_ptr; - ResourcePtr CreateResource(std::string_view name, - const mjpResourceProvider* provider); - - // Returns a mounted mjResource* that matches the given path. If no explicitly - // mounted mjResource* is found, returns a "default" mounting that uses the - // C file system. - mjResource* FindMount(const std::string& fullpath); - - // Invokes the `destructor_, but only if it has been set previously. This - // should be only called when resources_ is empty and callers should assume - // that `this` will be invalidated after this call. - void MaybeSelfDestruct(); - - mjVFS* self_; - std::mutex mutex_; // Protects open_resources_ and mounts_. - std::unordered_map open_resources_; - std::unordered_map mounts_; - mjResource default_mount_; - mjpResourceProvider default_provider_; - std::function destructor_; -}; - -} // namespace mujoco::user +const mjpResourceProvider* GetVfsResourceProvider(); #endif // MUJOCO_SRC_USER_USER_VFS_H_ diff --git a/test/user/user_resource_test.cc b/test/user/user_resource_test.cc index 4017c0ae..b783761f 100644 --- a/test/user/user_resource_test.cc +++ b/test/user/user_resource_test.cc @@ -301,7 +301,7 @@ TEST_F(ResourceTest, GeneralFailureTest) { error.data(), error.size()); ASSERT_THAT(resource, IsNull()); - EXPECT_THAT(error.data(), HasSubstr("Error opening file")); + EXPECT_THAT(error.data(), HasSubstr("could not open")); } TEST_F(ResourceTest, NameWithValidPrefix) { diff --git a/test/user/user_vfs_test.cc b/test/user/user_vfs_test.cc index 84d750c3..12ca78f2 100644 --- a/test/user/user_vfs_test.cc +++ b/test/user/user_vfs_test.cc @@ -26,7 +26,6 @@ namespace mujoco { namespace { -using ::testing::IsNull; using ::testing::NotNull; using UserVfsTest = MujocoTest; @@ -37,46 +36,6 @@ static bool HasFile(const mjVFS* vfs, const std::string& filename) { return result; } -struct TestProvider : public mjpResourceProvider { - // The TestProvider will increment the value at `addr` by these amounts when - // each callback is invoked. This can be used to verity that the correct - // callbacks are being invoked when expected. - enum CallbackValues { - Mounted = 100, - Unmounted = 200, - Opened = 300, - Read = 400, - Closed = 500, - }; - - explicit TestProvider(int* addr) { - mjp_defaultResourceProvider(this); - prefix = "test"; - data = addr; - - mount = [](mjResource* res) { - *(int*)res->provider->data += Mounted; - return 1; - }; - unmount = [](mjResource* res) { - *(int*)res->provider->data += Unmounted; - return 1; - }; - open = [](mjResource* res) { - *(int*)res->provider->data += Opened; - return 1; - }; - read = [](mjResource* res, const void** out) { - *(int*)res->provider->data += Read; - *out = res->provider->data; - return (int)sizeof(int); - }; - close = [](mjResource* res) { - *(int*)res->provider->data += Closed; - }; - } -}; - TEST_F(UserVfsTest, AddFile) { constexpr char path[] = "engine/testdata/actuation/"; const std::string dir = GetTestDataFilePath(path); @@ -286,126 +245,5 @@ TEST_F(UserVfsTest, Timestamps) { mj_deleteVFS(&vfs); } -TEST_F(UserVfsTest, MountUnmount) { - int test = 0; - int expect = 0; - TestProvider provider(&test); - - mjVFS vfs; - mj_defaultVFS(&vfs); - mjResource* res = mju_openResource("", "/some/path/foo", &vfs, nullptr, 0); - EXPECT_THAT(res, IsNull()); - EXPECT_EQ(test, expect); - - mj_mountVFS(&vfs, "/some/path", &provider); - expect += TestProvider::Mounted; - EXPECT_EQ(test, expect); - - res = mju_openResource("", "/some/path/foo", &vfs, nullptr, 0); - expect += TestProvider::Opened; - EXPECT_THAT(res, NotNull()); - EXPECT_EQ(test, expect); - - const void* buffer = nullptr; - const int size = mju_readResource(res, &buffer); - expect += TestProvider::Read; - EXPECT_THAT(buffer, NotNull()); - EXPECT_GT(size, 0); - EXPECT_EQ(test, expect); - - mju_closeResource(res); - expect += TestProvider::Closed; - EXPECT_EQ(test, expect); - - mj_unmountVFS(&vfs, "/some/path"); - expect += TestProvider::Unmounted; - EXPECT_EQ(test, expect); - - mj_deleteVFS(&vfs); -} - -TEST_F(UserVfsTest, AutoMountProviders) { - int test = 0; - int expect = 0; - - TestProvider provider(&test); - mjp_registerResourceProvider(&provider); - - mjVFS vfs; - mj_defaultVFS(&vfs); - - mjResource* res = mju_openResource("", "test:foo", &vfs, nullptr, 0); - expect += TestProvider::Mounted + TestProvider::Opened; - EXPECT_THAT(res, NotNull()); - - mju_closeResource(res); - expect += TestProvider::Closed; - EXPECT_EQ(test, expect); - - mj_deleteVFS(&vfs); - expect += TestProvider::Unmounted; - EXPECT_EQ(test, expect); -} - -TEST_F(UserVfsTest, StackedMounts) { - int test1 = 0; - int test2 = 1000; - int test3 = 1000000; - int expect1 = test1; - int expect2 = test2; - int expect3 = test3; - - TestProvider provider1(&test1); - TestProvider provider2(&test2); - TestProvider provider3(&test3); - - mjVFS vfs; - mj_defaultVFS(&vfs); - - mj_mountVFS(&vfs, "/some/path", &provider1); - mj_mountVFS(&vfs, "/some/path/further/down/very/deep", &provider2); - mj_mountVFS(&vfs, "/some/path/further/down", &provider3); - expect1 += TestProvider::Mounted; - expect2 += TestProvider::Mounted; - expect3 += TestProvider::Mounted; - EXPECT_EQ(test1, expect1); - EXPECT_EQ(test2, expect2); - EXPECT_EQ(test3, expect3); - - mjResource* res1 = mju_openResource("", "/some/path/foo", &vfs, nullptr, 0); - expect1 += TestProvider::Opened; - EXPECT_EQ(test1, expect1); - EXPECT_EQ(test2, expect2); - EXPECT_EQ(test3, expect3); - - mjResource* res2 = - mju_openResource("", "/some/path/further/down/foo", &vfs, nullptr, 0); - expect3 += TestProvider::Opened; - EXPECT_EQ(test1, expect1); - EXPECT_EQ(test2, expect2); - EXPECT_EQ(test3, expect3); - - mjResource* res3 = mju_openResource( - "", "/some/path/further/down/very/deep/foo", &vfs, nullptr, 0); - expect2 += TestProvider::Opened; - EXPECT_EQ(test1, expect1); - EXPECT_EQ(test2, expect2); - EXPECT_EQ(test3, expect3); - - mju_closeResource(res1); - mju_closeResource(res2); - mju_closeResource(res3); - expect1 += TestProvider::Closed; - expect2 += TestProvider::Closed; - expect3 += TestProvider::Closed; - - mj_deleteVFS(&vfs); - expect1 += TestProvider::Unmounted; - expect2 += TestProvider::Unmounted; - expect3 += TestProvider::Unmounted; - EXPECT_EQ(test1, expect1); - EXPECT_EQ(test2, expect2); - EXPECT_EQ(test3, expect3); -} } // namespace } // namespace mujoco diff --git a/test/xml/xml_api_test.cc b/test/xml/xml_api_test.cc index 4207dd16..c985d3d0 100644 --- a/test/xml/xml_api_test.cc +++ b/test/xml/xml_api_test.cc @@ -126,7 +126,7 @@ TEST_F(LoadXmlTest, InvalidFileFails) { std::array error; mjSpec* spec = mj_parseXML("invalid", nullptr, error.data(), error.size()); EXPECT_THAT(spec, IsNull()) << "Expected model loading to fail."; - EXPECT_THAT(error.data(), HasSubstr("Error opening file")); + EXPECT_THAT(error.data(), HasSubstr("No such file or directory")); } TEST_F(MujocoTest, SaveXmlShortString) {