*** BLURB HERE ***
David Shea (3): Remove unused methods for deselecting environments Remove optional groups from environmentGroups Move environment group selection logic to PackagePayload (#1179362)
pyanaconda/packaging/__init__.py | 7 ++++++- pyanaconda/packaging/dnfpayload.py | 36 ++---------------------------------- pyanaconda/packaging/yumpayload.py | 30 +----------------------------- 3 files changed, 9 insertions(+), 64 deletions(-)
--- pyanaconda/packaging/dnfpayload.py | 10 ---------- pyanaconda/packaging/yumpayload.py | 15 --------------- 2 files changed, 25 deletions(-)
diff --git a/pyanaconda/packaging/dnfpayload.py b/pyanaconda/packaging/dnfpayload.py index 5cde775..71cf5fb 100644 --- a/pyanaconda/packaging/dnfpayload.py +++ b/pyanaconda/packaging/dnfpayload.py @@ -443,16 +443,6 @@ class DNFPayload(packaging.PackagePayload): # DNF raises this when it is already selected log.debug(e)
- def _deselect_environment(self, env_id): - env = self._base.comps.environment_by_pattern(env_id) - if env is None: - raise packaging.NoSuchGroup(env_id) - try: - self._base.environment_remove(env) - except dnf.exceptions.CompsError as e: - # DNF raises this when it is already not selected - log.debug(e) - def _select_kernel_package(self): kernels = self.kernelPackages for kernel in kernels: diff --git a/pyanaconda/packaging/yumpayload.py b/pyanaconda/packaging/yumpayload.py index 16b7e24..58ace3a 100644 --- a/pyanaconda/packaging/yumpayload.py +++ b/pyanaconda/packaging/yumpayload.py @@ -1001,21 +1001,6 @@ reposdir=%s for group in environment.groups: self.selectGroup(group)
- def deselectEnvironment(self, environmentid): - groups = self._yumGroups - if not groups: - return - - with _yum_lock: - if not groups.has_environment(environmentid): - raise NoSuchGroup(environmentid) - - environment = groups.return_environment(environmentid) - for group in environment.groups: - self.deselectGroup(group) - for group in environment.options: - self.deselectGroup(group) - def environmentGroups(self, environmentid): groups = self._yumGroups if not groups:
This method is used in the software spoke to deselect groups added via the environment, and having the optional groups in there doesn't do any good since any selected optional groups would have been selected by the user. --- pyanaconda/packaging/dnfpayload.py | 5 +---- pyanaconda/packaging/yumpayload.py | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/pyanaconda/packaging/dnfpayload.py b/pyanaconda/packaging/dnfpayload.py index 71cf5fb..ffdbc0c 100644 --- a/pyanaconda/packaging/dnfpayload.py +++ b/pyanaconda/packaging/dnfpayload.py @@ -29,7 +29,6 @@ from pyanaconda.progress import progressQ
import ConfigParser import collections -import itertools import logging import multiprocessing import operator @@ -558,9 +557,7 @@ class DNFPayload(packaging.PackagePayload): env = self._base.comps.environment_by_pattern(environmentid) if env is None: raise packaging.NoSuchGroup(environmentid) - group_ids = (id_.name for id_ in env.group_ids) - option_ids = (id_.name for id_ in env.option_ids) - return list(itertools.chain(group_ids, option_ids)) + return [id_.name for id_ in env.group_ids]
def environmentHasOption(self, environmentid, grpid): env = self._base.comps.environment_by_pattern(environmentid) diff --git a/pyanaconda/packaging/yumpayload.py b/pyanaconda/packaging/yumpayload.py index 58ace3a..b7e9e0f 100644 --- a/pyanaconda/packaging/yumpayload.py +++ b/pyanaconda/packaging/yumpayload.py @@ -1011,7 +1011,7 @@ reposdir=%s raise NoSuchGroup(environmentid)
environment = groups.return_environment(environmentid) - return environment.groups + environment.options + return environment.groups
### ### METHODS FOR WORKING WITH GROUPS
On Wed, 2015-01-07 at 12:03 -0500, David Shea wrote:
This method is used in the software spoke to deselect groups added via the environment, and having the optional groups in there doesn't do any good since any selected optional groups would have been selected by the user.
I don't know exactly what the code is for, but just reading the commit message - is "any selected optional groups would have been selected by the user" 100% true? I believe comps allows the possibility of a 'default optional group', one that's selected by default (the difference between that and a non-optional one being that the user can choose not to have it). Not sure if that makes a difference to the change, but thought I'd point it out.
On 01/07/2015 12:32 PM, Adam Williamson wrote:
On Wed, 2015-01-07 at 12:03 -0500, David Shea wrote:
This method is used in the software spoke to deselect groups added via the environment, and having the optional groups in there doesn't do any good since any selected optional groups would have been selected by the user.
I don't know exactly what the code is for, but just reading the commit message - is "any selected optional groups would have been selected by the user" 100% true? I believe comps allows the possibility of a 'default optional group', one that's selected by default (the difference between that and a non-optional one being that the user can choose not to have it). Not sure if that makes a difference to the change, but thought I'd point it out.
Sure is. Well, that doesn't work with dnf, either. Also it would be mighty nice if anything about comps was documented anywhere.
On 01/07/2015 12:32 PM, Adam Williamson wrote:
On Wed, 2015-01-07 at 12:03 -0500, David Shea wrote:
This method is used in the software spoke to deselect groups added via the environment, and having the optional groups in there doesn't do any good since any selected optional groups would have been selected by the user.
I don't know exactly what the code is for, but just reading the commit message - is "any selected optional groups would have been selected by the user" 100% true? I believe comps allows the possibility of a 'default optional group', one that's selected by default (the difference between that and a non-optional one being that the user can choose not to have it). Not sure if that makes a difference to the change, but thought I'd point it out.
In depth analysis:
The situation you are describing is a groupid with a default="true" attribute in the optionlist. In rawhide there are all in xfce.
<environment> <id>xfce-desktop-environment</id> ... <optionlist> <groupid default="true">xfce-apps</groupid> <groupid default="true">xfce-media</groupid> ... </optionlist> </environment>
In the software spokes, such groups are detected and selected by using payload.environmentOptionIsDefault.
def environmentOptionIsDefault(self, environmentid, grpid): env = self._base.comps.environment_by_pattern(environmentid) if env is None: raise packaging.NoSuchGroup(environmentid) return False
What's wrong with that is left as an exercise to the reader.
But you are correct, this change breaks some cases of picking an environment and fiddling with the addons and picking a different environment. So another patch is forthcoming. And a bug to get dnf to implement something like yum's environment.defaultoptions.
The default is to include optionlist, which is the original behavior. This adds an option to exlucde it and return only the groups that are part of an environment's grouplist. --- pyanaconda/packaging/__init__.py | 2 +- pyanaconda/packaging/dnfpayload.py | 7 +++++-- pyanaconda/packaging/yumpayload.py | 7 +++++-- 3 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/pyanaconda/packaging/__init__.py b/pyanaconda/packaging/__init__.py index a5004bd..deff84c 100644 --- a/pyanaconda/packaging/__init__.py +++ b/pyanaconda/packaging/__init__.py @@ -1020,7 +1020,7 @@ class PackagePayload(Payload): def selectEnvironment(self, environmentid): raise NotImplementedError()
- def environmentGroups(self, environmentid): + def environmentGroups(self, environmentid, optional=True): raise NotImplementedError()
@property diff --git a/pyanaconda/packaging/dnfpayload.py b/pyanaconda/packaging/dnfpayload.py index 71cf5fb..3b6d59c 100644 --- a/pyanaconda/packaging/dnfpayload.py +++ b/pyanaconda/packaging/dnfpayload.py @@ -554,13 +554,16 @@ class DNFPayload(packaging.PackagePayload): raise packaging.NoSuchGroup(environmentid) return (env.ui_name, env.ui_description)
- def environmentGroups(self, environmentid): + def environmentGroups(self, environmentid, optional=True): env = self._base.comps.environment_by_pattern(environmentid) if env is None: raise packaging.NoSuchGroup(environmentid) group_ids = (id_.name for id_ in env.group_ids) option_ids = (id_.name for id_ in env.option_ids) - return list(itertools.chain(group_ids, option_ids)) + if optional: + return list(itertools.chain(group_ids, option_ids)) + else: + return list(group_ids)
def environmentHasOption(self, environmentid, grpid): env = self._base.comps.environment_by_pattern(environmentid) diff --git a/pyanaconda/packaging/yumpayload.py b/pyanaconda/packaging/yumpayload.py index 58ace3a..1910968 100644 --- a/pyanaconda/packaging/yumpayload.py +++ b/pyanaconda/packaging/yumpayload.py @@ -1001,7 +1001,7 @@ reposdir=%s for group in environment.groups: self.selectGroup(group)
- def environmentGroups(self, environmentid): + def environmentGroups(self, environmentid, optional=True): groups = self._yumGroups if not groups: return [] @@ -1011,7 +1011,10 @@ reposdir=%s raise NoSuchGroup(environmentid)
environment = groups.return_environment(environmentid) - return environment.groups + environment.options + if optional: + return environment.groups + environment.options + else: + return environment.groups
### ### METHODS FOR WORKING WITH GROUPS
The selectEnvironment method in yum only iterates over the groups within an environment and selects each, which is possible using the existing PackagePayload API. selectEnvironment in dnf selects the environment using dnf.Base.environment_install, which does not change the kickstart data and is probably premature. Replace both methods with one that calls selectGroup on each group in the environment. --- pyanaconda/packaging/__init__.py | 7 ++++++- pyanaconda/packaging/dnfpayload.py | 21 +-------------------- pyanaconda/packaging/yumpayload.py | 13 ------------- 3 files changed, 7 insertions(+), 34 deletions(-)
diff --git a/pyanaconda/packaging/__init__.py b/pyanaconda/packaging/__init__.py index a5004bd..721ab3d 100644 --- a/pyanaconda/packaging/__init__.py +++ b/pyanaconda/packaging/__init__.py @@ -1018,7 +1018,12 @@ class PackagePayload(Payload): raise NotImplementedError()
def selectEnvironment(self, environmentid): - raise NotImplementedError() + if environmentid not in self.environments: + raise NoSuchGroup(environmentid) + + # Select each group within the environment + for groupid in self.environmentGroups(environmentid): + self.selectGroup(groupid)
def environmentGroups(self, environmentid): raise NotImplementedError() diff --git a/pyanaconda/packaging/dnfpayload.py b/pyanaconda/packaging/dnfpayload.py index ffdbc0c..83a36a0 100644 --- a/pyanaconda/packaging/dnfpayload.py +++ b/pyanaconda/packaging/dnfpayload.py @@ -264,7 +264,7 @@ class DNFPayload(packaging.PackagePayload):
if env: try: - self._select_environment(env) + self.selectEnvironment(env) log.info("selected env: %s", env) except packaging.NoSuchGroup as e: self._miss(e) @@ -426,22 +426,6 @@ class DNFPayload(packaging.PackagePayload): # DNF raises this when it is already not selected log.debug(e)
- def _select_environment(self, env_id, default=True, optional=False, required=False): - env = self._base.comps.environment_by_pattern(env_id) - if env is None: - raise packaging.NoSuchGroup(env_id, required=required) - types = {'mandatory'} - if default: - types.add('default') - if optional: - types.add('optional') - exclude = self.data.packages.excludedList - try: - self._base.environment_install(env, types, exclude=exclude) - except dnf.exceptions.CompsError as e: - # DNF raises this when it is already selected - log.debug(e) - def _select_kernel_package(self): kernels = self.kernelPackages for kernel in kernels: @@ -671,9 +655,6 @@ class DNFPayload(packaging.PackagePayload): self.txID = None self._base.reset(sack=True, repos=True)
- def selectEnvironment(self, env_id): - self._select_environment(env_id) - def updateBaseRepo(self, fallback=True, checkmount=True): log.info('configuring base repo') self.reset() diff --git a/pyanaconda/packaging/yumpayload.py b/pyanaconda/packaging/yumpayload.py index b7e9e0f..bd6dd86 100644 --- a/pyanaconda/packaging/yumpayload.py +++ b/pyanaconda/packaging/yumpayload.py @@ -988,19 +988,6 @@ reposdir=%s
return (environment.ui_name, environment.ui_description)
- def selectEnvironment(self, environmentid): - groups = self._yumGroups - if not groups: - return - - with _yum_lock: - if not groups.has_environment(environmentid): - raise NoSuchGroup(environmentid) - - environment = groups.return_environment(environmentid) - for group in environment.groups: - self.selectGroup(group) - def environmentGroups(self, environmentid): groups = self._yumGroups if not groups:
The selectEnvironment method in yum only iterates over the groups within an environment and selects each, which is possible using the existing PackagePayload API. selectEnvironment in dnf selects the environment using dnf.Base.environment_install, which does not change the kickstart data and is probably premature. Replace both methods with one that calls selectGroup on each group in the environment. --- pyanaconda/packaging/__init__.py | 7 ++++++- pyanaconda/packaging/dnfpayload.py | 21 +-------------------- pyanaconda/packaging/yumpayload.py | 13 ------------- 3 files changed, 7 insertions(+), 34 deletions(-)
diff --git a/pyanaconda/packaging/__init__.py b/pyanaconda/packaging/__init__.py index deff84c..de85e80 100644 --- a/pyanaconda/packaging/__init__.py +++ b/pyanaconda/packaging/__init__.py @@ -1018,7 +1018,12 @@ class PackagePayload(Payload): raise NotImplementedError()
def selectEnvironment(self, environmentid): - raise NotImplementedError() + if environmentid not in self.environments: + raise NoSuchGroup(environmentid) + + # Select each group within the environment + for groupid in self.environmentGroups(environmentid, optional=False): + self.selectGroup(groupid)
def environmentGroups(self, environmentid, optional=True): raise NotImplementedError() diff --git a/pyanaconda/packaging/dnfpayload.py b/pyanaconda/packaging/dnfpayload.py index 3b6d59c..f8bfdea 100644 --- a/pyanaconda/packaging/dnfpayload.py +++ b/pyanaconda/packaging/dnfpayload.py @@ -265,7 +265,7 @@ class DNFPayload(packaging.PackagePayload):
if env: try: - self._select_environment(env) + self.selectEnvironment(env) log.info("selected env: %s", env) except packaging.NoSuchGroup as e: self._miss(e) @@ -427,22 +427,6 @@ class DNFPayload(packaging.PackagePayload): # DNF raises this when it is already not selected log.debug(e)
- def _select_environment(self, env_id, default=True, optional=False, required=False): - env = self._base.comps.environment_by_pattern(env_id) - if env is None: - raise packaging.NoSuchGroup(env_id, required=required) - types = {'mandatory'} - if default: - types.add('default') - if optional: - types.add('optional') - exclude = self.data.packages.excludedList - try: - self._base.environment_install(env, types, exclude=exclude) - except dnf.exceptions.CompsError as e: - # DNF raises this when it is already selected - log.debug(e) - def _select_kernel_package(self): kernels = self.kernelPackages for kernel in kernels: @@ -677,9 +661,6 @@ class DNFPayload(packaging.PackagePayload): self.txID = None self._base.reset(sack=True, repos=True)
- def selectEnvironment(self, env_id): - self._select_environment(env_id) - def updateBaseRepo(self, fallback=True, checkmount=True): log.info('configuring base repo') self.reset() diff --git a/pyanaconda/packaging/yumpayload.py b/pyanaconda/packaging/yumpayload.py index 1910968..5948349 100644 --- a/pyanaconda/packaging/yumpayload.py +++ b/pyanaconda/packaging/yumpayload.py @@ -988,19 +988,6 @@ reposdir=%s
return (environment.ui_name, environment.ui_description)
- def selectEnvironment(self, environmentid): - groups = self._yumGroups - if not groups: - return - - with _yum_lock: - if not groups.has_environment(environmentid): - raise NoSuchGroup(environmentid) - - environment = groups.return_environment(environmentid) - for group in environment.groups: - self.selectGroup(group) - def environmentGroups(self, environmentid, optional=True): groups = self._yumGroups if not groups:
anaconda-patches@lists.fedorahosted.org
-
Adam Williamson -
David Shea