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

Do not overwrite the contents of mkinitcpio.conf #2532

Merged
merged 1 commit into from
Aug 26, 2024

Conversation

ventureoo
Copy link
Contributor

  • This fix issue: N/A

PR Description:

After installation is complete, user may want to edit /etc/mkinitcpio.conf at will and may encounter a mismatch between the default configuration examples and what it has after installation since archinstall does not just change some values, but completely overwrites contents of the file removing all comments and other options. This PR fixes this by editing only parameters specified in code like hooks/modules/etc.

Tests and Checks

  • I have tested the code!
    (did not test install process, but tested the operation of a separate mkinitcpio function on default mkinitcpio.conf file).

@ventureoo ventureoo requested a review from Torxed as a code owner June 8, 2024 13:15
@Torxed
Copy link
Member

Torxed commented Jul 11, 2024

I'm not sure the re.sub() is working as intended:

>>> import re
>>> content = '\nMODULES=(vfio_pci vfio vfio_iommu_type1 btrfs nvidia nvidia_modeset nvidia_uvm nvidia_drm)'
>>> _modules = ['testmodule', 'anothermod']
>>> re.sub("\nMODULES=(.*)", f"\nMODULES=({' '.join(_modules)})", content)
'\nMODULES=(testmodule anothermod)'

@ventureoo
Copy link
Contributor Author

ventureoo commented Jul 12, 2024

I'm not sure the re.sub() is working as intended:

>>> import re
>>> content = '\nMODULES=(vfio_pci vfio vfio_iommu_type1 btrfs nvidia nvidia_modeset nvidia_uvm nvidia_drm)'
>>> _modules = ['testmodule', 'anothermod']
>>> re.sub("\nMODULES=(.*)", f"\nMODULES=({' '.join(_modules)})", content)
'\nMODULES=(testmodule anothermod)'

Well, yeah, re.sub() overwrites the MODULES, FILES, BINARIES lines, but leaves rest of the file intact like the comments and other options, which is the purpose of my PR. Also MODULES, FILES, BINARIES are empty by default in /etc/mkinitcpio.conf, so it's not a big problem as I think.

@Torxed
Copy link
Member

Torxed commented Aug 26, 2024

Apologies, I might have been a bit clear and nitpicking.
My intention was that if we're going to improve the code to be non-invasive we should perhaps include users modification to MODULES=() (assuming they've built their own ISO that has adaptions they need).

But coming back to this PR, I realized that it's a step in the right direction and nothing needs to be perfect to be added - as long as it's a step in the right direction.

Thank you for the improvement, I'll include it in the next release.

@Torxed Torxed merged commit 3fa7072 into archlinux:master Aug 26, 2024
6 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