Skip to content

Teardown after installation #4491#4536

Draft
h8d13 wants to merge 2 commits into
archlinux:masterfrom
evoquus:fix/teardown-install-layout
Draft

Teardown after installation #4491#4536
h8d13 wants to merge 2 commits into
archlinux:masterfrom
evoquus:fix/teardown-install-layout

Conversation

@h8d13
Copy link
Copy Markdown
Contributor

@h8d13 h8d13 commented May 12, 2026

Moves the large def out of installer into archinstall/lib/disk/utils.py

Also preserves @kill74 's commit in #4496 which did the heavy lifting

@h8d13 h8d13 requested a review from Torxed as a code owner May 12, 2026 11:03
@h8d13 h8d13 marked this pull request as draft May 15, 2026 10:45
@h8d13
Copy link
Copy Markdown
Contributor Author

h8d13 commented May 15, 2026

Reverted this a draft:

Test matrix is very large. Missing no enc generic umount + lazy fallback for when procs might still be active

@svartkanin svartkanin changed the title Continue #4491 Teardown after installation #4491 May 19, 2026
cmd = ['vgchange', '-a', active_flag, vg.name]

debug(f'vgchange volume group: {" ".join(cmd)}')
SysCommand(cmd)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I feel like an error handling might be useful here for debugging purposes

from archinstall.lib.output import debug, info, warn

if TYPE_CHECKING:
from archinstall.lib.installer import Installer
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is this causeing circular dependencies? If not can we move this to the top


info('Tearing down installation target')

with suppress(Exception):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We shouldn't supress things, either handle it or log it



def _teardown_lvm(installer: Installer) -> None:
from archinstall.lib.disk.lvm import lvm_vg_change, lvm_vol_change
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Move to the top

raise DiskError(f'Could not enable swap {path}:\n{err.message}')


def teardown(installer: Installer) -> None:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We should be passing the configuration here not the installer

if exc_type is not None:
error(str(exc_value))
try:
if exc_type is not None:
Copy link
Copy Markdown
Collaborator

@svartkanin svartkanin May 19, 2026

Choose a reason for hiding this comment

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

Can we move this whle thing out of the exit as that will only be run when Installer is used in the with context. So it would be better to have this as a function

def cleanup(...)

that can be called

installer = Installer(...)
# install steps
installer.cleanup()

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.

3 participants