You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "John Allen (JIRA)" <ji...@codehaus.org> on 2007/08/15 17:50:47 UTC

[jira] Issue Comment Edited: (MSITE-245) parent filesystem site.xml is never be found

    [ 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