From: Daan De Meyer Date: Sun, 13 Apr 2025 20:42:55 +0000 (+0200) Subject: Only write CLI arguments to history instead of full config X-Git-Tag: v26~242^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=cc45fe3bad2ef7f40b226e858a233b3f2ea024e4;p=thirdparty%2Fmkosi.git Only write CLI arguments to history instead of full config The huge downside of writing the full config to the history is that any modifications to config files are ignored until the image is rebuilt. If we make the argument that changes to config files are usually supposed to be permanent rather than ephemeral, it doesn't make sense to not take them into account when reusing the history. Especially since the primary purpose of the history is to avoid specifying the same CLI arguments over and over again without encoding those CLI arguments into a config file. So instead of saving the full config to the history and not reparsing the configuration when reusing the history, let's only save the CLI arguments to the history and always reparse the configuration, even when we end up reusing the history. This keeps the crucial benefit of the history intact (not having to repeat yourself endlessly) while still making sure any new settings added to the configuration files are taken into account immediately. It also simplifies the implementation quite a bit, as we can get away again with maintaining a single history file instead of two. Additionally, we opt to completely ignore the history when using the sandbox verb. The idea here is that the history only applies to the final image (and subimages). This was already the case as any --tools-tree-xxx CLI options aren't saved in the history as they are not fields of the Config object. --- diff --git a/mkosi/__init__.py b/mkosi/__init__.py index 3c922b1ae..82a860b0e 100644 --- a/mkosi/__init__.py +++ b/mkosi/__init__.py @@ -73,7 +73,6 @@ from mkosi.config import ( expand_delayed_specifiers, finalize_configdir, format_bytes, - have_history, in_sandbox, parse_boolean, parse_config, @@ -5070,11 +5069,6 @@ def run_verb(args: Args, tools: Optional[Config], images: Sequence[Config], *, r _, _, manifest = cache_tree_paths(tools) manifest.write_text(dump_json(tools.cache_manifest())) - Path(".mkosi-private/history/tools.json").unlink(missing_ok=True) - - if tools and last.history and not Path(".mkosi-private/history/tools.json").exists(): - Path(".mkosi-private/history").mkdir(parents=True, exist_ok=True) - Path(".mkosi-private/history/tools.json").write_text(dump_json(tools.to_dict())) if args.verb.needs_tools(): return { @@ -5165,27 +5159,22 @@ def run_verb(args: Args, tools: Optional[Config], images: Sequence[Config], *, r if args.force > 1 or not have_cache(initrd): remove_cache_entries(initrd) - if not have_history(args): - for i, config in enumerate(images): - if args.verb != Verb.build: - check_tools(config, args.verb) + for i, config in enumerate(images): + if args.verb != Verb.build: + check_tools(config, args.verb) - images[i] = config = run_configure_scripts(config) + images[i] = config = run_configure_scripts(config) - # The images array has been modified so we need to reevaluate last again. - # Also ensure that all other images are reordered in case their dependencies were modified. - last = images[-1] - images = resolve_deps(images[:-1], last.dependencies) + [last] + # The images array has been modified so we need to reevaluate last again. + # Also ensure that all other images are reordered in case their dependencies were modified. + last = images[-1] + images = resolve_deps(images[:-1], last.dependencies) + [last] if ( args.rerun_build_scripts or last.output_format == OutputFormat.none or not (last.output_dir_or_cwd() / last.output).exists() ): - history = (last.output_format == OutputFormat.none and not args.rerun_build_scripts) or ( - last.output_format != OutputFormat.none and not (last.output_dir_or_cwd() / last.output).exists() - ) - for config in images: if any( source.type != KeySourceType.file @@ -5268,12 +5257,6 @@ def run_verb(args: Args, tools: Optional[Config], images: Sequence[Config], *, r if args.auto_bump: bump_image_version() - if last.history and history: - Path(".mkosi-private/history").mkdir(parents=True, exist_ok=True) - Path(".mkosi-private/history/latest.json").write_text( - dump_json({"Images": [config.to_dict() for config in images]}) - ) - if args.verb == Verb.build: return diff --git a/mkosi/config.py b/mkosi/config.py index 8f9399c51..8c7b13ecf 100644 --- a/mkosi/config.py +++ b/mkosi/config.py @@ -2344,7 +2344,14 @@ class Config: return d @classmethod - def from_json(cls, s: Union[str, dict[str, Any], SupportsRead[str], SupportsRead[bytes]]) -> "Config": + def to_partial_dict(cls, partial: dict[str, Any]) -> dict[str, Any]: + return dict_with_capitalised_keys_factory([(k, v) for k, v in partial.items() if k in cls.fields()]) + + @classmethod + def from_partial_json( + cls, + s: Union[str, dict[str, Any], SupportsRead[str], SupportsRead[bytes]], + ) -> dict[str, Any]: """Instantiate a Config object from a (partial) JSON dump.""" if isinstance(s, str): j = json.loads(s) @@ -2374,9 +2381,13 @@ class Config: ) value_transformer = json_type_transformer(cls) - j = {(tk := key_transformer(k)): value_transformer(tk, v) for k, v in j.items()} + return {(tk := key_transformer(k)): value_transformer(tk, v) for k, v in j.items()} - return dataclasses.replace(cls.default(), **{k: v for k, v in j.items() if k in cls.fields()}) + @classmethod + def from_json(cls, s: Union[str, dict[str, Any], SupportsRead[str], SupportsRead[bytes]]) -> "Config": + return dataclasses.replace( + cls.default(), **{k: v for k, v in cls.from_partial_json(s).items() if k in cls.fields()} + ) def find_binary(self, *names: PathString, tools: bool = True) -> Optional[Path]: return find_binary(*names, root=self.tools() if tools else Path("/"), extra=self.extra_search_paths) @@ -4510,7 +4521,6 @@ class ParseContext: self.defaults: dict[str, Any] = {} # Compare inodes instead of paths so we can't get tricked by bind mounts and such. self.includes: set[tuple[int, int]] = set() - self.only_sections: tuple[str, ...] = tuple() def setting_prohibited(self, setting: ConfigSetting[T]) -> bool: image = self.config["image"] @@ -4791,9 +4801,6 @@ class ParseContext: if self.setting_prohibited(s): continue - if self.only_sections and s.section not in self.only_sections: - continue - for f in s.path_suffixes: f = f"mkosi.{f}" @@ -4833,7 +4840,7 @@ class ParseContext: for section, k, v in parse_ini( path, - only_sections=self.only_sections or {s.section for s in SETTINGS} | {"Host"}, + only_sections={s.section for s in SETTINGS} | {"Host"}, ): if not k and not v: continue @@ -4899,13 +4906,30 @@ class ParseContext: return ns -def have_history(args: Args, tools: bool = False) -> bool: - filename = "tools" if tools else "latest" +def want_new_history(args: Args) -> bool: + if args.directory is None: + return False + + if not args.verb.needs_build(): + return False + + if args.rerun_build_scripts: + return False + + if args.verb != Verb.build and args.force == 0: + return False + + return True + + +def have_history(args: Args) -> bool: + if want_new_history(args): + return False if args.directory is None: return False - if args.verb in (Verb.build, Verb.cat_config, Verb.clean): + if args.verb in (Verb.clean, Verb.sandbox): return False if args.verb == Verb.summary and args.force > 0: @@ -4914,10 +4938,13 @@ def have_history(args: Args, tools: bool = False) -> bool: if args.verb.needs_tools() and args.force > 0: return False - if args.verb.needs_build() and args.force > 0 and not args.rerun_build_scripts: + if args.verb.needs_build() and args.force > 0: return False - return Path(f".mkosi-private/history/{filename}.json").exists() + if args.verb == Verb.build and not args.rerun_build_scripts: + return False + + return Path(".mkosi-private/history/latest.json").exists() def finalize_default_tools( @@ -4989,7 +5016,6 @@ def parse_config( argv: Sequence[str] = (), *, resources: Path = Path("/"), - only_sections: Sequence[str] = (), ) -> tuple[Args, Optional[Config], tuple[Config, ...]]: argv = list(argv) @@ -5035,14 +5061,7 @@ def parse_config( return args, None, () if have_history(args): - try: - j = json.loads(Path(".mkosi-private/history/latest.json").read_text()) - *subimages, prev = [Config.from_json(c) for c in j["Images"]] - except (KeyError, ValueError): - die( - "Unable to parse history from .mkosi-private/history/latest.json", - hint="Build with -f to generate a new history file from scratch", - ) + history = Config.from_partial_json(Path(".mkosi-private/history/latest.json").read_text()) # If we're operating on a previously built image (vm, boot, shell, ...), we're not rebuilding the # image and the configuration of the latest build is available, we load the config that was used to @@ -5050,28 +5069,17 @@ def parse_config( # section settings which we allow changing without requiring a rebuild of the image. for s in SETTINGS: if s.section in ("Include", "Runtime"): + history.pop(s.dest, None) continue - if s.dest in context.cli and context.cli[s.dest] != getattr(prev, s.dest): + if s.dest in context.cli and context.cli[s.dest] != history[s.dest]: logging.warning( f"Ignoring {s.long} from the CLI. Run with -f to rebuild the image with this setting" ) - if hasattr(prev, s.dest): - context.cli[s.dest] = getattr(prev, s.dest) - if s.dest in context.config: - del context.config[s.dest] + context.cli |= history - context.only_sections = ("Include", "Runtime", "Host") - else: - context.only_sections = tuple(only_sections) - subimages = [] - prev = None - - if not in_sandbox() and have_history(args, tools=True): - tools = Config.from_json(json.loads(Path(".mkosi-private/history/tools.json").read_text())) - else: - tools = None + cli = copy.deepcopy(context.cli) # One of the specifiers needs access to the directory, so make sure it is available. context.config["directory"] = args.directory @@ -5089,9 +5097,11 @@ def parse_config( config = context.finalize() - if prev: - return args, tools, (*subimages, Config.from_dict(config)) + if config["history"] and want_new_history(args): + Path(".mkosi-private/history").mkdir(parents=True, exist_ok=True) + Path(".mkosi-private/history/latest.json").write_text(dump_json(Config.to_partial_dict(cli))) + tools = None if config.get("tools_tree") == Path("default"): if in_sandbox(): config["tools_tree"] = Path(os.environ["MKOSI_DEFAULT_TOOLS_TREE_PATH"]) diff --git a/mkosi/resources/man/mkosi.1.md b/mkosi/resources/man/mkosi.1.md index 9ff4c9be4..100dc0f30 100644 --- a/mkosi/resources/man/mkosi.1.md +++ b/mkosi/resources/man/mkosi.1.md @@ -1583,15 +1583,11 @@ boolean argument: either `1`, `yes`, or `true` to enable, or `0`, `no`, up in the generated XFS filesystem. `History=`, `--history=` -: Takes a boolean. If enabled, **mkosi** will write information about - the latest build to the `.mkosi-private` subdirectory in the - directory from which it was invoked. This information is then used - to restore the config of the latest build when running any verb that - does not rebuild the image or when running any verb that may rebuild - the image without specifying `--force`. - - Note that configure scripts will not be executed if we reuse the - history from a previous build. +: Takes a boolean. If enabled, **mkosi** will write the configuration + provided via the CLI for the latest build to the `.mkosi-private` + subdirectory in the directory from which it was invoked. These + arguments are then reused as long as the image is not rebuilt to + avoid having to specify them over and over again. To give an example of why this is useful, if you run `mkosi -O my-custom-output-dir -f` followed by `mkosi vm`, **mkosi**