From ccacce39c39d16977ab6c1592fcb6e941b05ddff Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sun, 26 Sep 2021 00:09:04 +0200 Subject: [PATCH] Add config location versioning (#2090). --- src/lib/analytics.cc | 6 +-- src/lib/config.cc | 84 ++++++++++++++++++++---------- src/lib/config.h | 6 ++- src/lib/cross.h | 2 +- src/lib/cross_linux.cc | 5 +- src/lib/cross_osx.cc | 5 +- src/lib/cross_windows.cc | 5 +- src/lib/film.cc | 2 +- src/lib/state.cc | 59 +++++++++++++++++---- src/lib/state.h | 3 +- src/tools/dcpomatic.cc | 2 +- src/tools/dcpomatic_disk.cc | 2 +- src/tools/dcpomatic_disk_writer.cc | 4 +- src/wx/full_config_dialog.cc | 6 +-- test/config_test.cc | 59 ++++++++++++++------- test/data | 2 +- 16 files changed, 176 insertions(+), 76 deletions(-) diff --git a/src/lib/analytics.cc b/src/lib/analytics.cc index ac0abc222..7483166ba 100644 --- a/src/lib/analytics.cc +++ b/src/lib/analytics.cc @@ -96,11 +96,11 @@ Analytics::write () const root->add_child("SuccessfulDCPEncodes")->add_child_text(raw_convert(_successful_dcp_encodes)); try { - doc.write_to_file_formatted(path("analytics.xml").string()); + doc.write_to_file_formatted(write_path("analytics.xml").string()); } catch (xmlpp::exception& e) { string s = e.what (); trim (s); - throw FileError (s, path("analytics.xml")); + throw FileError (s, write_path("analytics.xml")); } } @@ -110,7 +110,7 @@ Analytics::read () try { cxml::Document f ("Analytics"); - f.read_file (path("analytics.xml")); + f.read_file (read_path("analytics.xml")); _successful_dcp_encodes = f.number_child("SuccessfulDCPEncodes"); } catch (...) { /* Never mind */ diff --git a/src/lib/config.cc b/src/lib/config.cc index 15abff391..4aa4788bf 100644 --- a/src/lib/config.cc +++ b/src/lib/config.cc @@ -125,8 +125,8 @@ Config::set_defaults () #ifdef DCPOMATIC_WINDOWS _win32_console = false; #endif - _cinemas_file = path ("cinemas.xml"); - _dkdm_recipients_file = path ("dkdm_recipients.xml"); + _cinemas_file = read_path ("cinemas.xml"); + _dkdm_recipients_file = read_path ("dkdm_recipients.xml"); _show_hints_before_make_dcp = true; _confirm_kdm_email = true; _kdm_container_name_format = dcp::NameFormat ("KDM %f %c"); @@ -216,13 +216,13 @@ Config::backup () /* Make a copy of the configuration */ try { int n = 1; - while (n < 100 && boost::filesystem::exists(path(String::compose("config.xml.%1", n)))) { + while (n < 100 && boost::filesystem::exists(write_path(String::compose("config.xml.%1", n)))) { ++n; } - boost::filesystem::copy_file(path("config.xml", false), path(String::compose("config.xml.%1", n), false)); - boost::filesystem::copy_file(path("cinemas.xml", false), path(String::compose("cinemas.xml.%1", n), false)); - boost::filesystem::copy_file(path("dkdm_recipients.xml", false), path(String::compose("dkdm_recipients.xml.%1", n), false)); + boost::filesystem::copy_file(read_path("config.xml"), write_path(String::compose("config.xml.%1", n))); + boost::filesystem::copy_file(read_path("cinemas.xml"), write_path(String::compose("cinemas.xml.%1", n))); + boost::filesystem::copy_file(read_path("dkdm_recipients.xml"), write_path(String::compose("dkdm_recipients.xml.%1", n))); } catch (...) {} } @@ -231,7 +231,7 @@ Config::read () try { cxml::Document f ("Config"); - f.read_file (config_file ()); + f.read_file (config_read_file()); auto version = f.optional_number_child ("Version"); if (version && *version < _current_version) { @@ -467,8 +467,8 @@ try _dkdms->add (DKDMBase::read (i)); } } - _cinemas_file = f.optional_string_child("CinemasFile").get_value_or (path ("cinemas.xml").string ()); - _dkdm_recipients_file = f.optional_string_child("DKDMRecipientsFile").get_value_or (path("dkdm_recipients.xml").string()); + _cinemas_file = f.optional_string_child("CinemasFile").get_value_or(read_path("cinemas.xml").string()); + _dkdm_recipients_file = f.optional_string_child("DKDMRecipientsFile").get_value_or(read_path("dkdm_recipients.xml").string()); _show_hints_before_make_dcp = f.optional_bool_child("ShowHintsBeforeMakeDCP").get_value_or (true); _confirm_kdm_email = f.optional_bool_child("ConfirmKDMEmail").get_value_or (true); _kdm_container_name_format = dcp::NameFormat (f.optional_string_child("KDMContainerNameFormat").get_value_or ("KDM %f %c")); @@ -993,21 +993,23 @@ Config::write_config () const root->add_child("AddFilesPath")->add_child_text(_add_files_path->string()); } + auto target = config_write_file(); + try { auto const s = doc.write_to_string_formatted (); - boost::filesystem::path tmp (string(config_file().string()).append(".tmp")); + boost::filesystem::path tmp (string(target.string()).append(".tmp")); auto f = fopen_boost (tmp, "w"); if (!f) { throw FileError (_("Could not open file for writing"), tmp); } checked_fwrite (s.c_str(), s.bytes(), f, tmp); fclose (f); - boost::filesystem::remove (config_file()); - boost::filesystem::rename (tmp, config_file()); + boost::filesystem::remove (target); + boost::filesystem::rename (tmp, target); } catch (xmlpp::exception& e) { string s = e.what (); trim (s); - throw FileError (s, config_file()); + throw FileError (s, target); } } @@ -1201,12 +1203,14 @@ Config::clean_history_internal (vector& h) } } + bool Config::have_existing (string file) { - return boost::filesystem::exists (path (file, false)); + return boost::filesystem::exists (read_path(file)); } + void Config::read_cinemas (cxml::Document const & f) { @@ -1273,18 +1277,19 @@ Config::set_dkdm_recipients_file (boost::filesystem::path file) void Config::save_template (shared_ptr film, string name) const { - film->write_template (template_path (name)); + film->write_template (template_write_path(name)); } + list Config::templates () const { - if (!boost::filesystem::exists (path ("templates"))) { + if (!boost::filesystem::exists(read_path("templates"))) { return {}; } list n; - for (auto const& i: boost::filesystem::directory_iterator(path("templates"))) { + for (auto const& i: boost::filesystem::directory_iterator(read_path("templates"))) { n.push_back (i.path().filename().string()); } return n; @@ -1293,33 +1298,41 @@ Config::templates () const bool Config::existing_template (string name) const { - return boost::filesystem::exists (template_path (name)); + return boost::filesystem::exists (template_read_path(name)); } + boost::filesystem::path -Config::template_path (string name) const +Config::template_read_path (string name) const { - return path("templates") / tidy_for_filename (name); + return read_path("templates") / tidy_for_filename (name); } + +boost::filesystem::path +Config::template_write_path (string name) const +{ + return write_path("templates") / tidy_for_filename (name); +} + + void Config::rename_template (string old_name, string new_name) const { - boost::filesystem::rename (template_path (old_name), template_path (new_name)); + boost::filesystem::rename (template_read_path(old_name), template_write_path(new_name)); } void Config::delete_template (string name) const { - boost::filesystem::remove (template_path (name)); + boost::filesystem::remove (template_write_path(name)); } /** @return Path to the config.xml containing the actual settings, following a link if required */ boost::filesystem::path -Config::config_file () +config_file (boost::filesystem::path main) { cxml::Document f ("Config"); - auto main = path("config.xml", false); if (!boost::filesystem::exists (main)) { /* It doesn't exist, so there can't be any links; just return it */ return main; @@ -1341,6 +1354,21 @@ Config::config_file () return main; } + +boost::filesystem::path +Config::config_read_file () +{ + return config_file (read_path("config.xml")); +} + + +boost::filesystem::path +Config::config_write_file () +{ + return config_file (write_path("config.xml")); +} + + void Config::reset_cover_sheet () { @@ -1354,11 +1382,11 @@ Config::link (boost::filesystem::path new_file) const xmlpp::Document doc; doc.create_root_node("Config")->add_child("Link")->add_child_text(new_file.string()); try { - doc.write_to_file_formatted(path("config.xml", true).string()); + doc.write_to_file_formatted(write_path("config.xml").string()); } catch (xmlpp::exception& e) { string s = e.what (); trim (s); - throw FileError (s, path("config.xml")); + throw FileError (s, write_path("config.xml")); } } @@ -1366,14 +1394,14 @@ void Config::copy_and_link (boost::filesystem::path new_file) const { write (); - boost::filesystem::copy_file (config_file(), new_file, boost::filesystem::copy_option::overwrite_if_exists); + boost::filesystem::copy_file (config_read_file(), new_file, boost::filesystem::copy_option::overwrite_if_exists); link (new_file); } bool Config::have_write_permission () const { - auto f = fopen_boost (config_file(), "r+"); + auto f = fopen_boost (config_write_file(), "r+"); if (!f) { return false; } diff --git a/src/lib/config.h b/src/lib/config.h index 46bd390cc..5981e7994 100644 --- a/src/lib/config.h +++ b/src/lib/config.h @@ -1095,7 +1095,8 @@ public: void save_template (std::shared_ptr film, std::string name) const; bool existing_template (std::string name) const; std::list templates () const; - boost::filesystem::path template_path (std::string name) const; + boost::filesystem::path template_read_path (std::string name) const; + boost::filesystem::path template_write_path (std::string name) const; void rename_template (std::string old_name, std::string new_name) const; void delete_template (std::string name) const; @@ -1103,7 +1104,8 @@ public: static void drop (); static void restore_defaults (); static bool have_existing (std::string); - static boost::filesystem::path config_file (); + static boost::filesystem::path config_read_file (); + static boost::filesystem::path config_write_file (); private: Config (); diff --git a/src/lib/cross.h b/src/lib/cross.h index 919113305..ed1d0c8e7 100644 --- a/src/lib/cross.h +++ b/src/lib/cross.h @@ -64,7 +64,7 @@ extern int avio_open_boost (AVIOContext** s, boost::filesystem::path file, int f extern boost::filesystem::path home_directory (); extern bool running_32_on_64 (); extern void unprivileged (); -extern boost::filesystem::path config_path (); +extern boost::filesystem::path config_path (boost::optional version); extern boost::filesystem::path directory_containing_executable (); extern boost::filesystem::path fix_long_path (boost::filesystem::path path); extern bool show_in_file_manager (boost::filesystem::path dir, boost::filesystem::path select); diff --git a/src/lib/cross_linux.cc b/src/lib/cross_linux.cc index 2fcec2891..ee49d50bc 100644 --- a/src/lib/cross_linux.cc +++ b/src/lib/cross_linux.cc @@ -379,11 +379,14 @@ Drive::unmount () boost::filesystem::path -config_path () +config_path (optional version) { boost::filesystem::path p; p /= g_get_user_config_dir (); p /= "dcpomatic2"; + if (version) { + p /= *version; + } return p; } diff --git a/src/lib/cross_osx.cc b/src/lib/cross_osx.cc index d0cb9f216..ff40ffb70 100644 --- a/src/lib/cross_osx.cc +++ b/src/lib/cross_osx.cc @@ -552,7 +552,7 @@ Drive::get () boost::filesystem::path -config_path () +config_path (optional version) { boost::filesystem::path p; p /= g_get_home_dir (); @@ -560,6 +560,9 @@ config_path () p /= "Preferences"; p /= "com.dcpomatic"; p /= "2"; + if (version) { + p /= *version; + } return p; } diff --git a/src/lib/cross_windows.cc b/src/lib/cross_windows.cc index 723828d7e..b3d9a1558 100644 --- a/src/lib/cross_windows.cc +++ b/src/lib/cross_windows.cc @@ -672,11 +672,14 @@ Drive::unmount () boost::filesystem::path -config_path () +config_path (optional version) { boost::filesystem::path p; p /= g_get_user_config_dir (); p /= "dcpomatic2"; + if (version) { + p /= *version; + } return p; } diff --git a/src/lib/film.cc b/src/lib/film.cc index 3e6430ee3..0d277b82a 100644 --- a/src/lib/film.cc +++ b/src/lib/film.cc @@ -1921,7 +1921,7 @@ void Film::use_template (string name) { _template_film.reset (new Film (optional())); - _template_film->read_metadata (Config::instance()->template_path (name)); + _template_film->read_metadata (Config::instance()->template_read_path(name)); _use_isdcf_name = _template_film->_use_isdcf_name; _dcp_content_type = _template_film->_dcp_content_type; _container = _template_film->_container; diff --git a/src/lib/state.cc b/src/lib/state.cc index 5f7e9a701..e22f9e0b9 100644 --- a/src/lib/state.cc +++ b/src/lib/state.cc @@ -19,33 +19,70 @@ */ -#include "state.h" #include "cross.h" +#include "state.h" +#include "util.h" #include using std::string; +using boost::optional; boost::optional State::override_path; +/* List of config versions to look for in descending order of preference; + * i.e. look at the first one, and if that doesn't exist, try the second, etc. + */ +static std::vector config_versions = { "2.16" }; + + +static +boost::filesystem::path +config_path_or_override (optional version) +{ + if (State::override_path) { + auto p = *State::override_path; + if (version) { + p /= *version; + } + return p; + } + + return config_path (version); +} + + /** @param file State filename - * @return Full path to write @file to. + * @return Full path to read @file from. */ boost::filesystem::path -State::path (string file, bool create_directories) +State::read_path (string file) { - boost::filesystem::path p; - if (override_path) { - p = *override_path; - } else { - p = config_path (); + using namespace boost::filesystem; + + for (auto i: config_versions) { + auto full = config_path_or_override(i) / file; + if (exists(full)) { + return full; + } } + + return config_path_or_override({}) / file; +} + + +/** @param file State filename + * @return Full path to write @file to. + */ +boost::filesystem::path +State::write_path (string file) +{ + boost::filesystem::path p = config_path_or_override(config_versions.front()); boost::system::error_code ec; - if (create_directories) { - boost::filesystem::create_directories (p, ec); - } + boost::filesystem::create_directories (p, ec); p /= file; return p; } + diff --git a/src/lib/state.h b/src/lib/state.h index f1ed775a2..9338aae0c 100644 --- a/src/lib/state.h +++ b/src/lib/state.h @@ -40,7 +40,8 @@ public: /** If set, this overrides the standard path (in home, Library, AppData or wherever) for config.xml, cinemas.xml etc. */ static boost::optional override_path; - static boost::filesystem::path path (std::string file, bool create_directories = true); + static boost::filesystem::path read_path (std::string file); + static boost::filesystem::path write_path (std::string file); }; diff --git a/src/tools/dcpomatic.cc b/src/tools/dcpomatic.cc index ca45963f4..0b7f632e1 100644 --- a/src/tools/dcpomatic.cc +++ b/src/tools/dcpomatic.cc @@ -933,7 +933,7 @@ private: _("You are making a DKDM which is encrypted by a private key held in" "\n\n%s\n\nIt is VITALLY IMPORTANT " "that you BACK UP THIS FILE since if it is lost " - "your DKDMs (and the DCPs they protect) will become useless."), std_to_wx(Config::config_file().string()).data() + "your DKDMs (and the DCPs they protect) will become useless."), std_to_wx(Config::config_read_file().string()).data() ) ); diff --git a/src/tools/dcpomatic_disk.cc b/src/tools/dcpomatic_disk.cc index 7e43c0684..ec5d8e782 100644 --- a/src/tools/dcpomatic_disk.cc +++ b/src/tools/dcpomatic_disk.cc @@ -139,7 +139,7 @@ public: /* XXX: this is a hack, but I expect we'll need logs and I'm not sure if there's * a better place to put them. */ - dcpomatic_log.reset(new FileLog(config_path() / "disk.log")); + dcpomatic_log = make_shared(State::write_path("disk.log")); dcpomatic_log->set_types (dcpomatic_log->types() | LogEntry::TYPE_DISK); LOG_DISK("dcpomatic_disk %1 started", dcpomatic_git_commit); diff --git a/src/tools/dcpomatic_disk_writer.cc b/src/tools/dcpomatic_disk_writer.cc index c638b72eb..ef384bbba 100644 --- a/src/tools/dcpomatic_disk_writer.cc +++ b/src/tools/dcpomatic_disk_writer.cc @@ -27,7 +27,9 @@ #include "lib/exceptions.h" #include "lib/ext.h" #include "lib/file_log.h" +#include "lib/state.h" #include "lib/nanomsg.h" +#include "lib/util.h" #include "lib/version.h" #include "lib/warnings.h" @@ -288,7 +290,7 @@ main () /* XXX: this is a hack, but I expect we'll need logs and I'm not sure if there's * a better place to put them. */ - dcpomatic_log.reset(new FileLog(config_path() / "disk_writer.log", LogEntry::TYPE_DISK)); + dcpomatic_log.reset(new FileLog(State::write_path("disk_writer.log"), LogEntry::TYPE_DISK)); LOG_DISK_NC("dcpomatic_disk_writer started"); #endif diff --git a/src/wx/full_config_dialog.cc b/src/wx/full_config_dialog.cc index 1d0c1e01a..dcfcf394b 100644 --- a/src/wx/full_config_dialog.cc +++ b/src/wx/full_config_dialog.cc @@ -153,7 +153,7 @@ private: checked_set (_analyse_ebur128, config->analyse_ebur128 ()); #endif checked_set (_automatic_audio_analysis, config->automatic_audio_analysis ()); - checked_set (_config_file, config->config_file()); + checked_set (_config_file, config->config_read_file()); checked_set (_cinemas_file, config->cinemas_file()); GeneralPage::config_changed (); @@ -198,7 +198,7 @@ private: { auto config = Config::instance(); boost::filesystem::path new_file = wx_to_std(_config_file->GetPath()); - if (new_file == config->config_file()) { + if (new_file == config->config_read_file()) { return; } bool copy_and_link = true; @@ -212,7 +212,7 @@ private: if (copy_and_link) { config->write (); - if (new_file != config->config_file()) { + if (new_file != config->config_read_file()) { config->copy_and_link (new_file); } } else { diff --git a/test/config_test.cc b/test/config_test.cc index d78b9357b..173c95cbf 100644 --- a/test/config_test.cc +++ b/test/config_test.cc @@ -32,11 +32,11 @@ static void rewrite_bad_config () { boost::system::error_code ec; - boost::filesystem::remove ("build/test/bad_config/config.xml", ec); + boost::filesystem::remove ("build/test/bad_config/2.16/config.xml", ec); Config::override_path = "build/test/bad_config"; - boost::filesystem::create_directories ("build/test/bad_config"); - ofstream f ("build/test/bad_config/config.xml"); + boost::filesystem::create_directories ("build/test/bad_config/2.16"); + ofstream f ("build/test/bad_config/2.16/config.xml"); f << "\n" << "\n" << "\n" @@ -57,37 +57,37 @@ BOOST_AUTO_TEST_CASE (config_backup_test) Config::instance(); - BOOST_CHECK ( boost::filesystem::exists ("build/test/bad_config/config.xml.1")); - BOOST_CHECK (!boost::filesystem::exists ("build/test/bad_config/config.xml.2")); - BOOST_CHECK (!boost::filesystem::exists ("build/test/bad_config/config.xml.3")); - BOOST_CHECK (!boost::filesystem::exists ("build/test/bad_config/config.xml.4")); + BOOST_CHECK ( boost::filesystem::exists("build/test/bad_config/2.16/config.xml.1")); + BOOST_CHECK (!boost::filesystem::exists("build/test/bad_config/2.16/config.xml.2")); + BOOST_CHECK (!boost::filesystem::exists("build/test/bad_config/2.16/config.xml.3")); + BOOST_CHECK (!boost::filesystem::exists("build/test/bad_config/2.16/config.xml.4")); Config::drop(); rewrite_bad_config(); Config::instance(); - BOOST_CHECK ( boost::filesystem::exists ("build/test/bad_config/config.xml.1")); - BOOST_CHECK ( boost::filesystem::exists ("build/test/bad_config/config.xml.2")); - BOOST_CHECK (!boost::filesystem::exists ("build/test/bad_config/config.xml.3")); - BOOST_CHECK (!boost::filesystem::exists ("build/test/bad_config/config.xml.4")); + BOOST_CHECK ( boost::filesystem::exists("build/test/bad_config/2.16/config.xml.1")); + BOOST_CHECK ( boost::filesystem::exists("build/test/bad_config/2.16/config.xml.2")); + BOOST_CHECK (!boost::filesystem::exists("build/test/bad_config/2.16/config.xml.3")); + BOOST_CHECK (!boost::filesystem::exists("build/test/bad_config/2.16/config.xml.4")); Config::drop(); rewrite_bad_config(); Config::instance(); - BOOST_CHECK ( boost::filesystem::exists ("build/test/bad_config/config.xml.1")); - BOOST_CHECK ( boost::filesystem::exists ("build/test/bad_config/config.xml.2")); - BOOST_CHECK ( boost::filesystem::exists ("build/test/bad_config/config.xml.3")); - BOOST_CHECK (!boost::filesystem::exists ("build/test/bad_config/config.xml.4")); + BOOST_CHECK ( boost::filesystem::exists("build/test/bad_config/2.16/config.xml.1")); + BOOST_CHECK ( boost::filesystem::exists("build/test/bad_config/2.16/config.xml.2")); + BOOST_CHECK ( boost::filesystem::exists("build/test/bad_config/2.16/config.xml.3")); + BOOST_CHECK (!boost::filesystem::exists("build/test/bad_config/2.16/config.xml.4")); Config::drop(); rewrite_bad_config(); Config::instance(); - BOOST_CHECK (boost::filesystem::exists ("build/test/bad_config/config.xml.1")); - BOOST_CHECK (boost::filesystem::exists ("build/test/bad_config/config.xml.2")); - BOOST_CHECK (boost::filesystem::exists ("build/test/bad_config/config.xml.3")); - BOOST_CHECK (boost::filesystem::exists ("build/test/bad_config/config.xml.4")); + BOOST_CHECK (boost::filesystem::exists("build/test/bad_config/2.16/config.xml.1")); + BOOST_CHECK (boost::filesystem::exists("build/test/bad_config/2.16/config.xml.2")); + BOOST_CHECK (boost::filesystem::exists("build/test/bad_config/2.16/config.xml.3")); + BOOST_CHECK (boost::filesystem::exists("build/test/bad_config/2.16/config.xml.4")); /* This test has called Config::set_defaults(), so take us back to the config that we want for our tests. @@ -112,3 +112,24 @@ BOOST_AUTO_TEST_CASE (config_write_utf8_test) setup_test_config (); } + +BOOST_AUTO_TEST_CASE (config_upgrade_test) +{ + boost::filesystem::path dir = "build/test/config_upgrade_test"; + Config::override_path = dir; + Config::drop (); + boost::filesystem::remove_all (dir); + boost::filesystem::create_directories (dir); + + boost::filesystem::copy_file ("test/data/2.14.config.xml", dir / "config.xml"); + boost::filesystem::copy_file ("test/data/2.14.cinemas.xml", dir / "cinemas.xml"); + Config::instance(); + Config::instance()->write(); + + check_xml (dir / "config.xml", "test/data/2.14.config.xml", {}); + check_xml (dir / "cinemas.xml", "test/data/2.14.cinemas.xml", {}); + check_xml (dir / "2.16" / "config.xml", "test/data/2.16.config.xml", {}); + /* cinemas.xml is not copied into 2.16 as its format has not changed */ + BOOST_REQUIRE (!boost::filesystem::exists(dir / "2.16" / "cinemas.xml")); +} + diff --git a/test/data b/test/data index d8d9a1eed..56b37afdf 160000 --- a/test/data +++ b/test/data @@ -1 +1 @@ -Subproject commit d8d9a1eedc24b5f67f377b6e87e649cdee6fc3b7 +Subproject commit 56b37afdf96ecc83752ce70af061ee6c7ed4f78b -- 2.30.2