install: always assume yes; --force means reinstall, not "no prompts" by mobileoverlord · Pull Request #250 · avocado-linux/avocado-cli

mobileoverlord · 2026-09-07T15:56:00Z

Owner: this PR is mine (the fast-rebuilds series), not the feeds series.
Both series push under the same account, so authorship does not distinguish
them.

avocado install, ext install, runtime install and sdk install apply the
package set avocado.yaml and avocado.lock already declare. dnf's
confirmation offers no decision there: approving changes nothing, declining
leaves a half-configured sysroot. All four now pass -y unconditionally.

The bug this fixes

-y was passed only under --force, which conflated two unrelated things
and made the documented invocation the expensive one. --force also clears
every extension's sysroot and drops its build and image stamps, so anyone
passing -f merely to skip the prompts — which is what our own material tells
people to do — discarded all built extension content and paid a full rebuild on
the next avocado build, every iteration.

Worse, without -f the prompt is reached and simply waits. Under the task
renderer it is not even visible, so avocado install on a project whose BSP
extension resolves looks like a hang with no output. That is the failure this
was found through: Is this ok [y/N]: on an extension dependency install, on a
real project, invisible.

Measured on a four-extension project: install -f then build skipped 2 of 10
steps in 10.2s; without -f it skips all 10 in 6.6s.

What `--force` means now

Only what its name says: reinstall from scratch. Existing scripts and docs using
-f keep working but should drop the flag — the help text on the install
commands now says so, and only on those commands (uninstall, provision and
runtime build keep theirs).

sdk dnf, ext dnf and runtime dnf are untouched: they pass their arguments
to dnf verbatim and still prompt, which is correct when a person is driving dnf
directly.

No opt-out flag

Deliberate. An interactive confirmation cannot be answered usefully in CI or
under the renderer — that is why the renderer was gated on --force in the
first place — and the dnf pass-through commands already cover reviewing a
transaction by hand. If "show me what this would do" is wanted later, the useful
shape is a dry-run, not a prompt.

Verification

dnf_sync_step_passes_assume_yes pins the step that consumes the flag and
asserts no install site still derives -y from --force. End to end on a real
Jetson project under a pty: install then build twice, 0 prompts, no hang,
all steps skipping on the rebuild.

Companion fix in this PR

--output json used to coerce force = force || output.is_json(), for two
reasons that are both gone: dnf needed -y, which installs now pass
unconditionally, and docker run -it failed without a tty, which
utils::interactivity now decides from what the environment supports. Left in
place it would be actively harmful — with --force meaning
reinstall-from-scratch, every --output json install would clear the extension
sysroots and force a full rebuild, reintroducing exactly the trap this PR
removes, for anyone driving the CLI programmatically.

Copilot

🟡 Changes recommended

--force is now documented as “reinstall from scratch”, but JSON/renderer-related paths still couple behavior to --force, which can cause surprising destructive reinstalls and inconsistent renderer behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates Avocado’s install flows to run non-interactively by default, decoupling dnf confirmation prompts from --force so installs don’t hang under renderers/CI and --force can consistently mean “reinstall from scratch”.

Changes:

Pass -y to dnf unconditionally for install operations (SDK/runtime/ext/sysroot).
Update CLI help text to clarify --force semantics as “reinstall from scratch” (not “no prompts”).
Add a regression test pinning that the dnf sync step includes -y, and document the behavior change in the changelog.

File summaries

File	Description
src/main.rs	Updates `--force` help text across multiple command enums.
src/commands/sdk/install.rs	Always passes `-y` for SDK install dnf invocations.
src/commands/runtime/install.rs	Always passes `-y` for runtime install dnf invocations.
src/commands/rootfs/install.rs	Always passes `-y` for sysroot install, adds regression test.
src/commands/install.rs	Updates renderer-related comment text (install orchestration context).
src/commands/ext/install.rs	Always passes `-y` for extension install dnf invocations.
CHANGELOG.md	Documents the behavioral change and revised meaning of `--force`.

Review details

Files reviewed: 7/7 changed files
Comments generated: 3
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

🔵 Needs a closer look

The updated --force help text is inaccurate for rootfs install and initramfs install (it claims extension build content is discarded), which is user-facing CLI documentation that should be corrected before approval.

Review details

Suppressed comments (2)

src/main.rs:4719

The rootfs install --force help text says forcing “discards every extension's built content”, but this subcommand operates on the rootfs sysroot; the extension rebuild impact is not a direct/accurate description here and can mislead users about what will be cleared.

        /// Reinstall from scratch: clear the sysroots and re-seed them.
        ///
        /// Not needed to skip dnf's prompts — installs never prompt. Forcing
        /// discards every extension's built content, so the next build has to
        /// redo all of it.

src/main.rs:4790

The initramfs install --force help text says forcing “discards every extension's built content”, but this subcommand operates on the initramfs sysroot; the extension rebuild impact is not a direct/accurate description here and can mislead users about what will be cleared.

        /// Reinstall from scratch: clear the sysroots and re-seed them.
        ///
        /// Not needed to skip dnf's prompts — installs never prompt. Forcing
        /// discards every extension's built content, so the next build has to
        /// redo all of it.

Files reviewed: 7/7 changed files
Comments generated: 0 new
Review effort level: Lite

…mpts" The four install commands apply the package set avocado.yaml and avocado.lock already declare. dnf's confirmation offers no decision there: approving changes nothing, declining leaves a half-configured sysroot. They now pass -y always. -y used to come only with --force, which made the documented invocation the expensive one. --force also clears every extension sysroot and drops its build and image stamps, so `install -f` — what our material tells people to run — threw away all built extension content and forced a full rebuild on the next build, every iteration. Four-extension project: `install -f && build` skipped 2 of 10 steps in 10.2s; without -f it skips 10 in 6.6s. --force now means only reinstall-from-scratch, and its help says so on the install commands (and only those — uninstall, provision and runtime build were left alone). Old scripts keep working; they should drop the flag. The TUI gate goes with it. The renderer required --force only because a dnf prompt would have been hidden behind it. sdk dnf / ext dnf / runtime dnf are untouched: they pass arguments to dnf verbatim and still prompt, which is correct when a person drives dnf directly. That is also why no opt-out flag was added — a prompt cannot be answered in CI or under the TUI, and the pass-throughs already cover reviewing by hand.

Three review findings with one root cause: `--force` was still doing a second job after this branch narrowed its meaning. `--output json` set `force = true`, for two reasons that this branch removed. The TUI renderer was gated on `--force` because dnf could prompt without it and the renderer made the prompt invisible — the hang. And `docker run -it` fails where no terminal is attached. Installs now always pass `-y`, and `utils::interactivity` decides the container's stdio from what the environment can actually support, so neither holds. With `--force` meaning reinstall from scratch, the coercion turned every request for machine-readable output into a full rebuild. So the renderer gate loses `&& self.force` and both `force || output.is_json()` coercions are deleted. Removing the gate is the rest of the same fix rather than new scope: the gate's stated reason, in the commit that added it, was that "dnf may prompt for confirmation and the user needs direct terminal access". An interactive `avocado install` now gets the live checklist without having to ask for a rebuild to see it. **Verified against a real project** (jetson-agx-thor, cold): `install --output json` takes 6m31s and emits 52 NDJSON step events, so the renderer is still created in JSON mode; the same command again takes 41s rather than repeating the 6m31s, which is the regression this fixes. A plain `install` with no flags takes 45s, exits 0, and prompts for nothing. The assume-yes regression test was also too narrow — one exact string in one file, while the coupling can come back in any of the five install sites and be spelled several ways. It now scans all five for the shape: the assume-yes literal and a flag on the same line. Verified by planting a reintroduction in `ext/install.rs` and watching it fail there. The new `json_output_does_not_imply_force` test does the same for `main.rs`, with its needle assembled at runtime so the assertion cannot match its own source.

… lock is released `a_held_lock_is_live_and_excludes_a_second_up` fails intermittently under the full parallel suite — measured 2 failures in 20 runs on this branch — and passes every time when run alone. The assertion checks the lock is released immediately after the child is spawned, but there is a window between fork and exec where it is not yet, and a loaded machine lands in it. Same commit as cd9ce9b on #241 (`jschneck/feeds-named`), included here because this branch is off main too and would otherwise flake in CI until that one merges. The patches are identical, so whichever lands first carries it and the other merges cleanly. 0 failures in 15 runs with it, on the same machine that produced the 2 above.

nicksinas

Approved. One note on the new --force help text.

… command One help text was applied to all six install commands, and it was only true for two of them. It said `--force` clears the sysroots and discards every extension's built content. In fact: - `avocado install -f` reaches ext install and clears every extension's sysroot. - `ext install <name> -f` clears that one extension, not every extension. - `sdk install -f` runs the four sysroot tasks in parallel instead of serially. It clears nothing. - `runtime install -f` only suppresses the live checklist and the container TTY. - `rootfs install -f` and `initramfs install -f` only affect the container TTY. Either half of that is a bad outcome: a user avoids a flag they were told is destructive when it is not, or expects a reinstall that never happens. Each command now documents what the flag does there, and says plainly where it clears nothing. The line about not needing it to skip dnf's prompts stays on all six, since that is the misconception this series exists to remove. Reported in review by nicksinas, with the four wrong sites named.

Copilot AI lite review requested due to automatic review settings September 7, 2026 15:56

Copilot started reviewing on behalf of mobileoverlord September 7, 2026 15:56 View session

Copilot AI reviewed Sep 7, 2026

View reviewed changes

Comment thread src/commands/install.rs Outdated

Comment thread src/commands/rootfs/install.rs Outdated

Comment thread src/main.rs Outdated

mobileoverlord requested a lite review from Copilot September 7, 2026 17:56

Copilot started reviewing on behalf of mobileoverlord September 7, 2026 17:56 View session

Copilot AI reviewed Sep 7, 2026

View reviewed changes

mobileoverlord added 2 commits September 7, 2026 14:10

mobileoverlord force-pushed the jschneck/install-assume-yes branch from 3cffe3a to 81c29d6 Compare September 7, 2026 18:12

mobileoverlord requested a review from nicksinas September 7, 2026 21:38

nicksinas previously approved these changes Sep 8, 2026

View reviewed changes

Comment thread src/main.rs Outdated

mobileoverlord dismissed nicksinas’s stale review via 4760a23 September 9, 2026 19:23

Navigation Menu

Uh oh!

install: always assume yes; --force means reinstall, not "no prompts" - #250

install: always assume yes; --force means reinstall, not "no prompts"#250
mobileoverlord wants to merge 4 commits into
mainfrom
jschneck/install-assume-yes

mobileoverlord commented Sep 7, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

nicksinas left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

mobileoverlord commented Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The bug this fixes

What --force means now

No opt-out flag

Verification

Companion fix in this PR

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Suppressed comments (2)

Uh oh!

nicksinas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mobileoverlord commented Sep 7, 2026 •

edited

Loading

What `--force` means now