Skip to content

allow extending or overriding the patching config from the userpatche… #8690

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+3 −5
Open

allow extending or overriding the patching config from the userpatche… #8690

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions lib/tools/common/patching_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,9 @@ def __str__(self):
class PatchingConfig:
def __init__(self, yaml_config_file_paths: list[str]):
self.yaml_config_file_paths = yaml_config_file_paths
if len(yaml_config_file_paths) == 0:
self.yaml_config = {}
else:
# I'm lazy, single one for now.
self.yaml_config = self.read_yaml_config(yaml_config_file_paths[0])["config"]
self.yaml_config = {}
for p in yaml_config_file_paths:
self.yaml_config.update(self.read_yaml_config(p)["config"])
Comment on lines +55 to +57
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Current merge logic overwrites list settings, defeating the “extend” goal

Using dict.update() per fragment means any list-valued key from the Armbian-supplied config is wholesale replaced once the user fragment defines the same key. For example, if the base 0000.patching_config.yaml already ships with an overlay-directories entry and the user file only adds a new overlay, the final config will contain only the user-supplied list, silently discarding the base overlays. That directly contradicts the PR’s objective of letting userpatches extend the default config, and it will purge existing overlay/autopatch directives as soon as the user defines that section. Please switch to a merge routine that appends list values (and merges nested dicts) while still allowing explicit overrides when the user provides a non-list (e.g. null) value.

@@
-		self.yaml_config = {}
-		for p in yaml_config_file_paths:
-			self.yaml_config.update(self.read_yaml_config(p)["config"])
+		self.yaml_config: dict = {}
+		for p in yaml_config_file_paths:
+			self._merge_config(self.read_yaml_config(p)["config"])
+
+	def _merge_config(self, fragment: dict):
+		for key, value in fragment.items():
+			if key not in self.yaml_config or value is None:
+				self.yaml_config[key] = value
+				continue
+			current = self.yaml_config[key]
+			if isinstance(current, list) and isinstance(value, list):
+				self.yaml_config[key] = [*current, *value]
+			elif isinstance(current, dict) and isinstance(value, dict):
+				merged = current.copy()
+				merged.update(value)
+				self.yaml_config[key] = merged
+			else:
+				self.yaml_config[key] = value
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
self.yaml_config = {}
for p in yaml_config_file_paths:
self.yaml_config.update(self.read_yaml_config(p)["config"])
# in __init__ of PatchingConfig or similar class
self.yaml_config: dict = {}
for p in yaml_config_file_paths:
self._merge_config(self.read_yaml_config(p)["config"])
# Add this method to the class
def _merge_config(self, fragment: dict):
for key, value in fragment.items():
# If new key, or explicit null override, just assign
if key not in self.yaml_config or value is None:
self.yaml_config[key] = value
continue
current = self.yaml_config[key]
# Extend lists rather than replace
if isinstance(current, list) and isinstance(value, list):
self.yaml_config[key] = [*current, *value]
# Shallow-merge dicts
elif isinstance(current, dict) and isinstance(value, dict):
merged = current.copy()
merged.update(value)
self.yaml_config[key] = merged
# Fallback to override for other types
else:
self.yaml_config[key] = value
🤖 Prompt for AI Agents 
In lib/tools/common/patching_config.py around lines 55-57, the current loop uses
dict.update() which replaces list-valued keys instead of extending them; replace
that with a deep-merge routine: for each fragment, recursively merge into
self.yaml_config by merging nested dicts key-by-key, concatenating lists
(appending fragment list items to existing list) and only performing a full
override when the fragment value is not a list/dict (including explicit
null/None), so user fragments can extend default lists but still explicitly
override by providing non-list values; implement a small helper
merge_dicts(dest, src) and call it for each read_yaml_config(p)["config"]
instead of update().


self.patches_to_git_config: PatchingToGitConfig = PatchingToGitConfig(self.yaml_config.get("patches-to-git", {}))

Expand Down