From d3649c4435eefbc5d1edf0b3fbf54689c240b2e1 Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Tue, 9 Jan 2024 17:45:31 +0000 Subject: [PATCH 1/9] Add function to return list of all external files in a config Signed-off-by: Kevin Wheatley --- src/OpenColorIO/Config.cpp | 34 +++++++++++++--------------------- 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/src/OpenColorIO/Config.cpp b/src/OpenColorIO/Config.cpp index a4cf7c5fe..e90c7c527 100644 --- a/src/OpenColorIO/Config.cpp +++ b/src/OpenColorIO/Config.cpp @@ -1103,6 +1103,16 @@ class Config::Impl // That should never happen. return -1; } + + void GetAllFileReferences(std::set & files) const + { + ConstTransformVec allTransforms; + this->getAllInternalTransforms(allTransforms); + for(const auto & transform : allTransforms) + { + GetFileReferences(files, transform); + } + } }; @@ -1956,14 +1966,8 @@ void Config::validate() const ///// Resolve all file Transforms using context variables. { - ConstTransformVec allTransforms; - getImpl()->getAllInternalTransforms(allTransforms); - std::set files; - for (const auto & transform : allTransforms) - { - GetFileReferences(files, transform); - } + getImpl()->GetAllFileReferences(files); // Check that at least one of the search paths can be resolved into a valid path. // Note that a search path without context variable(s) always correctly resolves. @@ -4890,14 +4894,8 @@ const char * Config::getCacheID(const ConstContextRcPtr & context) const { std::ostringstream filehash; - ConstTransformVec allTransforms; - getImpl()->getAllInternalTransforms(allTransforms); - std::set files; - for(const auto & transform : allTransforms) - { - GetFileReferences(files, transform); - } + getImpl()->GetAllFileReferences(files); for(const auto & iter : files) { @@ -5590,14 +5588,8 @@ bool Config::isArchivable() const ///////////////////////////////// // FileTransform verification. // ///////////////////////////////// - ConstTransformVec allTransforms; - getImpl()->getAllInternalTransforms(allTransforms); - std::set files; - for(const auto & transform : allTransforms) - { - GetFileReferences(files, transform); - } + getImpl()->GetAllFileReferences(files); // Check that FileTransform sources are not absolute nor have context variables outside of // config working directory. From 04832faa06cdc0fc64b86a3884742a2c2feb4564 Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Mon, 15 Jan 2024 19:04:53 +0000 Subject: [PATCH 2/9] Add basic Archive flags for configuring archiving Signed-off-by: Kevin Wheatley --- docs/api/constants.rst | 8 +++++++ include/OpenColorIO/OpenColorIO.h | 5 ++++- include/OpenColorIO/OpenColorTypes.h | 31 ++++++++++++++++++++++++++++ src/OpenColorIO/Config.cpp | 7 ++++--- src/OpenColorIO/OCIOZArchive.cpp | 28 ++++++++++++++++++++----- src/OpenColorIO/OCIOZArchive.h | 9 +++++++- src/apps/ocioarchive/main.cpp | 11 +++++++++- src/apps/ocioview/ocioview/setup.py | 1 + src/bindings/python/PyConfig.cpp | 2 +- src/bindings/python/PyTypes.cpp | 1 + tests/cpu/OCIOZArchive_tests.cpp | 2 +- tests/python/ConstantsTest.py | 1 + 12 files changed, 93 insertions(+), 13 deletions(-) diff --git a/docs/api/constants.rst b/docs/api/constants.rst index 2652b8a14..4d033ea8c 100644 --- a/docs/api/constants.rst +++ b/docs/api/constants.rst @@ -47,6 +47,14 @@ These environmental variables are used by the OpenColorIO library Ex: OCIO_OPTIMIZATION_FLAGS="20479" or "0x4FFF" for OPTIMIZATION_LOSSLESS. + .. data:: PyOpenColorIO.OCIO_ARCHIVE_FLAGS_ENVVAR + + The envvar 'OCIO_ARCHIVE_FLAGS' provides a way to modify the parameters + used to create an ocioz archive. Remove the variable or set the value + to empty to not use it. Set the value of the variable to the desired + parameters as either an integer or hexadecimal value. + Ex: OCIO_ARCHIVE_FLAGS="256" or "0x0100" for ARCHIVE_FLAGS_MINIMAL. + .. group-tab:: C++ .. doxygengroup:: VarsEnvvar diff --git a/include/OpenColorIO/OpenColorIO.h b/include/OpenColorIO/OpenColorIO.h index b7183ce77..3ce6f8a04 100644 --- a/include/OpenColorIO/OpenColorIO.h +++ b/include/OpenColorIO/OpenColorIO.h @@ -1524,6 +1524,8 @@ class OCIOEXPORT Config * trying to resolve all the FileTransforms in the Config to specific files is because of the * goal to allow context variables to continue to work. * + * Archiving a minimal Config will try resolve these file references using the current context. + * * If a Config is created with CreateFromStream, CreateFromFile with an OCIOZ archive, or * CreateFromConfigIOProxy, it cannot be archived unless the working directory is manually set * to a directory that contains any necessary LUT files. @@ -1531,8 +1533,9 @@ class OCIOEXPORT Config * The provided output stream must be closed by the caller, if necessary (e.g., an ofstream). * * \param ostream The output stream to write to. + * \param flags Flags top control archive creation */ - void archive(std::ostream & ostream) const; + void archive(std::ostream & ostream, ArchiveFlags flags) const; Config(const Config &) = delete; Config& operator= (const Config &) = delete; diff --git a/include/OpenColorIO/OpenColorTypes.h b/include/OpenColorIO/OpenColorTypes.h index 21243eb36..8292f9c1d 100644 --- a/include/OpenColorIO/OpenColorTypes.h +++ b/include/OpenColorIO/OpenColorTypes.h @@ -774,6 +774,15 @@ extern OCIOEXPORT const char * OCIO_INACTIVE_COLORSPACES_ENVVAR; */ extern OCIOEXPORT const char * OCIO_OPTIMIZATION_FLAGS_ENVVAR; +/** + * The envvar 'OCIO_OPTIMIZATION_FLAGS' provides a way to force a given + * optimization level. Remove the variable or set the value to empty to + * not use it. Set the value of the variable to the desired optimization + * level as either an integer or hexadecimal value. + * Ex: OCIO_OPTIMIZATION_FLAGS="1" or "0x01" for ARCHIVE_FLAGS_MINIMAL. + */ +extern OCIOEXPORT const char * OCIO_ARCHIVE_FLAGS_ENVVAR; + /** * The envvar 'OCIO_USER_CATEGORIES' allows the end-user to filter color spaces shown by * applications. Only color spaces that include at least one of the supplied categories will be @@ -955,6 +964,28 @@ extern OCIOEXPORT const char * OCIO_CONFIG_DEFAULT_NAME; extern OCIOEXPORT const char * OCIO_CONFIG_DEFAULT_FILE_EXT; extern OCIOEXPORT const char * OCIO_CONFIG_ARCHIVE_FILE_EXT; +//!cpp:type:: Enum to control the behavior of archiving a :cpp:class:`Config`. +// +// TODO: extend docs +// TODO: Python bindings +// TODO: environment variable? +// +enum ArchiveFlags : unsigned long +{ + ARCHIVE_FLAGS_NONE = 0x0000, + + ARCHIVE_FLAGS_FAST_COMPRESSION = 0x02, // Compression level + ARCHIVE_FLAGS_NORMAL_COMPRESSION = 0x06, // Compression level + ARCHIVE_FLAGS_BEST_COMPRESSION = 0x09, // Compression level + ARCHIVE_FLAGS_COMPRESSION_MASK = 0x0F, // Compression level mask + + ARCHIVE_FLAGS_MINIMAL = 0x0100, // Generate a Minimal Archive + + ARCHIVE_FLAGS_DEFAULT = (ARCHIVE_FLAGS_NONE | ARCHIVE_FLAGS_BEST_COMPRESSION), + + ARCHIVE_FLAGS_SAFEST = (ARCHIVE_FLAGS_NONE | ARCHIVE_FLAGS_BEST_COMPRESSION) // TODO: name is poor +}; + // Built-in config feature // URI Prefix extern OCIOEXPORT const char * OCIO_BUILTIN_URI_PREFIX; diff --git a/src/OpenColorIO/Config.cpp b/src/OpenColorIO/Config.cpp index e90c7c527..c2544016a 100644 --- a/src/OpenColorIO/Config.cpp +++ b/src/OpenColorIO/Config.cpp @@ -49,6 +49,7 @@ const char * OCIO_ACTIVE_DISPLAYS_ENVVAR = "OCIO_ACTIVE_DISPLAYS"; const char * OCIO_ACTIVE_VIEWS_ENVVAR = "OCIO_ACTIVE_VIEWS"; const char * OCIO_INACTIVE_COLORSPACES_ENVVAR = "OCIO_INACTIVE_COLORSPACES"; const char * OCIO_OPTIMIZATION_FLAGS_ENVVAR = "OCIO_OPTIMIZATION_FLAGS"; +const char * OCIO_ARCHIVE_FLAGS_ENVVAR = "OCIO_ARCHIVE_FLAGS"; const char * OCIO_USER_CATEGORIES_ENVVAR = "OCIO_USER_CATEGORIES"; // Default filename (with extension) of a config and archived config. @@ -5559,7 +5560,7 @@ bool Config::isArchivable() const // 2) Path may not start with double dot ".." (going above working directory). pystring::startswith(normPath, "..") || // 3) A context variable may not be located at the start of the path. - (ContainsContextVariables(path) && + (ContainsContextVariables(path) && //TODO: if we can resolve context, do so and validate path to file (StringUtils::Find(path, "$") == 0 || StringUtils::Find(path, "%") == 0))) { @@ -5605,10 +5606,10 @@ bool Config::isArchivable() const return true; } -void Config::archive(std::ostream & ostream) const +void Config::archive(std::ostream & ostream, ArchiveFlags flags) const { // Using utility functions in OCIOZArchive.cpp. - archiveConfig(ostream, *this, getCurrentContext()->getWorkingDir()); + archiveConfig(ostream, *this, getCurrentContext()->getWorkingDir(), flags); } } // namespace OCIO_NAMESPACE diff --git a/src/OpenColorIO/OCIOZArchive.cpp b/src/OpenColorIO/OCIOZArchive.cpp index 982fce682..eaf690a3a 100644 --- a/src/OpenColorIO/OCIOZArchive.cpp +++ b/src/OpenColorIO/OCIOZArchive.cpp @@ -202,7 +202,18 @@ void addSupportedFiles(void * archiver, const char * path, const char * configWo } ////////////////////////////////////////////////////////////////////////////////////// -void archiveConfig(std::ostream & ostream, const Config & config, const char * configWorkingDirectory) +ArchiveFlags EnvironmentOverride(ArchiveFlags oFlags) // TODO: test override +{ + const std::string envFlag = GetEnvVariable(OCIO_ARCHIVE_FLAGS_ENVVAR); + if (!envFlag.empty()) + { + // Use 0 to allow base to be determined by the format. + oFlags = static_cast(std::stoul(envFlag, nullptr, 0)); + } + return oFlags; +} + +void archiveConfig(std::ostream & ostream, const Config & config, const char * configWorkingDirectory, ArchiveFlags flags) { void * archiver = nullptr; void *write_mem_stream = NULL; @@ -210,7 +221,9 @@ void archiveConfig(std::ostream & ostream, const Config & config, const char * c int32_t buffer_size = 0; mz_zip_file file_info; - if (!config.isArchivable()) + flags = EnvironmentOverride(flags); + + if (!config.isArchivable()) // TODO: pass in flags? { std::ostringstream os; os << "Config is not archivable."; @@ -238,8 +251,10 @@ void archiveConfig(std::ostream & ostream, const Config & config, const char * c ArchiveOptions options; // Make sure that the compression method is set to DEFLATE. options.compress_method = ArchiveCompressionMethods::DEFLATE; - // Make sure that the compression level is set to BEST. - options.compress_level = ArchiveCompressionLevels::BEST; + // Default compression level is set to BEST. + options.compress_level = flags & ARCHIVE_FLAGS_COMPRESSION_MASK + ? ArchiveCompressionLevels(flags & ARCHIVE_FLAGS_COMPRESSION_MASK) + : ArchiveCompressionLevels::BEST; // Create the writer handle. #if MZ_VERSION_BUILD >= 040000 @@ -305,7 +320,10 @@ void archiveConfig(std::ostream & ostream, const Config & config, const char * c /////////////////////// // Add all supported files to in-memory zip from any directories under working directory. // (recursive) - addSupportedFiles(archiver, configWorkingDirectory, configWorkingDirectory); + if (HasFlag(flags, ARCHIVE_FLAGS_MINIMAL)) + addSupportedFiles(archiver, configWorkingDirectory, configWorkingDirectory); + else + addSupportedFiles(archiver, configWorkingDirectory, configWorkingDirectory); // Close in-memory zip. mz_zip_writer_close(archiver); diff --git a/src/OpenColorIO/OCIOZArchive.h b/src/OpenColorIO/OCIOZArchive.h index 5bdbc6315..fc850d465 100644 --- a/src/OpenColorIO/OCIOZArchive.h +++ b/src/OpenColorIO/OCIOZArchive.h @@ -23,11 +23,13 @@ namespace OCIO_NAMESPACE * \param ostream Output stream to write the data into. * \param config Config object. * \param configWorkingDirectory Working directory of the current config. + * \param flags Archive flags used to . */ void archiveConfig( std::ostream & ostream, const Config & config, - const char * configWorkingDirectory); + const char * configWorkingDirectory, + ArchiveFlags flags); /** * \brief Get the content of a file inside an OCIOZ archive as a buffer. @@ -106,6 +108,11 @@ class CIOPOciozArchive : public ConfigIOProxy std::map m_entries; }; +// TODO: This "duplicates" flags function from Op.h could be a template? +inline bool HasFlag(ArchiveFlags flags, ArchiveFlags queryFlag) +{ + return (flags & queryFlag) == queryFlag; +} } // namespace OCIO_NAMESPACE #endif diff --git a/src/apps/ocioarchive/main.cpp b/src/apps/ocioarchive/main.cpp index 68054a6da..d7e5ecc8a 100644 --- a/src/apps/ocioarchive/main.cpp +++ b/src/apps/ocioarchive/main.cpp @@ -45,6 +45,7 @@ int main(int argc, const char **argv) // Default value is current directory. std::string extractDestination; + bool minimal = false; bool extract = false; bool list = false; bool help = false; @@ -61,6 +62,9 @@ int main(int argc, const char **argv) " ocioarchive myarchive\n\n" " # Archive myconfig/config.ocio into myarchive.ocioz\n" " ocioarchive myarchive --iconfig myconfig/config.ocio\n\n" + " # Archive from the OCIO environment variable into myarchive.ocioz\n" + " # requires all environment variables to be resolvable\n" + " ocioarchive myarchive --minimal\n\n" " # Extract myarchive.ocioz into new directory named myarchive\n" " ocioarchive --extract myarchive.ocioz\n\n" " # Extract myarchive.ocioz into new directory named ocio_config\n" @@ -70,6 +74,7 @@ int main(int argc, const char **argv) "%*", parse_end_args, "", "", "Options:", "--iconfig %s", &configFilename, "Config to archive (takes precedence over $OCIO)", + "--minimal", &minimal, "Create a minimal archive", "--extract", &extract, "Extract an OCIOZ config archive", "--dir %s", &extractDestination, "Path where to extract the files (folders are created if missing)", "--list", &list, "List the files inside an archive without extracting it", @@ -156,7 +161,11 @@ int main(int argc, const char **argv) std::ofstream ofstream(archiveName, std::ofstream::out | std::ofstream::binary); if (ofstream.good()) { - config->archive(ofstream); + OCIO::ArchiveFlags flags = OCIO::ARCHIVE_FLAGS_DEFAULT; + if (minimal) + flags = OCIO::ArchiveFlags(flags | OCIO::ARCHIVE_FLAGS_MINIMAL); + + config->archive(ofstream, flags); ofstream.close(); } else diff --git a/src/apps/ocioview/ocioview/setup.py b/src/apps/ocioview/ocioview/setup.py index 33699bad6..c3b9f48ee 100644 --- a/src/apps/ocioview/ocioview/setup.py +++ b/src/apps/ocioview/ocioview/setup.py @@ -47,6 +47,7 @@ def setup_env() -> None: ocio.OCIO_INACTIVE_COLORSPACES_ENVVAR, ocio.OCIO_OPTIMIZATION_FLAGS_ENVVAR, ocio.OCIO_USER_CATEGORIES_ENVVAR, + ocio.OCIO_ARCHIVE_FLAGS_ENVVAR, ): if env_var in os.environ: del os.environ[env_var] diff --git a/src/bindings/python/PyConfig.cpp b/src/bindings/python/PyConfig.cpp index 9534ddc35..4def23565 100644 --- a/src/bindings/python/PyConfig.cpp +++ b/src/bindings/python/PyConfig.cpp @@ -898,7 +898,7 @@ void bindPyConfig(py::module & m) .def("archive", [](ConfigRcPtr & self, const char * filepath) { std::ofstream f(filepath, std::ofstream::out | std::ofstream::binary); - self->archive(f); + self->archive(f, ARCHIVE_FLAGS_DEFAULT); // TODO: pass flags in rather than default f.close(); }, DOC(Config, archive)) diff --git a/src/bindings/python/PyTypes.cpp b/src/bindings/python/PyTypes.cpp index 5b22b5619..3bfd151bf 100644 --- a/src/bindings/python/PyTypes.cpp +++ b/src/bindings/python/PyTypes.cpp @@ -865,6 +865,7 @@ void bindPyTypes(py::module & m) m.attr("OCIO_ACTIVE_VIEWS_ENVVAR") = OCIO_ACTIVE_VIEWS_ENVVAR; m.attr("OCIO_INACTIVE_COLORSPACES_ENVVAR") = OCIO_INACTIVE_COLORSPACES_ENVVAR; m.attr("OCIO_OPTIMIZATION_FLAGS_ENVVAR") = OCIO_OPTIMIZATION_FLAGS_ENVVAR; + m.attr("OCIO_ARCHIVE_FLAGS_ENVVAR") = OCIO_ARCHIVE_FLAGS_ENVVAR; m.attr("OCIO_USER_CATEGORIES_ENVVAR") = OCIO_USER_CATEGORIES_ENVVAR; // Roles diff --git a/tests/cpu/OCIOZArchive_tests.cpp b/tests/cpu/OCIOZArchive_tests.cpp index bc1d2c497..a51d87552 100644 --- a/tests/cpu/OCIOZArchive_tests.cpp +++ b/tests/cpu/OCIOZArchive_tests.cpp @@ -498,7 +498,7 @@ OCIO_ADD_TEST(OCIOZArchive, archive_config_and_compare_to_original) // 2 - Archive the config of step 1. std::ostringstream ostringStream; - OCIO_CHECK_NO_THROW(configFromOcioFile->archive(ostringStream)); + OCIO_CHECK_NO_THROW(configFromOcioFile->archive(ostringStream, OCIO::ARCHIVE_FLAGS_DEFAULT)); // 3 - Verify that the binary data starts with "PK". OCIO_CHECK_EQUAL('P', ostringStream.str()[0]); diff --git a/tests/python/ConstantsTest.py b/tests/python/ConstantsTest.py index 852d6a907..95ededbac 100644 --- a/tests/python/ConstantsTest.py +++ b/tests/python/ConstantsTest.py @@ -16,6 +16,7 @@ def test_string_constants(self): self.assertEqual(OCIO.OCIO_ACTIVE_VIEWS_ENVVAR, 'OCIO_ACTIVE_VIEWS') self.assertEqual(OCIO.OCIO_INACTIVE_COLORSPACES_ENVVAR, 'OCIO_INACTIVE_COLORSPACES') self.assertEqual(OCIO.OCIO_OPTIMIZATION_FLAGS_ENVVAR, 'OCIO_OPTIMIZATION_FLAGS') + self.assertEqual(OCIO.OCIO_ARCHIVE_FLAGS_ENVVAR, 'OCIO_ARCHIVE_FLAGS') self.assertEqual(OCIO.OCIO_USER_CATEGORIES_ENVVAR, 'OCIO_USER_CATEGORIES') # Cache (env. variables). From b505ec925d30b0237056e3335be782d27e11e4f6 Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Tue, 16 Jan 2024 16:44:20 +0000 Subject: [PATCH 3/9] Add global comment to archives to record OCIO version used to create them Signed-off-by: Kevin Wheatley --- src/OpenColorIO/OCIOZArchive.cpp | 4 ++++ src/apps/ocioarchive/main.cpp | 29 ++++++++++++++++++----------- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/OpenColorIO/OCIOZArchive.cpp b/src/OpenColorIO/OCIOZArchive.cpp index eaf690a3a..e9f5f756e 100644 --- a/src/OpenColorIO/OCIOZArchive.cpp +++ b/src/OpenColorIO/OCIOZArchive.cpp @@ -325,6 +325,10 @@ void archiveConfig(std::ostream & ostream, const Config & config, const char * c else addSupportedFiles(archiver, configWorkingDirectory, configWorkingDirectory); + std::ostringstream comment; + comment << "Configuration written by archiveConfig() OCIO: " << GetVersion(); + mz_zip_writer_set_comment(archiver, comment.str().c_str()); + // Close in-memory zip. mz_zip_writer_close(archiver); } diff --git a/src/apps/ocioarchive/main.cpp b/src/apps/ocioarchive/main.cpp index d7e5ecc8a..b9bfc1ba1 100644 --- a/src/apps/ocioarchive/main.cpp +++ b/src/apps/ocioarchive/main.cpp @@ -123,7 +123,7 @@ int main(int argc, const char **argv) std::cerr << "ERROR: Could not load config: " << configFilename << std::endl; exit(1); } - + } else if (ocioEnv && *ocioEnv) { @@ -136,7 +136,7 @@ int main(int argc, const char **argv) catch (...) { // Capture any errors and display a custom message. - std::cerr << "ERROR: Could not load config from $OCIO variable: " + std::cerr << "ERROR: Could not load config from $OCIO variable: " << ocioEnv << std::endl; exit(1); } @@ -147,11 +147,11 @@ int main(int argc, const char **argv) exit(1); } - try + try { // The ocioz extension is added by the archive method. The assumption is that // archiveName is the filename without extension. - + // Do not add ocioz extension if already present. if (!StringUtils::EndsWith(archiveName, ".ocioz")) { @@ -163,14 +163,14 @@ int main(int argc, const char **argv) { OCIO::ArchiveFlags flags = OCIO::ARCHIVE_FLAGS_DEFAULT; if (minimal) - flags = OCIO::ArchiveFlags(flags | OCIO::ARCHIVE_FLAGS_MINIMAL); + flags = OCIO::ArchiveFlags(flags | OCIO::ARCHIVE_FLAGS_MINIMAL); config->archive(ofstream, flags); ofstream.close(); } else { - std::cerr << "Could not open output stream for: " + std::cerr << "Could not open output stream for: " << archiveName + std::string(OCIO::OCIO_CONFIG_ARCHIVE_FILE_EXT) << std::endl; exit(1); @@ -180,13 +180,13 @@ int main(int argc, const char **argv) { std::cerr << e.what() << std::endl; exit(1); - } + } } catch (OCIO::Exception & exception) { std::cerr << "ERROR: " << exception.what() << std::endl; exit(1); - } + } catch (std::exception& exception) { std::cerr << "ERROR: " << exception.what() << std::endl; @@ -210,7 +210,7 @@ int main(int argc, const char **argv) } archiveName = args[0].c_str(); - try + try { if (extractDestination.empty()) { @@ -250,7 +250,7 @@ int main(int argc, const char **argv) mz_zip_reader_create(&reader); #endif struct tm tmu_date; - + err = mz_zip_reader_open_file(reader, path.c_str()); if (err != MZ_OK) { @@ -273,7 +273,7 @@ int main(int argc, const char **argv) err = mz_zip_reader_entry_get_info(reader, &file_info); if (err != MZ_OK) { - std::cerr << "ERROR: Could not get information from entry: " << file_info->filename + std::cerr << "ERROR: Could not get information from entry: " << file_info->filename << std::endl; exit(1); } @@ -290,6 +290,13 @@ int main(int argc, const char **argv) err = mz_zip_reader_goto_next_entry(reader); } while (err == MZ_OK); + + const char *global_comment = nullptr; + if (mz_zip_reader_get_comment(reader, &global_comment) == MZ_OK) + { + if (global_comment) + std::cout << "\n" << global_comment << "\n"; + } } // Generic error handling. From f9a5f03eacaceaad2d8ccb4c98323aab8534da3b Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Tue, 16 Jan 2024 21:06:19 +0000 Subject: [PATCH 4/9] Add Basic minimal archive functionallity Needs documenting and testing May be nice to have a progress report and/or integration with OCIO Logging Signed-off-by: Kevin Wheatley --- include/OpenColorIO/OpenColorIO.h | 4 ++++ src/OpenColorIO/Config.cpp | 5 +++++ src/OpenColorIO/OCIOZArchive.cpp | 30 +++++++++++++++++++++++++++++- 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/include/OpenColorIO/OpenColorIO.h b/include/OpenColorIO/OpenColorIO.h index 3ce6f8a04..9aa8e7b55 100644 --- a/include/OpenColorIO/OpenColorIO.h +++ b/include/OpenColorIO/OpenColorIO.h @@ -13,6 +13,7 @@ #include #include #include +#include #include "OpenColorABI.h" #include "OpenColorTypes.h" @@ -1537,6 +1538,9 @@ class OCIOEXPORT Config */ void archive(std::ostream & ostream, ArchiveFlags flags) const; + //TODO: document + void GetAllFileReferences(std::set & files) const; + Config(const Config &) = delete; Config& operator= (const Config &) = delete; diff --git a/src/OpenColorIO/Config.cpp b/src/OpenColorIO/Config.cpp index c2544016a..74b75e3bd 100644 --- a/src/OpenColorIO/Config.cpp +++ b/src/OpenColorIO/Config.cpp @@ -5612,4 +5612,9 @@ void Config::archive(std::ostream & ostream, ArchiveFlags flags) const archiveConfig(ostream, *this, getCurrentContext()->getWorkingDir(), flags); } +void Config::GetAllFileReferences(std::set & files) const +{ + return getImpl()->GetAllFileReferences(files); +} + } // namespace OCIO_NAMESPACE diff --git a/src/OpenColorIO/OCIOZArchive.cpp b/src/OpenColorIO/OCIOZArchive.cpp index e9f5f756e..8d6c0b544 100644 --- a/src/OpenColorIO/OCIOZArchive.cpp +++ b/src/OpenColorIO/OCIOZArchive.cpp @@ -200,6 +200,34 @@ void addSupportedFiles(void * archiver, const char * path, const char * configWo mz_os_close_dir(dir); } } + +void addReferencedFiles(void * archiver, const Config & config) +{ + ConstContextRcPtr context = config.getCurrentContext(); + ContextRcPtr ctxFilepath = Context::Create(); + ctxFilepath->setSearchPath(context->getSearchPath()); + ctxFilepath->setWorkingDir(context->getWorkingDir()); + ctxFilepath->setConfigIOProxy(context->getConfigIOProxy()); + + auto prefixLength = std::string(context->getWorkingDir()).length() + 1; // +1 add trailing '/' TODO: improve this + + std::set files; + config.GetAllFileReferences(files); + for (const auto &file : files) + { + const std::string resolvedPath = context->resolveFileLocation(file.c_str(), ctxFilepath); + const std::string relativePath = resolvedPath.substr(prefixLength); + + auto returnCode = mz_zip_writer_add_file(archiver, resolvedPath.c_str(), relativePath.c_str()); + if (returnCode != MZ_OK) + { + std::ostringstream os; + os << "Could not write file " << resolvedPath << " to in-memory archive.()" << returnCode << ")"; + throw Exception(os.str().c_str()); + } + } +} + ////////////////////////////////////////////////////////////////////////////////////// ArchiveFlags EnvironmentOverride(ArchiveFlags oFlags) // TODO: test override @@ -321,7 +349,7 @@ void archiveConfig(std::ostream & ostream, const Config & config, const char * c // Add all supported files to in-memory zip from any directories under working directory. // (recursive) if (HasFlag(flags, ARCHIVE_FLAGS_MINIMAL)) - addSupportedFiles(archiver, configWorkingDirectory, configWorkingDirectory); + addReferencedFiles(archiver, config); else addSupportedFiles(archiver, configWorkingDirectory, configWorkingDirectory); From a3240801b8fc40fd065aa5af0730d257ac57c314 Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Mon, 22 Jan 2024 19:01:55 +0000 Subject: [PATCH 5/9] Add support for environment variables in minimal configuration archives If archiving a minimal config and environment variables are used, it must be possible for all the needed variables to be resolved and any files pointed to should also exist at the time of archiving. The values of the environment will be written into the saved configuration in the environment section of the configuration file. Ths configuration stored thus may be different to the initial config being passed for archiving. Needs tests adding and futher discussion in the TSC meetings. Signed-off-by: Kevin Wheatley --- include/OpenColorIO/OpenColorIO.h | 2 +- src/OpenColorIO/Config.cpp | 9 +-- src/OpenColorIO/OCIOZArchive.cpp | 98 ++++++++++++++++++++++--------- src/apps/ociocheck/main.cpp | 3 +- tests/cpu/OCIOZArchive_tests.cpp | 41 ++++++------- 5 files changed, 99 insertions(+), 54 deletions(-) diff --git a/include/OpenColorIO/OpenColorIO.h b/include/OpenColorIO/OpenColorIO.h index 9aa8e7b55..cbd74c577 100644 --- a/include/OpenColorIO/OpenColorIO.h +++ b/include/OpenColorIO/OpenColorIO.h @@ -1507,7 +1507,7 @@ class OCIOEXPORT Config * * \return bool Archivable if true. */ - bool isArchivable() const; + bool isArchivable(bool minimal) const; /** * \brief Archive the config and its LUTs into the specified output stream. diff --git a/src/OpenColorIO/Config.cpp b/src/OpenColorIO/Config.cpp index 74b75e3bd..b2fea6eee 100644 --- a/src/OpenColorIO/Config.cpp +++ b/src/OpenColorIO/Config.cpp @@ -5537,7 +5537,7 @@ ConfigIOProxyRcPtr Config::getConfigIOProxy() const return getImpl()->m_context->getConfigIOProxy(); } -bool Config::isArchivable() const +bool Config::isArchivable(bool minimal) const { ConstContextRcPtr context = getCurrentContext(); @@ -5550,7 +5550,7 @@ bool Config::isArchivable() const } // Utility lambda to check the following criteria. - auto validatePathForArchiving = [](const std::string & path) + auto validatePathForArchiving = [&minimal](const std::string & path) { // Using the normalized path. const std::string normPath = pystring::os::path::normpath(path); @@ -5561,8 +5561,9 @@ bool Config::isArchivable() const pystring::startswith(normPath, "..") || // 3) A context variable may not be located at the start of the path. (ContainsContextVariables(path) && //TODO: if we can resolve context, do so and validate path to file - (StringUtils::Find(path, "$") == 0 || - StringUtils::Find(path, "%") == 0))) + (!minimal && + (StringUtils::Find(path, "$") == 0 || + StringUtils::Find(path, "%") == 0)))) { return false; } diff --git a/src/OpenColorIO/OCIOZArchive.cpp b/src/OpenColorIO/OCIOZArchive.cpp index 8d6c0b544..2965ecaf2 100644 --- a/src/OpenColorIO/OCIOZArchive.cpp +++ b/src/OpenColorIO/OCIOZArchive.cpp @@ -16,6 +16,7 @@ #include "Platform.h" #include "utils/StringUtils.h" #include "transforms/FileTransform.h" +#include "Logging.h" #include "OCIOZArchive.h" @@ -183,6 +184,9 @@ void addSupportedFiles(void * archiver, const char * path, const char * configWo if (!possibleFormats.empty()) { // The extension is supported. Add the current file to the OCIOZ archive. + std::ostringstream logMessage; + logMessage << "Adding file: " << absPath; + LogInfo(logMessage.str()); if (mz_zip_writer_add_path( archiver, absPath.c_str(), configWorkingDirectory, 0, 1) != MZ_OK) @@ -201,9 +205,9 @@ void addSupportedFiles(void * archiver, const char * path, const char * configWo } } -void addReferencedFiles(void * archiver, const Config & config) +ContextRcPtr addReferencedFiles(void * archiver, const ConfigRcPtr & config) { - ConstContextRcPtr context = config.getCurrentContext(); + ConstContextRcPtr context = config->getCurrentContext(); ContextRcPtr ctxFilepath = Context::Create(); ctxFilepath->setSearchPath(context->getSearchPath()); ctxFilepath->setWorkingDir(context->getWorkingDir()); @@ -212,20 +216,42 @@ void addReferencedFiles(void * archiver, const Config & config) auto prefixLength = std::string(context->getWorkingDir()).length() + 1; // +1 add trailing '/' TODO: improve this std::set files; - config.GetAllFileReferences(files); + config->GetAllFileReferences(files); + std::set added_files; for (const auto &file : files) { const std::string resolvedPath = context->resolveFileLocation(file.c_str(), ctxFilepath); const std::string relativePath = resolvedPath.substr(prefixLength); + std::ostringstream logMessage; - auto returnCode = mz_zip_writer_add_file(archiver, resolvedPath.c_str(), relativePath.c_str()); - if (returnCode != MZ_OK) + if (added_files.find(relativePath) == added_files.end()) { - std::ostringstream os; - os << "Could not write file " << resolvedPath << " to in-memory archive.()" << returnCode << ")"; - throw Exception(os.str().c_str()); + logMessage << "Adding file: " << file << " -> " << relativePath; + LogInfo(logMessage.str()); + auto returnCode = mz_zip_writer_add_file(archiver, resolvedPath.c_str(), relativePath.c_str()); + if (returnCode != MZ_OK) + { + std::ostringstream os; + os << "Could not write file " << resolvedPath << " to in-memory archive.()" << returnCode << ")"; + throw Exception(os.str().c_str()); + } } + else + { + logMessage << "Skipping already added file: " << file << " -> " << relativePath; + LogInfo(logMessage.str()); + } + added_files.insert(relativePath); } + + const auto variableCount = ctxFilepath->getNumStringVars(); + for (auto i = 0; i != variableCount; ++i) + { + std::ostringstream logMessage; + logMessage << "Used Variable: " << ctxFilepath->getStringVarNameByIndex(i) << " -> " << ctxFilepath->getStringVarByIndex(i); + LogInfo(logMessage.str()); + } + return ctxFilepath; } ////////////////////////////////////////////////////////////////////////////////////// @@ -250,8 +276,9 @@ void archiveConfig(std::ostream & ostream, const Config & config, const char * c mz_zip_file file_info; flags = EnvironmentOverride(flags); + const bool minimal = HasFlag(flags, ARCHIVE_FLAGS_MINIMAL); - if (!config.isArchivable()) // TODO: pass in flags? + if (!config.isArchivable(minimal)) // TODO: pass in flags? { std::ostringstream os; os << "Config is not archivable."; @@ -261,11 +288,6 @@ void archiveConfig(std::ostream & ostream, const Config & config, const char * c // Initialize. memset(&file_info, 0, sizeof(file_info)); - // Retrieve and store the config as string. - std::stringstream ss; - config.serialize(ss); - std::string configStr = ss.str(); - // Write zip to memory stream. #if MZ_VERSION_BUILD >= 040000 write_mem_stream = mz_stream_mem_create(); @@ -303,10 +325,38 @@ void archiveConfig(std::ostream & ostream, const Config & config, const char * c // Open the in-memory zip. if (mz_zip_writer_open(archiver, write_mem_stream, 0) == MZ_OK) { + ConfigRcPtr editiableConfig = config.createEditableCopy(); // TODO: is this a little heavy handed just so we can modify the envronment? + /////////////////////// + // Adding LUT files // + /////////////////////// + if (minimal) + { + ContextRcPtr archivedContext = addReferencedFiles(archiver, editiableConfig); + + // Need to make sure the used evironment context is in the config + const auto variableCount = archivedContext->getNumStringVars(); + for (auto i = 0; i != variableCount; ++i) + { + editiableConfig->addEnvironmentVar(archivedContext->getStringVarNameByIndex(i), archivedContext->getStringVarByIndex(i)); + } + } + else + { + // Add all supported files to in-memory zip from any directories under working directory. + // (recursive) + addSupportedFiles(archiver, configWorkingDirectory, configWorkingDirectory); + } + + ////////////////////////////// + // Adding config to archive // + ////////////////////////////// // Use a hardcoded name for the config's filename inside the archive. std::string configFullname = std::string(OCIO_CONFIG_DEFAULT_NAME) + std::string(OCIO_CONFIG_DEFAULT_FILE_EXT); + std::stringstream ss; + editiableConfig->serialize(ss); + std::string configStr = ss.str(); // Get config string size. int32_t configSize = (int32_t) configStr.size(); @@ -316,11 +366,13 @@ void archiveConfig(std::ostream & ostream, const Config & config, const char * c file_info.version_madeby = MZ_VERSION_MADEBY; file_info.compression_method = MZ_COMPRESS_METHOD_DEFLATE; file_info.flag = MZ_ZIP_FLAG_UTF8; - file_info.uncompressed_size = configSize; + file_info.uncompressed_size = configSize; // Retrieve and store the config as string. + + constexpr uint32_t posix_attrib = 0100644; // File with -rw-r--r-- permissions + constexpr uint32_t msdos_attrib = 0200; // MSDOS equivalent + file_info.external_fa = msdos_attrib; + file_info.external_fa |= (posix_attrib << 16); - ////////////////////////////// - // Adding config to archive // - ////////////////////////////// int32_t written = 0; // Opens an entry in the in-memory zip file for writing. if (mz_zip_writer_entry_open(archiver, &file_info) == MZ_OK) @@ -343,16 +395,6 @@ void archiveConfig(std::ostream & ostream, const Config & config, const char * c throw Exception(os.str().c_str()); } - /////////////////////// - // Adding LUT files // - /////////////////////// - // Add all supported files to in-memory zip from any directories under working directory. - // (recursive) - if (HasFlag(flags, ARCHIVE_FLAGS_MINIMAL)) - addReferencedFiles(archiver, config); - else - addSupportedFiles(archiver, configWorkingDirectory, configWorkingDirectory); - std::ostringstream comment; comment << "Configuration written by archiveConfig() OCIO: " << GetVersion(); mz_zip_writer_set_comment(archiver, comment.str().c_str()); diff --git a/src/apps/ociocheck/main.cpp b/src/apps/ociocheck/main.cpp index b3c8c3385..acf8a1e66 100644 --- a/src/apps/ociocheck/main.cpp +++ b/src/apps/ociocheck/main.cpp @@ -504,6 +504,7 @@ int main(int argc, const char **argv) std::string cacheID; bool isArchivable = false; + const bool minimal = false; try { LogGuard logGuard; @@ -512,7 +513,7 @@ int main(int argc, const char **argv) std::cout << logGuard.output(); cacheID = config->getCacheID(); - isArchivable = config->isArchivable(); + isArchivable = config->isArchivable(minimal); // Passed if there are no Error level logs. StringUtils::StringVec svec = StringUtils::SplitByLines(logGuard.output()); diff --git a/tests/cpu/OCIOZArchive_tests.cpp b/tests/cpu/OCIOZArchive_tests.cpp index a51d87552..ba69be0e3 100644 --- a/tests/cpu/OCIOZArchive_tests.cpp +++ b/tests/cpu/OCIOZArchive_tests.cpp @@ -116,6 +116,7 @@ OCIO_ADD_TEST(OCIOZArchive, is_config_archivable) std::istringstream iss; iss.str(CONFIG); + const bool minimal = false; OCIO::ConfigRcPtr cfg; OCIO_CHECK_NO_THROW(cfg = OCIO::Config::CreateFromStream(iss)->createEditableCopy()); @@ -138,39 +139,39 @@ OCIO_ADD_TEST(OCIOZArchive, is_config_archivable) // Valid search path. cfg->setSearchPath("luts"); - OCIO_CHECK_EQUAL(true, cfg->isArchivable()); + OCIO_CHECK_EQUAL(true, cfg->isArchivable(minimal)); cfg->setSearchPath(R"(luts/myluts1)"); - OCIO_CHECK_EQUAL(true, cfg->isArchivable()); + OCIO_CHECK_EQUAL(true, cfg->isArchivable(minimal)); cfg->setSearchPath(R"(luts\myluts1)"); - OCIO_CHECK_EQUAL(true, cfg->isArchivable()); + OCIO_CHECK_EQUAL(true, cfg->isArchivable(minimal)); // Valid Search path starting with "./" or ".\". cfg->setSearchPath(R"(./myLuts)"); - OCIO_CHECK_EQUAL(true, cfg->isArchivable()); + OCIO_CHECK_EQUAL(true, cfg->isArchivable(minimal)); cfg->setSearchPath(R"(.\myLuts)"); - OCIO_CHECK_EQUAL(true, cfg->isArchivable()); + OCIO_CHECK_EQUAL(true, cfg->isArchivable(minimal)); // Valid search path starting with "./" or ".\" and a context variable. cfg->setSearchPath(R"(./$SHOT/myluts)"); - OCIO_CHECK_EQUAL(true, cfg->isArchivable()); + OCIO_CHECK_EQUAL(true, cfg->isArchivable(minimal)); cfg->setSearchPath(R"(.\$SHOT\myluts)"); - OCIO_CHECK_EQUAL(true, cfg->isArchivable()); + OCIO_CHECK_EQUAL(true, cfg->isArchivable(minimal)); cfg->setSearchPath(R"(luts/$SHOT)"); - OCIO_CHECK_EQUAL(true, cfg->isArchivable()); + OCIO_CHECK_EQUAL(true, cfg->isArchivable(minimal)); cfg->setSearchPath(R"(luts/$SHOT/luts1)"); - OCIO_CHECK_EQUAL(true, cfg->isArchivable()); + OCIO_CHECK_EQUAL(true, cfg->isArchivable(minimal)); cfg->setSearchPath(R"(luts\$SHOT)"); - OCIO_CHECK_EQUAL(true, cfg->isArchivable()); + OCIO_CHECK_EQUAL(true, cfg->isArchivable(minimal)); cfg->setSearchPath(R"(luts\$SHOT\luts1)"); - OCIO_CHECK_EQUAL(true, cfg->isArchivable()); + OCIO_CHECK_EQUAL(true, cfg->isArchivable(minimal)); /* * Illegal scenarios @@ -178,34 +179,34 @@ OCIO_ADD_TEST(OCIOZArchive, is_config_archivable) // Illegal search path starting with "..". cfg->setSearchPath(R"(luts:../luts)"); - OCIO_CHECK_EQUAL(false, cfg->isArchivable()); + OCIO_CHECK_EQUAL(false, cfg->isArchivable(minimal)); cfg->setSearchPath(R"(luts:..\myLuts)"); - OCIO_CHECK_EQUAL(false, cfg->isArchivable()); + OCIO_CHECK_EQUAL(false, cfg->isArchivable(minimal)); // Illegal search path starting with a context variable. cfg->setSearchPath(R"(luts:$SHOT)"); - OCIO_CHECK_EQUAL(false, cfg->isArchivable()); + OCIO_CHECK_EQUAL(false, cfg->isArchivable(minimal)); // Illegal search path with absolute path. cfg->setSearchPath(R"(luts:/luts)"); - OCIO_CHECK_EQUAL(false, cfg->isArchivable()); + OCIO_CHECK_EQUAL(false, cfg->isArchivable(minimal)); cfg->setSearchPath(R"(luts:/$SHOT)"); - OCIO_CHECK_EQUAL(false, cfg->isArchivable()); + OCIO_CHECK_EQUAL(false, cfg->isArchivable(minimal)); #ifdef _WIN32 cfg->clearSearchPaths(); cfg->addSearchPath(R"(C:\luts)"); - OCIO_CHECK_EQUAL(false, cfg->isArchivable()); + OCIO_CHECK_EQUAL(false, cfg->isArchivable(minimal)); cfg->clearSearchPaths(); cfg->addSearchPath(R"(C:\)"); - OCIO_CHECK_EQUAL(false, cfg->isArchivable()); + OCIO_CHECK_EQUAL(false, cfg->isArchivable(minimal)); cfg->clearSearchPaths(); cfg->addSearchPath(R"(C:\$SHOT)"); - OCIO_CHECK_EQUAL(false, cfg->isArchivable()); + OCIO_CHECK_EQUAL(false, cfg->isArchivable(minimal)); #endif } @@ -224,7 +225,7 @@ OCIO_ADD_TEST(OCIOZArchive, is_config_archivable) cs->setTransform(ft, OCIO::COLORSPACE_DIR_TO_REFERENCE); cfg->addColorSpace(cs); - OCIO_CHECK_EQUAL(isArchivable, cfg->isArchivable()); + OCIO_CHECK_EQUAL(isArchivable, cfg->isArchivable(minimal)); cfg->removeColorSpace("csTest"); }; From fa13166e963cb9f7cb7bfa1606708bbaa6f964fb Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Mon, 22 Jan 2024 20:37:02 +0000 Subject: [PATCH 6/9] Add minimal code to make the python tests pass Adds default parameter value for isArchivable function to bindings. This is not matching the behaviour of the C++ API. Signed-off-by: Kevin Wheatley --- src/bindings/python/PyConfig.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/bindings/python/PyConfig.cpp b/src/bindings/python/PyConfig.cpp index 4def23565..6b43ccb4c 100644 --- a/src/bindings/python/PyConfig.cpp +++ b/src/bindings/python/PyConfig.cpp @@ -894,13 +894,18 @@ void bindPyConfig(py::module & m) DOC(Config, setProcessorCacheFlags)) // Archiving - .def("isArchivable", &Config::isArchivable, DOC(Config, isArchivable)) + .def("isArchivable", [](ConfigRcPtr & self, bool minimal) + { + return self->isArchivable(minimal); + }, + py::arg("minimal") = false, + DOC(Config, isArchivable)) .def("archive", [](ConfigRcPtr & self, const char * filepath) { std::ofstream f(filepath, std::ofstream::out | std::ofstream::binary); self->archive(f, ARCHIVE_FLAGS_DEFAULT); // TODO: pass flags in rather than default f.close(); - }, + }, DOC(Config, archive)) // Conversion to string From 0d14c738f9e182937e95b08337abedf2c1e503d3 Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Tue, 23 Jan 2024 11:30:33 +0000 Subject: [PATCH 7/9] Add minimal boolean to lambda captures for MSVC Signed-off-by: Kevin Wheatley --- tests/cpu/OCIOZArchive_tests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cpu/OCIOZArchive_tests.cpp b/tests/cpu/OCIOZArchive_tests.cpp index ba69be0e3..7f796e8bc 100644 --- a/tests/cpu/OCIOZArchive_tests.cpp +++ b/tests/cpu/OCIOZArchive_tests.cpp @@ -214,7 +214,7 @@ OCIO_ADD_TEST(OCIOZArchive, is_config_archivable) cfg->clearSearchPaths(); // Lambda function to facilitate adding a new FileTransform to a config. - auto addFTAndTestIsArchivable = [&cfg](const std::string & path, bool isArchivable) + auto addFTAndTestIsArchivable = [&cfg, &minimal](const std::string & path, bool isArchivable) { std::string fullPath = pystring::os::path::join(path, "fake_lut.clf"); auto ft = OCIO::FileTransform::Create(); From 9a471c0f61f5f19fb8593b2a0efc5d6298445c6b Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Tue, 23 Jan 2024 11:54:59 +0000 Subject: [PATCH 8/9] Rework lambda to pass MSVC and GCC MSVC thinks you need the capture, GCC thinks the capture is not needed, so pass variable directly into the lambda function call. Ugly to pass several booleans, but should work. Signed-off-by: Kevin Wheatley --- tests/cpu/OCIOZArchive_tests.cpp | 40 ++++++++++++++++---------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/tests/cpu/OCIOZArchive_tests.cpp b/tests/cpu/OCIOZArchive_tests.cpp index 7f796e8bc..00c37d8ba 100644 --- a/tests/cpu/OCIOZArchive_tests.cpp +++ b/tests/cpu/OCIOZArchive_tests.cpp @@ -214,7 +214,7 @@ OCIO_ADD_TEST(OCIOZArchive, is_config_archivable) cfg->clearSearchPaths(); // Lambda function to facilitate adding a new FileTransform to a config. - auto addFTAndTestIsArchivable = [&cfg, &minimal](const std::string & path, bool isArchivable) + auto addFTAndTestIsArchivable = [&cfg](const std::string & path, bool isArchivable, bool minimal) { std::string fullPath = pystring::os::path::join(path, "fake_lut.clf"); auto ft = OCIO::FileTransform::Create(); @@ -237,41 +237,41 @@ OCIO_ADD_TEST(OCIOZArchive, is_config_archivable) */ // Valid FileTransform path. - addFTAndTestIsArchivable("luts", true); - addFTAndTestIsArchivable(R"(luts/myluts1)", true); - addFTAndTestIsArchivable(R"(luts\myluts1)", true); + addFTAndTestIsArchivable("luts", true, false); + addFTAndTestIsArchivable(R"(luts/myluts1)", true, false); + addFTAndTestIsArchivable(R"(luts\myluts1)", true, false); // Valid Search path starting with "./" or ".\". - addFTAndTestIsArchivable(R"(./myLuts)", true); - addFTAndTestIsArchivable(R"(.\myLuts)", true); + addFTAndTestIsArchivable(R"(./myLuts)", true, false); + addFTAndTestIsArchivable(R"(.\myLuts)", true, false); // Valid search path starting with "./" or ".\" and a context variable. - addFTAndTestIsArchivable(R"(./$SHOT/myluts)", true); - addFTAndTestIsArchivable(R"(.\$SHOT\myluts)", true); - addFTAndTestIsArchivable(R"(luts/$SHOT)", true); - addFTAndTestIsArchivable(R"(luts/$SHOT/luts1)", true); - addFTAndTestIsArchivable(R"(luts\$SHOT)", true); - addFTAndTestIsArchivable(R"(luts\$SHOT\luts1)", true); + addFTAndTestIsArchivable(R"(./$SHOT/myluts)", true, false); + addFTAndTestIsArchivable(R"(.\$SHOT\myluts)", true, false); + addFTAndTestIsArchivable(R"(luts/$SHOT)", true, false); + addFTAndTestIsArchivable(R"(luts/$SHOT/luts1)", true, false); + addFTAndTestIsArchivable(R"(luts\$SHOT)", true, false); + addFTAndTestIsArchivable(R"(luts\$SHOT\luts1)", true, false); /* * Illegal scenarios */ // Illegal search path starting with "..". - addFTAndTestIsArchivable(R"(../luts)", false); - addFTAndTestIsArchivable(R"(..\myLuts)", false); + addFTAndTestIsArchivable(R"(../luts)", false, false); + addFTAndTestIsArchivable(R"(..\myLuts)", false, false); // Illegal search path starting with a context variable. - addFTAndTestIsArchivable(R"($SHOT)", false); + addFTAndTestIsArchivable(R"($SHOT)", false, false); // Illegal search path with absolute path. - addFTAndTestIsArchivable(R"(/luts)", false); - addFTAndTestIsArchivable(R"(/$SHOT)", false); + addFTAndTestIsArchivable(R"(/luts)", false, false); + addFTAndTestIsArchivable(R"(/$SHOT)", false, false); #ifdef _WIN32 - addFTAndTestIsArchivable(R"(C:\luts)", false); - addFTAndTestIsArchivable(R"(C:\)", false); - addFTAndTestIsArchivable(R"(\$SHOT)", false); + addFTAndTestIsArchivable(R"(C:\luts)", false, false); + addFTAndTestIsArchivable(R"(C:\)", false, false); + addFTAndTestIsArchivable(R"(\$SHOT)", false, false); #endif } } From b42159f1422e8d825c48636fc40df467ef758556 Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Fri, 20 Dec 2024 11:26:37 +0000 Subject: [PATCH 9/9] Constify the flags Signed-off-by: Kevin Wheatley --- include/OpenColorIO/OpenColorIO.h | 2 +- src/OpenColorIO/Config.cpp | 2 +- src/OpenColorIO/OCIOZArchive.cpp | 4 ++-- src/OpenColorIO/OCIOZArchive.h | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/include/OpenColorIO/OpenColorIO.h b/include/OpenColorIO/OpenColorIO.h index cbd74c577..d30ff3d13 100644 --- a/include/OpenColorIO/OpenColorIO.h +++ b/include/OpenColorIO/OpenColorIO.h @@ -1536,7 +1536,7 @@ class OCIOEXPORT Config * \param ostream The output stream to write to. * \param flags Flags top control archive creation */ - void archive(std::ostream & ostream, ArchiveFlags flags) const; + void archive(std::ostream & ostream, const ArchiveFlags & flags) const; //TODO: document void GetAllFileReferences(std::set & files) const; diff --git a/src/OpenColorIO/Config.cpp b/src/OpenColorIO/Config.cpp index b2fea6eee..52679b693 100644 --- a/src/OpenColorIO/Config.cpp +++ b/src/OpenColorIO/Config.cpp @@ -5607,7 +5607,7 @@ bool Config::isArchivable(bool minimal) const return true; } -void Config::archive(std::ostream & ostream, ArchiveFlags flags) const +void Config::archive(std::ostream & ostream, const ArchiveFlags & flags) const { // Using utility functions in OCIOZArchive.cpp. archiveConfig(ostream, *this, getCurrentContext()->getWorkingDir(), flags); diff --git a/src/OpenColorIO/OCIOZArchive.cpp b/src/OpenColorIO/OCIOZArchive.cpp index 2965ecaf2..aafbccaef 100644 --- a/src/OpenColorIO/OCIOZArchive.cpp +++ b/src/OpenColorIO/OCIOZArchive.cpp @@ -267,7 +267,7 @@ ArchiveFlags EnvironmentOverride(ArchiveFlags oFlags) // TODO: test override return oFlags; } -void archiveConfig(std::ostream & ostream, const Config & config, const char * configWorkingDirectory, ArchiveFlags flags) +void archiveConfig(std::ostream & ostream, const Config & config, const char * configWorkingDirectory, const ArchiveFlags & archiveFlags) { void * archiver = nullptr; void *write_mem_stream = NULL; @@ -275,7 +275,7 @@ void archiveConfig(std::ostream & ostream, const Config & config, const char * c int32_t buffer_size = 0; mz_zip_file file_info; - flags = EnvironmentOverride(flags); + ArchiveFlags flags = EnvironmentOverride(archiveFlags); const bool minimal = HasFlag(flags, ARCHIVE_FLAGS_MINIMAL); if (!config.isArchivable(minimal)) // TODO: pass in flags? diff --git a/src/OpenColorIO/OCIOZArchive.h b/src/OpenColorIO/OCIOZArchive.h index fc850d465..015ddc3bf 100644 --- a/src/OpenColorIO/OCIOZArchive.h +++ b/src/OpenColorIO/OCIOZArchive.h @@ -23,13 +23,13 @@ namespace OCIO_NAMESPACE * \param ostream Output stream to write the data into. * \param config Config object. * \param configWorkingDirectory Working directory of the current config. - * \param flags Archive flags used to . + * \param archiveFlags Archive flags used to configure archive generation. */ void archiveConfig( std::ostream & ostream, const Config & config, const char * configWorkingDirectory, - ArchiveFlags flags); + const ArchiveFlags & archiveFlags); /** * \brief Get the content of a file inside an OCIOZ archive as a buffer.