[
http://jira.codehaus.org/browse/MSITE-245?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_104922
]
John Allen edited comment on MSITE-245 at 8/15/07 10:49 AM:
------------------------------------------------------------
I'm not sure I exactly follow you John, I've had a look at the patch and the
opening line in it is to remove the existing relativePath logic altogether:
{code}
- String relativePath = getRelativePath(
siteDirectory.getAbsolutePath(), basedir.getAbsolutePath() );
{code}
When i think about this method I see a very simple purpose, namely to try and
find a site+locale+xml file for the passed in project, and if a locale specific
one can not be found, to assume that a default site.xml will exist. Clients of
the method obviously know that the returned File object may or may not exist so
it's not necessary for this methods to check for that.
Ideally we would pass in the MavenProject object we're interrogating here
rather than it's basedir as this would provide more opportunities in the future
to try and find the real site directory for the operand project, however as
that is not a model parameter and is instead a configuration parameter of the
site plugin itself, we would need a way of looking at the configuration
settings of the site plugin for the passed in project and I don't think we have
a ppropriate way of doing this at the moment.
This method, as you know, is used for two things. One, to get this project's
site.xml (prefering a locale based one) and secondly to try and find other
project's site.xml files as the client code tries to peek up the project
inheritance hierarchy. Neither of these uses cases seem to warrant a relative
path approach, after all there is no need to use a relative path when we have
explicit paths to work with and, once we find the file, (or not as the case
maybe) we instantiate a File object from it, so we loose any interest in
whether the File object's location (i.e. the site.xml filename and path) was
found via a relative path approach or not.
The method does have one concession to supporting a site.xml file that is being
stored in a 'non default' location but that only comes when looking for 'this'
project's site.xml file as we have access to this project's site plugin
configuration.
We could take a stance that when trying to find any project's site xml file we
assume it will use the same site directory location that this project is using:
{code}
protected File getSiteDescriptorFile( File basedir, Locale locale )
{
String sitePath;
if ( basedir.equals( project.getBasedir() ))
{
// it's this project's basedir so use our siteDirectory
sitePath = siteDirectory.getAbsolutePath();
}
else
{
// it's not this project's basedir so it must be one of our parent's
// so lets assume they store their site.xml in the same location
relative
// to their basedir as we do
String siteRelativeDirectory = getRelativePath(
siteDirectory.getAbsolutePath(), project.getBasedir().getAbsolutePath() );
sitePath = basedir.getAbsolutePath() + "/" + siteRelativeDirectory;
}
File siteDescriptor = new File( sitePath, "site_" +
locale.getLanguage() + ".xml" );
if ( !siteDescriptor.exists() )
{
siteDescriptor = new File( sitePath, "site.xml" );
}
return siteDescriptor;
{code}
Thus if this project stores its site files in ${basedir}/src/foo then we will
try and find all site.xml files in the directory src/foo. In fact we could go a
step further and try that first and if we fail then fall back to trying the
standard default location of src/site.
Don't know if I've helped here:)
ps. The code block above was written in JIRA and has not been tested, its just
for illustration
was:
I'm not sure I exactly follow you John, I've had a look at the patch and the
opening line in it is to remove the existing relativePath logic altogether:
{code}
- String relativePath = getRelativePath(
siteDirectory.getAbsolutePath(), basedir.getAbsolutePath() );
{code}
When i think about this method I see a very simple purpose, namely to try and
find a site+locale+xml file for the passed in project, and if a locale specific
one can not be found, to assume that a default site.xml will exist. Clients of
the method obviously know that the returned File object may or may not exist so
it's not necessary for this methods to check for that.
Ideally we would pass in the MavenProject object we're interrogating here
rather than it's basedir as this would provide more opportunities in the future
to try and find the real site directory for the operand project, however as
that is not a model parameter and is instead a configuration parameter of the
site plugin itself, we would need a way of looking at the configuration
settings of the site plugin for the passed in project and I don't think we have
a ppropriate way of doing this at the moment.
This method, as you know, is used for two things. One, to get this project's
site.xml (prefering a locale based one) and secondly to try and find other
project's site.xml files as the client code tries to peek up the project
inheritance hierarchy. Neither of these uses cases seem to warrant a relative
path approach, after all there is no need to use a relative path when we have
explicit paths to work with and, once we find the file, (or not as the case
maybe) we instantiate a File object from it, so we loose any interest in
whether the File object's location (i.e. the site.xml filename and path) was
found via a relative path approach or not.
The method does have one concession to supporting a site.xml file that is being
stored in a 'non default' location but that only comes when looking for 'this'
project's site.xml file as we have access to this project's site plugin
configuration.
We could take a stance that when trying to find any project's site xml file we
assume it will use the same site directory location that this project is using:
{code}
protected File getSiteDescriptorFile( File basedir, Locale locale )
{
String sitePath;
if ( basedir.equals( project.getBasedir() ))
{
// it's this project's basedir so use our siteDirectory
sitePath = siteDirectory.getAbsolutePath();
}
else
{
// it's not this project's basedir so it must be one of our parent's
// so lets assume they store their site.xml in the same location
relative
// to their basedir as we do
String siteRelativeDirectory = getRelativePath(
siteDirectory.getAbsolutePath(), project.getBasedir().getAbsolutePath() );
sitePath = basedir.getAbsolutePath() + "/" + siteRelativeDirectory;
}
File siteDescriptor = new File( sitePath, "site_" +
locale.getLanguage() + ".xml" );
if ( !siteDescriptor.exists() )
{
siteDescriptor = new File( sitePath, "site.xml" );
}
return siteDescriptor;
{code}
Thus if this project stores its site files in ${basedir}/src/foo then we will
try and find all site.xml files in the directory src/foo. In fact we could go a
step further and try that first and if we fail then fall back to trying the
standard default location of src/site.
Don;t know if I've helped here:)
> parent filesystem site.xml is never be found
> --------------------------------------------
>
> Key: MSITE-245
> URL: http://jira.codehaus.org/browse/MSITE-245
> Project: Maven 2.x Site Plugin
> Issue Type: Bug
> Affects Versions: 2.0-beta-5
> Environment: 2.0.7
> Reporter: John Allen
> Assignee: John Casey
> Priority: Blocker
> Attachments: site-patch.txt
>
>
> The current approach used by the getSiteDescriptorFile(File, Locale) is wrong
> as the basedir passed in to it is not just the project's own basedir, it's
> potentially a parent project's basedir and thus the previous code makes no
> sense as we would need to add on the parent's site.xml site directory and
> then try and find the relative path to it and as there's no way (that I know
> of) of a) finding that out from the parent project's object model (even if we
> passed it in) and b) the current code does not append anything to the passed
> in basedir so is always looking for a site.xml in the parents root directory.
> What's more the use of a relative path here is pointless too as we simply
> read in the descriptor file, not persist it into links where relativePaths
> are useful.
> Current code:
> {code}
> protected File getSiteDescriptorFile( File basedir, Locale locale )
> {
> String relativePath = getRelativePath(
> siteDirectory.getAbsolutePath(), basedir.getAbsolutePath() );
> File siteDescriptor = new File( relativePath, "site_" +
> locale.getLanguage() + ".xml" );
> if ( !siteDescriptor.exists() )
> {
> siteDescriptor = new File( relativePath, "site.xml" );
> }
> return siteDescriptor;
> }
> {code}
> Fixed code
> {code}
> protected File getSiteDescriptorFile( File basedir, Locale locale )
> {
> String sitePath;
>
> if ( basedir.equals( project.getBasedir() ))
> {
> // it's this project's basedir so use our siteDirectory (allows
> this project
> // to use a custom site location)
>
> sitePath = siteDirectory.getAbsolutePath();
> }
> else
> {
> // it's not this project's basedir so it must be one of our
> parent's,
> // so we'll just have to assume they store their site.xml in the
> // standard location (src/site)
>
> sitePath = basedir.getAbsolutePath() + "/src/site";
> }
>
> File siteDescriptor = new File( sitePath, "site_" +
> locale.getLanguage() + ".xml" );
> if ( !siteDescriptor.exists() )
> {
> siteDescriptor = new File( sitePath, "site.xml" );
> }
> return siteDescriptor;
> {code}
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://jira.codehaus.org/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
