From 42284477b912f0275e28455b7aaccd11e5b4ddd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Tarti=C3=A8re?= Date: Sun, 31 Mar 2024 13:12:14 -1000 Subject: [PATCH 1/2] Revert "(CAT-1646) - Remove section if its not have any settings" PR #532 introduced 4ebe03056c8da9466d03bb3237604f06ee1885cd which breaks backwards compatibility and is part of a patch release, as reported here: https://github.com/puppetlabs/puppetlabs-inifile/pull/532#issuecomment-2028513997 This reverts commit 4ebe03056c8da9466d03bb3237604f06ee1885cd. This will allow us to do a new patch release that fix the unexpected backwards incompatible change, and leave the room to implement this as part of a next major version of the module. --- lib/puppet/util/ini_file.rb | 5 +-- lib/puppet/util/ini_file/section.rb | 5 ++- .../puppet/provider/ini_setting/ruby_spec.rb | 32 ------------------- 3 files changed, 3 insertions(+), 39 deletions(-) diff --git a/lib/puppet/util/ini_file.rb b/lib/puppet/util/ini_file.rb index 3acbf7a4..b7529812 100644 --- a/lib/puppet/util/ini_file.rb +++ b/lib/puppet/util/ini_file.rb @@ -222,7 +222,6 @@ def read_section(name, start_line, line_iter) end_line_num = start_line min_indentation = nil empty = true - empty_line_count = 0 loop do line, line_num = line_iter.peek if line_num.nil? || @section_regex.match(line) @@ -230,7 +229,7 @@ def read_section(name, start_line, line_iter) # when it's empty, we must be sure it's thought of as new, # which is signalled with a nil ending line end_line_num = nil if name == '' && empty - return Section.new(name, start_line, end_line_num, settings, min_indentation, empty_line_count) + return Section.new(name, start_line, end_line_num, settings, min_indentation) end if (match = @setting_regex.match(line)) settings[match[2]] = match[4] @@ -238,8 +237,6 @@ def read_section(name, start_line, line_iter) min_indentation = [indentation, min_indentation || indentation].min end end_line_num = line_num - empty_line_count += 1 if line == "\n" - empty = false line_iter.next end diff --git a/lib/puppet/util/ini_file/section.rb b/lib/puppet/util/ini_file/section.rb index 4e29b5fd..46e01477 100644 --- a/lib/puppet/util/ini_file/section.rb +++ b/lib/puppet/util/ini_file/section.rb @@ -14,14 +14,13 @@ class Section # `end_line` of `nil`. # * `start_line` and `end_line` will be set to `nil` for a new non-global # section. - def initialize(name, start_line, end_line, settings, indentation, empty_line_count = 0) + def initialize(name, start_line, end_line, settings, indentation) @name = name @start_line = start_line @end_line = end_line @existing_settings = settings.nil? ? {} : settings @additional_settings = {} @indentation = indentation - @empty_line_count = empty_line_count end attr_reader :name, :start_line, :end_line, :additional_settings, :indentation @@ -51,7 +50,7 @@ def existing_setting?(setting_name) # the global section is empty whenever it's new; # other sections are empty when they have no lines def empty? - global? ? new_section? : (start_line == end_line || (end_line && (end_line - @empty_line_count)) == start_line) + global? ? new_section? : start_line == end_line end def update_existing_setting(setting_name, value) diff --git a/spec/unit/puppet/provider/ini_setting/ruby_spec.rb b/spec/unit/puppet/provider/ini_setting/ruby_spec.rb index ef7da0eb..a250bcde 100644 --- a/spec/unit/puppet/provider/ini_setting/ruby_spec.rb +++ b/spec/unit/puppet/provider/ini_setting/ruby_spec.rb @@ -1141,38 +1141,6 @@ def self.file_path end end - context 'when section has only empty line' do - let(:orig_content) do - <<~INIFILE - [section1] - foo=foovalue - - - [section2] - - foo= foovalue2 - baz=bazvalue - url = http:// - INIFILE - end - - expected_content = <<~INIFILE - [section2] - - foo= foovalue2 - baz=bazvalue - url = http:// - INIFILE - - it 'remove empty section' do - resource = Puppet::Type::Ini_setting.new(common_params.merge(section: 'section1', setting: 'foo', ensure: 'absent')) - provider = described_class.new(resource) - expect(provider.exists?).to be true - provider.destroy - validate_file(expected_content, tmpfile) - end - end - context 'when dealing with indentation in sections' do let(:orig_content) do <<~INIFILE From 5635e76f23abac85a0234d4661c8152070cbc2c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Tarti=C3=A8re?= Date: Sun, 31 Mar 2024 13:50:16 -1000 Subject: [PATCH 2/2] Keep sections when they become empty When the last setting of a section was removed, the whole section was removed unless it contained white space of comments. In #532, this was changed to also remove sections that only contained space (blank lines), but it caused regressions and was reverted in #535. For consistency, we completely suppress the auto-removal of "empty" sections: removing the last setting of a section will not remove this section anymore, just like what happens for sections with only blank lines and comments. --- lib/puppet/util/ini_file.rb | 8 ----- spec/acceptance/ini_setting_spec.rb | 5 +-- spec/acceptance/ini_subsetting_spec.rb | 2 +- .../puppet/provider/ini_setting/ruby_spec.rb | 32 +------------------ .../provider/ini_subsetting/ruby_spec.rb | 4 ++- 5 files changed, 8 insertions(+), 43 deletions(-) diff --git a/lib/puppet/util/ini_file.rb b/lib/puppet/util/ini_file.rb index b7529812..478cc208 100644 --- a/lib/puppet/util/ini_file.rb +++ b/lib/puppet/util/ini_file.rb @@ -122,14 +122,6 @@ def remove_setting(section_name, setting) # was modified. section_index = @section_names.index(section_name) decrement_section_line_numbers(section_index + 1) - - return unless section.empty? - - # By convention, it's time to remove this newly emptied out section - lines.delete_at(section.start_line) - decrement_section_line_numbers(section_index + 1) - @section_names.delete_at(section_index) - @sections_hash.delete(section.name) end def save diff --git a/spec/acceptance/ini_setting_spec.rb b/spec/acceptance/ini_setting_spec.rb index 140a35e5..93c75cc4 100644 --- a/spec/acceptance/ini_setting_spec.rb +++ b/spec/acceptance/ini_setting_spec.rb @@ -97,7 +97,7 @@ subject { super().content } it { is_expected.to match %r{four = five} } - it { is_expected.not_to match %r{\[one\]} } + it { is_expected.to match %r{\[one\]} } it { is_expected.not_to match %r{two = three} } end end @@ -296,7 +296,8 @@ describe '#content' do subject { super().content } - it { is_expected.to be_empty } + it { is_expected.to match %r{\[section1\]} } + it { is_expected.not_to match %r{valueinsection1 = newValue} } end end end diff --git a/spec/acceptance/ini_subsetting_spec.rb b/spec/acceptance/ini_subsetting_spec.rb index 297ce4c7..af16b12d 100644 --- a/spec/acceptance/ini_subsetting_spec.rb +++ b/spec/acceptance/ini_subsetting_spec.rb @@ -130,7 +130,7 @@ describe '#content' do subject { super().content } - it { is_expected.not_to match %r{\[one\]} } + it { is_expected.to match %r{\[one\]} } it { is_expected.not_to match %r{key =} } it { is_expected.not_to match %r{alphabet} } it { is_expected.not_to match %r{betatrons} } diff --git a/spec/unit/puppet/provider/ini_setting/ruby_spec.rb b/spec/unit/puppet/provider/ini_setting/ruby_spec.rb index a250bcde..30c5e693 100644 --- a/spec/unit/puppet/provider/ini_setting/ruby_spec.rb +++ b/spec/unit/puppet/provider/ini_setting/ruby_spec.rb @@ -1002,6 +1002,7 @@ def self.file_path #another comment ; yet another comment + -nonstandard- INIFILE it 'removes a setting with pre/suffix that exists' do resource = Puppet::Type::Ini_setting.new(common_params.merge(section: 'nonstandard', setting: 'shoes', ensure: 'absent', section_prefix: '-', section_suffix: '-')) @@ -1108,37 +1109,6 @@ def self.file_path provider.destroy validate_file(expected_content_five, tmpfile) end - - expected_content_six = <<~INIFILE - [section1] - ; This is also a comment - foo=foovalue - - bar = barvalue - main = true - [section2] - - foo= foovalue2 - baz=bazvalue - url = http://192.168.1.1:8080 - [section3] - # com = ment - uncom = ment - [section:sub] - subby=bar - #another comment - ; yet another comment - - -nonstandard- - shoes = purple - INIFILE - it 'removes the section when removing the last line in the section' do - resource = Puppet::Type::Ini_setting.new(common_params.merge(section: 'section4', setting: 'uncom', ensure: 'absent')) - provider = described_class.new(resource) - expect(provider.exists?).to be true - provider.destroy - validate_file(expected_content_six, tmpfile) - end end context 'when dealing with indentation in sections' do diff --git a/spec/unit/puppet/provider/ini_subsetting/ruby_spec.rb b/spec/unit/puppet/provider/ini_subsetting/ruby_spec.rb index 9236fb68..49a186a2 100644 --- a/spec/unit/puppet/provider/ini_subsetting/ruby_spec.rb +++ b/spec/unit/puppet/provider/ini_subsetting/ruby_spec.rb @@ -345,7 +345,9 @@ def validate_file(expected_content, tmpfile) something = else INIFILE - expected_content_two = '' + expected_content_two = <<-INIFILE + [main] + INIFILE it 'removes the subsetting when the it is empty' do resource = Puppet::Type::Ini_subsetting.new(common_params.merge(setting: 'reports', subsetting: 'http', subsetting_separator: ','))