On 05/17/2011 08:27 PM, Chris Lumens wrote:
I see a couple things that concern me.
I will try to do something about those where possible, see below:
First, Dispatch.dispatch still exists and with it, all the "if dir == DISPATCH_" crud in the UI steps. At this past FUDCon, I thought the plan we discussed was that each step would keep its own state so we didn't have to check for the direction we were moving through anaconda, and that
When user keeps hitting back in the gui, steps should revert what they did previously so they can be reexecuted later(at least in theory, it's not always necessary). Tell me if you think otherwise, but we need to keep this ability. To do that right now, yes, they check the dispatch.dir variable and react accordingly.
My patchset allows for an alternative where there would be one method for going forward and one for going back, like 'storageinit' and 'storageinit_revert'. The steps that don't need it would not implement the '_revert' method.
steps would add each other to a sort of queue instead of having the big master list.
This just depends on how you think about what we have now. The 'one big list' is like a registry of all steps existing, a superset of all the steps running. An additional thing it provides is a total ordering on all the steps: given two steps you can decide which one should run sooner. Then steps can dynamically schedule what other steps will or will not run, just like adding stuff to a queue.
I don't think we need different structure at the moment. How would we for instance benefit from a queue? Simplifying the main loop, perhaps. But since it is not possible to determine all the steps that need to be taken ahead of the dispatch start, there would need to be a mechanism that can remove stuff from the queue without running them (like user chooses upgrade and we are no longer going to do bootloader). There still is a problem of ordering, suppose there are steps A, B and C where each following step depends on some information that can be affected by the previous step. If A adds C to the queue before B the two steps would just run in reverse order and the information from B would never be seen by C.
I can only speak for myself here, but I know I have always hated the tests at the beginning of the various steps. They have led to a variety of bugs over the years. By keeping this kind of stuff, I don't see how this is a big improvement.
Improvement is in splitting the concepts of Dispatcher and Step: Step is a class now and can have attributes, for instance a separate attribute to tell it what to do on revert. This way we can get rid of the checks.
Second, there's still the concept of steps that take a function to call and steps that look up the function to call in a stepToClass dict in the UI. This really needs to get unified.
I am totally open to suggestions here. The problem is that executing the step 'bootloader' in GUI does something completely different then in text. IOW, what a step with a screen does is given by what interface it is running under. So this information should be kept in the interface anyway (or we need interface-agnostic steps, i.e. no calls to gtk/snack from there: a whole different and bigger job).
Third, I was really hoping that any control flow rework would also provide us with a proper main loop. Peter's been talking about wanting that for years, and we'll need one if we ever want anaconda to be a dbus message producer in addition to consumer. And I've already got at least one reason for wanting that.
Yeah, this is a pain in the ass. A 'proper' main loop what do you mean by that?
The Dispatch.dispatch should act as the main loop and I tried to make it that way as possible, yet it still has to exit and be reentered. This is because we have another loop, gtk.main and one of the loops needs to be running the other loop (one of them needs to be on the stack above the other one): in our case it must be gtk.main, because we don't want to quit it and start it again over and over. And we can't just keep gtk.main sitting above either, the gui would never do anything.
The solution to this is having gtk.main run in a separate thread. But I had to start on this feature first and having the steps with attributes is the first, huh, step.
So all in all, this set enables a lot of things we couldn't do before and breaks (hopefully) none. Plus I remember from fudcon the main complain was it was never possible to tell if a step is going to be run or not and I think I helped that by making the scheduling interface more strict.
Ales _______________________________________________ Anaconda-devel-list mailing list Anaconda-devel-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/anaconda-devel-list
