From c3d1bd0dde9f4553b825a4720208e4d5e17abe6b Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Thu, 10 Apr 2025 13:45:39 +0200 Subject: [PATCH] Rework history <=> sandbox integration Until now, we read the tools tree configuration both inside and outside the sandbox. This is problematic for a few reasons: - For a number of reasons, the tools tree configuration inside and outside the sandbox may be different. This is especially true when the history is used, as when invoking mkosi sandbox and mkosi is invoked again within the sandbox, we don't know when invoking mkosi sandbox whether the nested invocation will reuse the history or not. This can lead to situations where mkosi sandbox thinks the tools tree is up-to-date whereas mkosi inside the sandbox thinks the tools tree is out of date, which is a situation that is very hard to resolve for users. - Even if we read the tools tree configuration inside the sandbox, there's no real point to it except maybe for showing the summary. Since the tools tree has already been built and mounted at that point, we can't build or use a new tools tree, or clean up an old one. Let's fix these issues by *not* reading the tools tree configuration inside the sandbox, neither from configuration not from history. The one piece of tools tree configuration we do need to pass into the sandbox is the default tools tree path as that gets included in the history of the other images so we pass it in via an environment variable $MKOSI_DEFAULT_TOOLS_TREE_PATH. As a consequence of this approach, we cannot write the tools tree history from within the sandbox anymore, so we write it to a separate file from outside the sandbox instead. Additionally, we make more commands use the history. Up until now summary and other verbs did not make use of the history. This doesn't really make sense, so let's make sure they make use of the history as well. summary in particular is important as it may be used to query information about the built image after building it which needs to take the history into account. We still allow summary to ignore the history by passing --force. Also we still ignore the history for clean as there are use cases both for having it use the history and not yet the --force flag already has meaning there. --- mkosi/__init__.py | 42 +++++++++++++++-------------- mkosi/config.py | 48 +++++++++++++++++++++++++--------- mkosi/resources/man/mkosi.1.md | 3 ++- 3 files changed, 60 insertions(+), 33 deletions(-) diff --git a/mkosi/__init__.py b/mkosi/__init__.py index 9187e1b3a..0fcb26deb 100644 --- a/mkosi/__init__.py +++ b/mkosi/__init__.py @@ -4126,6 +4126,8 @@ def run_sandbox(args: Args, config: Config) -> None: env |= {"MKOSI_HOST_DISTRIBUTION": str(hd)} if hr: env |= {"MKOSI_HOST_RELEASE": hr} + if config.tools_tree: + env |= {"MKOSI_DEFAULT_TOOLS_TREE_PATH": os.fspath(config.tools_tree)} cmdline = [*args.cmdline] @@ -4933,13 +4935,8 @@ def run_build( ) -def dump_history_json(tools: Optional[Config], images: Sequence[Config]) -> str: - return json.dumps( - {"Tools": tools.to_dict() if tools else None, "Images": [config.to_dict() for config in images]}, - cls=JsonEncoder, - indent=4, - sort_keys=True, - ) +def dump_json(dict: dict[str, Any]) -> str: + return json.dumps(dict, cls=JsonEncoder, indent=4, sort_keys=True) def run_verb(args: Args, tools: Optional[Config], images: Sequence[Config], *, resources: Path) -> None: @@ -4994,7 +4991,12 @@ def run_verb(args: Args, tools: Optional[Config], images: Sequence[Config], *, r if args.verb == Verb.summary: if args.json: - text = dump_history_json(tools, images) + text = dump_json( + { + "Tools": tools.to_dict() if tools else None, + "Images": [config.to_dict() for config in images], + } + ) else: text = f"{summary(tools)}\n" if tools else "" text += "\n".join(summary(config) for config in images) @@ -5032,12 +5034,6 @@ def run_verb(args: Args, tools: Optional[Config], images: Sequence[Config], *, r "been built yet", hint="Make sure to (re)build the tools tree first with 'mkosi build' or use '--force'", ) - elif in_sandbox(): - die( - "Cannot rebuild out-of-date default tools tree from inside mkosi sandbox", - hint="Exit mkosi sandbox and re-enter the sandbox again with mkosi -f sandbox to " - "rebuild the default tools tree", - ) elif last.incremental == Incremental.strict: die( "Default tools tree is out-of-date but the strict incremental mode is enabled", @@ -5075,12 +5071,16 @@ def run_verb(args: Args, tools: Optional[Config], images: Sequence[Config], *, r keyring_dir=Path(keyring_dir), metadata_dir=Path(metadata_dir), ) - _, _, manifest = cache_tree_paths(tools) - manifest.write_text( - json.dumps(tools.cache_manifest(), cls=JsonEncoder, indent=4, sort_keys=True) - ) - if not args.verb.needs_build(): + _, _, 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 { Verb.ssh: run_ssh, Verb.journalctl: run_journalctl, @@ -5281,7 +5281,9 @@ def run_verb(args: Args, tools: Optional[Config], images: Sequence[Config], *, r if last.history and history: Path(".mkosi-private/history").mkdir(parents=True, exist_ok=True) - Path(".mkosi-private/history/latest.json").write_text(dump_history_json(tools, images)) + 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 b3fc41992..9e8ec00f4 100644 --- a/mkosi/config.py +++ b/mkosi/config.py @@ -108,6 +108,9 @@ class Verb(StrEnum): Verb.dependencies, ) + def needs_tools(self) -> bool: + return self in (Verb.sandbox, Verb.journalctl, Verb.coredumpctl, Verb.ssh) + def needs_build(self) -> bool: return self in ( Verb.build, @@ -4904,13 +4907,25 @@ class ParseContext: return ns -def have_history(args: Args) -> bool: - return ( - args.directory is not None - and args.verb.needs_build() - and ((args.verb != Verb.build and not args.force) or args.rerun_build_scripts) - and Path(".mkosi-private/history/latest.json").exists() - ) +def have_history(args: Args, tools: bool = False) -> bool: + filename = "tools" if tools else "latest" + + if args.directory is None: + return False + + if args.verb in (Verb.build, Verb.cat_config, Verb.clean): + return False + + if args.verb == Verb.summary and args.force > 0: + return False + + 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: + return False + + return Path(f".mkosi-private/history/{filename}.json").exists() def finalize_default_tools( @@ -5007,8 +5022,11 @@ def parse_config( if args.debug_sandbox: ARG_DEBUG_SANDBOX.set(args.debug_sandbox) + if args.rerun_build_scripts and not args.verb.needs_build(): + die(f"--rerun-build-scripts cannot be used with the '{args.verb}' command") + if args.cmdline and not args.verb.supports_cmdline(): - die(f"Arguments after verb are not supported for {args.verb}.") + die(f"Arguments after verb are not supported for the '{args.verb}' command") # If --debug was passed, apply it as soon as possible. if ARG_DEBUG.get(): @@ -5024,7 +5042,6 @@ def parse_config( if have_history(args): try: j = json.loads(Path(".mkosi-private/history/latest.json").read_text()) - tools = Config.from_json(j["Tools"]) if j["Tools"] is not None else None *subimages, prev = [Config.from_json(c) for c in j["Images"]] except (KeyError, ValueError): die( @@ -5053,10 +5070,14 @@ def parse_config( context.only_sections = ("Include", "Runtime", "Host") else: context.only_sections = tuple(only_sections) - tools = None 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 + # One of the specifiers needs access to the directory, so make sure it is available. context.config["directory"] = args.directory @@ -5077,8 +5098,11 @@ def parse_config( return args, tools, (*subimages, Config.from_dict(config)) if config.get("tools_tree") == Path("default"): - tools = finalize_default_tools(context, config, configdir=configdir, resources=resources) - config["tools_tree"] = tools.output_dir_or_cwd() / tools.output + if in_sandbox(): + config["tools_tree"] = os.environ["MKOSI_DEFAULT_TOOLS_TREE_PATH"] + else: + tools = finalize_default_tools(context, config, configdir=configdir, resources=resources) + config["tools_tree"] = tools.output_dir_or_cwd() / tools.output images = [] diff --git a/mkosi/resources/man/mkosi.1.md b/mkosi/resources/man/mkosi.1.md index 1d553727f..600e292db 100644 --- a/mkosi/resources/man/mkosi.1.md +++ b/mkosi/resources/man/mkosi.1.md @@ -1587,7 +1587,8 @@ boolean argument: either `1`, `yes`, or `true` to enable, or `0`, `no`, 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 - needs a build without specifying `--force`. + 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. -- 2.47.3