diff --git a/NEWS b/NEWS index 6062cee5..dea3f300 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,8 @@ 2.0.6.dev0 * #1068: Fix a warning from LVM about leaked file descriptors. * #1086: Fix for the "spot" check breaking when the "--progress" flag is used. + * #1091: Fix for the "config generate" action generating invalid configuration when upgrading + deprecated command hooks. * Add support for Borg 2's "s3:" and "b2:" repository URLs, so you can backup to S3 or B2 cloud storage services even without using Rclone. diff --git a/borgmatic/config/generate.py b/borgmatic/config/generate.py index 734457f2..3a20837f 100644 --- a/borgmatic/config/generate.py +++ b/borgmatic/config/generate.py @@ -51,7 +51,7 @@ def schema_to_sample_configuration(schema, source_config=None, level=0, parent_i add_comments_to_configuration_sequence(config, schema, indent=(level * INDENT)) elif borgmatic.config.schema.compare_types(schema_type, {'object'}): if source_config and isinstance(source_config, list) and isinstance(source_config[0], dict): - source_config = dict(collections.ChainMap(*source_config)) + source_config = source_config[0] config = ( ruamel.yaml.comments.CommentedMap( @@ -71,7 +71,7 @@ def schema_to_sample_configuration(schema, source_config=None, level=0, parent_i ) indent = (level * INDENT) + (SEQUENCE_INDENT if parent_is_sequence else 0) add_comments_to_configuration_object( - config, schema, source_config, indent=indent, skip_first=parent_is_sequence + config, schema, source_config, indent=indent, skip_first_field=parent_is_sequence ) elif borgmatic.config.schema.compare_types(schema_type, SCALAR_SCHEMA_TYPES, match=all): return example @@ -108,17 +108,30 @@ def comment_out_optional_configuration(rendered_config): ''' lines = [] optional = False + indent_characters = None + indent_characters_at_sentinel = None for line in rendered_config.split('\n'): + indent_characters = len(line) - len(line.lstrip()) + # Upon encountering an optional configuration option, comment out lines until the next blank # line. if line.strip().startswith(f'# {COMMENTED_OUT_SENTINEL}'): optional = True + indent_characters_at_sentinel = indent_characters continue # Hit a blank line, so reset commenting. if not line.strip(): optional = False + indent_characters_at_sentinel = None + # Dedented, so reset commenting. + elif ( + indent_characters_at_sentinel is not None + and indent_characters < indent_characters_at_sentinel + ): + optional = False + indent_characters_at_sentinel = None lines.append(comment_out_line(line) if optional else line) @@ -198,25 +211,28 @@ COMMENTED_OUT_SENTINEL = 'COMMENT_OUT' def add_comments_to_configuration_object( - config, schema, source_config=None, indent=0, skip_first=False + config, schema, source_config=None, indent=0, skip_first_field=False ): ''' Using descriptions from a schema as a source, add those descriptions as comments to the given configuration dict, putting them before each field. Indent the comment the given number of characters. + If skip_first_field is True, omit the comment for the initial field. This is useful for + sequences, where the comment for the first field goes before the sequence itself. + And a sentinel for commenting out options that are neither in DEFAULT_KEYS nor the the given source configuration dict. The idea is that any options used in the source configuration should stay active in the generated configuration. ''' for index, field_name in enumerate(config.keys()): - if skip_first and index == 0: + if skip_first_field and index == 0: continue field_schema = borgmatic.config.schema.get_properties(schema).get(field_name, {}) description = field_schema.get('description', '').strip() - # If this isn't a default key, add an indicator to the comment flagging it to be commented + # If this isn't a default key, add an indicator to the comment, flagging it to be commented # out from the sample configuration. This sentinel is consumed by downstream processing that # does the actual commenting out. if field_name not in DEFAULT_KEYS and ( @@ -299,6 +315,12 @@ def generate_sample_configuration( source_config = load.load_configuration(source_filename) normalize.normalize(source_filename, source_config) + # The borgmatic.config.normalize.normalize() function tacks on an empty "bootstrap" if + # needed, so the hook gets used by default. But we don't want it to end up in the generated + # config unless the user has set it explicitly, as an empty "bootstrap:" won't validate. + if source_config and source_config.get('bootstrap') == {}: + del source_config['bootstrap'] + destination_config = merge_source_configuration_into_destination( schema_to_sample_configuration(schema, source_config), source_config ) diff --git a/tests/integration/config/test_generate.py b/tests/integration/config/test_generate.py index 71c0abc5..5e0f98e5 100644 --- a/tests/integration/config/test_generate.py +++ b/tests/integration/config/test_generate.py @@ -126,6 +126,45 @@ def test_schema_to_sample_configuration_comments_out_non_source_config_options_i assert 'field3' not in config[0].ca.items +def test_schema_to_sample_configuration_comments_out_non_source_config_options_in_sequence_of_maps_with_different_subschemas(): + schema = { + 'type': 'array', + 'items': { + 'type': 'object', + 'oneOf': [ + { + 'properties': dict( + [ + ('field1', {'type': 'string', 'example': 'Example 1'}), + ('field2', {'type': 'string', 'example': 'Example 2'}), + ] + ) + }, + { + 'properties': dict( + [ + ('field2', {'type': 'string', 'example': 'Example 2'}), + ('field3', {'type': 'string', 'example': 'Example 3'}), + ] + ) + }, + ], + }, + } + source_config = [{'field1': 'value'}, {'field3': 'value'}] + + config = module.schema_to_sample_configuration(schema, source_config) + + assert config == [ + dict([('field1', 'Example 1'), ('field2', 'Example 2'), ('field3', 'Example 3')]) + ] + + # The first field in a sequence does not get commented. + assert 'field1' not in config[0].ca.items + assert 'COMMENT_OUT' in config[0].ca.items['field2'][1][-1]._value + assert 'COMMENT_OUT' in config[0].ca.items['field3'][1][-1]._value + + def test_comment_out_line_skips_blank_line(): line = ' \n' @@ -169,8 +208,11 @@ foo: - quux repositories: - - one - - two + - path: foo + # COMMENT_OUT + label: bar + - path: baz + label: quux # This comment should be kept. # COMMENT_OUT @@ -185,8 +227,10 @@ other: thing # - quux repositories: - - one - - two + - path: foo +# label: bar + - path: baz + label: quux # This comment should be kept. # other: thing @@ -297,14 +341,28 @@ def test_add_comments_to_configuration_comments_out_non_source_config_options(): assert 'baz' not in config.ca.items -def test_add_comments_to_configuration_object_with_skip_first_does_not_comment_out_first_option(): +def test_add_comments_to_configuration_object_with_skip_first_field_does_not_comment_out_first_option(): config = module.ruamel.yaml.comments.CommentedMap([('foo', 33), ('bar', 44), ('baz', 55)]) schema = { 'type': 'object', 'properties': {'foo': {'description': 'Foo'}, 'bar': {'description': 'Bar'}}, } - module.add_comments_to_configuration_object(config, schema, skip_first=True) + module.add_comments_to_configuration_object(config, schema, skip_first_field=True) + + assert 'foo' not in config.ca.items + assert 'COMMENT_OUT' in config.ca.items['bar'][1][-1]._value + assert 'baz' not in config.ca.items + + +def test_add_comments_to_configuration_object_with_skip_first_field_does_not_comment_out_first_option(): + config = module.ruamel.yaml.comments.CommentedMap([('foo', 33), ('bar', 44), ('baz', 55)]) + schema = { + 'type': 'object', + 'properties': {'foo': {'description': 'Foo'}, 'bar': {'description': 'Bar'}}, + } + + module.add_comments_to_configuration_object(config, schema, skip_first_field=True) assert 'foo' not in config.ca.items assert 'COMMENT_OUT' in config.ca.items['bar'][1][-1]._value @@ -326,15 +384,39 @@ def test_generate_sample_configuration_does_not_raise(): module.generate_sample_configuration(False, None, 'dest.yaml', 'schema.yaml') -def test_generate_sample_configuration_with_source_filename_does_not_raise(): +def test_generate_sample_configuration_with_source_filename_omits_empty_bootstrap_field(): builtins = flexmock(sys.modules['builtins']) builtins.should_receive('open').with_args('schema.yaml').and_return('') flexmock(module.ruamel.yaml).should_receive('YAML').and_return( flexmock(load=lambda filename: {}) ) - flexmock(module.load).should_receive('load_configuration') + flexmock(module.load).should_receive('load_configuration').and_return( + {'bootstrap': {}, 'foo': 'bar'} + ) flexmock(module.normalize).should_receive('normalize') - flexmock(module).should_receive('schema_to_sample_configuration') + flexmock(module).should_receive('schema_to_sample_configuration').with_args( + object, {'foo': 'bar'} + ).once() + flexmock(module).should_receive('merge_source_configuration_into_destination') + flexmock(module).should_receive('render_configuration') + flexmock(module).should_receive('comment_out_optional_configuration') + flexmock(module).should_receive('write_configuration') + + module.generate_sample_configuration(False, 'source.yaml', 'dest.yaml', 'schema.yaml') + + +def test_generate_sample_configuration_with_source_filename_keeps_non_empty_bootstrap_field(): + builtins = flexmock(sys.modules['builtins']) + builtins.should_receive('open').with_args('schema.yaml').and_return('') + flexmock(module.ruamel.yaml).should_receive('YAML').and_return( + flexmock(load=lambda filename: {}) + ) + source_config = {'bootstrap': {'stuff': 'here'}, 'foo': 'bar'} + flexmock(module.load).should_receive('load_configuration').and_return(source_config) + flexmock(module.normalize).should_receive('normalize') + flexmock(module).should_receive('schema_to_sample_configuration').with_args( + object, source_config + ).once() flexmock(module).should_receive('merge_source_configuration_into_destination') flexmock(module).should_receive('render_configuration') flexmock(module).should_receive('comment_out_optional_configuration')