You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@forrest.apache.org by Mathieu Champlon <m....@free.fr> on 2006/08/07 10:07:01 UTC
adding site-wide configuration files (was: [jira] Closed: (FOR-913)
failure when ${project.home}/forrest.properties does not exist)
Ross Gardler a écrit :
> Mathieu Champlon wrote:
>>>
>>> Ross Gardler closed FOR-913.
>>> ----------------------------
>>>
>>> Resolution: Invalid
>>>
>>> forrest.properties has nothing to do with ant. It provides
>>> configuration values for forrest.
>>>
>>> A forrest.properties file is required.
> ...
>> What I was trying to explain is that a forrest.properties file seems
>> indeed required while its content isn't.
>> If you seed a new project and remove everything from the
>> forrest.properties file, it builds fine (using default configuration).
>> But if you remove the (now empty) file, the build breaks.
>>
>> In the context of several projects with an automated/centralized
>> build process, it isn't very convenient to maintain a
>> forrest.properties file for each project.
> ...
>> For example if the proxy configuration must be changed, it's much
>> easier to change it in one place rather than going through every
>> project.
> ...
>
> OK, that all makes sense. But how is this changed by having a blank
> forrest.properties file?
It isn't.
The point was more something like : if the file can be blank then it
should probably be made optional.
> Then why not provide us with a patch? (again a rhetorical question,
> keep reading, I just want to point out we need your help)
(...)
> I've applied a guard around the necessary code. It should now work
> without a forrest.properties file.
Thanks !
I was actually hoping/waiting for the re-opening of the issue so I could
attach the patch.
> Perhaps your use case requries a new site-wide properties file. It
> would be easier to manage than setting properties in your ant build
> file and is easy to implement.
Yes, good point !
(...)
> It is also worth noting that in a future version of Forrest (probably
> 0.9) we will be moving to a new XML property configuration system.
> This provides much more flexibility than the existing system. If you
> add a site-wide file please be sure to add an equivalent XML conf file
> (loaded in the same code).
I took a look at the configuration management before adding this new
feature and I have a few refactoring suggestions :
1/ ForrestConfModule#getSystemProperty is never used and therefore
neither is defaultHome
2/ import java.io.FileNotFoundException; is not needed anymore
3/ configuration variables precedence order
It seems from reading ForrestConfModule#initialize that configuration
variables come from four different sources, in this order (the first
defined wins) :
. system properties
. xml configuration files
. property configuration files
. plugin xml configuration files
System properties are defined in the ant scripts as the properties
starting with either 'forrest.' or 'project.', including -D properties
from command-line and properties loaded from files with <property
file=.../>.
Therefore all configuration variables defined in property files loaded
by forrest.build.xml are accessible as system properties in
ForrestConfModule (well actually not all of them, because 'proxy' isn't,
but it's used only in ant scripts).
So there is no need to reload ${project.home}/forrest.properties in
ForrestConfModule#initialize.
Moreover, as the system properties are loaded at the end but replace all
other properties (in ForrestConfModule#loadSystemProperties),
configuration values from xml files are actually replaced by values from
property files.
So the actual precedence order is :
. command-line properties
. property configuration files
. xml configuration files
. plugin xml configuration files
What I suggest is to refactor ForrestConfModule#initialize so that it
clearly reflects all this, simplifying it at the same time, mostly by
removing the unnecessary code and re-arranging the loading order.
Among other benefits it also removes ant property files from
ForrestConfModule.
4/ ForrestConfModule#initialize is called twice
I don't exactly know why, it seems to be called from avalon, because
this method is actually an implementation of
org.apache.avalon.framework.activity.Initializable#initialize().
I haven't really looked further...
5/ there is no user.home forrest.properties.xml
There is a ${user.home}/forrest.properties loaded from the ant scripts,
but there is no corresponding xml configuration file. Is it on purpose ?
The user.home property from ant is not passed as system property because
it does not start with neither 'forrest.' nor 'project.', but it can be
specifically added.
6/ finally : adding site-wide properties files
Not much to say, only that the configuration files are only looked for
if a property named global.home exists (naming it site.home can be
confusing because 'site' has a special meaning in forrest already, so I
picked 'global').
Please tell me which modifications you think are worthwhile (and
possibly create an issue for this ?) and I will upload them for a step
by step patching.
MAT.
Re: adding site-wide configuration files
Posted by David Crossley <cr...@apache.org>.
Adding a note for the mail archives. Discussion fragmented to
"Re: svn commit: r430235 - in /forrest/trunk/main/webapp/WEB-INF"
http://marc.theaimsgroup.com/?t=115518302800001
-David
Re: adding site-wide configuration files
Posted by Ross Gardler <rg...@apache.org>.
David Crossley wrote:
> Thorsten Scherler wrote:
>
>>Ross Gardler escribi??:
>>
>>>Mathieu Champlon wrote:
>>>
>>>
>>>>3/ configuration variables precedence order
>>>>
>>>>It seems from reading ForrestConfModule#initialize that configuration
>>>>variables come from four different sources, in this order (the first
>>>>defined wins) :
>>>>. system properties
>>>>. xml configuration files
>>>>. property configuration files
>>>>. plugin xml configuration files
>>>>
>>>>System properties are defined in the ant scripts as the properties
>>>>starting with either 'forrest.' or 'project.', including -D properties
>>>>from command-line and properties loaded from files with <property
>>>>file=.../>.
>>>>Therefore all configuration variables defined in property files loaded
>>>>by forrest.build.xml are accessible as system properties in
>>>>ForrestConfModule (well actually not all of them, because 'proxy' isn't,
>>>>but it's used only in ant scripts).
>>>>So there is no need to reload ${project.home}/forrest.properties in
>>>>ForrestConfModule#initialize.
>>>>Moreover, as the system properties are loaded at the end but replace all
>>>>other properties (in ForrestConfModule#loadSystemProperties),
>>>>configuration values from xml files are actually replaced by values from
>>>>property files.
>>>>
>>>>So the actual precedence order is :
>>>>. command-line properties
>>>>. property configuration files
>>>>. xml configuration files
>>>>. plugin xml configuration files
>>>>
>>>>What I suggest is to refactor ForrestConfModule#initialize so that it
>>>>clearly reflects all this, simplifying it at the same time, mostly by
>>>>removing the unnecessary code and re-arranging the loading order.
>>>
>>>That all sounds fine by me.
>>
>>You forgot the ones from forrest-core.xconf.
>>
>>ForrestConfModule does not implement configure(...) meaning it will use
>>the one from super (DefaultsModule). This is adding properties from the
>>configuration (which are different for defaults vs. project).
>
>
> There is also another way to add properties.
>
> Cocoon has a new properties system which we have not
> been using yet. The version of Cocoon that we are
> currently using is not the most recent, but it includes
> an implementation which does work.
>
> It reads properties from a set of configuration files
> under WEB-INF/properties/ (in our version of Cocoon).
>
> These properties can then be used in Cocoon's xconf
> and sitemaps.
>
> http://cocoon.zones.apache.org/daisy/documentation/g1/1162.html
> (That is for the current Cocoon trunk.)
>
> See http://issues.apache.org/jira/browse/FOR-917
As far as I am aware, there isn't currently a way to add files to
WEB-INF from within the project directory. So this method brings us back
to the same problem of needing to edit Forrest core.
Having said that a config system that uses standard Cocoon methods can't
be a bad thing. So, anyone got the time to add the ability to add files
to WEB-INF from within a project. Hint... it is not simple, at least
given my initial thoughts, since it would break the in place editing
features).
Ross
Re: adding site-wide configuration files (was: [jira] Closed: (FOR-913) failure when ${project.home}/forrest.properties does not exist)
Posted by David Crossley <cr...@apache.org>.
Thorsten Scherler wrote:
> Ross Gardler escribi??:
> > Mathieu Champlon wrote:
> >
> > > 3/ configuration variables precedence order
> > >
> > > It seems from reading ForrestConfModule#initialize that configuration
> > > variables come from four different sources, in this order (the first
> > > defined wins) :
> > > . system properties
> > > . xml configuration files
> > > . property configuration files
> > > . plugin xml configuration files
> > >
> > > System properties are defined in the ant scripts as the properties
> > > starting with either 'forrest.' or 'project.', including -D properties
> > > from command-line and properties loaded from files with <property
> > > file=.../>.
> > > Therefore all configuration variables defined in property files loaded
> > > by forrest.build.xml are accessible as system properties in
> > > ForrestConfModule (well actually not all of them, because 'proxy' isn't,
> > > but it's used only in ant scripts).
> > > So there is no need to reload ${project.home}/forrest.properties in
> > > ForrestConfModule#initialize.
> > > Moreover, as the system properties are loaded at the end but replace all
> > > other properties (in ForrestConfModule#loadSystemProperties),
> > > configuration values from xml files are actually replaced by values from
> > > property files.
> > >
> > > So the actual precedence order is :
> > > . command-line properties
> > > . property configuration files
> > > . xml configuration files
> > > . plugin xml configuration files
> > >
> > > What I suggest is to refactor ForrestConfModule#initialize so that it
> > > clearly reflects all this, simplifying it at the same time, mostly by
> > > removing the unnecessary code and re-arranging the loading order.
> >
> > That all sounds fine by me.
>
> You forgot the ones from forrest-core.xconf.
>
> ForrestConfModule does not implement configure(...) meaning it will use
> the one from super (DefaultsModule). This is adding properties from the
> configuration (which are different for defaults vs. project).
There is also another way to add properties.
Cocoon has a new properties system which we have not
been using yet. The version of Cocoon that we are
currently using is not the most recent, but it includes
an implementation which does work.
It reads properties from a set of configuration files
under WEB-INF/properties/ (in our version of Cocoon).
These properties can then be used in Cocoon's xconf
and sitemaps.
http://cocoon.zones.apache.org/daisy/documentation/g1/1162.html
(That is for the current Cocoon trunk.)
See http://issues.apache.org/jira/browse/FOR-917
-David
Re: adding site-wide configuration files (was: [jira] Closed:
(FOR-913) failure when ${project.home}/forrest.properties does not exist)
Posted by Thorsten Scherler <th...@wyona.com>.
El lun, 07-08-2006 a las 10:45 +0100, Ross Gardler escribió:
> Mathieu Champlon wrote:
> > Ross Gardler a écrit :
> >
> >> Mathieu Champlon wrote:
>
> ...
>
> >> Then why not provide us with a patch? (again a rhetorical question,
> >> keep reading, I just want to point out we need your help)
> >
> > (...)
> >
> >> I've applied a guard around the necessary code. It should now work
> >> without a forrest.properties file.
>
> OK, I get it. Well I kind of got it before, hence my patch.
>
> > I was actually hoping/waiting for the re-opening of the issue so I could
> > attach the patch.
>
> Well, like you said it was real simple, almost as easy to do the change
> as it was to open the issue again. Sorry I missed this in your post
> though, help is always appreciated, I should have left it for you to do ;-)
>
> >> Perhaps your use case requries a new site-wide properties file. It
> >> would be easier to manage than setting properties in your ant build
> >> file and is easy to implement.
> >
> >
> > Yes, good point !
> >
> > (...)
> >
> >> It is also worth noting that in a future version of Forrest (probably
> >> 0.9) we will be moving to a new XML property configuration system.
> >> This provides much more flexibility than the existing system. If you
> >> add a site-wide file please be sure to add an equivalent XML conf file
> >> (loaded in the same code).
> >
> >
> > I took a look at the configuration management before adding this new
> > feature and I have a few refactoring suggestions :
> >
> > 1/ ForrestConfModule#getSystemProperty is never used and therefore
> > neither is defaultHome
> > 2/ import java.io.FileNotFoundException; is not needed anymore
>
> OK.
>
> > 3/ configuration variables precedence order
> >
> > It seems from reading ForrestConfModule#initialize that configuration
> > variables come from four different sources, in this order (the first
> > defined wins) :
> > . system properties
> > . xml configuration files
> > . property configuration files
> > . plugin xml configuration files
> >
> > System properties are defined in the ant scripts as the properties
> > starting with either 'forrest.' or 'project.', including -D properties
> > from command-line and properties loaded from files with <property
> > file=.../>.
> > Therefore all configuration variables defined in property files loaded
> > by forrest.build.xml are accessible as system properties in
> > ForrestConfModule (well actually not all of them, because 'proxy' isn't,
> > but it's used only in ant scripts).
> > So there is no need to reload ${project.home}/forrest.properties in
> > ForrestConfModule#initialize.
> > Moreover, as the system properties are loaded at the end but replace all
> > other properties (in ForrestConfModule#loadSystemProperties),
> > configuration values from xml files are actually replaced by values from
> > property files.
> >
> > So the actual precedence order is :
> > . command-line properties
> > . property configuration files
> > . xml configuration files
> > . plugin xml configuration files
> >
> > What I suggest is to refactor ForrestConfModule#initialize so that it
> > clearly reflects all this, simplifying it at the same time, mostly by
> > removing the unnecessary code and re-arranging the loading order.
>
> That all sounds fine by me.
You forgot the ones from forrest-core.xconf.
ForrestConfModule does not implement configure(...) meaning it will use
the one from super (DefaultsModule). This is adding properties from the
configuration (which are different for defaults vs. project).
>
> > 4/ ForrestConfModule#initialize is called twice
> >
> > I don't exactly know why, it seems to be called from avalon, because
> > this method is actually an implementation of
> > org.apache.avalon.framework.activity.Initializable#initialize().
> > I haven't really looked further...
>
> I have nothing to add to this. I didn't realise it was called twice.
> Since this is a Cocoon input module, then this is a Cocoon thing. So we
> need to take it up with them once we have a little more understanding of
> why it is happening.
I think the logical explanation is that we use it *twice*. Once for the
"defaults" and the other for "projects".
http://svn.apache.org/viewvc/forrest/trunk/main/webapp/WEB-INF/xconf/forrest-core.xconf?view=markup
I do not like this at all and may explain further (in another mail) why,
but bottom line is I wrote a input-module parameter generator (will
check it in shortly) and playing around with it I encountered problems
with this architecture.
>
> > 5/ there is no user.home forrest.properties.xml
> >
> > There is a ${user.home}/forrest.properties loaded from the ant scripts,
> > but there is no corresponding xml configuration file. Is it on purpose ?
> > The user.home property from ant is not passed as system property because
> > it does not start with neither 'forrest.' nor 'project.', but it can be
> > specifically added.
>
> This is not on purpose. The forrest.properties.xml was added for the
> specific purpose of allowing plugins to extend the configuration options
> available in the sitemap without having to touch core.
>
> It is still experimental (i.e. incomplete) code, so adding this feature
> would be a good idea.
Actually one big thing we forgot.
public Iterator getAttributeNames is not implemented but used form
super. That returns only the properties we have defined in the
forrest-core.xconf, that is way to less, since we added a lot more.
Now we can either only return the keys defined in the xconf (this is
what super does) or all keys defined in our module
(filteringProperties). The problem is now that one can reach the same
value like {project:something} and {defaults:something}, which is not
good at all because we share the input module in 2 different module
confs.
Further the whole defaults input module does not makes sense anymore,
since the new properties system is fallback enabled, meaning we always
have a default.
salu2
>
> > 6/ finally : adding site-wide properties files
> >
> > Not much to say, only that the configuration files are only looked for
> > if a property named global.home exists (naming it site.home can be
> > confusing because 'site' has a special meaning in forrest already, so I
> > picked 'global').
>
> Sounds fine.
>
> We look forward to your patches. Thanks for your contribuion.
>
> Ross
--
Thorsten Scherler
COO Spain
Wyona Inc. - Open Source Content Management - Apache Lenya
http://www.wyona.com http://lenya.apache.org
thorsten.scherler@wyona.com thorsten@apache.org
Re: adding site-wide configuration files (was: [jira] Closed: (FOR-913)
failure when ${project.home}/forrest.properties does not exist)
Posted by Ross Gardler <rg...@apache.org>.
Mathieu Champlon wrote:
> Ross Gardler a écrit :
>
>> Mathieu Champlon wrote:
...
>> Then why not provide us with a patch? (again a rhetorical question,
>> keep reading, I just want to point out we need your help)
>
> (...)
>
>> I've applied a guard around the necessary code. It should now work
>> without a forrest.properties file.
OK, I get it. Well I kind of got it before, hence my patch.
> I was actually hoping/waiting for the re-opening of the issue so I could
> attach the patch.
Well, like you said it was real simple, almost as easy to do the change
as it was to open the issue again. Sorry I missed this in your post
though, help is always appreciated, I should have left it for you to do ;-)
>> Perhaps your use case requries a new site-wide properties file. It
>> would be easier to manage than setting properties in your ant build
>> file and is easy to implement.
>
>
> Yes, good point !
>
> (...)
>
>> It is also worth noting that in a future version of Forrest (probably
>> 0.9) we will be moving to a new XML property configuration system.
>> This provides much more flexibility than the existing system. If you
>> add a site-wide file please be sure to add an equivalent XML conf file
>> (loaded in the same code).
>
>
> I took a look at the configuration management before adding this new
> feature and I have a few refactoring suggestions :
>
> 1/ ForrestConfModule#getSystemProperty is never used and therefore
> neither is defaultHome
> 2/ import java.io.FileNotFoundException; is not needed anymore
OK.
> 3/ configuration variables precedence order
>
> It seems from reading ForrestConfModule#initialize that configuration
> variables come from four different sources, in this order (the first
> defined wins) :
> . system properties
> . xml configuration files
> . property configuration files
> . plugin xml configuration files
>
> System properties are defined in the ant scripts as the properties
> starting with either 'forrest.' or 'project.', including -D properties
> from command-line and properties loaded from files with <property
> file=.../>.
> Therefore all configuration variables defined in property files loaded
> by forrest.build.xml are accessible as system properties in
> ForrestConfModule (well actually not all of them, because 'proxy' isn't,
> but it's used only in ant scripts).
> So there is no need to reload ${project.home}/forrest.properties in
> ForrestConfModule#initialize.
> Moreover, as the system properties are loaded at the end but replace all
> other properties (in ForrestConfModule#loadSystemProperties),
> configuration values from xml files are actually replaced by values from
> property files.
>
> So the actual precedence order is :
> . command-line properties
> . property configuration files
> . xml configuration files
> . plugin xml configuration files
>
> What I suggest is to refactor ForrestConfModule#initialize so that it
> clearly reflects all this, simplifying it at the same time, mostly by
> removing the unnecessary code and re-arranging the loading order.
That all sounds fine by me.
> 4/ ForrestConfModule#initialize is called twice
>
> I don't exactly know why, it seems to be called from avalon, because
> this method is actually an implementation of
> org.apache.avalon.framework.activity.Initializable#initialize().
> I haven't really looked further...
I have nothing to add to this. I didn't realise it was called twice.
Since this is a Cocoon input module, then this is a Cocoon thing. So we
need to take it up with them once we have a little more understanding of
why it is happening.
> 5/ there is no user.home forrest.properties.xml
>
> There is a ${user.home}/forrest.properties loaded from the ant scripts,
> but there is no corresponding xml configuration file. Is it on purpose ?
> The user.home property from ant is not passed as system property because
> it does not start with neither 'forrest.' nor 'project.', but it can be
> specifically added.
This is not on purpose. The forrest.properties.xml was added for the
specific purpose of allowing plugins to extend the configuration options
available in the sitemap without having to touch core.
It is still experimental (i.e. incomplete) code, so adding this feature
would be a good idea.
> 6/ finally : adding site-wide properties files
>
> Not much to say, only that the configuration files are only looked for
> if a property named global.home exists (naming it site.home can be
> confusing because 'site' has a special meaning in forrest already, so I
> picked 'global').
Sounds fine.
We look forward to your patches. Thanks for your contribuion.
Ross