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

Fixed the default behaviour of the autopartitioning (#1380767) #941

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions pyanaconda/installclass.py
Expand Up @@ -34,6 +34,7 @@
log = logging.getLogger("anaconda")

from pyanaconda.kickstart import getAvailableDiskSpace
from pyanaconda.constants import DEFAULT_AUTOPART_TYPE

class BaseInstallClass(object):
# default to not being hidden
Expand All @@ -58,6 +59,10 @@ class BaseInstallClass(object):
# Blivet uses by default.
defaultFS = None

# The default partitioning scheme to use for autopartitioning.
# It has to be always set.
default_autopart_type = DEFAULT_AUTOPART_TYPE

# help
help_folder = "/usr/share/anaconda/help"
help_main_page = "Installation_Guide.xml"
Expand Down Expand Up @@ -115,6 +120,9 @@ def configure(self, anaconda):
anaconda.bootloader.timeout = self.bootloaderTimeoutDefault
anaconda.bootloader.boot_args.update(self.bootloaderExtraArgs)

# The default partitioning should be always set.
self.setDefaultPartitioning(anaconda.storage)

# sets default ONBOOT values and updates ksdata accordingly
def setNetworkOnbootDefault(self, ksdata):
pass
Expand Down
1 change: 0 additions & 1 deletion pyanaconda/installclasses/fedora.py
Expand Up @@ -37,7 +37,6 @@ class FedoraBaseInstallClass(BaseInstallClass):

def configure(self, anaconda):
BaseInstallClass.configure(self, anaconda)
BaseInstallClass.setDefaultPartitioning(self, anaconda.storage)

def setNetworkOnbootDefault(self, ksdata):
if any(nd.onboot for nd in ksdata.network.network if nd.device):
Expand Down
1 change: 0 additions & 1 deletion pyanaconda/installclasses/rhel.py
Expand Up @@ -53,7 +53,6 @@ class RHELBaseInstallClass(BaseInstallClass):

def configure(self, anaconda):
BaseInstallClass.configure(self, anaconda)
self.setDefaultPartitioning(anaconda.storage)

def setNetworkOnbootDefault(self, ksdata):
if any(nd.onboot for nd in ksdata.network.network if nd.device):
Expand Down
2 changes: 2 additions & 0 deletions pyanaconda/installclasses/rhv.py
Expand Up @@ -24,6 +24,7 @@
from blivet.platform import platform
from blivet.devicelibs import swap
from blivet.size import Size
from pykickstart.constants import AUTOPART_TYPE_LVM_THINP


class OvirtBaseInstallClass(BaseInstallClass):
Expand All @@ -32,6 +33,7 @@ class OvirtBaseInstallClass(BaseInstallClass):
hidden = not productName.startswith("oVirt")

efi_dir = "centos"
default_autopart_type = AUTOPART_TYPE_LVM_THINP

def configure(self, anaconda):
BaseInstallClass.configure(self, anaconda)
Expand Down
4 changes: 4 additions & 0 deletions pyanaconda/iutil.py
Expand Up @@ -1242,3 +1242,7 @@ def touch(file_path):
# even when the path points to dirrectory
if not os.path.exists(file_path):
os.mknod(file_path)

def firstNotNone(iterable):
"""Return the first item that is not None."""
return next((item for item in iterable if item is not None), None)
2 changes: 1 addition & 1 deletion pyanaconda/kickstart.py
Expand Up @@ -290,7 +290,7 @@ def execute(self, storage, ksdata, instClass):

# sets up default autopartitioning. use clearpart separately
# if you want it
instClass.setDefaultPartitioning(storage)
refreshAutoSwapSize(storage)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please also describe this particular change in the commit message.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

storage.doAutoPart = True

if self.encrypted:
Expand Down
8 changes: 7 additions & 1 deletion pyanaconda/ui/gui/spokes/custom.py
Expand Up @@ -35,7 +35,7 @@
from pyanaconda.product import productName, productVersion, translated_new_install_name
from pyanaconda.threads import AnacondaThread, threadMgr
from pyanaconda.constants import THREAD_EXECUTE_STORAGE, THREAD_STORAGE, THREAD_CUSTOM_STORAGE_INIT
from pyanaconda.iutil import lowerASCII
from pyanaconda.iutil import lowerASCII, firstNotNone
from pyanaconda.bootloader import BootLoaderError
from pyanaconda.kickstart import refreshAutoSwapSize
from pyanaconda import isys
Expand Down Expand Up @@ -153,6 +153,7 @@ def __init__(self, data, storage, payload, instclass):
self._hidden_disks = []
self._fs_types = [] # list of supported fstypes
self._free_space = Size(0)
self._default_autopart_type = None

self._device_size_text = None
self._device_disks = []
Expand Down Expand Up @@ -274,6 +275,10 @@ def initialize(self):
# this list will include things like PVs and RAID members.
self._fsStore.clear()

# Set the default partitioning scheme.
self._default_autopart_type = firstNotNone((self.data.autopart.type,
self.instclass.default_autopart_type))

# Connect viewport scrolling with accordion focus events
self._accordion.set_focus_hadjustment(self._partitionsViewport.get_hadjustment())
self._accordion.set_focus_vadjustment(self._partitionsViewport.get_vadjustment())
Expand Down Expand Up @@ -501,6 +506,7 @@ def _populate_accordion(self):
page = CreateNewPage(translated_new_install_name(),
self.on_create_clicked,
self._change_autopart_type,
self._default_autopart_type,
partitionsToReuse=bool(ui_roots) or bool(unused_devices))
self._accordion.addPage(page, cb=self.on_page_clicked)

Expand Down
5 changes: 2 additions & 3 deletions pyanaconda/ui/gui/spokes/lib/accordion.py
Expand Up @@ -24,7 +24,6 @@
from pyanaconda.i18n import _
from pyanaconda.product import productName, productVersion
from pyanaconda.ui.gui.utils import escape_markup, really_hide, really_show
from pyanaconda.constants import DEFAULT_AUTOPART_TYPE
from pyanaconda.storage_utils import AUTOPART_CHOICES

from gi.repository.AnacondaWidgets import MountpointSelector
Expand Down Expand Up @@ -263,7 +262,7 @@ def removeSelector(self, selector):
# of this class will be packed into the Accordion first and then when the new installation
# is created, it will be removed and replaced with a Page for it.
class CreateNewPage(Page):
def __init__(self, title, createClickedCB, autopartTypeChangedCB, partitionsToReuse=True):
def __init__(self, title, createClickedCB, autopartTypeChangedCB, defaultAutopartType, partitionsToReuse=True):
# pylint: disable=super-init-not-called,non-parent-init-called
Gtk.Box.__init__(self, orientation=Gtk.Orientation.VERTICAL, spacing=6)
self.members = []
Expand Down Expand Up @@ -329,7 +328,7 @@ def __init__(self, title, createClickedCB, autopartTypeChangedCB, partitionsToRe

for item in (AUTOPART_CHOICES):
itr = store.append([_(item[0]), item[1]])
if item[1] == DEFAULT_AUTOPART_TYPE:
if item[1] == defaultAutopartType:
default = itr

combo.set_margin_left(18)
Expand Down
12 changes: 5 additions & 7 deletions pyanaconda/ui/gui/spokes/storage.py
Expand Up @@ -41,6 +41,7 @@

from gi.repository import Gdk, GLib, AnacondaWidgets

from pyanaconda.iutil import firstNotNone
from pyanaconda.ui.communication import hubQ
from pyanaconda.ui.lib.disks import getDisks, isLocalDisk, applyDiskSelection, checkDiskSelection
from pyanaconda.ui.gui import GUIObject
Expand Down Expand Up @@ -72,7 +73,7 @@
from pyanaconda import constants, iutil, isys
from pyanaconda.bootloader import BootLoaderError

from pykickstart.constants import CLEARPART_TYPE_NONE, AUTOPART_TYPE_LVM
from pykickstart.constants import CLEARPART_TYPE_NONE
from pykickstart.errors import KickstartValueError

import sys
Expand Down Expand Up @@ -234,7 +235,6 @@ def __init__(self, *args, **kwargs):
self.applyOnSkip = True

self._ready = False
self.autoPartType = None
self.encrypted = False
self.passphrase = ""
self.selected_disks = self.data.ignoredisk.onlyuse[:]
Expand All @@ -251,7 +251,8 @@ def __init__(self, *args, **kwargs):
self.data.autopart.autopart = True

self.autopart = self.data.autopart.autopart
self.autoPartType = None
self.default_autopart_type = firstNotNone((self.data.autopart.type,
self.instclass.default_autopart_type))
self.clearPartType = CLEARPART_TYPE_NONE

if arch.isS390() and (self.data.zerombr.zerombr or self.data.clearpart.cdl):
Expand All @@ -275,7 +276,7 @@ def _grabObjects(self):
def apply(self):
applyDiskSelection(self.storage, self.data, self.selected_disks)
self.data.autopart.autopart = self.autopart
self.data.autopart.type = self.autoPartType
self.data.autopart.type = self.default_autopart_type
self.data.autopart.encrypted = self.encrypted
self.data.autopart.passphrase = self.passphrase

Expand Down Expand Up @@ -527,9 +528,6 @@ def refresh(self):
self._unhide_disks()

self.autopart = self.data.autopart.autopart
self.autoPartType = self.data.autopart.type
if self.autoPartType is None:
self.autoPartType = AUTOPART_TYPE_LVM
self.encrypted = self.data.autopart.encrypted
self.passphrase = self.data.autopart.passphrase

Expand Down
31 changes: 18 additions & 13 deletions pyanaconda/ui/tui/spokes/storage.py
Expand Up @@ -21,7 +21,7 @@
# Some of the code here is copied from pyanaconda/ui/gui/spokes/storage.py
# which has the same license and authored by David Lehman <dlehman@redhat.com>
#

from pyanaconda.iutil import firstNotNone
from pyanaconda.ui.lib.disks import getDisks, applyDiskSelection, checkDiskSelection
from pyanaconda.ui.categories.system import SystemCategory
from pyanaconda.ui.tui.spokes import NormalTUISpoke
Expand All @@ -37,20 +37,20 @@
from pyanaconda.flags import flags
from pyanaconda.kickstart import doKickstartStorage, resetCustomStorageData
from pyanaconda.threads import threadMgr, AnacondaThread
from pyanaconda.constants import THREAD_STORAGE, THREAD_STORAGE_WATCHER, THREAD_DASDFMT, DEFAULT_AUTOPART_TYPE
from pyanaconda.constants import THREAD_STORAGE, THREAD_STORAGE_WATCHER, THREAD_DASDFMT
from pyanaconda.constants_text import INPUT_PROCESSED
from pyanaconda.i18n import _, P_, N_, C_
from pyanaconda.bootloader import BootLoaderError

from pykickstart.constants import CLEARPART_TYPE_ALL, CLEARPART_TYPE_LINUX, CLEARPART_TYPE_NONE, AUTOPART_TYPE_LVM
from pykickstart.constants import CLEARPART_TYPE_ALL, CLEARPART_TYPE_LINUX, CLEARPART_TYPE_NONE
from pykickstart.errors import KickstartValueError

from collections import OrderedDict

import logging
log = logging.getLogger("anaconda")

__all__ = ["StorageSpoke", "AutoPartSpoke"]
__all__ = ["StorageSpoke"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this change intentional? What is the purpose?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In tui, it does not make sense to enable automatic import of indirect spokes, because we always create them and use them only in other spokes. With automatic import, they will be found by a hub, created, initialized and forgotten.

Also, I've added a new parameter to the AutoPartSpoke.__init__ method and the initialization in a hub does not work anymore.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the extra explanation. I think this deserves a separate commit or at least some explanation in the commit message, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.


CLEARALL = N_("Use All Space")
CLEARLINUX = N_("Replace Existing Linux system(s)")
Expand Down Expand Up @@ -93,6 +93,10 @@ def __init__(self, app, data, storage, payload, instclass):
# default to using autopart for interactive installs
self.data.autopart.autopart = True

# Set the default partitioning scheme.
self.default_autopart_type = firstNotNone((self.data.autopart.type,
self.instclass.default_autopart_type))

@property
def completed(self):
retval = bool(self.storage.rootDevice and not self.errors)
Expand Down Expand Up @@ -290,8 +294,8 @@ def input(self, args, key):
# proceed.
return None

newspoke = AutoPartSpoke(self.app, self.data, self.storage,
self.payload, self.instclass)
newspoke = AutoPartSpoke(self.app, self.data, self.storage, self.payload,
self.instclass, self.default_autopart_type)
self.app.switch_screen_modal(newspoke)
self.apply()
self.execute()
Expand Down Expand Up @@ -376,7 +380,7 @@ def apply(self):
self.data.clearpart.drives = self.selected_disks[:]

if self.data.autopart.type is None:
self.data.autopart.type = AUTOPART_TYPE_LVM
self.data.autopart.type = self.default_autopart_type

if self.autopart:
self.clearPartType = CLEARPART_TYPE_ALL
Expand Down Expand Up @@ -474,10 +478,11 @@ class AutoPartSpoke(NormalTUISpoke):
title = N_("Autopartitioning Options")
category = SystemCategory

def __init__(self, app, data, storage, payload, instclass):
def __init__(self, app, data, storage, payload, instclass, default_autopart_type):
NormalTUISpoke.__init__(self, app, data, storage, payload, instclass)
self.clearPartType = self.data.clearpart.type
self.parttypelist = sorted(PARTTYPES.keys())
self.default_autopart_type = default_autopart_type

@property
def indirect(self):
Expand Down Expand Up @@ -521,8 +526,8 @@ def input(self, args, key):
except ValueError:
# TRANSLATORS: 'c' to continue
if key.lower() == C_('TUI|Spoke Navigation', 'c'):
newspoke = PartitionSchemeSpoke(self.app, self.data, self.storage,
self.payload, self.instclass)
newspoke = PartitionSchemeSpoke(self.app, self.data, self.storage, self.payload,
self.instclass, self.default_autopart_type)
self.app.switch_screen_modal(newspoke)
self.apply()
self.close()
Expand All @@ -540,13 +545,13 @@ class PartitionSchemeSpoke(NormalTUISpoke):
title = N_("Partition Scheme Options")
category = SystemCategory

def __init__(self, app, data, storage, payload, instclass):
def __init__(self, app, data, storage, payload, instclass, default_autopart_type):
NormalTUISpoke.__init__(self, app, data, storage, payload, instclass)
self.partschemes = OrderedDict()
pre_select = self.data.autopart.type or DEFAULT_AUTOPART_TYPE

for i, item in enumerate(AUTOPART_CHOICES):
self.partschemes[item[0]] = item[1]
if item[1] == pre_select:
if item[1] == default_autopart_type:
self._selection = i

@property
Expand Down
12 changes: 12 additions & 0 deletions tests/pyanaconda_tests/iutil_test.py
Expand Up @@ -752,6 +752,18 @@ def parent_dir_test(self):
for d, r in dirs:
self.assertEquals(iutil.parent_dir(d), r)

def first_not_none_test(self):
self.assertEquals(iutil.firstNotNone([]), None)
self.assertEquals(iutil.firstNotNone([None, None, None]), None)

self.assertEquals(iutil.firstNotNone([1, None, None]), 1)
self.assertEquals(iutil.firstNotNone([None, 2, None]), 2)
self.assertEquals(iutil.firstNotNone([None, None, 3]), 3)

self.assertEquals(iutil.firstNotNone([0, None, None]), 0)
self.assertEquals(iutil.firstNotNone([1, 2, 3]), 1)
self.assertEquals(iutil.firstNotNone([None, 2, 3]), 2)

class EncryptPasswordTests(unittest.TestCase):
def encrypt_password_test(self):
""" Test the encrypt_password function"""
Expand Down