From f516f47c8e1b1925985ef303fbb07098f0e06c9c Mon Sep 17 00:00:00 2001 From: SoftFever Date: Wed, 1 Jul 2026 21:55:19 +0800 Subject: [PATCH] add profile validator checks --- .github/workflows/check_profiles.yml | 27 ++++- .../OrcaSlicer_profile_validator.cpp | 4 +- src/libslic3r/PresetBundle.cpp | 102 +++++++++++++++--- src/libslic3r/PresetBundle.hpp | 9 +- .../libslic3r/test_preset_bundle_loading.cpp | 87 +++++++++++++++ 5 files changed, 211 insertions(+), 18 deletions(-) diff --git a/.github/workflows/check_profiles.yml b/.github/workflows/check_profiles.yml index 9a5272a314..15d6c6ba54 100644 --- a/.github/workflows/check_profiles.yml +++ b/.github/workflows/check_profiles.yml @@ -57,6 +57,20 @@ jobs: set +e ./OrcaSlicer_profile_validator -p ${{ github.workspace }}/resources/profiles -l 2 -v BBL -f 2>&1 | tee ${{ runner.temp }}/validate_filament_subtypes.log exit ${PIPESTATUS[0]} + # Flag inherits/compatible_printers/compatible_prints references that point at a deleted or + # renamed preset. Opt-in per vendor for now (via -r); enabled for BBL and Qidi until other + # vendors' profiles are cleaned up. Runs before the custom-preset injection below. + - name: validate preset references for BBL and Qidi profiles + id: validate_preset_references + continue-on-error: true + run: | + set +e + rc=0 + for v in BBL Qidi; do + ./OrcaSlicer_profile_validator -p ${{ github.workspace }}/resources/profiles -l 2 -v "$v" -r 2>&1 | tee -a ${{ runner.temp }}/validate_preset_references.log + [ ${PIPESTATUS[0]} -ne 0 ] && rc=1 + done + exit $rc - name: validate custom presets id: validate_custom @@ -76,7 +90,7 @@ jobs: echo "${{ github.event.pull_request.number }}" > ${{ runner.temp }}/profile-check-results/pr_number.txt - name: Prepare comment artifact - if: ${{ always() && github.event_name == 'pull_request' && (steps.extra_json_check.outcome == 'failure' || steps.validate_system.outcome == 'failure' || steps.validate_filament_subtypes.outcome == 'failure' || steps.validate_custom.outcome == 'failure') }} + if: ${{ always() && github.event_name == 'pull_request' && (steps.extra_json_check.outcome == 'failure' || steps.validate_system.outcome == 'failure' || steps.validate_filament_subtypes.outcome == 'failure' || steps.validate_preset_references.outcome == 'failure' || steps.validate_custom.outcome == 'failure') }} run: | { # Marker matched by check_profiles_comment.yml to delete prior comments. @@ -111,6 +125,15 @@ jobs: echo "" fi + if [ "${{ steps.validate_preset_references.outcome }}" = "failure" ]; then + echo "### BBL/Qidi Preset Reference Validation Failed" + echo "" + echo '```' + head -c 30000 ${{ runner.temp }}/validate_preset_references.log || echo "No output captured" + echo '```' + echo "" + fi + if [ "${{ steps.validate_custom.outcome }}" = "failure" ]; then echo "### Custom Preset Validation Failed" echo "" @@ -133,7 +156,7 @@ jobs: retention-days: 1 - name: Fail if any check failed - if: ${{ always() && (steps.extra_json_check.outcome == 'failure' || steps.validate_system.outcome == 'failure' || steps.validate_filament_subtypes.outcome == 'failure' || steps.validate_custom.outcome == 'failure') }} + if: ${{ always() && (steps.extra_json_check.outcome == 'failure' || steps.validate_system.outcome == 'failure' || steps.validate_filament_subtypes.outcome == 'failure' || steps.validate_preset_references.outcome == 'failure' || steps.validate_custom.outcome == 'failure') }} run: | echo "One or more profile checks failed. See above for details." exit 1 diff --git a/src/dev-utils/OrcaSlicer_profile_validator.cpp b/src/dev-utils/OrcaSlicer_profile_validator.cpp index 3208f55452..baee58258c 100644 --- a/src/dev-utils/OrcaSlicer_profile_validator.cpp +++ b/src/dev-utils/OrcaSlicer_profile_validator.cpp @@ -96,6 +96,7 @@ int main(int argc, char* argv[]) ("vendor,v", po::value()->default_value(""), "Vendor name. Optional, all profiles present in the folder will be validated if not specified") ("generate_presets,g", po::value()->default_value(false), "Generate user presets for mock test") ("check_filament_subtypes,f", po::bool_switch()->default_value(false), "Also flag printers with duplicate (ambiguous) filament subtypes. Off unless this flag is present.") + ("check_preset_references,r", po::bool_switch()->default_value(false), "Also flag presets whose inherits/compatible_printers/compatible_prints reference a deleted or renamed preset. Off unless this flag is present.") ("log_level,l", po::value()->default_value(2), "Log level. Optional, default is 2 (warning). Higher values produce more detailed logs."); // clang-format on @@ -120,6 +121,7 @@ int main(int argc, char* argv[]) int log_level = vm["log_level"].as(); bool generate_user_preset = vm["generate_presets"].as(); bool check_filament_subtypes = vm["check_filament_subtypes"].as(); + bool check_preset_references = vm["check_preset_references"].as(); // check if path is valid, and return error if not if (!fs::exists(path) || !fs::is_directory(path)) { @@ -166,7 +168,7 @@ int main(int argc, char* argv[]) return 0; } - if (preset_bundle->has_errors(check_filament_subtypes)) { + if (preset_bundle->has_errors(check_filament_subtypes, check_preset_references)) { std::cout << "Validation failed" << std::endl; return 1; } diff --git a/src/libslic3r/PresetBundle.cpp b/src/libslic3r/PresetBundle.cpp index 853b61b093..d267d19f8d 100644 --- a/src/libslic3r/PresetBundle.cpp +++ b/src/libslic3r/PresetBundle.cpp @@ -532,8 +532,16 @@ PresetsConfigSubstitutions PresetBundle::load_presets(AppConfig &config, Forward load_user_presets(dir_user_presets, substitution_rule); } - // Rewrite renamed compatible_printers / compatible_prints references before selection. - this->normalize_compatible_presets(); + // Rewrite renamed compatible_printers / compatible_prints references before selection. Skipped + // in validation mode so the profile validator (has_errors -> check_preset_references) sees the + // raw vendor-JSON references instead of the silently-repaired ones. + if (!validation_mode) + this->normalize_compatible_presets(); + // Rewrite renamed compatible_printers / compatible_prints references before selection. Skipped + // in validation mode so the profile validator (has_errors -> check_preset_references) sees the + // raw vendor-JSON references instead of the silently-repaired ones. + if (!validation_mode) + this->normalize_compatible_presets(); this->update_multi_material_filament_presets(); this->update_compatible(PresetSelectCompatibleType::Never); @@ -5278,21 +5286,20 @@ static void normalize_compatible_field(Preset &preset, const char *field_key, Pr void PresetBundle::normalize_compatible_presets() { // compatible_printers references a printer preset; compatible_prints (filaments / SLA materials) - // references a process preset. Skip system presets: they are the targets of renames, not holders - // of stale references, and their vendor data must not be mutated. (begin()/end() skip defaults.) - auto normalize = [](PresetCollection &holders, PresetCollection &printers, PresetCollection *processes) { + // references a process preset. System presets are normalized too: a vendor profile can itself + // reference a sibling preset by a name that was later renamed, and the rewrite is in-memory only + // (system presets are never persisted back to vendor JSON). (begin()/end() skip defaults.) + auto normalize = [this](PresetCollection &holders, PresetCollection *processes) { for (Preset &p : holders) { - if (p.is_system) - continue; - normalize_compatible_field(p, "compatible_printers", printers); + normalize_compatible_field(p, "compatible_printers", this->printers); if (processes != nullptr) normalize_compatible_field(p, "compatible_prints", *processes); } }; - normalize(this->prints, this->printers, nullptr); - normalize(this->filaments, this->printers, &this->prints); - normalize(this->sla_prints, this->printers, nullptr); - normalize(this->sla_materials, this->printers, &this->sla_prints); + normalize(this->prints, nullptr); + normalize(this->filaments, &this->prints); + normalize(this->sla_prints, nullptr); + normalize(this->sla_materials, &this->sla_prints); } void PresetBundle::update_compatible(PresetSelectCompatibleType select_other_print_if_incompatible, PresetSelectCompatibleType select_other_filament_if_incompatible) @@ -5526,7 +5533,8 @@ void PresetBundle::set_default_suppressed(bool default_suppressed) printers.set_default_suppressed(default_suppressed); } -bool PresetBundle::has_errors(bool check_duplicate_filament_subtypes) const +bool PresetBundle::has_errors(bool check_duplicate_filament_subtypes, bool check_references) const +bool PresetBundle::has_errors(bool check_duplicate_filament_subtypes, bool check_references) const { if (m_errors != 0 || printers.m_errors != 0 || filaments.m_errors != 0 || prints.m_errors != 0) return true; @@ -5549,6 +5557,12 @@ bool PresetBundle::has_errors(bool check_duplicate_filament_subtypes) const if (check_duplicate_filament_subtypes && this->check_duplicate_filament_subtypes()) has_errors = true; + if (check_references && this->check_preset_references()) + has_errors = true; + + if (check_references && this->check_preset_references()) + has_errors = true; + return has_errors; } @@ -5579,6 +5593,68 @@ static std::string preset_file_uri(const std::string &file) return uri; } +// Orca: validator-only. Flag any system preset whose inherits / compatible_printers / +// compatible_prints references a name that no longer resolves. Uses find_preset (exact match, then +// the renamed_from map - no fuzzy find_preset2, no alias resolution), so: +// nullptr -> the referenced preset was deleted/renamed away (name is dangling), +// resolved != name -> the reference uses an old name that renamed_from maps to a current one. +// Both should be fixed at the source rather than relying on load-time normalization - which is why +// normalize_compatible_presets() is skipped in validation mode (see load_presets), so this sees the +// raw vendor-JSON references. Safe under a single-vendor run (-v) too: inherits / compatible_printers +// / compatible_prints only name same-vendor or OrcaFilamentLibrary presets (both loaded), so a +// reference that does not resolve is genuinely dangling rather than an unloaded cross-vendor preset. +bool PresetBundle::check_preset_references() const +{ + bool found = false; + + // Resolve one reference (an inherits parent or a compatible_* entry) against its target + // collection and log if it is dangling (unknown) or uses a renamed preset's old name. + auto report_ref = [&](const Preset &p, const std::string &name, const PresetCollection &target, + const char *verb, const char *noun) { + const Preset *resolved = target.find_preset(name, false); + if (resolved == nullptr) { + found = true; + BOOST_LOG_TRIVIAL(error) << "Preset \"" << p.name << "\" " << verb << " unknown " << noun << " \"" << name << "\":\n" + << preset_file_uri(p.file); + } else if (resolved->name != name) { + found = true; + BOOST_LOG_TRIVIAL(error) << "Preset \"" << p.name << "\" " << verb << " renamed " << noun << " \"" << name + << "\" (now \"" << resolved->name << "\"):\n" << preset_file_uri(p.file); + } + }; + + auto check_list = [&](const Preset &p, const char *key, const PresetCollection &target) { + const auto *opt = p.config.option(key); + if (opt == nullptr) + return; + for (const std::string &name : opt->values) + if (!name.empty()) + report_ref(p, name, target, "references", key); + }; + + auto check_collection = [&](const PresetCollection &holders, const PresetCollection *processes) { + for (const Preset &p : holders) { + if (!p.is_system) + continue; + if (const std::string &inh = p.inherits(); !inh.empty()) + report_ref(p, inh, holders, "inherits", "parent"); + check_list(p, "compatible_printers", this->printers); + if (processes != nullptr) + check_list(p, "compatible_prints", *processes); + } + }; + + // Printers carry no compatible_printers/compatible_prints (those name a printer, so a printer + // holding them makes no sense); check_list is a no-op for them, so only their inherits is checked. + check_collection(this->printers, nullptr); + check_collection(this->prints, nullptr); + check_collection(this->filaments, &this->prints); + check_collection(this->sla_prints, nullptr); + check_collection(this->sla_materials, &this->sla_prints); + + return found; +} + // Orca: a filament is matched from the AMS by (filament_id + printer compatibility). // For any one printer, at most one instantiated filament preset with a given // filament_id may be compatible - otherwise the AMS match is ambiguous and the diff --git a/src/libslic3r/PresetBundle.hpp b/src/libslic3r/PresetBundle.hpp index 3acf9ae4c6..59f6bee309 100644 --- a/src/libslic3r/PresetBundle.hpp +++ b/src/libslic3r/PresetBundle.hpp @@ -485,8 +485,13 @@ public: return { Preset::TYPE_PRINTER, Preset::TYPE_SLA_PRINT, Preset::TYPE_SLA_MATERIAL }; } - // Orca: for validation only. The duplicate filament subtype check is opt-in for now - bool has_errors(bool check_duplicate_filament_subtypes = false) const; + // Orca: for validation only. The duplicate filament subtype and preset-reference checks are + // opt-in for now (enabled per-vendor by the profile-check CI as vendors are cleaned up). + bool has_errors(bool check_duplicate_filament_subtypes = false, bool check_preset_references = false) const; + + // Orca: for validation only. Flag any system preset whose inherits / compatible_printers / + // compatible_prints references a deleted (unknown) or renamed (old) preset name. + bool check_preset_references() const; private: // Orca: validation only - flag any printer with two or more compatible diff --git a/tests/libslic3r/test_preset_bundle_loading.cpp b/tests/libslic3r/test_preset_bundle_loading.cpp index 4e2ddb480f..b3bf1c15d3 100644 --- a/tests/libslic3r/test_preset_bundle_loading.cpp +++ b/tests/libslic3r/test_preset_bundle_loading.cpp @@ -355,6 +355,40 @@ TEST_CASE("Renamed printer/process names are normalized into compatible lists on CHECK(compatible_list(bundle.prints, "My Process", "compatible_printers") == std::vector{ "New Printer" }); } +TEST_CASE("Renamed names are normalized into a SYSTEM preset's compatible lists", "[Preset][Rename]") +{ + PresetBundle bundle; + + // Current printer + process, each renamed from an older name. + add_inmemory_preset(bundle.printers, "New Printer"); + set_renamed_from(bundle.printers, "New Printer", { "Old Printer" }); + add_inmemory_preset(bundle.prints, "New Process"); + set_renamed_from(bundle.prints, "New Process", { "Old Process" }); + + // A *system* (vendor) filament whose own compatible lists still reference the OLD names. A vendor + // profile can point at a sibling preset that was later renamed, so system presets must be + // normalized too (they are skipped by neither collection walk). + add_inmemory_preset(bundle.filaments, "System Filament").is_system = true; + compatible_list(bundle.filaments, "System Filament", "compatible_printers") = { "Old Printer" }; + compatible_list(bundle.filaments, "System Filament", "compatible_prints") = { "Old Process" }; + + AppConfig app_config; + bundle.load_installed_printers(app_config); // build the rename maps + bundle.normalize_compatible_presets(); + + // The stale references in the system preset are rewritten to the current names. + CHECK(compatible_list(bundle.filaments, "System Filament", "compatible_printers") == + std::vector{ "New Printer" }); + CHECK(compatible_list(bundle.filaments, "System Filament", "compatible_prints") == + std::vector{ "New Process" }); + + // The rewrite does not flag the system preset dirty, and is idempotent. + CHECK_FALSE(bundle.filaments.find_preset("System Filament", false, true)->is_dirty); + bundle.normalize_compatible_presets(); + CHECK(compatible_list(bundle.filaments, "System Filament", "compatible_printers") == + std::vector{ "New Printer" }); +} + TEST_CASE("compatible_prints on SLA materials resolves against sla_prints, not prints", "[Preset][Rename]") { PresetBundle bundle; @@ -377,3 +411,56 @@ TEST_CASE("compatible_prints on SLA materials resolves against sla_prints, not p std::vector{ "New SLA Process" }); } +TEST_CASE("Profile validator flags dangling and renamed preset references", "[Preset][Validate]") +{ + PresetBundle bundle; + + // Current printers: a real one, and a renamed one (its old name resolves via renamed_from). + add_inmemory_preset(bundle.printers, "Real Printer"); + add_inmemory_preset(bundle.printers, "New Printer"); + set_renamed_from(bundle.printers, "New Printer", { "Old Printer" }); + + // A real process, referenced from a filament's compatible_prints. + add_inmemory_preset(bundle.prints, "Real Process").is_system = true; + + // A fully valid system filament: references only current names. + add_inmemory_preset(bundle.filaments, "Good Filament").is_system = true; + compatible_list(bundle.filaments, "Good Filament", "compatible_printers") = { "Real Printer" }; + compatible_list(bundle.filaments, "Good Filament", "compatible_prints") = { "Real Process" }; + + AppConfig app_config; + bundle.load_installed_printers(app_config); // build the rename maps + + // With only valid references, the validator is clean. + CHECK_FALSE(bundle.check_preset_references()); + + SECTION("deleted compatible_printers is flagged") { + add_inmemory_preset(bundle.filaments, "Ghost Ref Filament").is_system = true; + compatible_list(bundle.filaments, "Ghost Ref Filament", "compatible_printers") = { "Ghost Printer" }; + CHECK(bundle.check_preset_references()); + } + + SECTION("renamed compatible_printers (old name) is flagged") { + add_inmemory_preset(bundle.filaments, "Old Ref Filament").is_system = true; + compatible_list(bundle.filaments, "Old Ref Filament", "compatible_printers") = { "Old Printer" }; + CHECK(bundle.check_preset_references()); + } + + SECTION("deleted compatible_prints is flagged") { + add_inmemory_preset(bundle.filaments, "Bad Process Ref").is_system = true; + compatible_list(bundle.filaments, "Bad Process Ref", "compatible_prints") = { "Ghost Process" }; + CHECK(bundle.check_preset_references()); + } + + SECTION("deleted inherits parent is flagged") { + add_inmemory_preset(bundle.filaments, "Orphan Filament", "Ghost Parent").is_system = true; + CHECK(bundle.check_preset_references()); + } + + SECTION("non-system preset with a dangling reference is ignored") { + add_inmemory_preset(bundle.filaments, "User Filament"); // is_system stays false + compatible_list(bundle.filaments, "User Filament", "compatible_printers") = { "Ghost Printer" }; + CHECK_FALSE(bundle.check_preset_references()); + } +} +