Hi All,

I've been working on getting PyChecker to run on anaconda in a way that the
output is usable, I'm making nice progress here. As a result I've a first patch
fixing various issues which I believe to be real issues.


As we are close to the F-10 preview release, and as I'm not at my best atm
(sick), I don't want to directly apply my fixes. So here is a patch fixing
this. Please review and apply it, I believe it fixes various real issues.


Thanks & Regards,

Hans
diff --git a/anaconda b/anaconda
index 8612d37..44dd15a 100755
--- a/anaconda
+++ b/anaconda
@@ -259,7 +259,7 @@ def setVNCFromKickstart(opts):
try:
opts.ksfile = preprocessKickstart(opts.ksfile)
except KickstartError, msg:
- stdoutLog.critical(_("Error processing %%ksappend lines: %s") % e)
+ stdoutLog.critical(_("Error processing %%ksappend lines: %s") % msg)
sys.exit(1)
except Exception, e:
stdoutLog.critical(_("Unknown error processing %%ksappend lines: %s") % e)
@@ -305,7 +305,7 @@ def setRescueModeFromKickstart(opts):
try:
opts.ksfile = preprocessKickstart(opts.ksfile)
except KickstartError, msg:
- stdoutLog.critical(_("Error processing %%ksappend lines: %s") % e)
+ stdoutLog.critical(_("Error processing %%ksappend lines: %s") % msg)
sys.exit(1)
except Exception, e:
stdoutLog.critical(_("Unknown error processing %%ksappend lines: %s") % e)
@@ -840,7 +840,7 @@ if __name__ == "__main__":
os.environ["DISPLAY"] = ":1"
doStartupX11Actions(opts.runres)
xserver_pid = proc.pid
- except OSError, RuntimeError:
+ except (OSError, RuntimeError):
stdoutLog.warning(" X startup failed, falling back to text mode")
opts.display_mode = 't'
graphical_failed = 1
diff --git a/dmraid.py b/dmraid.py
index 90b5ad7..58aa559 100644
--- a/dmraid.py
+++ b/dmraid.py
@@ -265,7 +265,7 @@ def scanForMPath(drives):
return reduce(lambda x,y: x + [updateName(y),], mpaths, [])

def renameMPath(mpath, name):
- cacheDrives.rename(rs, name)
+ cacheDrives.rename(mpath, name)

def startMPath(mpath):
if flags.mpath == 0:
diff --git a/fsset.py b/fsset.py
index a1fdaef..d37ec6c 100644
--- a/fsset.py
+++ b/fsset.py
@@ -2723,7 +2723,7 @@ def readFstab (anaconda):
sys.exit(0)
else:
log.warning("Duplicate labels for %s, but no intf so trying "
- "to continue" %(info,))
+ "to continue" %(label,))

path = anaconda.rootPath + '/etc/fstab'
intf = anaconda.intf
diff --git a/instdata.py b/instdata.py
index 7e64b06..ae629c9 100644
--- a/instdata.py
+++ b/instdata.py
@@ -52,6 +52,9 @@ from pykickstart.version import versionToString, DEVEL
import logging
log = logging.getLogger("anaconda")

+import gettext
+_ = lambda x: gettext.ldgettext("anaconda", x)
+
# Collector class for all data related to an install/upgrade.

class InstallData:
diff --git a/iscsi.py b/iscsi.py
index 90cba14..a42a03e 100644
--- a/iscsi.py
+++ b/iscsi.py
@@ -317,7 +317,7 @@ class iscsi(object):

for pidstr in psout.split():
pid = string.atoi(pidstr)
- login.info("killing %s %d" % (ISCSID, pid))
+ log.info("killing %s %d" % (ISCSID, pid))

os.kill(pid, signal.SIGKILL)

diff --git a/iw/GroupSelector.py b/iw/GroupSelector.py
index 4a60d78..b56833a 100644
--- a/iw/GroupSelector.py
+++ b/iw/GroupSelector.py
@@ -97,7 +97,7 @@ def _deselectPackage(ayum, group, pkg):
except mdErrors.PackageSackError:
log = logging.getLogger("yum.verbose")
log.debug("no such package %s from group %s" %(pkg,
- self.group.groupid))
+ group.groupid))
if pkgs:
pkgs = ayum.bestPackagesFromList(pkgs)
for po in pkgs:
diff --git a/iw/task_gui.py b/iw/task_gui.py
index ea8f0d0..e18aef1 100644
--- a/iw/task_gui.py
+++ b/iw/task_gui.py
@@ -145,7 +145,7 @@ class RepoEditor:
self.intf.messageWindow(_("Error"),
_("The repository %s has already been added. Please "
"choose a different repository name and "
- "URL.") % reponame, type="ok", custom_icon="error")
+ "URL.") % self.repo.name, type="ok", custom_icon="error")
return False

return True
diff --git a/kickstart.py b/kickstart.py
index e6b1f59..af2e063 100644
--- a/kickstart.py
+++ b/kickstart.py
@@ -36,6 +36,7 @@ import lvm
import warnings
import upgrade
import pykickstart.commands as commands
+import cryptodev
from pykickstart.constants import *
from pykickstart.errors import *
from pykickstart.parser import *
@@ -382,7 +383,7 @@ class LogVol(commands.logvol.F9_LogVol):
if lvd.encrypted:
if lvd.passphrase and
not self.handler.anaconda.id.partitions.encryptionPass phrase:
- self.anaconda.id.partitions.encryptionPassphrase = lvd.passphrase
+ self.handler.anaconda.id.partitions.encryptionPass phrase = lvd.passphrase
request.encryption = cryptodev.LUKSDevice(passphrase=lvd.passphrase, format=lvd.format)

addPartRequest(self.handler.anaconda, request)
@@ -626,7 +627,7 @@ class Partition(commands.partition.F9_Partition):
if pd.encrypted:
if pd.passphrase and
not self.handler.anaconda.id.partitions.encryptionPass phrase:
- self.anaconda.id.partitions.encryptionPassphrase = pd.passphrase
+ self.handler.anaconda.id.partitions.encryptionPass phrase = pd.passphrase
request.encryption = cryptodev.LUKSDevice(passphrase=pd.passphrase, format=pd.format)

addPartRequest(self.handler.anaconda, request)
@@ -709,7 +710,7 @@ class Raid(commands.raid.F9_Raid):
if rd.encrypted:
if rd.passphrase and
not self.handler.anaconda.id.partitions.encryptionPass phrase:
- self.anaconda.id.partitions.encryptionPassphrase = rd.passphrase
+ self.handler.anaconda.id.partitions.encryptionPass phrase = rd.passphrase
request.encryption = cryptodev.LUKSDevice(passphrase=rd.passphrase, format=rd.format)

addPartRequest(self.handler.anaconda, request)
diff --git a/partedUtils.py b/partedUtils.py
index 1ead010..de8eff4 100644
--- a/partedUtils.py
+++ b/partedUtils.py
@@ -1027,7 +1027,7 @@ class DiskSet:
"-F",
"-P",
"-f",
- "/dev/%s" % (dev,)]
+ "/dev/%s" % (drive,)]

fd = os.open("/dev/null", os.O_RDWR | os.O_CREAT | os.O_APPEND)
p = os.pipe()
diff --git a/textw/grpselect_text.py b/textw/grpselect_text.py
index f74b54c..ff4a402 100644
--- a/textw/grpselect_text.py
+++ b/textw/grpselect_text.py
@@ -39,7 +39,7 @@ class GroupSelectionWindow:
pkgs = self.ayum.pkgSack.returnNewestByName(pkg)
except yum.Errors.PackageSackError:
log.debug("no such package %s from group %s" %
- (pkg, self.group.groupid))
+ (pkg, grpid))
if pkgs:
pkgs = self.ayum.bestPackagesFromList(pkgs)
for po in pkgs:
diff --git a/upgrade.py b/upgrade.py
index 2578ac0..4aee58c 100644
--- a/upgrade.py
+++ b/upgrade.py
@@ -36,6 +36,7 @@ from constants import *
from product import productName

import rhpl
+import rhpl.arch

import gettext
_ = lambda x: gettext.ldgettext("anaconda", x)
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

These all look fine to go ahead and commit, with the exception of the
couple comments below. I say go ahead and commit to master since they
all look like relevant potential bugzilla entries to me.

> diff --git a/iw/GroupSelector.py b/iw/GroupSelector.py
> index 4a60d78..b56833a 100644
> --- a/iw/GroupSelector.py
> +++ b/iw/GroupSelector.py
> @@ -97,7 +97,7 @@ def _deselectPackage(ayum, group, pkg):
> except mdErrors.PackageSackError:
> log = logging.getLogger("yum.verbose")
> log.debug("no such package %s from group %s" %(pkg,
> - self.group.groupid))
> + group.groupid))
> if pkgs:
> pkgs = ayum.bestPackagesFromList(pkgs)
> for po in pkgs:

This can just be groupid, not group.groupid. See slightly above.

> diff --git a/partedUtils.py b/partedUtils.py
> index 1ead010..de8eff4 100644
> --- a/partedUtils.py
> +++ b/partedUtils.py
> @@ -1027,7 +1027,7 @@ class DiskSet:
> "-F",
> "-P",
> "-f",
> - "/dev/%s" % (dev,)]
> + "/dev/%s" % (drive,)]
>
> fd = os.open("/dev/null", os.O_RDWR | os.O_CREAT | os.O_APPEND)
> p = os.pipe()

Can drive be "/dev/whatever" here, or is it just going to be "whatever"?
The call to progressWindow just above makes me wonder what we're
expecting drive to be here. Might want to throw in a check. It'd be
nice if we were at all consistent about /dev/ vs. not in anaconda.

- Chris

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

On Tue, 2008-10-28 at 11:51 +0100, Hans de Goede wrote:
> - stdoutLog.critical(_("Error processing %%ksappend lines: %s")
> % e)
> + stdoutLog.critical(_("Error processing %%ksappend lines: %s")
> % msg)

I see a bunch of these, which could be a product of not having a project
standard for these try/except blocks. Would it be worthwhile to convert
everything to one standard and keep it that way in the future (as to
avoid silly head mistakes?)

--
Jesse Keating
Fedora -- Freedom² is a feature!
identi.ca: http://identi.ca/jkeating
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

Chris Lumens wrote:

These all look fine to go ahead and commit, with the exception of the
couple comments below. I say go ahead and commit to master since they
all look like relevant potential bugzilla entries to me.



Thanks for the review!


diff --git a/iw/GroupSelector.py b/iw/GroupSelector.py
index 4a60d78..b56833a 100644
--- a/iw/GroupSelector.py
+++ b/iw/GroupSelector.py
@@ -97,7 +97,7 @@ def _deselectPackage(ayum, group, pkg):
except mdErrors.PackageSackError:
log = logging.getLogger("yum.verbose")
log.debug("no such package %s from group %s" %(pkg,
- self.group.groupid))
+ group.groupid))
if pkgs:
pkgs = ayum.bestPackagesFromList(pkgs)
for po in pkgs:


This can just be groupid, not group.groupid. See slightly above.



Erm, that should be grpid not groupid, but yes using that is better, fixed.


diff --git a/partedUtils.py b/partedUtils.py
index 1ead010..de8eff4 100644
--- a/partedUtils.py
+++ b/partedUtils.py
@@ -1027,7 +1027,7 @@ class DiskSet:
"-F",
"-P",
"-f",
- "/dev/%s" % (dev,)]
+ "/dev/%s" % (drive,)]

fd = os.open("/dev/null", os.O_RDWR | os.O_CREAT | os.O_APPEND)

p = os.pipe()


Can drive be "/dev/whatever" here, or is it just going to be "whatever"?


Note this is s390 only code, so I've looked at what RHEL-5.3 does as there we
actually have testing on s390, and it seems that its always just "whatever". So
I'm keeping this as is in the patch. In any case its better then the old
reference a non existing variable code.


Applying now.

Regards,

Hans

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list