From 68cb3b7b5bfdb2858f8ca0d5350c40e402237a74 Mon Sep 17 00:00:00 2001 From: Michael Karlesky Date: Thu, 22 Aug 2024 14:56:19 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Proper=20default=20handling=20fo?= =?UTF-8?q?r=20tool=20merging?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Missing user configuration options could cause default tools to be merged (and validated) or not merged at all. Previous fixes here were expanded to cover all cases. - Brought in defaults from actual defaults.rb instead of duplicating. - Reworked backtrace tool handling and conditional validation to use existing patterns. This is simpler and more robust than the previous approach. --- lib/ceedling/configurator.rb | 38 +++++++++++++++++++++++++----- lib/ceedling/configurator_setup.rb | 8 ------- lib/ceedling/defaults.rb | 13 ++++++---- 3 files changed, 40 insertions(+), 19 deletions(-) diff --git a/lib/ceedling/configurator.rb b/lib/ceedling/configurator.rb index 2d78c2dc..b4a4a077 100644 --- a/lib/ceedling/configurator.rb +++ b/lib/ceedling/configurator.rb @@ -92,18 +92,43 @@ def merge_tools_defaults(config, default_config) msg = @reportinator.generate_progress( 'Collecting default tool configurations' ) @loginator.log( msg, Verbosity::OBNOXIOUS ) - # config[:project] is guaranteed to exist / validated to exist + # config[:project] is guaranteed to exist / validated to exist but may not include elements referenced below # config[:test_build] and config[:release_build] are optional in a user project configuration - release_assembly, _ = @config_walkinator.fetch_value( :release_build, :use_assembly, hash:config, default:false ) - test_assembly, _ = @config_walkinator.fetch_value( :test_build, :use_assembly, hash:config, default:false) + + + release_build, _ = @config_walkinator.fetch_value( :project, :release_build, + hash:config, + default: DEFAULT_CEEDLING_PROJECT_CONFIG[:project][:release_build] + ) + + test_preprocessing, _ = @config_walkinator.fetch_value( :project, :use_test_preprocessor, + hash:config, + default: DEFAULT_CEEDLING_PROJECT_CONFIG[:project][:use_test_preprocessor] + ) + + backtrace, _ = @config_walkinator.fetch_value( :project, :use_backtrace, + hash:config, + default: DEFAULT_CEEDLING_PROJECT_CONFIG[:project][:use_backtrace] + ) + + release_assembly, _ = @config_walkinator.fetch_value( :release_build, :use_assembly, + hash:config, + default: DEFAULT_CEEDLING_PROJECT_CONFIG[:release_build][:use_assembly] + ) + + test_assembly, _ = @config_walkinator.fetch_value( :test_build, :use_assembly, + hash:config, + default: DEFAULT_CEEDLING_PROJECT_CONFIG[:test_build][:use_assembly] + ) default_config.deep_merge( DEFAULT_TOOLS_TEST.deep_clone() ) - default_config.deep_merge( DEFAULT_TOOLS_TEST_PREPROCESSORS.deep_clone() ) if (config[:project][:use_test_preprocessor] != :none) + default_config.deep_merge( DEFAULT_TOOLS_TEST_PREPROCESSORS.deep_clone() ) if (test_preprocessing != :none) default_config.deep_merge( DEFAULT_TOOLS_TEST_ASSEMBLER.deep_clone() ) if test_assembly + default_config.deep_merge( DEFAULT_TOOLS_TEST_GDB_BACKTRACE.deep_clone() ) if (backtrace == :gdb) - default_config.deep_merge( DEFAULT_TOOLS_RELEASE.deep_clone() ) if (config[:project][:release_build]) - default_config.deep_merge( DEFAULT_TOOLS_RELEASE_ASSEMBLER.deep_clone() ) if (config[:project][:release_build] and release_assembly) + default_config.deep_merge( DEFAULT_TOOLS_RELEASE.deep_clone() ) if release_build + default_config.deep_merge( DEFAULT_TOOLS_RELEASE_ASSEMBLER.deep_clone() ) if (release_build and release_assembly) end @@ -437,6 +462,7 @@ def eval_environment_variables(config) # Set the environment variable for our session @system_wrapper.env_set( key.to_s.upcase, hash[key] ) + @loginator.log( " - #{key.to_s.upcase}: \"#{hash[key]}\"", Verbosity::DEBUG ) end end diff --git a/lib/ceedling/configurator_setup.rb b/lib/ceedling/configurator_setup.rb index 3e41fb3d..ee06e742 100644 --- a/lib/ceedling/configurator_setup.rb +++ b/lib/ceedling/configurator_setup.rb @@ -171,14 +171,6 @@ def validate_tools(config) valid &= @configurator_validator.validate_tool( config:config, key:tool ) end - if config[:project][:use_backtrace] == :gdb - valid &= @configurator_validator.validate_tool( - config:config, - key: :test_backtrace_gdb, - respect_optional: false - ) - end - return valid end diff --git a/lib/ceedling/defaults.rb b/lib/ceedling/defaults.rb index 9ae6e0f6..e7b1a71b 100644 --- a/lib/ceedling/defaults.rb +++ b/lib/ceedling/defaults.rb @@ -234,9 +234,7 @@ DEFAULT_TEST_BACKTRACE_GDB_TOOL = { :executable => ENV['GDB'].nil? ? FilePathUtils.os_executable_ext('gdb').freeze : ENV['GDB'], :name => 'default_test_backtrace_gdb'.freeze, - # Must be optional because validation is contingent on backtrace configuration. - # (Don't break a build if `gdb` is unavailable but backtrace does not require it.) - :optional => true.freeze, + :optional => false.freeze, :arguments => [ '-q'.freeze, '--batch'.freeze, @@ -253,8 +251,13 @@ :test_compiler => DEFAULT_TEST_COMPILER_TOOL, :test_linker => DEFAULT_TEST_LINKER_TOOL, :test_fixture => DEFAULT_TEST_FIXTURE_TOOL, - :test_fixture_simple_backtrace => DEFAULT_TEST_FIXTURE_SIMPLE_BACKTRACE_TOOL, - :test_backtrace_gdb => DEFAULT_TEST_BACKTRACE_GDB_TOOL, + :test_fixture_simple_backtrace => DEFAULT_TEST_FIXTURE_SIMPLE_BACKTRACE_TOOL + } + } + +DEFAULT_TOOLS_TEST_GDB_BACKTRACE = { + :tools => { + :test_backtrace_gdb => DEFAULT_TEST_BACKTRACE_GDB_TOOL } }