Jeremy Katz wrote:
On Fri, 2007-04-20 at 13:34 +0200, Alexander Todorov wrote:
What's in the tar.gz:
FYI -- in the future, it's a little easier to review if patches are
attached directly rather than inside of archives where you have to open
more apps to read through them. Comments are also a lot easier then.
Thanks. Minor changes follow.
* anaconda.process_and_launch_dogtail_script.diff
The biggest patch of all. This adds a --dogtail option to the command
line options for anaconda. Checks all possible conditions specifying a
dogtail script: kickstart, boot cmd line, anaconda cmd line option.
Downloads the script, fork and execute it before anaconda.intf.run().
The boot command line should just lead to the option being passed on the
anaconda command line from the loader. See loader2/loader.c's handling
of the various args like resolution, xdriver, etc.
So there is no need to check both? Only one of them is ok, right?
Do I have to patch loader.c to parse dogtail=url and pass it to anaconda
script? I suppose I will have to.
It's also far more
normal to set the value of anaconda.dogtail to None instead of "" for
unset.
Done.
We'll also eventually need to have the filename not be
hard-coded as that opens things up to race conditions, especially as
anaconda starts being run on "live" systems in cases like the live CD
install.
Done, now uses tempfile.mkstemp to store the tescase in
/tmp/ramfs/testcase.py.XXXXXX
* gui.py.always_load_a11y.diff
Removed checking for dogtail. Always loads accessibility.
What's the reasoning for always loading? Loading when needed instead is
a lot nicer as it helps to reduce the memory footprint in cases where
it's not needed.
in /usr/bin/anaconda
line 863: anaconda.setInstallInterface(opts.display_mode)
Imports gui.py and gtk if graphical install is selected.
line 926: instClass.setInstallData(anaconda)
Parses kickstart data and sets instClass.ksdata.dogtail.
I was not sure how to modify anaconda to parse ks before loading gui
without breaking something. That's why accessibility is loaded by default.
In gui.py only boot cmd line is checked. If dogtail command is added in
kickstart it is not checked.
What is the prefered way to solve this issue? Should I parse kickstart
file and check only for dogtail command just before `import gui' or
leave it like that? Or should I make more modifications and have ks
parsed only once before `import gui'?
* installdata.py.save_dogtail_kickstart.diff
Added code that writes the dogtail option to the /root/anaconda-ks.cfg
after install is complete.
"" vs None again
Done
* scripts.upd-instroot.dogtail_files.diff
Added code to extract the missing files into stage2.img.
Works fine, already sent to anaconda-devel-list.
Why are the gconf bits needed here? At least in the past, loading the
modules was good enough to get things initialized and that was all that
the gconf key affected. ie, what's changed?
In dogtail/utils.py
---
import gconf
gconfClient = gconf.client_get_default()
a11yGConfKey = '/desktop/gnome/interface/accessibility'
def isA11yEnabled():
"""
Checks if accessibility is enabled via gconf.
"""
return gconfClient.get_bool(a11yGConfKey)
---
This requires gconf.so and gconfd-2 otherwise fails.
libgconfbackend-xml.so is required to parse XML config file.
.gconf directory and %gconf.xml are config files.
It may be easier to have a patch against dogtail that returns always
true for isA11yEnabled() and does not use GConf
* pykickstart.data.add_dogtail.diff
* pykickstart.parser.add_dogtail_command.diff
Looks okay at a glance.
Now incorporated in scripts.upd-instroot.dogtail_files.diff.
Is it better this way or leave pykickstart patches separated from
anaconda patches?
Please review these patches.
Overall, looking good... I do want to hold off on applying them until
after we branch anaconda for F7 at this point just to avoid breaking
things in the final freeze. But that shouldn't be a big deal.
Jeremy
Thanks for review and comments.
Alexander.
--- anaconda.orig 2007-04-18 17:04:00.000000000 +0200
+++ anaconda 2007-04-30 16:45:22.000000000 +0200
@@ -243,6 +243,7 @@
op.add_option("--module", action="append", default=[])
op.add_option("--nomount", dest="rescue_nomount", action="store_true", default=False)
op.add_option("--updates", dest="updateSrc", action="store", type="string")
+ op.add_option("--dogtail", dest="dogtail", action="store", type="string")
return op.parse_args()
@@ -481,6 +482,7 @@
def __init__(self):
self.intf = None
self.dir = None
+ self.dogtail = None
self.id = None
self.method = None
self.methodstr = None
@@ -925,6 +927,44 @@
anaconda.id.setDisplayMode(opts.display_mode)
instClass.setInstallData(anaconda)
+ # we have parsed the kickstart data in the function above
+ # search for dogtail script in this priority: kickstart->anaconda options
+ if opts.ksfile and instClass.ksdata and instClass.ksdata.dogtail:
+ anaconda.dogtail = instClass.ksdata.dogtail
+ else:
+ if opts.dogtail:
+ anaconda.dogtail = opts.dogtail
+
+ if anaconda.dogtail:
+ try:
+ anaconda.id.setDogtail(anaconda.dogtail)
+ import urlgrabber
+ try:
+ fr = urlgrabber.urlopen(anaconda.dogtail)
+ except urlgrabber.grabber.URLGrabError, e:
+ log.error("Could not retrieve %s error was\n%s" % (anaconda.dogtail, e))
+ fr = None
+
+ if fr:
+ from tempfile import mkstemp
+ (fw, testcase) = mkstemp(prefix='testcase.py.', dir='/tmp/ramfs')
+ os.write(fw, fr.read())
+ fr.close()
+ os.close(fw)
+
+ # download completed now run the test
+ if not os.fork():
+ # we are in the child
+ os.chmod(testcase, 0755)
+ os.execv(testcase, [testcase])
+ sys.exit(1)
+ else:
+ # we are in the parent, sleep to give time for the testcase to initialize
+ # todo: is this needed, how to avoid race conditions
+ time.sleep(1)
+ except Exception, e:
+ log.error("Exception %s while running Dogtail testcase" % e)
+
# We need to copy the VNC-related kickstart stuff into the new ksdata
if opts.ksfile is not None:
instClass.ksdata.vnc = vncksdata
--- instdata.py.orig 2007-04-19 14:29:34.000000000 +0200
+++ instdata.py 2007-04-30 16:46:22.000000000 +0200
@@ -92,6 +92,9 @@
def setDisplayMode(self, display_mode):
self.displayMode = display_mode
+ def setDogtail(self, dogtail):
+ self.dogtail = dogtail
+
# expects a Keyboard object
def setKeyboard(self, keyboard):
self.keyboard = keyboard
@@ -204,6 +207,9 @@
f.write("upgrade\n");
else:
f.write("install\n");
+
+ if self.dogtail:
+ f.write("dogtail %s\n" % self.dogtail)
# figure out the install method and write out a line
if self.methodstr.startswith('ftp://') or self.methodstr.startswith('http://'):
@@ -283,6 +289,7 @@
self.floppyDevice = floppyDevice
self.fsset = fsset.FileSystemSet(anaconda)
self.excludeDocs = 0
+ self.dogtail = anaconda.dogtail
if flags.cmdline.has_key("excludedocs"):
self.excludeDocs = 1
--- upd-instroot.orig 2007-04-30 16:49:19.000000000 +0200
+++ upd-instroot 2007-04-30 18:02:23.000000000 +0200
@@ -234,7 +234,7 @@
fi
# dogtail stuff...
-PACKAGESGR="$PACKAGESGR gail at-spi libbonobo ORBit2"
+PACKAGESGR="$PACKAGESGR gail at-spi libbonobo ORBit2 pyspi GConf2 dogtail libXevie libXtst gnome-python2-gconf"
# modular xorg...
XORGLIBS="libICE libSM libX11 libXcursor libXext libXfixes libXft libXi libxkbfile libXmu libXpm libXrandr libXrender libXt libXxf86misc libXaw liblbxutil libXfont libfontenc libXau libXdmcp libXfont libXinerama"
@@ -809,12 +809,20 @@
usr/$LIBDIR/gtk-2.0/modules/libgail.so
usr/libexec/bonobo-activation-server
usr/libexec/at-spi-registryd
+usr/libexec/gconfd-2
usr/$LIBDIR/libORBit*
usr/$LIBDIR/libbonobo*
usr/$LIBDIR/libspi*
usr/$LIBDIR/libcspi*
usr/$LIBDIR/bonobo/servers/*
usr/$LIBDIR/libXevie*
+usr/$LIBDIR/libXtst*
+usr/$LIBDIR/libgconf-2*
+usr/$LIBDIR/GConf/2/libgconfbackend-xml.so
+usr/$LIBDIR/python?.?/site-packages/gtk-2.0/atk.so
+usr/$LIBDIR/python?.?/site-packages/gtk-2.0/gconf.so
+usr/$LIBDIR/python?.?/site-packages/atspi.so
+usr/lib/python?.?/site-packages/dogtail/*.py
EOF
@@ -951,6 +959,64 @@
echo "Expanding graphical packages..."
expandPackageSet "$RPMSGR" $DESTGR $KEEPFILEGR
+# more dogtail stuff
+if [ -n "$NEEDGR" ]; then
+ echo "Creating customized GConf2 settings for root"
+ mkdir -p $DESTGR/.gconf/desktop/gnome/interface
+ touch $DESTGR/.gconf/desktop/%gconf.xml
+ touch $DESTGR/.gconf/desktop/gnome/%gconf.xml
+ cat > $DESTGR/.gconf/desktop/gnome/interface/%gconf.xml <<EOF
+<?xml version="1.0"?>
+<gconf>
+ <entry name="accessibility" mtime="1176200664" type="bool" value="true">
+ </entry>
+</gconf>
+EOF
+#todo: fix file permissions
+
+ # add dogtail <url> command to kickstart
+ echo "Patching pykickstart ..."
+ cd $DESTGR/usr/lib/python?.?/site-packages/pykickstart
+ patch -p0 > /dev/null <<EOF
+--- data.py.orig 2007-04-18 16:26:35.000000000 +0200
++++ data.py 2007-04-18 16:26:51.000000000 +0200
+@@ -30,6 +30,7 @@
+ self.device = ""
+ self.deviceprobe = ""
+ self.displayMode = DISPLAY_MODE_GRAPHICAL
++ self.dogtail = None
+ self.driverdisk = ""
+ self.firewall = {"enabled": True, "ports": [], "trusts": []}
+ self.firstboot = FIRSTBOOT_SKIP
+EOF
+
+ patch -p0 > /dev/null <<EOF
+--- parser.py.orig 2007-04-18 14:43:10.000000000 +0200
++++ parser.py 2007-04-18 14:59:51.000000000 +0200
+@@ -233,6 +233,7 @@
+ "cmdline" : self.doDisplayMode,
+ "device" : self.doDevice,
+ "deviceprobe" : self.doDeviceProbe,
++ "dogtail" : self.doDogtail,
+ "driverdisk" : self.doDriverDisk,
+ "firewall" : self.doFirewall,
+ "firstboot" : self.doFirstboot,
+@@ -370,6 +371,12 @@
+ elif self.currentCmd == "text":
+ self.ksdata.displayMode = DISPLAY_MODE_TEXT
+
++ def doDogtail(self, args):
++ if len(args) > 1:
++ raise KickstartValueError, formatErrorMsg(self.lineno, msg=_("Command %s only takes one argument") % "dogtail")
++
++ self.ksdata.dogtail = args[0]
++
+ def doDriverDisk(self, args):
+ self.ksdata.driverdisk = string.join(args)
+EOF
+ cd -
+fi
+
echo "retrieving timezones"
TZDIR=/tmp/glibc-timezone-$$
mkdir -p $TZDIR/usr/share/zoneinfo
