Skip to content

Conversation

@cgwalters
Copy link
Collaborator

The /sysroot writability regressed this, and add a missing test case for this.

@bootc-bot bootc-bot bot requested a review from jmarrero November 19, 2025 14:32
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request fixes a regression in bootc usroverlay where /sysroot was being unnecessarily made writable. The fix correctly detects the boot environment without invoking storage preparation logic that has side effects. A new test case is also added to cover the usroverlay functionality, verifying that it creates a transient writable overlay on /usr that is discarded on reboot. My review includes a couple of suggestions to improve the naming and description in the new test files for better clarity and maintainability.

@@ -0,0 +1,3 @@
summary: Execute tests for bootc usrover
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

There's a typo in the summary. It should be usroverlay.

summary: Execute tests for bootc usroverlay

@jmarrero
Copy link
Contributor

code lgtm, but it's weird that only stream10 is failing with:

  x Eval block failed with pipeline input
    ,-[/var/ARTIFACTS/work-readonly-testsjy4ax59d/tmt/plans/integration/readonly-tests/discover/default-0/tests/tmt/tests/booted/readonly/030-test-locking-read.nu:19:1]
 18 | # Await completion
 19 | 0..$n | each { |v|
    : ^^|^^
    :   `-- source value
 20 |     loop {
    `----

Error: nu::shell::non_zero_exit_code

  x External command had a non-zero exit code
    ,-[/var/ARTIFACTS/work-readonly-testsjy4ax59d/tmt/plans/integration/readonly-tests/discover/default-0/tests/tmt/tests/booted/readonly/030-test-locking-read.nu:26:9]
 25 |         # check status
 26 |         systemctl status $"bootc-status-($v)" out> /dev/null
    :         ^^^^|^^^^
    :             `-- exited with code 3
 27 |         # Clean it up
    `----

I am re-running these and see if it's some glitch as this nu test was not changed here.

@cgwalters cgwalters added this to the 1.11.0 milestone Nov 19, 2025
@cgwalters
Copy link
Collaborator Author

OK right we need the same workaround here that landed in the factory reset test.

The `/sysroot` writability regressed this, and add a missing test
case for this.

Signed-off-by: Colin Walters <walters@verbum.org>
Since other tests are hitting it.

Signed-off-by: Colin Walters <walters@verbum.org>
@cgwalters cgwalters enabled auto-merge (rebase) November 19, 2025 21:06
@cgwalters cgwalters merged commit 29e5c9a into bootc-dev:main Nov 19, 2025
29 of 37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants