Just a couple things stood out to me from doing a quick review...
> @@ -2361,6 +2368,26 @@ def writeSysconfigKernel(storage, version):
> f.write("HYPERVISOR_ARGS=logging=vga,serial,memory\n")
> f.close()
>
> +def writeBootLoaderFinal(storage, payload, instClass, ksdata):
> + """ Do the final write of the bootloader. """
> +
> + from pyanaconda.errors import errorHandler, ERROR_RAISE
> +
> + # set up dracut/fips boot args
> + # XXX FIXME: do this from elsewhere?
> + #storage.bootloader.set_boot_args(keyboard=anaconda.keyboard,
> + # storage=anaconda.storage,
> + # language=anaconda.instLanguage,
> + # network=anaconda.network)
> + storage.bootloader.set_boot_args(storage=storage,
> + payload=payload,
> + keyboard=ksdata.keyboard)
> + try:
> + storage.bootloader.write()
> + except BootLoaderError as e:
> + if errorHandler.cb(e) == ERROR_RAISE:
> + raise
> +
> def writeBootLoader(storage, payload, instClass, ksdata):
> """ Write bootloader configuration to disk.
>
> @@ -2376,6 +2403,13 @@ def writeBootLoader(storage, payload, instClass, ksdata):
> stage2_device = storage.bootloader.stage2_device
> log.info("bootloader stage2 target device is %s" % stage2_device.name)
>
> + if payload.handlesBootloaderConfiguration:
> + if storage.bootloader.skip_bootloader:
> + log.info("skipping bootloader install per user request")
> + return
> + writeBootLoaderFinal(storage, payload, instClass, ksdata)
> + return
> +
> # get a list of installed kernel packages
> kernel_versions = payload.kernelVersionList
> if not kernel_versions:
> @@ -2420,19 +2454,4 @@ def writeBootLoader(storage, payload, instClass, ksdata):
> label=label, short=short)
> storage.bootloader.add_image(image)
>
> - # set up dracut/fips boot args
> - # XXX FIXME: do this from elsewhere?
> - #storage.bootloader.set_boot_args(keyboard=anaconda.keyboard,
> - # storage=anaconda.storage,
> - # language=anaconda.instLanguage,
> - # network=anaconda.network)
> - storage.bootloader.set_boot_args(storage=storage,
> - payload=payload,
> - keyboard=ksdata.keyboard)
> -
> - try:
> - storage.bootloader.write()
> - except BootLoaderError as e:
> - if errorHandler.cb(e) == ERROR_RAISE:
> - raise
> -
> + writeBootLoaderFinal(storage, payload, instClass, ksdata)
Vratislav changed the set_boot_args calls a little recently to remove
passing keyboard, so this won't apply cleanly.
> diff --git a/pyanaconda/kickstart.py b/pyanaconda/kickstart.py
> index df7b5e6..c788074 100644
> --- a/pyanaconda/kickstart.py
> +++ b/pyanaconda/kickstart.py
> @@ -490,6 +490,13 @@ class Realm(commands.realm.F19_Realm):
>
> log.info("Joined realm %s", self.join_realm)
>
> +class OSTreeSetup(commands.ostreesetup.F20_OSTreeSetup):
> + def __init__(self, *args):
> + commands.ostreesetup.F20_OSTreeSetup.__init__(self, *args)
> +
> + def execute(self, *args):
> + # This is implemented in packaging/ostreepayload.py
> + pass
>
> class ClearPart(commands.clearpart.F17_ClearPart):
> def parse(self, args):
> @@ -1580,6 +1588,7 @@ commandMap = {
> "logvol": LogVol,
> "multipath": MultiPath,
> "network": Network,
> + "ostreesetup": OSTreeSetup,
> "part": Partition,
> "partition": Partition,
> "raid": Raid,
If you're not adding anaconda-specific behavior for a kickstart command,
you don't need to do either of these chunks.
> diff --git a/pyanaconda/packaging/yumpayload.py b/pyanaconda/packaging/yumpayload.py
> index 824c8a4..7b863b5 100644
> --- a/pyanaconda/packaging/yumpayload.py
> +++ b/pyanaconda/packaging/yumpayload.py
> @@ -1273,6 +1273,10 @@ reposdir=%s
> ### METHODS FOR WORKING WITH PACKAGES
> ###
> @property
> + def supportsPackages(self):
> + return True
> +
> + @property
> def packages(self):
> from yum.Errors import RepoError
dnfpayload will also support packages.
> diff --git a/pyanaconda/ui/gui/hubs/__init__.py b/pyanaconda/ui/gui/hubs/__init__.py
> index f8b016e..e739f1d 100644
> --- a/pyanaconda/ui/gui/hubs/__init__.py
> +++ b/pyanaconda/ui/gui/hubs/__init__.py
> @@ -287,7 +287,17 @@ class Hub(GUIObject, common.Hub):
>
> @property
> def continuePossible(self):
> - return len(self._incompleteSpokes) == 0 and len(self._notReadySpokes) == 0 and getattr(self._checker, "success", True)
> + if len(self._incompleteSpokes) > 0:
> + log.info("Incomplete spokes: %r" % (self._incompleteSpokes, ))
> + return False
> + if len(self._notReadySpokes) > 0:
> + log.info("NotReady spokes: %r" % (self._notReadySpokes, ))
> + return False
> + checkSuccess=getattr(self._checker, "success", True)
> + if not checkSuccess:
> + log.info("Checker returned %r" % (checkSuccess, ))
> + return False
> + return True
>
> def _updateContinueButton(self):
> self.continueButton.set_sensitive(self.continuePossible)
Is this chunk just for debugging?
> diff --git a/pyanaconda/ui/gui/spokes/software.py b/pyanaconda/ui/gui/spokes/software.py
> index b21866b..46e7ede 100644
> --- a/pyanaconda/ui/gui/spokes/software.py
> +++ b/pyanaconda/ui/gui/spokes/software.py
> @@ -164,7 +164,9 @@ class SoftwareSelectionSpoke(NormalSpoke):
>
> @property
> def showable(self):
> - return not flags.livecdInstall and not self.data.method.method == "liveimg"
> + return (self.payload.supportsPackages
> + and not flags.livecdInstall
> + and not self.data.method.method == "liveimg")
>
> @property
> def status(self):
> diff --git a/pyanaconda/ui/gui/spokes/source.py b/pyanaconda/ui/gui/spokes/source.py
> index 59ff82c..add7fbc 100644
> --- a/pyanaconda/ui/gui/spokes/source.py
> +++ b/pyanaconda/ui/gui/spokes/source.py
> @@ -761,7 +761,9 @@ class SourceSpoke(NormalSpoke):
>
> @property
> def showable(self):
> - return not flags.livecdInstall and not self.data.method.method == "liveimg"
> + return (self.payload.supportsPackages
> + and not flags.livecdInstall
> + and not self.data.method.method == "liveimg")
>
> def _mirror_active(self):
> return self._protocolComboBox.get_active() == PROTOCOL_MIRROR
"not flags.livecdInstall and not self.data.method.method == 'liveimg'"
is basically the same as saying self.payload.supportsPackages. It's
certainly what we were going for there. So you might as well just
completely replace the existing tests with your new one.
> >From cad071d736cb35e67245ca11cf8750931c0d98ba Mon Sep 17 00:00:00 2001
> From: Colin Walters <walters@xxxxxxxxxx>
> Date: Tue, 11 Mar 2014 09:33:36 -0400
> Subject: [PATCH] ostreesetup: New command
>
> This tells the installer to handle an OSTree repository.
> ---
> pykickstart/commands/__init__.py | 3 +-
> pykickstart/commands/ostreesetup.py | 74 +++++++++++++++++++++++++++++++++++++
> pykickstart/handlers/control.py | 2 +
> tests/commands/ostreesetup.py | 37 +++++++++++++++++++
> 4 files changed, 115 insertions(+), 1 deletion(-)
> create mode 100644 pykickstart/commands/ostreesetup.py
> create mode 100644 tests/commands/ostreesetup.py
>
> diff --git a/pykickstart/commands/__init__.py b/pykickstart/commands/__init__.py
> index c5145ba..ed4d649 100644
> --- a/pykickstart/commands/__init__.py
> +++ b/pykickstart/commands/__init__.py
> @@ -21,6 +21,7 @@ import authconfig, autopart, autostep, bootloader, btrfs, clearpart, cdrom, devi
> import deviceprobe, displaymode, dmraid, driverdisk, eula, fcoe, firewall, firstboot
> import group, harddrive, ignoredisk, interactive, iscsi, iscsiname, key, keyboard, lang
> import langsupport, lilocheck, liveimg, logging, logvol, mediacheck, method, monitor
> -import mouse, multipath, network, nfs, partition, raid, realm, reboot, repo, rescue
> +import mouse, multipath, network, nfs, ostreesetup
> +import partition, raid, realm, reboot, repo, rescue
> import rootpw, selinux, services, skipx, sshpw, timezone, updates, upgrade, url, user
> import unsupported_hardware, vnc, volgroup, xconfig, zerombr, zfcp
I changed this file to make pylint happier, so this chunk won't apply
cleanly. Look at pykickstart in git and you'll see the obvious thing to
change. It's not hard.
> diff --git a/pykickstart/commands/ostreesetup.py b/pykickstart/commands/ostreesetup.py
> new file mode 100644
> index 0000000..b722c2c
> --- /dev/null
> +++ b/pykickstart/commands/ostreesetup.py
> @@ -0,0 +1,74 @@
> +#
> +# Copyright (C) 2014 Red Hat, Inc.
> +#
> +# This copyrighted material is made available to anyone wishing to use,
> +# modify, copy, or redistribute it subject to the terms and conditions of
> +# the GNU General Public License v.2, or (at your option) any later version.
> +# This program is distributed in the hope that it will be useful, but WITHOUT
> +# ANY WARRANTY expressed or implied, including the implied warranties of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General
> +# Public License for more details. You should have received a copy of the
> +# GNU General Public License along with this program; if not, write to the
> +# Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> +# 02110-1301, USA. Any Red Hat trademarks that are incorporated in the
> +# source code or documentation are not subject to the GNU General Public
> +# License and may only be used or replicated with the express permission of
> +# Red Hat, Inc.
> +#
> +
> +from pykickstart.base import *
> +from pykickstart.errors import *
> +from pykickstart.options import *
I've converted pykickstart away from doing global imports, so you'll
want to make changes here too. The changes will look like this:
from pykickstart.base import KickstartCommand
from pykickstart.options import KSOptionParser
You're not using anything out of pykickstart.errors.
> +
> +import gettext
> +_ = lambda x: gettext.ldgettext("pykickstart", x)
> +
> +class F20_OSTreeSetup(KickstartCommand):
> + removedKeywords = KickstartCommand.removedKeywords
> + removedAttrs = KickstartCommand.removedAttrs
> +
> + def __init__(self, *args, **kwargs):
> + KickstartCommand.__init__(self, *args, **kwargs)
> + self.op = self._getParser()
> + self.osname = kwargs.get('osname', None)
> + self.remote = kwargs.get("remote", self.osname)
> + self.url = kwargs.get('url', None)
> + self.ref = kwargs.get('ref', None)
> + self.noGpg = kwargs.get('noGpg', True)
> +
> + def __str__(self):
> + retval = KickstartCommand.__str__(self)
> +
> + if self.osname:
> + retval += "# OSTree setup\n"
> + retval += "ostreesetup %s\n" % self._getArgsAsStr()
> +
> + return retval
> +
> + def _getArgsAsStr(self):
> + retval = ""
> + if self.osname:
> + retval += "--osname=%s" % self.osname
> + if self.remote:
> + retval += "--remote=%s" % self.remote
> + if self.url:
> + retval += "--url=%s" % self.url
> + if self.ref:
> + retval += "--ref=%s" % self.ref
> + if self.noGpg:
> + retval += "--nogpg"
> + return retval
You will want to put a space in front of the -- in all of these, or
using multiple options will result in a string that looks like:
ostreesetup --osname=blah--url=http://blah--nogpg
You may wish to add a test case that uses multiple arguments.
Also... can osname, remote, or ref have a space in it? If so, you'll
want to surround the %s with quotes.
- Chris
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
