Browse Source

When deep merging common configuration, merge colliding list values by appending them (#531).

Dan Helfman 3 years ago
parent
commit
255cc6ec23

+ 4 - 2
NEWS

@@ -1,8 +1,10 @@
 1.6.1.dev0
 1.6.1.dev0
  * #528: Improve the error message when a configuration override contains an invalid value.
  * #528: Improve the error message when a configuration override contains an invalid value.
+ * #531: BREAKING: When deep merging common configuration, merge colliding list values by appending
+   them. Previously, one list replaced the other.
  * #532: When a configuration include is a relative path, load it from either the current working
  * #532: When a configuration include is a relative path, load it from either the current working
-   directory or from the directory containing the file doing the including. (Previously, only the
-   working directory was used.)
+   directory or from the directory containing the file doing the including. Previously, only the
+   working directory was used.
  * Add a randomized delay to the sample systemd timer to spread out the load on a server.
  * Add a randomized delay to the sample systemd timer to spread out the load on a server.
  * Add emojis to documentation table of contents to make it easier to find particular how-to and
  * Add emojis to documentation table of contents to make it easier to find particular how-to and
    reference guides at a glance.
    reference guides at a glance.

+ 14 - 0
borgmatic/config/load.py

@@ -157,6 +157,20 @@ def deep_merge_nodes(nodes):
                             anchor=b_value.anchor,
                             anchor=b_value.anchor,
                         ),
                         ),
                     )
                     )
+                # If we're dealing with SequenceNodes, merge by appending one sequence to the other.
+                elif isinstance(b_value, ruamel.yaml.nodes.SequenceNode):
+                    replaced_nodes[(b_key, b_value)] = (
+                        b_key,
+                        ruamel.yaml.nodes.SequenceNode(
+                            tag=b_value.tag,
+                            value=a_value.value + b_value.value,
+                            start_mark=b_value.start_mark,
+                            end_mark=b_value.end_mark,
+                            flow_style=b_value.flow_style,
+                            comment=b_value.comment,
+                            anchor=b_value.anchor,
+                        ),
+                    )
 
 
     return [
     return [
         replaced_nodes.get(node, node) for node in nodes if replaced_nodes.get(node) != DELETED_NODE
         replaced_nodes.get(node, node) for node in nodes if replaced_nodes.get(node) != DELETED_NODE

+ 3 - 3
docs/how-to/make-per-application-backups.md

@@ -125,9 +125,9 @@ Once this include gets merged in, the resulting configuration would have a
 When there's an option collision between the local file and the merged
 When there's an option collision between the local file and the merged
 include, the local file's option takes precedence. And as of borgmatic 1.6.0,
 include, the local file's option takes precedence. And as of borgmatic 1.6.0,
 this feature performs a deep merge, meaning that values are merged at all
 this feature performs a deep merge, meaning that values are merged at all
-levels in the two configuration files. This allows you to include common
-configuration—up to full borgmatic configuration files—while overriding only
-the parts you want to customize.
+levels in the two configuration files. Colliding list values are appended
+together. This allows you to include common configuration—up to full borgmatic
+configuration files—while overriding only the parts you want to customize.
 
 
 Note that this `<<` include merging syntax is only for merging in mappings
 Note that this `<<` include merging syntax is only for merging in mappings
 (configuration options and their values). But if you'd like to include a
 (configuration options and their values). But if you'd like to include a

+ 46 - 0
tests/integration/config/test_load.py

@@ -326,3 +326,49 @@ def test_deep_merge_nodes_keeps_deeply_nested_values():
     assert nested_options[0][1].value == '--init-option'
     assert nested_options[0][1].value == '--init-option'
     assert nested_options[1][0].value == 'prune'
     assert nested_options[1][0].value == 'prune'
     assert nested_options[1][1].value == '--prune-option'
     assert nested_options[1][1].value == '--prune-option'
+
+
+def test_deep_merge_nodes_appends_colliding_sequence_values():
+    node_values = [
+        (
+            ruamel.yaml.nodes.ScalarNode(tag='tag:yaml.org,2002:str', value='hooks'),
+            ruamel.yaml.nodes.MappingNode(
+                tag='tag:yaml.org,2002:map',
+                value=[
+                    (
+                        ruamel.yaml.nodes.ScalarNode(
+                            tag='tag:yaml.org,2002:str', value='before_backup'
+                        ),
+                        ruamel.yaml.nodes.SequenceNode(
+                            tag='tag:yaml.org,2002:int', value=['echo 1', 'echo 2']
+                        ),
+                    ),
+                ],
+            ),
+        ),
+        (
+            ruamel.yaml.nodes.ScalarNode(tag='tag:yaml.org,2002:str', value='hooks'),
+            ruamel.yaml.nodes.MappingNode(
+                tag='tag:yaml.org,2002:map',
+                value=[
+                    (
+                        ruamel.yaml.nodes.ScalarNode(
+                            tag='tag:yaml.org,2002:str', value='before_backup'
+                        ),
+                        ruamel.yaml.nodes.SequenceNode(
+                            tag='tag:yaml.org,2002:int', value=['echo 3', 'echo 4']
+                        ),
+                    ),
+                ],
+            ),
+        ),
+    ]
+
+    result = module.deep_merge_nodes(node_values)
+    assert len(result) == 1
+    (section_key, section_value) = result[0]
+    assert section_key.value == 'hooks'
+    options = section_value.value
+    assert len(options) == 1
+    assert options[0][0].value == 'before_backup'
+    assert options[0][1].value == ['echo 1', 'echo 2', 'echo 3', 'echo 4']