Fixed loading of system presets with incompatible system profile keys

before the "reconfigure" dialog is shown.

Replaced boost::filesystem::copy_file() with Slic3r::copy_file()
in config snapshot loading code.

src/libslic3r/Config.cpp

@@ -551,7 +551,8 @@ bool ConfigBase::set_deserialize_raw(const t_config_option_key &opt_key_src, con
         else
             opt->set(optdef->default_value.get());
 
-        if (substitutions_ctxt.rule == ForwardCompatibilitySubstitutionRule::Enable) {
+        if (substitutions_ctxt.rule == ForwardCompatibilitySubstitutionRule::Enable ||
+            substitutions_ctxt.rule == ForwardCompatibilitySubstitutionRule::EnableSystemSilent) {
             // Log the substitution.
             ConfigSubstitution config_substitution;
             config_substitution.opt_def = optdef;

src/libslic3r/Config.hpp

@@ -123,9 +123,16 @@ enum PrinterTechnology : unsigned char
 
 enum ForwardCompatibilitySubstitutionRule
 {
+    // Disable susbtitution, throw exception if an option value is not recognized.
     Disable,
+    // Enable substitution of an unknown option value with default. Log the substitution.
     Enable,
+    // Enable substitution of an unknown option value with default. Don't log the substitution.
     EnableSilent,
+    // Enable substitution of an unknown option value with default. Log substitutions in user profiles, don't log substitutions in system profiles.
+    EnableSystemSilent,
+    // Enable silent substitution of an unknown option value with default when loading user profiles. Throw on an unknown option value in a system profile.
+    EnableSilentDisableSystem,
 };
 
 class  ConfigOption;

src/libslic3r/PresetBundle.cpp

@@ -165,7 +165,9 @@ void PresetBundle::setup_directories()
 PresetsConfigSubstitutions PresetBundle::load_presets(AppConfig &config, ForwardCompatibilitySubstitutionRule substitution_rule, const std::string &preferred_model_id)
 {
     // First load the vendor specific system presets.
-    std::string errors_cummulative = this->load_system_presets();
+    PresetsConfigSubstitutions substitutions;
+    std::string errors_cummulative;
+    std::tie(substitutions, errors_cummulative) = this->load_system_presets(substitution_rule);
 
     const std::string dir_user_presets = data_dir()
 #ifdef SLIC3R_PROFILE_USE_PRESETS_SUBDIR
@@ -176,7 +178,6 @@ PresetsConfigSubstitutions PresetBundle::load_presets(AppConfig &config, Forward
 #endif
         ;
 
-    PresetsConfigSubstitutions substitutions;
     try {
         this->prints.load_presets(dir_user_presets, "print", substitutions, substitution_rule);
     } catch (const std::runtime_error &err) {
@@ -219,12 +220,20 @@ PresetsConfigSubstitutions PresetBundle::load_presets(AppConfig &config, Forward
 
 // Load system presets into this PresetBundle.
 // For each vendor, there will be a single PresetBundle loaded.
-std::string PresetBundle::load_system_presets()
+std::pair<PresetsConfigSubstitutions, std::string> PresetBundle::load_system_presets(ForwardCompatibilitySubstitutionRule compatibility_rule)
 {
+    if (compatibility_rule == ForwardCompatibilitySubstitutionRule::EnableSystemSilent)
+        // Loading system presets, don't log substitutions.
+        compatibility_rule = ForwardCompatibilitySubstitutionRule::EnableSilent;
+    else if (compatibility_rule == ForwardCompatibilitySubstitutionRule::EnableSilentDisableSystem)
+        // Loading system presets, throw on unknown option value.
+        compatibility_rule = ForwardCompatibilitySubstitutionRule::Disable;
+
     // Here the vendor specific read only Config Bundles are stored.
-    boost::filesystem::path dir = (boost::filesystem::path(data_dir()) / "vendor").make_preferred();
+    boost::filesystem::path     dir = (boost::filesystem::path(data_dir()) / "vendor").make_preferred();
-    std::string errors_cummulative;
+    PresetsConfigSubstitutions  substitutions;
-    bool        first = true;
+    std::string                 errors_cummulative;
+    bool                        first = true;
     for (auto &dir_entry : boost::filesystem::directory_iterator(dir))
         if (Slic3r::is_ini_file(dir_entry)) {
             std::string name = dir_entry.path().filename().string();
@@ -234,13 +243,13 @@ std::string PresetBundle::load_system_presets()
                 // Load the config bundle, flatten it.
                 if (first) {
                     // Reset this PresetBundle and load the first vendor config.
-                    this->load_configbundle(dir_entry.path().string(), PresetBundle::LoadSystem);
+                    append(substitutions, this->load_configbundle(dir_entry.path().string(), PresetBundle::LoadSystem, compatibility_rule).first);
                     first = false;
                 } else {
                     // Load the other vendor configs, merge them with this PresetBundle.
                     // Report duplicate profiles.
                     PresetBundle other;
-                    other.load_configbundle(dir_entry.path().string(), PresetBundle::LoadSystem);
+                    append(substitutions, other.load_configbundle(dir_entry.path().string(), PresetBundle::LoadSystem, compatibility_rule).first);
                     std::vector<std::string> duplicates = this->merge_presets(std::move(other));
                     if (! duplicates.empty()) {
                         errors_cummulative += "Vendor configuration file " + name + " contains the following presets with names used by other vendors: ";
@@ -262,7 +271,7 @@ std::string PresetBundle::load_system_presets()
 	}
 
 	this->update_system_maps();
-    return errors_cummulative;
+    return std::make_pair(std::move(substitutions), errors_cummulative);
 }
 
 // Merge one vendor's presets with the other vendor's presets, report duplicates.
@@ -713,7 +722,7 @@ ConfigSubstitutions PresetBundle::load_config_file(const std::string &path, Forw
         return config_substitutions;
     }
     case CONFIG_FILE_TYPE_CONFIG_BUNDLE:
-        return load_config_file_config_bundle(path, tree);
+        return load_config_file_config_bundle(path, tree, compatibility_rule);
     }
 
     // This shall never happen. Suppres compiler warnings.
@@ -890,13 +899,14 @@ void PresetBundle::load_config_file_config(const std::string &name_or_path, bool
 }
 
 // Load the active configuration of a config bundle from a boost property_tree. This is a private method called from load_config_file.
-ConfigSubstitutions PresetBundle::load_config_file_config_bundle(const std::string &path, const boost::property_tree::ptree &tree)
+ConfigSubstitutions PresetBundle::load_config_file_config_bundle(
+    const std::string &path, const boost::property_tree::ptree &tree, ForwardCompatibilitySubstitutionRule compatibility_rule)
 {
     // 1) Load the config bundle into a temp data.
     PresetBundle tmp_bundle;
     // Load the config bundle, but don't save the loaded presets to user profile directory, as only the presets marked as active in the loaded preset bundle
     // will be loaded into the master PresetBundle and activated.
-    auto [presets_substitutions, presets_imported] = tmp_bundle.load_configbundle(path, {});
+    auto [presets_substitutions, presets_imported] = tmp_bundle.load_configbundle(path, {}, compatibility_rule);
 
     std::string bundle_name = std::string(" - ") + boost::filesystem::path(path).filename().string();
 
@@ -1103,15 +1113,11 @@ static void flatten_configbundle_hierarchy(boost::property_tree::ptree &tree, co
 
 // Load a config bundle file, into presets and store the loaded presets into separate files
 // of the local configuration directory.
-std::pair<PresetsConfigSubstitutions, size_t> PresetBundle::load_configbundle(const std::string &path, LoadConfigBundleAttributes flags)
+std::pair<PresetsConfigSubstitutions, size_t> PresetBundle::load_configbundle(
+    const std::string &path, LoadConfigBundleAttributes flags, ForwardCompatibilitySubstitutionRule compatibility_rule)
 {
     // Enable substitutions for user config bundle, throw an exception when loading a system profile.
-    ConfigSubstitutionContext substitution_context {
+    ConfigSubstitutionContext  substitution_context { compatibility_rule };
-        flags.has(LoadConfigBundleAttribute::LoadSystem) ?
-            ForwardCompatibilitySubstitutionRule::Disable :
-            ForwardCompatibilitySubstitutionRule::Enable 
-        };
-
     PresetsConfigSubstitutions substitutions;
 
     if (flags.has(LoadConfigBundleAttribute::ResetUserProfile) || flags.has(LoadConfigBundleAttribute::LoadSystem))

src/libslic3r/PresetBundle.hpp

@@ -102,7 +102,8 @@ public:
     using LoadConfigBundleAttributes = enum_bitmask<LoadConfigBundleAttribute>;
     // Load the config bundle based on the flags.
     // Don't do any config substitutions when loading a system profile, perform and report substitutions otherwise.
-    std::pair<PresetsConfigSubstitutions, size_t> load_configbundle(const std::string &path, LoadConfigBundleAttributes flags);
+    std::pair<PresetsConfigSubstitutions, size_t> load_configbundle(
+        const std::string &path, LoadConfigBundleAttributes flags, ForwardCompatibilitySubstitutionRule compatibility_rule);
 
     // Export a config bundle file containing all the presets and the names of the active presets.
     void                        export_configbundle(const std::string &path, bool export_system_settings = false, bool export_physical_printers = false);
@@ -139,7 +140,7 @@ public:
 
     static const char *PRUSA_BUNDLE;
 private:
-    std::string                 load_system_presets();
+    std::pair<PresetsConfigSubstitutions, std::string> load_system_presets(ForwardCompatibilitySubstitutionRule compatibility_rule);
     // Merge one vendor's presets with the other vendor's presets, report duplicates.
     std::vector<std::string>    merge_presets(PresetBundle &&other);
     // Update renamed_from and alias maps of system profiles.
@@ -158,7 +159,8 @@ private:
     // and the external config is just referenced, not stored into user profile directory.
     // If it is not an external config, then the config will be stored into the user profile directory.
     void                        load_config_file_config(const std::string &name_or_path, bool is_external, DynamicPrintConfig &&config);
-    ConfigSubstitutions         load_config_file_config_bundle(const std::string &path, const boost::property_tree::ptree &tree);
+    ConfigSubstitutions         load_config_file_config_bundle(
+        const std::string &path, const boost::property_tree::ptree &tree, ForwardCompatibilitySubstitutionRule compatibility_rule);
 
     DynamicPrintConfig          full_fff_config() const;
     DynamicPrintConfig          full_sla_config() const;

src/slic3r/Config/Snapshot.cpp

@@ -10,6 +10,7 @@
 #include <boost/filesystem/operations.hpp>
 
 #include "libslic3r/PresetBundle.hpp"
+#include "libslic3r/format.hpp"
 #include "libslic3r/libslic3r.h"
 #include "libslic3r/Time.hpp"
 #include "libslic3r/Config.hpp"
@@ -358,11 +359,12 @@ static void copy_config_dir_single_level(const boost::filesystem::path &path_src
 {
     if (! boost::filesystem::is_directory(path_dst) && 
         ! boost::filesystem::create_directory(path_dst))
-        throw Slic3r::RuntimeError(std::string("Slic3r was unable to create a directory at ") + path_dst.string());
+        throw Slic3r::RuntimeError(std::string("PrusaSlicer was unable to create a directory at ") + path_dst.string());
 
     for (auto &dir_entry : boost::filesystem::directory_iterator(path_src))
         if (Slic3r::is_ini_file(dir_entry))
-		    boost::filesystem::copy_file(dir_entry.path(), path_dst / dir_entry.path().filename(), boost::filesystem::copy_option::overwrite_if_exists);
+            if (std::string error_message; copy_file(dir_entry.path().string(), (path_dst / dir_entry.path().filename()).string(), error_message, false) != SUCCESS)
+                throw Slic3r::RuntimeError(format("Failed copying \"%1%\" to \"%2%\": %3%", path_src.string(), path_dst.string(), error_message));
 }
 
 static void delete_existing_ini_files(const boost::filesystem::path &path)
@@ -413,7 +415,7 @@ const Snapshot&	SnapshotDB::take_snapshot(const AppConfig &app_config, Snapshot:
                 ++ it;
         // Read the active config bundle, parse the config version.
         PresetBundle bundle;
-        bundle.load_configbundle((data_dir / "vendor" / (cfg.name + ".ini")).string(), PresetBundle::LoadConfigBundleAttribute::LoadVendorOnly);
+        bundle.load_configbundle((data_dir / "vendor" / (cfg.name + ".ini")).string(), PresetBundle::LoadConfigBundleAttribute::LoadVendorOnly, ForwardCompatibilitySubstitutionRule::EnableSilent);
         for (const auto &vp : bundle.vendors)
             if (vp.second.id == cfg.name)
                 cfg.version.config_version = vp.second.config_version;

src/slic3r/GUI/ConfigWizard.cpp

@@ -58,7 +58,9 @@ bool Bundle::load(fs::path source_path, bool ais_in_resources, bool ais_prusa_bu
     this->is_prusa_bundle = ais_prusa_bundle;
 
     std::string path_string = source_path.string();
-    auto [config_substitutions, presets_loaded] = preset_bundle->load_configbundle(path_string, PresetBundle::LoadConfigBundleAttribute::LoadSystem);
+    // Throw when parsing invalid configuration. Only valid configuration is supposed to be provided over the air.
+    auto [config_substitutions, presets_loaded] = preset_bundle->load_configbundle(
+        path_string, PresetBundle::LoadConfigBundleAttribute::LoadSystem, ForwardCompatibilitySubstitutionRule::Disable);
     // No substitutions shall be reported when loading a system config bundle, no substitutions are allowed.
     assert(config_substitutions.empty());
     auto first_vendor = preset_bundle->vendors.begin();
@@ -2463,7 +2465,8 @@ void ConfigWizard::priv::apply_config(AppConfig *app_config, PresetBundle *prese
     // Reloading the configs after some modifications were done to PrusaSlicer.ini.
     // Just perform the substitutions silently, as the substitutions were already presented to the user on application start-up
     // and the Wizard shall not create any new values that would require substitution.
-    PresetsConfigSubstitutions substitutions = preset_bundle->load_presets(*app_config, ForwardCompatibilitySubstitutionRule::EnableSilent, preferred_model);
+    // Throw on substitutions in system profiles, as the system profiles provided over the air should be compatible with this PrusaSlicer version.
+    preset_bundle->load_presets(*app_config, ForwardCompatibilitySubstitutionRule::EnableSilentDisableSystem, preferred_model);
 
     if (page_custom->custom_wanted()) {
         page_firmware->apply_custom_config(*custom_config);

src/slic3r/GUI/GUI_App.cpp

@@ -892,7 +892,10 @@ bool GUI_App::on_init_inner()
     // Suppress the '- default -' presets.
     preset_bundle->set_default_suppressed(app_config->get("no_defaults") == "1");
     try {
-        init_params->preset_substitutions = preset_bundle->load_presets(*app_config, ForwardCompatibilitySubstitutionRule::Enable);
+        // Enable all substitutions (in both user and system profiles), but log the substitutions in user profiles only.
+        // If there are substitutions in system profiles, then a "reconfigure" event shall be triggered, which will force
+        // installation of a compatible system preset, thus nullifying the system preset substitutions.
+        init_params->preset_substitutions = preset_bundle->load_presets(*app_config, ForwardCompatibilitySubstitutionRule::EnableSystemSilent);
     } catch (const std::exception &ex) {
         show_error(nullptr, ex.what());
     }
@@ -1676,6 +1679,9 @@ void GUI_App::add_config_menu(wxMenuBar *menu)
                         Config::SnapshotDB::singleton().take_snapshot(*app_config, Config::Snapshot::SNAPSHOT_BEFORE_ROLLBACK);
                     try {
                         app_config->set("on_snapshot", Config::SnapshotDB::singleton().restore_snapshot(dlg.snapshot_to_activate(), *app_config).id);
+                        // Enable substitutions, log both user and system substitutions. There should not be any substitutions performed when loading system
+                        // presets because compatibility of profiles shall be verified using the min_slic3r_version keys in config index, but users
+                        // are known to be creative and mess with the config files in various ways.
                         if (PresetsConfigSubstitutions all_substitutions = preset_bundle->load_presets(*app_config, ForwardCompatibilitySubstitutionRule::Enable);
                             ! all_substitutions.empty())
                             show_substitutions_info(all_substitutions);

src/slic3r/GUI/MainFrame.cpp

@@ -1578,7 +1578,9 @@ void MainFrame::load_configbundle(wxString file/* = wxEmptyString, const bool re
     size_t presets_imported = 0;
     PresetsConfigSubstitutions config_substitutions;
     try {
-        std::tie(config_substitutions, presets_imported) = wxGetApp().preset_bundle->load_configbundle(file.ToUTF8().data(), PresetBundle::LoadConfigBundleAttribute::SaveImported);
+        // Report all substitutions.
+        std::tie(config_substitutions, presets_imported) = wxGetApp().preset_bundle->load_configbundle(
+            file.ToUTF8().data(), PresetBundle::LoadConfigBundleAttribute::SaveImported, ForwardCompatibilitySubstitutionRule::Enable);
     } catch (const std::exception &ex) {
         show_error(this, ex.what());
         return;

src/slic3r/Utils/PresetUpdater.cpp

@@ -65,6 +65,10 @@ void copy_file_fix(const fs::path &source, const fs::path &target)
 				_L("Copying of file %1% to %2% failed: %3%"),
 				source, target, error_message));
 	}
+	// Permissions should be copied from the source file by copy_file(). We are not sure about the source
+	// permissions, let's rewrite them with 644.
+	static constexpr const auto perms = fs::owner_read | fs::owner_write | fs::group_read | fs::others_read;
+	fs::permissions(target, perms);
 }
 
 struct Update
@@ -611,7 +615,8 @@ void PresetUpdater::priv::perform_updates(Updates &&updates, bool snapshot) cons
 			update.install();
 
 			PresetBundle bundle;
-			bundle.load_configbundle(update.source.string(), PresetBundle::LoadConfigBundleAttribute::LoadSystem);
+			// Throw when parsing invalid configuration. Only valid configuration is supposed to be provided over the air.
+			bundle.load_configbundle(update.source.string(), PresetBundle::LoadConfigBundleAttribute::LoadSystem, ForwardCompatibilitySubstitutionRule::Disable);
 
 			BOOST_LOG_TRIVIAL(info) << format("Deleting %1% conflicting presets", bundle.prints.size() + bundle.filaments.size() + bundle.printers.size());
 
@@ -715,7 +720,8 @@ static void reload_configs_update_gui()
 	auto* app_config = GUI::wxGetApp().app_config;
 	// System profiles should not trigger any substitutions, user profiles may trigger substitutions, but these substitutions
 	// were already presented to the user on application start up. Just do substitutions now and keep quiet about it.
-	GUI::wxGetApp().preset_bundle->load_presets(*app_config, ForwardCompatibilitySubstitutionRule::EnableSilent);
+	// However throw on substitutions in system profiles, those shall never happen with system profiles installed over the air.
+	GUI::wxGetApp().preset_bundle->load_presets(*app_config, ForwardCompatibilitySubstitutionRule::EnableSilentDisableSystem);
 	GUI::wxGetApp().load_current_presets();
 	GUI::wxGetApp().plater()->set_bed_shape();
 }