Thanks for this Benjamin. Vincent
2008/8/29 <[EMAIL PROTECTED]>: > Author: bentmann > Date: Fri Aug 29 14:06:00 2008 > New Revision: 690389 > > URL: http://svn.apache.org/viewvc?rev=690389&view=rev > Log: > [MNGSITE-48] Consolidate coding pitfalls into a standalone document > > Added: > maven/site/trunk/src/site/apt/plugin-developers/common-bugs.apt (with > props) > Modified: > > maven/site/trunk/src/site/apt/guides/plugin/guide-java-plugin-development.apt > maven/site/trunk/src/site/apt/plugin-developers/index.apt > > Modified: > maven/site/trunk/src/site/apt/guides/plugin/guide-java-plugin-development.apt > URL: > http://svn.apache.org/viewvc/maven/site/trunk/src/site/apt/guides/plugin/guide-java-plugin-development.apt?rev=690389&r1=690388&r2=690389&view=diff > ============================================================================== > --- > maven/site/trunk/src/site/apt/guides/plugin/guide-java-plugin-development.apt > (original) > +++ > maven/site/trunk/src/site/apt/guides/plugin/guide-java-plugin-development.apt > Fri Aug 29 14:06:00 2008 > @@ -100,8 +100,9 @@ > * The <<<getLog>>> method (defined in <<<AbstractMojo>>>) returns a > log4j-like logger object which allows plugins to create messages at levels > of "debug", "info", "warn", and "error". This logger is the accepted > means > - to display information to the user. Please have a look at the section > about > - pitfalls for a hint on its proper usage. > + to display information to the user. Please have a look at the section > + > {{{../../plugin-developers/common-bugs.html#Retrieving_the_Mojo_Logger}Retrieving > the Mojo Logger}} for a hint on > + its proper usage. > > [] > > @@ -642,135 +643,6 @@ > <myObject>test</myObject> > +-----+ > > -* Some Pitfalls > - > - The following are some subtle anti-patterns that plugin developers should > avoid or take care. > - > -** Retrieving the Mojo Logger > - > - Defining an instance field in your mojo to save the logger like the > following is bad: > - > -+-----+ > -public MyMojo extends AbstractMojo > -{ > - private Log log = getLog(); > - > - public void execute() throws MojoExecutionException > - { > - log.debug( "..." ); > - } > -} > -+-----+ > - > - <<Explanation>>: Getting the logger this way (i.e. during the construction > of the mojo) will retrieve some default logger. In contrast, calling > - <<<getLog()>>> later in the <<<execute()>>> method will retrieve a logger > that has been injected by the plexus container. > - This is easily noticed by the different output styles of the log messages > and the fact that one gets <<<[debug]>>> messages without having "-X" enabled > - when using the wrong logger. Just call <<<getLog()>>> whenever you need it, > it is fast enough and needs not be saved in an instance field. > - > -** Using Relative File Path Parameters > - > - Defining a file path parameter of type <<<java.lang.String>>> is dangerous: > - > -+-----+ > -public MyMojo extends AbstractMojo > -{ > - /** > - * This parameter will take a file path (file paths are > platform-dependent...) > - * > - * @parameter > - */ > - private String outputDirectory; > - > - public void execute() throws MojoExecutionException > - { > - File outputDir = new File( outputDirectory ).getAbsoluteFile(); > - outputDir.mkdirs(); > - } > -} > -+-----+ > - > - <<Explanation>>: Users of your mojo will usually provide relative paths > for its parameters (i.e. outputDirectory = "target/something") and > - expect the mojo to resolve these paths against the base directory of the > project (i.e. outputDirectory = "$\{basedir\}/target/something"). > - However, whenever you use <<<java.io.File>>> with a relative path to > access the file system, the relative path will be resolved against > - the current working directory. But the current working directory might be > anything and is <not> necessarily the project's base directory. > - This is especially true during a reactor build when the current working > directory is usually the base directory of the parent project > - and not the base directory of the currently executed sub module. For this > reason, mojo parameters taking paths should be of type > - <<<java.io.File>>>. The important difference compared to > <<<java.lang.String>>> is that Maven will automatically push properly > resolved paths > - into the mojo fields. If you really need to use <<<java.lang.String>>> for > the parameter type (e.g. to allow the user to alternatively specify > - a class path resource or URL), be sure to always resolve relative file > paths manually against the base directory of the project. > - > -** Creating Resource Bundles > - > - Omitting an explicit resource bundle for the default locale provided by the > base bundle is not allowed. For example, the following family > - of resource bundles does not provide reliable internationalization for your > mojo: > - > -+-----+ > -mymojo-report.properties > -mymojo-report_de.properties > -+-----+ > - > - <<Explanation>>: As described in the method javadoc about > - > <<<{{{http://java.sun.com/javase/6/docs/api/java/util/ResourceBundle.html#getBundle(java.lang.String,%20java.util.Locale,%20java.lang.ClassLoader)}ResourceBundle.getBundle(String, > Locale, ClassLoader)}}>>>, > - the lookup strategy to find a bundle for a specific locale will prefer the > bundle for the JVM's default locale over the base bundle. > - However, the JVM's default locale needs not to match the locale of the > base bundle. In the example above, a request for the locale "en" will > - retrieve the bundle <<<mymojo-report_de.properties>>> instead of > <<<mymojo-report.properties>>> if the JVM's default locale happens to be "de". > - Therefore, always provide a dedicated bundle for the default locale of > your bundle family (e.g. <<<mymojo-report_en.properties>>>). This > - bundle should be empty such that it retrieves strings via the parent chain > from the base bundle. > - > -** Getting the Output Directory for a Report Mojo > - > - A report mojo that subclasses <<<AbstractReportMojo>>> needs to implement > <<<AbstractReportMojo.getOutputDirectory()>>>. This method > - should <never> be called during the generation of the report, i.e. the code > shown below is buggy: > - > -+-----+ > -public MyReportMojo extends AbstractReportMojo > -{ > - /** > - * The base directory of the report output. Only relevant if the mojo is > not run as part of a site generation. > - * > - * @parameter expression="${project.reporting.outputDirectory}" > - */ > - private File outputDirectory; > - > - /** > - * @see > org.apache.maven.reporting.AbstractMavenReport#getOutputDirectory() > - */ > - protected String getOutputDirectory() > - { > - return outputDirectory.toString(); > - } > - > - /** > - * @see org.apache.maven.reporting.AbstractMavenReport#executeReport( > java.util.Locale ) > - */ > - public void executeReport( Locale locale ) > - { > - // will fail during a site generation > - File outputFile = new File( getOutputDirectory(), "summary.txt" ); > - } > -} > -+-----+ > - > - <<Explanation>>: There are in principal two situations in which a report > mojo could be invoked. The mojo might be run > - directly from the command line or it might be run indirectly as part of > the site generation along with other report mojos. > - The glaring difference between these two invocations is the way the output > directory is controlled. In the first case, > - the parameter <<<outputDirectory>>> from the mojo itself is used. In the > second case however, the Maven Site Plugin > - will set the output directory according to its own configuration by > calling <<<MavenReport.setReportOutputDirectory()>>> > - on the reports being generated. Therefore, developers should always use > <<<MavenReport.getReportOutputDirectory()>>> if they > - need to query the effective output directory for the report. > - > -** Using Plexus Utilities > - > - If you depend on a version of plexus-utils later than 1.1, be sure to > properly specify your prerequisites on Maven: > - > -+-----+ > - <prerequisites> > - <maven>2.0.6</maven> > - </prerequisites> > -+-----+ > - > - <<Explanation>>: Before Maven 2.0.6, Maven always used plexus-utils 1.1 > for the plugin class realm regardless of the > - version of plexus-utils that the plugin specified in its POM. > > * Resources > > @@ -784,6 +656,6 @@ > > [[5]] {{{http://commons.apache.org/io/}Commons IO}}: Set of utilities > classes useful for file/path handling. > > - [[6]] {{{http://www.nabble.com/Common-Bugs-td14783703s177.html}Common > Bugs Thread}}. > + [[6]] {{{../../plugin-developers/common-bugs.html}Common Bugs and > Pitfalls}}: Overview of problematic coding patterns. > > [] > > Added: maven/site/trunk/src/site/apt/plugin-developers/common-bugs.apt > URL: > http://svn.apache.org/viewvc/maven/site/trunk/src/site/apt/plugin-developers/common-bugs.apt?rev=690389&view=auto > ============================================================================== > --- maven/site/trunk/src/site/apt/plugin-developers/common-bugs.apt (added) > +++ maven/site/trunk/src/site/apt/plugin-developers/common-bugs.apt Fri Aug > 29 14:06:00 2008 > @@ -0,0 +1,458 @@ > + ------ > + Common Bugs and Pitfalls > + ------ > + Benjamin Bentmann > + ------ > + 2008-08-29 > + ------ > + > +~~ Licensed to the Apache Software Foundation (ASF) under one > +~~ or more contributor license agreements. See the NOTICE file > +~~ distributed with this work for additional information > +~~ regarding copyright ownership. The ASF licenses this file > +~~ to you under the Apache License, Version 2.0 (the > +~~ "License"); you may not use this file except in compliance > +~~ with the License. You may obtain a copy of the License at > +~~ > +~~ http://www.apache.org/licenses/LICENSE-2.0 > +~~ > +~~ Unless required by applicable law or agreed to in writing, > +~~ software distributed under the License is distributed on an > +~~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY > +~~ KIND, either express or implied. See the License for the > +~~ specific language governing permissions and limitations > +~~ under the License. > + > +~~ NOTE: For help with the syntax of this file, see: > +~~ http://maven.apache.org/doxia/references/apt-format.html > + > +Common Bugs and Pitfalls > + > + Maven is not the smallest project in terms of source code and has as such > already suffered from many bugs. Having a > + closer look at all the issues revealed some coding problems that had > widespread among the various subcomponents. This > + document lists these commonly occurring anti patterns in order to help the > Maven community to prevent rather than fix > + bugs. Note that the primary focus is on pointing out problems that are > subtle in their nature rather than giving a > + comprehensive guide for Java or Maven development. > + > + * {{{Reading_and_Writing_Text_Files}Reading and Writing Text Files}} > + > + * {{{Converting_between_URLs_and_Filesystem_Paths}Converting between URLs > and Filesystem Paths}} > + > + * {{{Handling_Strings_Case-insensitively}Handling Strings > Case-insensitively}} > + > + * {{{Creating_Resource_Bundle_Families}Creating Resource Bundle Families}} > + > + * {{{Using_System_Properties}Using System Properties}} > + > + * {{{Resolving_Relative_Paths}Resolving Relative Paths}} > + > + * {{{Determining_the_Output_Directory_for_a_Site_Report}Determining the > Output Directory for a Site Report}} > + > + * {{{Retrieving_the_Mojo_Logger}Retrieving the Mojo Logger}} > + > + * {{{Depending_on_Plexus_Utilities_1.1}Depending on Plexus Utilities 1.1+}} > + > +* {Reading and Writing Text Files} > + > + Textual content is composed of characters while file systems merely store > byte streams. A file encoding (aka charset) > + is used to convert between bytes and characters. The challenge is using > the right file encoding. > + > + The JVM has this notion of a default encoding (given by the > <<<file.encoding>>> property) which it derives from a > + system's locale. While this might be a convenient feature sometimes, using > this default encoding for a project build > + is in general a bad idea: The build output will depend on the > machine/developer who runs the build. As such, usage > + of the default encoding threatens the dream of a reproducible build. > + > + For example, if developer A has UTF-8 as default encoding while developer > B uses ISO-8859-1, text files are very > + likely to get messed up during resource filtering or similar tasks. > + > + Therefore, plugin developers should avoid any direct or indirect usage of > the classes/methods that simply employ the > + platform's default encoding. For instance, <<<FileWriter>>> and > <<<FileReader>>> should usually be avoided: > + > ++--- > +/* > + * FIXME: This assumes the source file is using the platform's default > encoding. > + */ > +Reader reader = new FileReader( javaFile ); > ++--- > + > + Instead, the classes <<<OutputStreamWriter>>> and <<<OutputStreamReader>>> > can be used in combination with an > + explicit encoding value. This encoding value can be retrieved from a mojo > parameter such that the user can configure > + the plugin to fit his/her needs. > + > + To save the user from configuring each plugin individually, conventions > have been established that allow a user to > + centrally configure the file encoding per POM. Plugin developers should > respect these conventions whereever possible: > + > + * > {{{http://docs.codehaus.org/display/MAVENUSER/POM+Element+for+Source+File+Encoding}Source > File Encoding}} > + > + * > {{{http://docs.codehaus.org/display/MAVEN/Reporting+Encoding+Configuration}Report > Output Encoding}} > + > + [] > + > + Finally note that XML files require special handling because they are > equipped with an encoding declaration in the > + XML prolog. Reading or writing XML files with an encoding that does not > match their XML prolog's <<<encoding>>> > + attribute is a bad idea: > + > ++--- > +/* > + * FIXME: This assumes the XML encoding declaration matches the platform's > default encoding. > + */ > +Writer writer = new FileWriter( xmlFile ); > +writer.write( xmlContent ); > ++--- > + > + To ease the correct processing of XML files, developers are encouraged to > use > + > <<<{{{http://plexus.codehaus.org/plexus-utils/apidocs/org/codehaus/plexus/util/ReaderFactory.html#newXmlReader(java.io.File)}ReaderFactory.newXmlReader()}}>>> > + and > + > <<<{{{http://plexus.codehaus.org/plexus-utils/apidocs/org/codehaus/plexus/util/WriterFactory.html#newXmlWriter(java.io.File)}WriterFactory.newXmlWriter()}}>>> > + from the Plexus Utilities. > + > +* {Converting between URLs and Filesystem Paths} > + > + URLs and filesystem paths are really two different things and converting > between them is not trivial. The main source > + of problems is that different encoding rules apply for the strings that > make up a URL or filesystem path. For example, > + consider the following code snippet and its associated console output: > + > ++--- > +File file = new File( "foo bar+foo" ); > +URL url = file.toURI().toURL(); > + > +System.out.println( file.toURL() ); > +> file:/C:/temp/foo bar+foo > + > +System.out.println( url ); > +> file:/C:/temp/foo%20bar+foo > + > +System.out.println( url.getPath() ); > +> /C:/temp/foo%20bar+foo > + > +System.out.println( URLDecoder.decode( url.getPath(), "UTF-8" ) ); > +> /C:/temp/foo bar foo > ++--- > + > + First of all, please note that > + > <<<{{{http://java.sun.com/javase/6/docs/api/java/io/File.html#toURL()}File.toURL()}}>>> > does not escape the space > + character (and others). This yields an invalid URL, as per > + {{{http://www.faqs.org/rfcs/rfc2396.html}RFC 2396, section 2.4.3 "Excluded > US-ASCII Characters"}}. The class > + <<<java.net.URL>>> will silently accept such invalid URLs, in contrast > <<<java.net.URI>>> will not (see also > + > <<<{{{http://java.sun.com/javase/6/docs/api/java/net/URL.html#toURI()}URL.toURI()}}>>>). > For this reason, > + <<<File.toURL()>>> has been deprecated and should be replaced with > <<<File.toURI().toURL()>>>. > + > + Next, > <<<{{{http://java.sun.com/javase/6/docs/api/java/net/URL.html#getPath()}URL.getPath()}}>>> > does in general not > + return a string that can be used as a filesystem path. It returns a > substring of the URL and as such can contain > + escape sequences. The prominent example is the space character which will > show up as "%20". People sometimes hack > + around this by means of <<<replace("%20", " ")>>> but that does simply not > cover all cases. It's worth to mention > + that on the other hand the related method > + > <<<{{{http://java.sun.com/javase/6/docs/api/java/net/URI.html#getPath()}URI.getPath()}}>>> > does decode escapes but > + still the result is not a filesystem path (compare the source for the > constructor <<<File(URI)>>>). To summarize, > + the following idiom is to be avoided: > + > ++--- > +URL url = new URL( "file:/C:/Program%20Files/Java/bin/java.exe" ); > + > +/* > + * FIXME: This does not decode percent encoded characters. > + */ > +File path = new File( url.getPath() ); > ++--- > + > + To decode a URL, people sometimes also choose > + > <<<{{{http://java.sun.com/javase/6/docs/api/java/net/URLDecoder.html}java.net.URLDecoder}}>>>. > The pitfall with this > + class is that is actually performs HTML form decoding which is yet another > encoding and not the same as the URL > + encoding (compare the last paragraph in class javadoc about > + > <<<{{{http://java.sun.com/javase/6/docs/api/java/net/URL.html}java.net.URL}}>>>). > For instance, a <<<URLDecoder>>> > + will erroneously convert the character "+" into a space as illustrated by > the last sysout in the example above. > + > + In an ideal world, code targetting JRE 1.4+ could easily avoid these > problems by using the constructor > + > <<<{{{http://java.sun.com/javase/6/docs/api/java/io/File.html#File(java.net.URI)}File(URI)}}>>> > as suggested by the > + following snippet: > + > ++--- > +URL url = new URL( "file:/C:/Documents and Settings/user/.m2/settings.xml" ); > + > +/* > + * FIXME: This assumes the URL is fully compliant with RFC 3986. > + */ > +File path = new File( new URI( url.toExternalForm() ) ); > ++--- > + > + The remaining source of frustration is the conversion from <<<URL>>> to > <<<URI>>>. As already said, the <<<URL>>> > + class accepts malformed URLs which will make the constructor of <<<URI>>> > throw an exception. And indeed, class > + loaders from Sun JREs up to Java 1.4 will deliver malformed URLs when > queried for a resource. Likewise, the class > + loaders employed by Maven 2.x deliver malformed resource URLs regardless > of the JRE version (see > + {{{http://jira.codehaus.org/browse/MNG-3607}MNG-3607}}). > + > + For all these reasons, it is recommended to use > + > <<<{{{http://commons.apache.org/io/api-release/org/apache/commons/io/FileUtils.html#toFile(java.net.URL)}FileUtils.toFile()}}>>> > + from Commons IO or > + > <<<{{{http://plexus.codehaus.org/plexus-utils/apidocs/org/codehaus/plexus/util/FileUtils.html#toFile(java.net.URL)}FileUtils.toFile()}}>>> > + from a recent Plexus Utilities. > + > +* {Handling Strings Case-insensitively} > + > + When developers need to compare strings without regard to case or want to > realize a map with case-insensitive string > + keys, they often employ > + > <<<{{{http://java.sun.com/javase/6/docs/api/java/lang/String.html#toLowerCase()}String.toLowerCase()}}>>> > or > + > <<<{{{http://java.sun.com/javase/6/docs/api/java/lang/String.html#toUpperCase()}String.toUpperCase()}}>>> > to create > + a "normalized" string before doing a simple <<<String.equals()>>>. Now, > the <<<to*Case()>>> methods are overloaded: > + One takes no arguments and one takes a <<<Locale>>> object. > + > + The gotcha with the arg-less methods is that their output depends on the > default locale of the JVM but the default > + locale is out of control of the developer. That means the string expected > by the developer (who runs/tests his code > + in a JVM using locale <<<xy>>>) does not necessarily match the string seen > by another user (that runs a JVM with > + locale <<<ab>>>). For example, the comparison shown in the next code > snippet is likely to fail for systems with > + default locale Turkish because Turkish has unusual casing rules for the > characters "i" and "I": > + > ++--- > +/* > + * FIXME: This assumes the casing rules of the current platform > + * match the rules for the English locale. > + */ > +if ( "info".equals( debugLevel.toLowerCase() ) ) > + logger.info( message ); > ++--- > + > + For case-insensitive string comparisions which should be > locale-insensitive, the method > + > <<<{{{http://java.sun.com/javase/6/docs/api/java/lang/String.html#equalsIgnoreCase(java.lang.String)}String.equalsIgnoreCase()}}>>> > + should be used instead. If only a substring like a prefix/suffix should be > compared, the method > + > <<<{{{http://java.sun.com/javase/6/docs/api/java/lang/String.html#regionMatches(boolean,%20int,%20java.lang.String,%20int,%20int)}String.regionMatches()}}>>> > + can be used instead. > + > + If the usage of <<<String.to*Case()>>> cannot be avoided, the overloaded > version taking a <<<Locale>>> object should > + be used, passing in > + > <<<{{{http://java.sun.com/javase/6/docs/api/java/util/Locale.html#ENGLISH}Locale.ENGLISH}}>>>. > The resulting code > + will still run on Non-English systems, the parameter only locks down the > casing rules used for the string comparison > + such that the code delivers the same results on all platforms. > + > +* {Creating Resource Bundle Families} > + > + Especially reporting plugins employ resource bundles to support > internationalization. One language (usually English) > + is provided as the fallback/default language in the base resource bundle. > Due to the lookup strategy performed by > + > <<<{{{http://java.sun.com/javase/6/docs/api/java/util/ResourceBundle.html#getBundle(java.lang.String,%20java.util.Locale,%20java.lang.ClassLoader)}ResourceBundle.getBundle()}}>>>, > + one must always provide a dedicated resource bundle for this default > language, too. This bundle should be empty > + because it inherits the strings via the parent chain from the base bundle, > but it must exist. > + > + The following example illustrates this requirement. Imagine the broken > resource bundle family shown below which is > + intended to provide localization for English, German and French: > + > ++--- > +src/ > ++- main/ > + +- resources/ > + +- mymojo-report.properties > + +- mymojo-report_de.properties > + +- mymojo-report_fr.properties > ++--- > + > + Now, if a resource bundle is to be looked up for English on a JVM whose > default locale happens to be French, the > + bundle <<<mymojo-report_fr.properties>>> will be loaded instead of the > intended bundle <<<mymojo-report.properties>>>. > + > + Reporting plugins that suffer from this bug can easily be detected by > executing <<<mvn site -D locales=xy,en>>> where > + <<<xy>>> denotes any other language code supported by the particular > plugin. Specifying <<<xy>>> as the first locale > + will have the Maven Site Plugin change the JVM's default locale to > <<<xy>>> which in turn causes the lookup for > + <<<en>>> to fail as outlined above unless the plugin has a dedicated > resource bundle for English. > + > +* {Using System Properties} > + > + Maven's command line supports the definition of system properties via > arguments of the form <<<-D key=value>>>. > + While these properties are called system properties, plugins should never > use > + > <<<{{{http://java.sun.com/javase/6/docs/api/java/lang/System.html#getProperty(java.lang.String)}System.getProperty()}}>>> > + and related methods to query these properties. For example, the following > code snippet will not work reliably when > + Maven is embedded, say into an IDE or a CI server: > + > ++--- > +public MyMojo extends AbstractMojo > +{ > + public void execute() > + { > + /* > + * FIXME: This prevents proper embedding into IDEs or CI systems. > + */ > + String value = System.getProperty( "maven.test.skip" ); > + } > +} > ++--- > + > + The problem is that the properties managed by the <<<System>>> class are > global, i.e. shared among all threads in the > + current JVM. To prevent conflicts with other code running in the same JVM, > Maven plugins should instead query the > + execution properties. These can be obtained from > + > <<<{{{http://maven.apache.org/ref/current/maven-core/apidocs/org/apache/maven/execution/MavenSession.html#getExecutionProperties()}MavenSession.getExecutionProperties()}}>>>. > + > +* {Resolving Relative Paths} > + > + It is common practice for users of Maven to specify relative paths in the > POM, not to mention that the Super POM does > + so, too. The intention is to resolve such relative paths against the base > directory of the current project. In other > + words, the paths <<<target/classes>>> and > <<<$\{basedir\}/target/classes>>> should resolve to the same directory for a > + given POM. > + > + Unfortunately, the class > <<<{{{http://java.sun.com/javase/6/docs/api/java/io/File.html}java.io.File}}>>> > does not > + resolve relative paths against the project's base directory. As mentioned > in its class javadoc, it resolves relative > + paths against the current working directory. In plain English: Unless a > Maven component has complete control over the > + current working directory, any usage of <<<java.io.File>>> in combination > with a relative path is a bug. > + > + At first glance, one might be tempted to argue that the project base > directory is equal to the current working > + directory. However, this assumption is generally not true. Consider the > following scenarios: > + > + [[a]] Reactor Builds > + > + When a child module is build during a reactor build, the current > working is usually the base directory of the > + parent project, not the base directory of the current module. That > is the most common scenario where users are > + faced with the bug. > + > + [[b]] Embedded Maven Invocations > + > + Other tools, most notably IDEs, that run Maven under the hood may > have set the current working directory to > + their installation folder or whatever they like. > + > + [[c]] Maven Invocations using the <<<-f>>> Switch > + > + While it is surely an uncommon use-case, the user is free to invoke > Maven from an arbitrary working directory > + by specifying an absolute path like <<<mvn -f > /home/me/projects/demo/pom.xml>>>. > + > + [] > + > + Hence this example code is prone to misbehave: > + > ++--- > +public MyMojo extends AbstractMojo > +{ > + /** > + * @parameter > + */ > + private String outputDirectory; > + > + public void execute() > + { > + /* > + * FIXME: This will resolve relative paths like "target/classes" > against > + * the user's working directory instead of the project's base > directory. > + */ > + File outputDir = new File( outputDirectory ).getAbsoluteFile(); > + } > +} > ++--- > + > + In order to guarantee reliable builds, Maven and its plugins must manually > resolve relative paths against the > + project's base directory. A simple idiom like the following will do just > fine: > + > ++--- > +File file = new File( path ); > +if ( !file.isAbsolute() ) > +{ > + file = new File( project.getBasedir(), file ); > +} > ++--- > + > + Many Maven plugins can get this resolution automatically if they declare > their affected mojo parameters of type > + <<<java.io.File>>> instead of <<<java.lang.String>>>. This subtle > difference in parameter types will trigger a > + feature known as <path translation>, i.e. the Maven core will > automatically resolve relative paths when it pumps the > + XML configuration into a mojo. > + > +* {Determining the Output Directory for a Site Report} > + > + Most reporting plugins inherit from <<<AbstractMavenReport>>>. In doing > so, they need to implement the inherited but > + abstract method <<<getOutputDirectory()>>>. To implement this method, > plugins usually declare a field named > + <<<outputDirectory>>> which they return in the method. Nothing wrong so > far. > + > + Now, some plugins need to create additional files in the report output > directory that accompany the report generated > + via the sink interface. While it is tempting to use either the method > <<<getOutputDirectory()>>> or the field > + <<<outputDirectory>>> directly in order to setup a path for the output > files, this leads most likely to a bug. More > + precisely, those plugins will not properly output files when run by the > Maven Site Plugin as part of the site > + lifecycle. This is best noticed when the output directory for the site is > configured directly in the Maven Site Plugin > + such that it deviates from the expression > <<<$\{project.reporting.outputDirectory\}>>> that the plugins use by default. > + Multi-language site generation is another scenario to exploit this bug > which is illustrated below: > + > ++--- > +public MyReportMojo extends AbstractMavenReport > +{ > + /** > + * @parameter default-value="${project.reporting.outputDirectory}" > + */ > + private File outputDirectory; > + > + protected String getOutputDirectory() > + { > + return outputDirectory.getAbsolutePath(); > + } > + > + public void executeReport( Locale locale ) > + { > + /* > + * FIXME: This assumes the mojo parameter reflects the effective > + * output directory as used by the Maven Site Plugin. > + */ > + outputDirectory.mkdirs(); > + } > +} > ++--- > + > + There are in principal two situations in which a report mojo could be > invoked. The mojo might be run directly from > + the command line or the default build lifecycle or it might be run > indirectly as part of the site generation along > + with other report mojos. The glaring difference between these two > invocations is the way the output directory is > + controlled. In the first case, the parameter <<<outputDirectory>>> from > the mojo itself is used. In the second case > + however, the Maven Site Plugin takes over control and will set the output > directory according to its own configuration > + by calling <<<MavenReport.setReportOutputDirectory()>>> on the reports > being generated. > + > + Therefore, developers should always use > <<<MavenReport.getReportOutputDirectory()>>> if they need to query the > + effective output directory for the report. The implementation of > <<<AbstractMavenReport.getOutputDirectory()>>> > + is only intended as a fallback in case the mojo is not run as part of the > site generation. > + > +* {Retrieving the Mojo Logger} > + > + Maven employs an IoC container named Plexus to setup a plugin's mojos > before their execution. In other words, > + components required by a mojo will be provided by means of dependency > injection, more precisely field injection. The > + important point to keep in mind is that this field injection happens > <after> the mojo's constructor has finished. > + This means that references to injected components are invalid during the > construction time of the mojo. > + > + For example, the next snippet tries to retrieve the mojo logger during > construction time but the mojo logger is an > + injected component and as such has not been properly initialized yet: > + > ++--- > +public MyMojo extends AbstractMojo > +{ > + /* > + * FIXME: This will retrieve a wrong logger instead of the intended mojo > logger. > + */ > + private Log log = getLog(); > + > + public void execute() > + { > + log.debug( "..." ); > + } > +} > ++--- > + > + In case of the logger, the above mojo will simply use a default console > logger, i.e. the code defect is not > + immediately noticeable by a <<<NullPointerException>>>. This default > logger will however use a different message > + format for its output and also outputs debug messages even if Maven's > debug mode was not enabled. For this reason, > + developers must not try to cache the logger during construction time. The > method <<<getLog()>>> is fast enough and > + can simply be called whenever one needs it. > + > +* {Depending on Plexus Utilities 1.1+} > + > + Up to Maven 2.0.5, version 1.1 of the artifact <<<plexus-utils>>> was > included in the Maven core class loader which > + is shared with the plugin class realm. This effectively prevented plugins > from using another/newer version of > + <<<plexus-utils>>>. This has been solved starting with Maven 2.0.6 by > shading (most of) the classes from > + <<<plexus-utils>>> (see > {{{http://jira.codehaus.org/browse/MNG-2892}MNG-2892}}). > + > + In short, plugins that strictly require a newer version of > <<<plexus-utils>>> also require Maven 2.0.6 as a minimum. > + Hence, a POM snippet for a Maven plugin like shown below is misleading: > + > ++--- > +<project> > + <packaging>maven-plugin</packaging> > + ... > + <prerequisites> > + <!-- FIXME: This assumes the plugin works with plexus-utils:1.1, too --> > + <maven>2.0</maven> > + </prerequisites> > + ... > + <dependencies> > + <dependency> > + <groupId>org.codehaus.plexus</groupId> > + <artifactId>plexus-utils</artifactId> > + <version>1.5.6</version> > + </dependency> > + </dependencies> > + ... > +</project> > ++--- > > Propchange: maven/site/trunk/src/site/apt/plugin-developers/common-bugs.apt > ------------------------------------------------------------------------------ > svn:eol-style = native > > Propchange: maven/site/trunk/src/site/apt/plugin-developers/common-bugs.apt > ------------------------------------------------------------------------------ > svn:keywords = Author Date Id Revision > > Modified: maven/site/trunk/src/site/apt/plugin-developers/index.apt > URL: > http://svn.apache.org/viewvc/maven/site/trunk/src/site/apt/plugin-developers/index.apt?rev=690389&r1=690388&r2=690389&view=diff > ============================================================================== > --- maven/site/trunk/src/site/apt/plugin-developers/index.apt (original) > +++ maven/site/trunk/src/site/apt/plugin-developers/index.apt Fri Aug 29 > 14:06:00 2008 > @@ -26,6 +26,8 @@ > > * {{{./cookbook/index.html} Plugins Cookbook}} - Examples for how to > perform common tasks in plugins > > + * {{{./common-bugs.html} Common Bugs and Pitfalls}} - Overview of > problematic coding patterns > + > [] > > ~~TODO: trails > > > --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
