feat(bootstrapper): record bootstrap stack state on each loop iteration#1159
feat(bootstrapper): record bootstrap stack state on each loop iteration#1159dhellmann wants to merge 1 commit into
Conversation
Write `bootstrap-stack.json` to the work directory before each iteration of the DFS loop, capturing the full stack as a JSON array (index 0 = next item to pop). Enables debugging, progress inspection, and post-mortem analysis of interrupted runs. Co-Authored-By: Claude <claude@anthropic.com> Signed-off-by: Doug Hellmann <dhellmann@redhat.com>
📝 WalkthroughWalkthroughThe PR adds stack persistence to the bootstrap resolver. The Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/fromager/bootstrapper.py`:
- Around line 1305-1307: The current bootstrap() snapshot write using
open(self._stack_filename, "w") may truncate the file and abort bootstrap on
error; instead, serialize the reversed(stack) into records and write JSON
atomically by writing to a temporary file in the same directory (e.g., use
tempfile.NamedTemporaryFile or a .tmp path) and then atomically replace the
target with os.replace; catch and log any exceptions (use the existing logger on
the object, e.g., self._logger or fallback logger) as a non-fatal warning so
bootstrap() continues on failure, referencing the existing serialize() call and
self._stack_filename to locate the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 876da36e-8403-4513-abee-eb324d8b2989
📒 Files selected for processing (2)
src/fromager/bootstrapper.pytests/test_bootstrapper.py
| records = [serialize(item) for item in reversed(stack)] | ||
| with open(self._stack_filename, "w") as f: | ||
| json.dump(records, f, indent=2, default=str) |
There was a problem hiding this comment.
Make stack snapshot writes atomic and non-fatal.
A failed snapshot write currently aborts bootstrap(), and direct writes can leave a truncated JSON file if interrupted. For a diagnostics artifact, this should degrade gracefully and write atomically.
Suggested fix
records = [serialize(item) for item in reversed(stack)]
- with open(self._stack_filename, "w") as f:
- json.dump(records, f, indent=2, default=str)
+ tmp_filename = self._stack_filename.with_suffix(".tmp")
+ try:
+ with open(tmp_filename, "w", encoding="utf-8") as f:
+ json.dump(records, f, indent=2, default=str)
+ os.replace(tmp_filename, self._stack_filename)
+ except OSError as err:
+ logger.warning(
+ "failed to write bootstrap stack state to %s: %s",
+ self._stack_filename,
+ err,
+ )
+ with contextlib.suppress(FileNotFoundError):
+ tmp_filename.unlink()📝 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.
| records = [serialize(item) for item in reversed(stack)] | |
| with open(self._stack_filename, "w") as f: | |
| json.dump(records, f, indent=2, default=str) | |
| records = [serialize(item) for item in reversed(stack)] | |
| tmp_filename = self._stack_filename.with_suffix(".tmp") | |
| try: | |
| with open(tmp_filename, "w", encoding="utf-8") as f: | |
| json.dump(records, f, indent=2, default=str) | |
| os.replace(tmp_filename, self._stack_filename) | |
| except OSError as err: | |
| logger.warning( | |
| "failed to write bootstrap stack state to %s: %s", | |
| self._stack_filename, | |
| err, | |
| ) | |
| with contextlib.suppress(FileNotFoundError): | |
| tmp_filename.unlink() |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/fromager/bootstrapper.py` around lines 1305 - 1307, The current
bootstrap() snapshot write using open(self._stack_filename, "w") may truncate
the file and abort bootstrap on error; instead, serialize the reversed(stack)
into records and write JSON atomically by writing to a temporary file in the
same directory (e.g., use tempfile.NamedTemporaryFile or a .tmp path) and then
atomically replace the target with os.replace; catch and log any exceptions (use
the existing logger on the object, e.g., self._logger or fallback logger) as a
non-fatal warning so bootstrap() continues on failure, referencing the existing
serialize() call and self._stack_filename to locate the change.
1 participant
Summary
Writes
bootstrap-stack.jsonto the work directory before each iteration of the DFS loop inBootstrapper.bootstrap(). The file is a JSON array where index 0 is the next item to be processed (stack[-1]), overwritten on each iteration. Each entry captures identity and accumulated state fields (req,req_type,phase,resolved_version,source_url,why,parent, dep sets); heavy build objects (build_env, paths,build_result) are intentionally excluded.Enables debugging, progress inspection, and post-mortem analysis of interrupted runs.
🤖 Generated with Claude Code