You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@maven.apache.org by "elharo (via GitHub)" <gi...@apache.org> on 2023/04/21 11:33:51 UTC
[GitHub] [maven-shared-incremental] elharo commented on a diff in pull request #23: [MCOMPILER-333] Clean generatedSourcesDirectory along with outputDirectory
elharo commented on code in PR #23:
URL: https://github.com/apache/maven-shared-incremental/pull/23#discussion_r1173657637
##########
src/main/java/org/apache/maven/shared/incremental/IncrementalBuildHelper.java:
##########
@@ -129,6 +146,30 @@ public void setDirectoryScanner( DirectoryScanner directoryScanner )
this.directoryScanner = directoryScanner;
}
+ /**
+ * Get the existing DirectoryScanner used by this helper,
+ * or create new a DirectoryScanner if none is yet set.
+ * The DirectoryScanner is used for detecting changes in a directory
+ */
+ public DirectoryScanner getGeneratedSourcesDirectoryScanner()
+ {
+ if ( generatedSourcesDirectoryScanner == null )
+ {
+ generatedSourcesDirectoryScanner = new DirectoryScanner();
+ }
+
+ return generatedSourcesDirectoryScanner;
+ }
+
+ /**
+ * Set the DirectoryScanner which shall get used by this build helper.
Review Comment:
which shall get used by this build helper --> that scans the target/generated sources directory
##########
src/main/java/org/apache/maven/shared/incremental/IncrementalBuildHelper.java:
##########
@@ -66,12 +71,24 @@
*/
private DirectoryScanner directoryScanner;
+ /**
+ * Used for detecting changes between the Mojo execution.
+ * @see #getGeneratedSourcesDirectoryScanner() ;
+ */
+ private DirectoryScanner generatedSourcesDirectoryScanner;
+
/**
* Once the {@link #beforeRebuildExecution(org.apache.maven.shared.incremental.IncrementalBuildHelperRequest)} got
* called, this will contain the list of files in the build directory.
*/
private String[] filesBeforeAction = new String[0];
+ /**
+ * Once the {@link #beforeRebuildExecution(org.apache.maven.shared.incremental.IncrementalBuildHelperRequest)} got
Review Comment:
After the {@link #beforeRebuildExecution(org.apache.maven.shared.incremental.IncrementalBuildHelperRequest)} is
* called, this contains the list of files in the build directory.
##########
src/main/java/org/apache/maven/shared/incremental/IncrementalBuildHelper.java:
##########
@@ -129,6 +146,30 @@ public void setDirectoryScanner( DirectoryScanner directoryScanner )
this.directoryScanner = directoryScanner;
}
+ /**
+ * Get the existing DirectoryScanner used by this helper,
+ * or create new a DirectoryScanner if none is yet set.
+ * The DirectoryScanner is used for detecting changes in a directory
+ */
+ public DirectoryScanner getGeneratedSourcesDirectoryScanner()
Review Comment:
Do these methods need to be public? It looks like they can be package protected/default access
##########
src/main/java/org/apache/maven/shared/incremental/IncrementalBuildHelper.java:
##########
@@ -129,6 +146,30 @@ public void setDirectoryScanner( DirectoryScanner directoryScanner )
this.directoryScanner = directoryScanner;
}
+ /**
+ * Get the existing DirectoryScanner used by this helper,
+ * or create new a DirectoryScanner if none is yet set.
+ * The DirectoryScanner is used for detecting changes in a directory
+ */
+ public DirectoryScanner getGeneratedSourcesDirectoryScanner()
+ {
+ if ( generatedSourcesDirectoryScanner == null )
+ {
+ generatedSourcesDirectoryScanner = new DirectoryScanner();
+ }
+
+ return generatedSourcesDirectoryScanner;
+ }
+
+ /**
+ * Set the DirectoryScanner which shall get used by this build helper.
+ * @param generatedSourcesDirectoryScanner
+ */
+ public void setGeneratedSourcesDirectoryScanner( DirectoryScanner generatedSourcesDirectoryScanner )
Review Comment:
I'm not sure why these methods (or this class for that matter) is needed. It seems a directory scanner could be easily created without it.
##########
src/main/java/org/apache/maven/shared/incremental/IncrementalBuildHelper.java:
##########
@@ -323,25 +362,23 @@ public String[] beforeRebuildExecution( IncrementalBuildHelperRequest incrementa
public void afterRebuildExecution( IncrementalBuildHelperRequest incrementalBuildHelperRequest )
throws MojoExecutionException
{
- DirectoryScanner diffScanner = getDirectoryScanner();
- // now scan the same directory again and create a diff
- diffScanner.scan();
- DirectoryScanResult scanResult = diffScanner.diffIncludedFiles( filesBeforeAction );
-
File mojoConfigBase = getMojoStatusDirectory();
- File mojoConfigFile = new File( mojoConfigBase, CREATED_FILES_LST_FILENAME );
- try
- {
- FileUtils.fileWriteArray( mojoConfigFile, scanResult.getFilesAdded() );
- }
- catch ( IOException e )
- {
- throw new MojoExecutionException( "Error while storing the mojo status", e );
- }
+ writeChangedFiles(
+ getDirectoryScanner(),
Review Comment:
These getter methods don't seem to help. They're an unnecessary layer of indirection. Just use the constructor directly.
##########
src/main/java/org/apache/maven/shared/incremental/IncrementalBuildHelper.java:
##########
@@ -323,25 +362,23 @@ public String[] beforeRebuildExecution( IncrementalBuildHelperRequest incrementa
public void afterRebuildExecution( IncrementalBuildHelperRequest incrementalBuildHelperRequest )
throws MojoExecutionException
{
- DirectoryScanner diffScanner = getDirectoryScanner();
- // now scan the same directory again and create a diff
- diffScanner.scan();
- DirectoryScanResult scanResult = diffScanner.diffIncludedFiles( filesBeforeAction );
-
File mojoConfigBase = getMojoStatusDirectory();
- File mojoConfigFile = new File( mojoConfigBase, CREATED_FILES_LST_FILENAME );
- try
- {
- FileUtils.fileWriteArray( mojoConfigFile, scanResult.getFilesAdded() );
- }
- catch ( IOException e )
- {
- throw new MojoExecutionException( "Error while storing the mojo status", e );
- }
+ writeChangedFiles(
+ getDirectoryScanner(),
+ filesBeforeAction,
+ mojoConfigBase,
+ CREATED_FILES_LST_FILENAME );
+
+ writeChangedFiles(
+ getGeneratedSourcesDirectoryScanner(),
+ generatedSourcesBeforeAction,
+ mojoConfigBase,
+ GENERATED_FILES_LST_FILENAME );
// in case of clean compile the file is not created so next compile won't see it
// we mus create it here
+ File mojoConfigFile;
Review Comment:
declare on same line as initialize
##########
src/main/java/org/apache/maven/shared/incremental/IncrementalBuildHelper.java:
##########
@@ -358,6 +395,73 @@ public void afterRebuildExecution( IncrementalBuildHelperRequest incrementalBuil
}
+ private void writeChangedFiles(
+ DirectoryScanner directoryScanner,
+ String[] filesBeforeAction,
+ File mojoConfigBase,
+ String createdFilesLstFilename ) throws MojoExecutionException
+ {
+ if ( directoryScanner.getBasedir() == null )
+ {
+ return;
+ }
+
+ DirectoryScanResult outputDirectoryScanResult = scanDirectoryDiff(
+ directoryScanner, filesBeforeAction );
+
+ try
+ {
+ writeChangedFiles( outputDirectoryScanResult, mojoConfigBase, createdFilesLstFilename );
+ }
+ catch ( IOException e )
+ {
+ throw new MojoExecutionException( "Error while storing the mojo status", e );
+ }
+ }
+
+ private static void writeChangedFiles(
+ DirectoryScanResult outputDirectoryScanResult,
+ File mojoConfigBase,
+ String createdFilesLstFilename )
Review Comment:
Lst --> List
##########
src/main/java/org/apache/maven/shared/incremental/IncrementalBuildHelper.java:
##########
@@ -323,25 +362,23 @@ public String[] beforeRebuildExecution( IncrementalBuildHelperRequest incrementa
public void afterRebuildExecution( IncrementalBuildHelperRequest incrementalBuildHelperRequest )
throws MojoExecutionException
{
- DirectoryScanner diffScanner = getDirectoryScanner();
- // now scan the same directory again and create a diff
- diffScanner.scan();
- DirectoryScanResult scanResult = diffScanner.diffIncludedFiles( filesBeforeAction );
-
File mojoConfigBase = getMojoStatusDirectory();
- File mojoConfigFile = new File( mojoConfigBase, CREATED_FILES_LST_FILENAME );
- try
- {
- FileUtils.fileWriteArray( mojoConfigFile, scanResult.getFilesAdded() );
- }
- catch ( IOException e )
- {
- throw new MojoExecutionException( "Error while storing the mojo status", e );
- }
+ writeChangedFiles(
+ getDirectoryScanner(),
+ filesBeforeAction,
+ mojoConfigBase,
+ CREATED_FILES_LST_FILENAME );
+
+ writeChangedFiles(
+ getGeneratedSourcesDirectoryScanner(),
+ generatedSourcesBeforeAction,
+ mojoConfigBase,
+ GENERATED_FILES_LST_FILENAME );
// in case of clean compile the file is not created so next compile won't see it
// we mus create it here
Review Comment:
mus --> must
##########
src/main/java/org/apache/maven/shared/incremental/IncrementalBuildHelper.java:
##########
@@ -358,6 +395,73 @@ public void afterRebuildExecution( IncrementalBuildHelperRequest incrementalBuil
}
+ private void writeChangedFiles(
+ DirectoryScanner directoryScanner,
+ String[] filesBeforeAction,
+ File mojoConfigBase,
+ String createdFilesLstFilename ) throws MojoExecutionException
Review Comment:
Lst --> List
##########
src/main/java/org/apache/maven/shared/incremental/IncrementalBuildHelper.java:
##########
@@ -358,6 +395,73 @@ public void afterRebuildExecution( IncrementalBuildHelperRequest incrementalBuil
}
+ private void writeChangedFiles(
+ DirectoryScanner directoryScanner,
+ String[] filesBeforeAction,
+ File mojoConfigBase,
+ String createdFilesLstFilename ) throws MojoExecutionException
+ {
+ if ( directoryScanner.getBasedir() == null )
+ {
+ return;
+ }
+
+ DirectoryScanResult outputDirectoryScanResult = scanDirectoryDiff(
+ directoryScanner, filesBeforeAction );
+
+ try
+ {
+ writeChangedFiles( outputDirectoryScanResult, mojoConfigBase, createdFilesLstFilename );
+ }
+ catch ( IOException e )
+ {
+ throw new MojoExecutionException( "Error while storing the mojo status", e );
+ }
+ }
+
+ private static void writeChangedFiles(
+ DirectoryScanResult outputDirectoryScanResult,
+ File mojoConfigBase,
+ String createdFilesLstFilename )
+ throws IOException
+ {
+ File createdFiles = new File( mojoConfigBase, createdFilesLstFilename );
+ FileUtils.fileWriteArray( createdFiles, outputDirectoryScanResult.getFilesAdded() );
Review Comment:
This method is deprecated. Use
java.nio.files.Files.write(file.toPath(), data.getBytes(encoding), StandardOpenOption.CREATE)
and specify UTF-8 encoding
##########
src/main/java/org/apache/maven/shared/incremental/IncrementalBuildHelper.java:
##########
@@ -358,6 +395,73 @@ public void afterRebuildExecution( IncrementalBuildHelperRequest incrementalBuil
}
+ private void writeChangedFiles(
+ DirectoryScanner directoryScanner,
+ String[] filesBeforeAction,
+ File mojoConfigBase,
+ String createdFilesLstFilename ) throws MojoExecutionException
+ {
+ if ( directoryScanner.getBasedir() == null )
+ {
+ return;
+ }
+
+ DirectoryScanResult outputDirectoryScanResult = scanDirectoryDiff(
+ directoryScanner, filesBeforeAction );
+
+ try
+ {
+ writeChangedFiles( outputDirectoryScanResult, mojoConfigBase, createdFilesLstFilename );
+ }
+ catch ( IOException e )
+ {
+ throw new MojoExecutionException( "Error while storing the mojo status", e );
+ }
+ }
+
+ private static void writeChangedFiles(
+ DirectoryScanResult outputDirectoryScanResult,
+ File mojoConfigBase,
+ String createdFilesLstFilename )
+ throws IOException
+ {
+ File createdFiles = new File( mojoConfigBase, createdFilesLstFilename );
+ FileUtils.fileWriteArray( createdFiles, outputDirectoryScanResult.getFilesAdded() );
+ }
+
+ private static DirectoryScanResult scanDirectoryDiff(
+ DirectoryScanner directoryScanner,
+ String[] filesBeforeAction )
+ {
+ // now scan the same directory again and create a diff
+ directoryScanner.scan();
+ DirectoryScanResult outputScanResult =
+ directoryScanner.diffIncludedFiles( filesBeforeAction );
+ return outputScanResult;
+ }
+
+ private static String[] deleteFiles( File fileNameIndex, File parent ) throws IOException
+ {
+ String[] oldFiles = FileUtils.fileReadArray( fileNameIndex );
Review Comment:
deprecated. use java.nio.files.Files.readAllLines() instead and specify UTF-8
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org