Skip to content

Commit

Permalink
🐛 Proper default handling for tool merging
Browse files Browse the repository at this point in the history
- 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.
  • Loading branch information
mkarlesky committed Aug 22, 2024
1 parent db3c219 commit 68cb3b7
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 19 deletions.
38 changes: 32 additions & 6 deletions lib/ceedling/configurator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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

Expand Down
8 changes: 0 additions & 8 deletions lib/ceedling/configurator_setup.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
13 changes: 8 additions & 5 deletions lib/ceedling/defaults.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
}
}

Expand Down

0 comments on commit 68cb3b7

Please sign in to comment.