You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@maven.apache.org by Benjamin Bentmann <be...@udo.edu> on 2008/09/17 15:43:49 UTC
Re: svn commit: r696052 - /maven/shared/trunk/maven-filtering/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java
> Author: olamy
> Date: Tue Sep 16 13:55:35 2008
> New Revision: 696052
>
> URL: http://svn.apache.org/viewvc?rev=696052&view=rev
> Log:
> use new method copyDirectoryLayout from FileUtils
>
> Modified:
> maven/shared/trunk/maven-filtering/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java
>
> Modified: maven/shared/trunk/maven-filtering/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java
> URL: http://svn.apache.org/viewvc/maven/shared/trunk/maven-filtering/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java?rev=696052&r1=696051&r2=696052&view=diff
> ==============================================================================
> --- maven/shared/trunk/maven-filtering/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java (original)
> +++ maven/shared/trunk/maven-filtering/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java Tue Sep 16 13:55:35 2008
> @@ -246,7 +246,7 @@
> {
> try
> {
> - FileUtils.copyDirectoryStructure( resourceDirectory, targetPath == null ? outputDirectory
> + FileUtils.copyDirectoryLayout( resourceDirectory, targetPath == null ? outputDirectory
> : new File( outputDirectory,
> targetPath ),
> includes, excludes );
Hm, do we really need to do another scan here? The DirectoryScanner is
already hanging around and scanner.getIncludedDirectories() knows the
directories to copy, doesn't it?
I am basically concerned about performance here: IO is expensive and
scanning a big directory twice when the results are the same doesn't
look right IMHO. Not to mention the increased memory consumption since
two populated DirectoryScanners will be hanging around when the code
dives into copyDirectoryLayout().
Benjamin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: svn commit: r696052 - /maven/shared/trunk/maven-filtering/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java
Posted by Olivier Lamy <ol...@apache.org>.
ok.
Having a private method in DefaultMavenResourcesFiltering which do the
job sounds better (as in InvokerMojo)
--
Olivier
2008/9/17 Benjamin Bentmann <be...@udo.edu>:
> Olivier Lamy wrote:
>
>> Changing the method in p-u to FileUtils.copyDirectoryLayout( source,
>> target, directoryScanner ) ?
>
> Not sure whether this is the right approach, it would complicate the usage
> of this method in other contexts where people might not have the
> DirectoryScanner by hand.
>
> My original thought was just to copy the few lines of the required for-loop
> directly over to DefaultMavenResourcesFiltering. There's not too much magic
> in this loop.
>
> If reusage of plexus-utils is the ultimate goal, we could also consider to
> create something like
>
> FileUtils.copyDirectory(File srcDir, File dstDir,
> String[] includes, String[] excludes,
> boolean copyEmptyDirs,
> CopyHook fileCopyHook)
>
> i.e. a method that basically does all the work but allows to customize the
> actual file copy via a hook. The DefaultMavenResourcesFiltering would then
> provide a custom hook that does the filtering stuff. Just an idea.
>
>
> Benjamin
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: svn commit: r696052 - /maven/shared/trunk/maven-filtering/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java
Posted by Benjamin Bentmann <be...@udo.edu>.
Olivier Lamy wrote:
> Changing the method in p-u to FileUtils.copyDirectoryLayout( source,
> target, directoryScanner ) ?
Not sure whether this is the right approach, it would complicate the
usage of this method in other contexts where people might not have the
DirectoryScanner by hand.
My original thought was just to copy the few lines of the required
for-loop directly over to DefaultMavenResourcesFiltering. There's not
too much magic in this loop.
If reusage of plexus-utils is the ultimate goal, we could also consider
to create something like
FileUtils.copyDirectory(File srcDir, File dstDir,
String[] includes, String[] excludes,
boolean copyEmptyDirs,
CopyHook fileCopyHook)
i.e. a method that basically does all the work but allows to customize
the actual file copy via a hook. The DefaultMavenResourcesFiltering
would then provide a custom hook that does the filtering stuff. Just an
idea.
Benjamin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: svn commit: r696052 - /maven/shared/trunk/maven-filtering/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java
Posted by Olivier Lamy <ol...@apache.org>.
Hi Benjamin,
Thanks for review.
Changing the method in p-u to FileUtils.copyDirectoryLayout( source,
target, directoryScanner ) ?
Assuming scan() method already called in the directoryScanner instance.
--
Olivier
2008/9/17 Benjamin Bentmann <be...@udo.edu>:
>> Author: olamy
>> Date: Tue Sep 16 13:55:35 2008
>> New Revision: 696052
>>
>> URL: http://svn.apache.org/viewvc?rev=696052&view=rev
>> Log:
>> use new method copyDirectoryLayout from FileUtils
>>
>> Modified:
>>
>> maven/shared/trunk/maven-filtering/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java
>>
>> Modified:
>> maven/shared/trunk/maven-filtering/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java
>> URL:
>> http://svn.apache.org/viewvc/maven/shared/trunk/maven-filtering/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java?rev=696052&r1=696051&r2=696052&view=diff
>>
>> ==============================================================================
>> ---
>> maven/shared/trunk/maven-filtering/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java
>> (original)
>> +++
>> maven/shared/trunk/maven-filtering/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java
>> Tue Sep 16 13:55:35 2008
>> @@ -246,7 +246,7 @@
>> {
>> try
>> {
>> - FileUtils.copyDirectoryStructure( resourceDirectory,
>> targetPath == null ? outputDirectory
>> + FileUtils.copyDirectoryLayout( resourceDirectory,
>> targetPath == null ? outputDirectory
>>
>> : new File( outputDirectory,
>>
>> targetPath ),
>> includes, excludes
>> );
>
> Hm, do we really need to do another scan here? The DirectoryScanner is
> already hanging around and scanner.getIncludedDirectories() knows the
> directories to copy, doesn't it?
>
> I am basically concerned about performance here: IO is expensive and
> scanning a big directory twice when the results are the same doesn't look
> right IMHO. Not to mention the increased memory consumption since two
> populated DirectoryScanners will be hanging around when the code dives into
> copyDirectoryLayout().
>
>
> Benjamin
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org