Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix imports and add checks for BTRFS tools #3123

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hugo22dr
Copy link

Description

This pull request fixes imports and adds checks to ensure that the BTRFS tools (btrfs-progs) are available on the system before performing BTRFS-related operations. It also ensures that the mount point is available before mounting the file system.

Major Changes

  1. Fixed imports of the SysCommand and SysCommandWorker modules.
  2. Added _check_btrfs_tools and _ensure_mountpoint_available functions in the DeviceHandler class to check the availability of the BTRFS tools and ensure that the mount point is available.
  3. Modified the create_btrfs_volumes function in the FilesystemHandler class to use the newly added functions.

Tests

  • Verified that the BTRFS tools are available before performing BTRFS operations.
  • Ensured that the mount point is available before mounting the filesystem.
  • Tested in a local environment to ensure that the changes work as expected.

Contributor

This is my first contribution to this repository. I appreciate the opportunity to contribute and welcome feedback to improve this pull request.

@hugo22dr hugo22dr requested a review from Torxed as a code owner January 15, 2025 05:51
@svartkanin
Copy link
Collaborator

I don't think explicitly checking for btrfs to be available is required to do. Archinstall is meant to run on a live ISO and by defining the relevant dependencies in the pkg build file it is to be assumed that they are present

@hugo22dr
Copy link
Author

I don't think explicitly checking for btrfs to be available is required to do. Archinstall is meant to run on a live ISO and by defining the relevant dependencies in the pkg build file it is to be assumed that they are present

Thanks for the feedback. I would like to provide more context on the errors we are experiencing and how the proposed changes aim to address them.

Error Context
We are experiencing errors during the Arch Linux installation with the archinstall parameter. The errors start at init.py on lines 337/339 and continue through importlib/init.py on line 88. These errors appear to be related to the lack of BTRFS tools in the runtime.

The proposed changes include:

BTRFS Tools Check: We have added a check to ensure that the BTRFS tools (btrfs-progs) are available on the system before performing BTRFS-related operations. This is to avoid errors during the installation when these tools are not present.

Mount Point Availability Guarantee: We have added a function to ensure that the mount point is available before mounting the file system. This helps to avoid mount conflicts that can cause the installation to fail.

We understand that Archinstall is intended to run on a live ISO and that the relevant dependencies must be present. However, in some cases, there may be environments where these tools are not available by default, causing the installation to fail. The proposed changes aim to make the installation process more robust by explicitly checking for the presence of the required tools and ensuring that the mount point is available.

If the explicit check for BTRFS tools is not necessary, we are open to removing this part of the changes. However, we would like to discuss possible solutions to ensure that the installation environment has all the necessary dependencies to avoid errors during installation.

I appreciate the opportunity to contribute and we welcome additional feedback to improve this pull request.

@svartkanin
Copy link
Collaborator

You mentioned errors, can you provide an error log file from an installation?

When installing archinstall via pacman then all necessary packages and dependencies should also be installed

@hugo22dr
Copy link
Author

You mentioned errors, can you provide an error log file from an installation?

When installing archinstall via pacman then all necessary packages and dependencies should also be installed

Sure! I'll do that right away, sorry for not presenting the problem.

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