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.
This commit is contained in:
Vojtech Bubnik 2021-06-28 17:26:24 +02:00
parent 1e60acde12
commit 856da036eb
9 changed files with 67 additions and 32 deletions

View File

@ -553,7 +553,8 @@ bool ConfigBase::set_deserialize_raw(const t_config_option_key &opt_key_src, con
opt->set(optdef->default_value.get()); 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. // Log the substitution.
ConfigSubstitution config_substitution; ConfigSubstitution config_substitution;
config_substitution.opt_def = optdef; config_substitution.opt_def = optdef;

View File

@ -166,9 +166,16 @@ enum PrinterTechnology : unsigned char
enum ForwardCompatibilitySubstitutionRule enum ForwardCompatibilitySubstitutionRule
{ {
// Disable susbtitution, throw exception if an option value is not recognized.
Disable, Disable,
// Enable substitution of an unknown option value with default. Log the substitution.
Enable, Enable,
// Enable substitution of an unknown option value with default. Don't log the substitution.
EnableSilent, 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; class ConfigOption;

View File

@ -191,7 +191,9 @@ void PresetBundle::setup_directories()
PresetsConfigSubstitutions PresetBundle::load_presets(AppConfig &config, ForwardCompatibilitySubstitutionRule substitution_rule, const std::string &preferred_model_id) PresetsConfigSubstitutions PresetBundle::load_presets(AppConfig &config, ForwardCompatibilitySubstitutionRule substitution_rule, const std::string &preferred_model_id)
{ {
// First load the vendor specific system presets. // 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() const std::string dir_user_presets = data_dir()
#ifdef SLIC3R_PROFILE_USE_PRESETS_SUBDIR #ifdef SLIC3R_PROFILE_USE_PRESETS_SUBDIR
@ -202,7 +204,6 @@ PresetsConfigSubstitutions PresetBundle::load_presets(AppConfig &config, Forward
#endif #endif
; ;
PresetsConfigSubstitutions substitutions;
try { try {
this->prints.load_presets(dir_user_presets, "print", substitutions, substitution_rule); this->prints.load_presets(dir_user_presets, "print", substitutions, substitution_rule);
} catch (const std::runtime_error &err) { } catch (const std::runtime_error &err) {
@ -245,12 +246,20 @@ PresetsConfigSubstitutions PresetBundle::load_presets(AppConfig &config, Forward
// Load system presets into this PresetBundle. // Load system presets into this PresetBundle.
// For each vendor, there will be a single PresetBundle loaded. // 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. // 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)) for (auto &dir_entry : boost::filesystem::directory_iterator(dir))
if (Slic3r::is_ini_file(dir_entry)) { if (Slic3r::is_ini_file(dir_entry)) {
std::string name = dir_entry.path().filename().string(); std::string name = dir_entry.path().filename().string();
@ -260,13 +269,13 @@ std::string PresetBundle::load_system_presets()
// Load the config bundle, flatten it. // Load the config bundle, flatten it.
if (first) { if (first) {
// Reset this PresetBundle and load the first vendor config. // 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; first = false;
} else { } else {
// Load the other vendor configs, merge them with this PresetBundle. // Load the other vendor configs, merge them with this PresetBundle.
// Report duplicate profiles. // Report duplicate profiles.
PresetBundle other; 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)); std::vector<std::string> duplicates = this->merge_presets(std::move(other));
if (! duplicates.empty()) { if (! duplicates.empty()) {
errors_cummulative += "Vendor configuration file " + name + " contains the following presets with names used by other vendors: "; errors_cummulative += "Vendor configuration file " + name + " contains the following presets with names used by other vendors: ";
@ -288,7 +297,7 @@ std::string PresetBundle::load_system_presets()
} }
this->update_system_maps(); 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. // Merge one vendor's presets with the other vendor's presets, report duplicates.
@ -739,7 +748,7 @@ ConfigSubstitutions PresetBundle::load_config_file(const std::string &path, Forw
return config_substitutions; return config_substitutions;
} }
case CONFIG_FILE_TYPE_CONFIG_BUNDLE: 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. // This shall never happen. Suppres compiler warnings.
@ -916,13 +925,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. // 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. // 1) Load the config bundle into a temp data.
PresetBundle tmp_bundle; 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 // 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. // 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);
UNUSED(presets_imported); UNUSED(presets_imported);
std::string bundle_name = std::string(" - ") + boost::filesystem::path(path).filename().string(); std::string bundle_name = std::string(" - ") + boost::filesystem::path(path).filename().string();
@ -1135,15 +1145,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 // Load a config bundle file, into presets and store the loaded presets into separate files
// of the local configuration directory. // 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. // 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; PresetsConfigSubstitutions substitutions;
if (flags.has(LoadConfigBundleAttribute::ResetUserProfile) || flags.has(LoadConfigBundleAttribute::LoadSystem)) if (flags.has(LoadConfigBundleAttribute::ResetUserProfile) || flags.has(LoadConfigBundleAttribute::LoadSystem))

View File

@ -102,7 +102,8 @@ public:
using LoadConfigBundleAttributes = enum_bitmask<LoadConfigBundleAttribute>; using LoadConfigBundleAttributes = enum_bitmask<LoadConfigBundleAttribute>;
// Load the config bundle based on the flags. // Load the config bundle based on the flags.
// Don't do any config substitutions when loading a system profile, perform and report substitutions otherwise. // 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. // 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); 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; static const char *PRUSA_BUNDLE;
private: 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. // Merge one vendor's presets with the other vendor's presets, report duplicates.
std::vector<std::string> merge_presets(PresetBundle &&other); std::vector<std::string> merge_presets(PresetBundle &&other);
// Update renamed_from and alias maps of system profiles. // 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. // 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. // 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); 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_fff_config() const;
DynamicPrintConfig full_sla_config() const; DynamicPrintConfig full_sla_config() const;

View File

@ -10,6 +10,7 @@
#include <boost/filesystem/operations.hpp> #include <boost/filesystem/operations.hpp>
#include "libslic3r/PresetBundle.hpp" #include "libslic3r/PresetBundle.hpp"
#include "libslic3r/format.hpp"
#include "libslic3r/libslic3r.h" #include "libslic3r/libslic3r.h"
#include "libslic3r/Time.hpp" #include "libslic3r/Time.hpp"
#include "libslic3r/Config.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) && if (! boost::filesystem::is_directory(path_dst) &&
! boost::filesystem::create_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)) for (auto &dir_entry : boost::filesystem::directory_iterator(path_src))
if (Slic3r::is_ini_file(dir_entry)) 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) 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; ++ it;
// Read the active config bundle, parse the config version. // Read the active config bundle, parse the config version.
PresetBundle bundle; 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) for (const auto &vp : bundle.vendors)
if (vp.second.id == cfg.name) if (vp.second.id == cfg.name)
cfg.version.config_version = vp.second.config_version; cfg.version.config_version = vp.second.config_version;

View File

@ -65,7 +65,9 @@ bool Bundle::load(fs::path source_path, bool ais_in_resources, bool ais_prusa_bu
this->is_prusa_bundle = ais_prusa_bundle; this->is_prusa_bundle = ais_prusa_bundle;
std::string path_string = source_path.string(); 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);
UNUSED(config_substitutions); UNUSED(config_substitutions);
// No substitutions shall be reported when loading a system config bundle, no substitutions are allowed. // No substitutions shall be reported when loading a system config bundle, no substitutions are allowed.
assert(config_substitutions.empty()); assert(config_substitutions.empty());
@ -2590,7 +2592,8 @@ void ConfigWizard::priv::apply_config(AppConfig *app_config, PresetBundle *prese
// Reloading the configs after some modifications were done to PrusaSlicer.ini. // 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 // 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. // 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()) { if (page_custom->custom_wanted()) {
page_firmware->apply_custom_config(*custom_config); page_firmware->apply_custom_config(*custom_config);

View File

@ -912,7 +912,10 @@ bool GUI_App::on_init_inner()
// Suppress the '- default -' presets. // Suppress the '- default -' presets.
preset_bundle->set_default_suppressed(app_config->get("no_defaults") == "1"); preset_bundle->set_default_suppressed(app_config->get("no_defaults") == "1");
try { 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) { } catch (const std::exception &ex) {
show_error(nullptr, ex.what()); show_error(nullptr, ex.what());
} }
@ -1874,6 +1877,9 @@ void GUI_App::add_config_menu(wxMenuBar *menu)
Config::SnapshotDB::singleton().take_snapshot(*app_config, Config::Snapshot::SNAPSHOT_BEFORE_ROLLBACK); Config::SnapshotDB::singleton().take_snapshot(*app_config, Config::Snapshot::SNAPSHOT_BEFORE_ROLLBACK);
try { try {
app_config->set("on_snapshot", Config::SnapshotDB::singleton().restore_snapshot(dlg.snapshot_to_activate(), *app_config).id); 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); if (PresetsConfigSubstitutions all_substitutions = preset_bundle->load_presets(*app_config, ForwardCompatibilitySubstitutionRule::Enable);
! all_substitutions.empty()) ! all_substitutions.empty())
show_substitutions_info(all_substitutions); show_substitutions_info(all_substitutions);

View File

@ -1809,7 +1809,9 @@ void MainFrame::load_configbundle(wxString file/* = wxEmptyString, const bool re
size_t presets_imported = 0; size_t presets_imported = 0;
PresetsConfigSubstitutions config_substitutions; PresetsConfigSubstitutions config_substitutions;
try { 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) { } catch (const std::exception &ex) {
show_error(this, ex.what()); show_error(this, ex.what());
return; return;

View File

@ -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%"), _L("Copying of file %1% to %2% failed: %3%"),
source, target, error_message)); 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 struct Update
@ -611,7 +615,8 @@ void PresetUpdater::priv::perform_updates(Updates &&updates, bool snapshot) cons
update.install(); update.install();
PresetBundle bundle; 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()); 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; auto* app_config = GUI::wxGetApp().app_config;
// System profiles should not trigger any substitutions, user profiles may trigger substitutions, but these substitutions // 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. // 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().load_current_presets();
GUI::wxGetApp().plater()->set_bed_shape(); GUI::wxGetApp().plater()->set_bed_shape();
} }