Hi,
Those are great changes to merge. Well, the xbean-naming comment
could be improved thought :)
Thanks,
Gianny
On 18/03/2009, at 9:47 PM, Gianny Damour wrote:
On 18/03/2009, at 6:24 PM, David Jencks wrote:
After more work than I expected I have the server running with
Configuration being basically a pojo rather than constructing the
classloaders itself in its constructor. I commited my current
work in sandbox/djencks/framework together with a patch (cl-
factore.patch.1 -- I seem to have misspelled factory) for the
rest of geronimo. The server starts and seems to work although a
bunch of testsuite tests fail when run automatically.... when I
try the same thing by hand they seem to work.
Some of the changes:
-- separate classloader construction into a factory
-- track resolved configuration dependencies in a separate object
-- configuration now is a data container
-- I basically eliminated the EditableKernelConfigurationManager
which is used in a few places to modify existing configurations.
I've always thought this ability was a rather bad idea. I would
rather replace it with something like always storing the plan into
plugins (done now with car-maven-plugin, we just need to add it
for stuff deployed into geronimo directly) and providing a way to
edit the plan and redeploy the plugin with a new version number
(or only letting you edit snapshot plugins)
-- There's been a long-standing problem that many of the
"auxillary" builders can't figure out whether they need to add
dependencies to the environment until after they get to scan for
annotations at which time it was too late to modify the
classloader. As a result most of these builders always add their
dependencies whether or not they are actually needed. I think
that since the classloader construction is separated from the
configuration initialization we can now solve this problem and
create new classloaders each time we update the dependencies. I
haven't verified this but am rather hopeful.
I think that this is a bit of a conceptual simplification of some
of the work the configuration manager is doing and that it would
be ok to commit these changes to trunk. The main objection I
could see would be to the EditiableConfigurationManager change.
This functionality can probably be added back without too much
difficulty although I really think it is a mistake.
Thoughts?
The main change is in rev 755494, I removed an accidental file in
rev 755496.
I haven't had a chance to look at Gianny's classloader-per-jar
patch but I'm hopeful that can be merged into this without
excessive difficulty.
Based on a cursory review of the changes, a simple change to
JarFileClassLoaderFactory should be enough to merge the proposed
changes. If the patch makes sense, then I can easily merge against
this branch.
I will review in more details the overall change, however the
extraction of a classloader factory is certainly good.
Thanks,
Gianny
many thanks
david jencks
