Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[MCOMPILER-499] Display recompilation causes #129

Closed
wants to merge 1 commit into from
Closed

[MCOMPILER-499] Display recompilation causes #129

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 93 additions & 7 deletions src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@
import org.apache.maven.shared.incremental.IncrementalBuildHelperRequest;
import org.apache.maven.shared.utils.ReaderFactory;
import org.apache.maven.shared.utils.StringUtils;
import org.apache.maven.shared.utils.io.DirectoryScanResult;
import org.apache.maven.shared.utils.io.DirectoryScanner;
import org.apache.maven.shared.utils.io.FileUtils;
import org.apache.maven.shared.utils.logging.MessageBuilder;
import org.apache.maven.shared.utils.logging.MessageUtils;
Expand Down Expand Up @@ -100,6 +102,8 @@ public abstract class AbstractCompilerMojo
{
protected static final String PS = System.getProperty( "path.separator" );

private static final String INPUT_FILES_LST_FILENAME = "inputFiles.lst";

static final String DEFAULT_SOURCE = "1.7";

static final String DEFAULT_TARGET = "1.7";
Expand Down Expand Up @@ -560,6 +564,8 @@ public abstract class AbstractCompilerMojo
@Parameter( defaultValue = "true", property = "maven.compiler.createMissingPackageInfoClass" )
private boolean createMissingPackageInfoClass = true;

@Parameter( defaultValue = "false", property = "maven.compiler.showCompilationChanges" )
private boolean showCompilationChanges = false;
/**
* Resolves the artifacts needed.
*/
Expand Down Expand Up @@ -875,14 +881,34 @@ else if ( CompilerConfiguration.CompilerReuseStrategy.ReuseSame.getStrategy().eq

incrementalBuildHelperRequest = new IncrementalBuildHelperRequest().inputFiles( sources );

DirectoryScanResult dsr = computeInputFileTreeChanges( incrementalBuildHelper, sources );

boolean idk = compiler.getCompilerOutputStyle()
.equals( CompilerOutputStyle.ONE_OUTPUT_FILE_FOR_ALL_INPUT_FILES ) && !canUpdateTarget;
boolean dependencyChanged = isDependencyChanged();
boolean sourceChanged = isSourceChanged( compilerConfiguration, compiler );
boolean inputFileTreeChanged = hasInputFileTreeChanged( dsr );
// CHECKSTYLE_OFF: LineLength
if ( ( compiler.getCompilerOutputStyle().equals( CompilerOutputStyle.ONE_OUTPUT_FILE_FOR_ALL_INPUT_FILES ) && !canUpdateTarget )
|| isDependencyChanged()
|| isSourceChanged( compilerConfiguration, compiler )
|| incrementalBuildHelper.inputFileTreeChanged( incrementalBuildHelperRequest ) )
if ( idk
|| dependencyChanged
|| sourceChanged
|| inputFileTreeChanged )
// CHECKSTYLE_ON: LineLength
{
getLog().info( "Changes detected - recompiling the module!" );
String cause = idk ? "idk" : ( dependencyChanged ? "dependency"
: ( sourceChanged ? "source" : "input tree" ) );
getLog().info( "Changes detected - recompiling the module! :" + cause );
if ( showCompilationChanges )
{
for ( String fileAdded : dsr.getFilesAdded() )
{
getLog().info( "\t+ " + fileAdded );
}
for ( String fileRemoved : dsr.getFilesRemoved() )
{
getLog().info( "\t- " + fileRemoved );
}
}

compilerConfiguration.setSourceFiles( sources );
}
Expand Down Expand Up @@ -1515,7 +1541,14 @@ private boolean isSourceChanged( CompilerConfiguration compilerConfiguration, Co
{
for ( File f : staleSources )
{
getLog().debug( "Stale source detected: " + f.getAbsolutePath() );
if ( showCompilationChanges )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we cannot see it in this GH ui diff but we go in this path only if ( getLog().isDebugEnabled() ) this must be changed to something such
if ( !staleSources.isEmpty() && (getLog().isDebugEnabled() || showCompilationChanges))
I guess the idea is to display those files when using -Dmaven.compiler.showCompilationChanges=true but without using full debug with -X

{
getLog().info( "Stale source detected: " + f.getAbsolutePath() );
}
else
{
getLog().debug( "Stale source detected: " + f.getAbsolutePath() );
}
}
}
return !staleSources.isEmpty();
Expand Down Expand Up @@ -1760,7 +1793,14 @@ protected boolean isDependencyChanged()
{
if ( hasNewFile( artifactPath, buildStartTime ) )
{
getLog().debug( "New dependency detected: " + artifactPath.getAbsolutePath() );
if ( showCompilationChanges )
{
getLog().info( "New dependency detected: " + artifactPath.getAbsolutePath() );
}
else
{
getLog().debug( "New dependency detected: " + artifactPath.getAbsolutePath() );
}
return true;
}
}
Expand Down Expand Up @@ -1909,6 +1949,52 @@ private String getMavenCompilerPluginVersion()
return pomProperties.getProperty( "version" );
}

private DirectoryScanResult computeInputFileTreeChanges( IncrementalBuildHelper ibh, Set<File> inputFiles )
throws MojoExecutionException
{
File mojoConfigBase = ibh.getMojoStatusDirectory();
File mojoConfigFile = new File( mojoConfigBase, INPUT_FILES_LST_FILENAME );

String[] oldInputFiles = new String[0];

if ( mojoConfigFile.exists() )
{
try
{
oldInputFiles = FileUtils.fileReadArray( mojoConfigFile );
}
catch ( IOException e )
{
throw new MojoExecutionException( "Error reading old mojo status " + mojoConfigFile, e );
}
}

String[] inputFileNames = new String[ inputFiles.size() ];
Copy link
Member

@olamy olamy Jul 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
String[] inputFileNames = new String[ inputFiles.size() ];
String[] inputFileNames = inputFiles.stream().map(File::getAbsolutePath).toArray(String[]::new);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simply replace the block with a single line. yup even Maven can use modern syntax 😜

int i = 0;
for ( File inputFile : inputFiles )
{
inputFileNames[ i++ ] = inputFile.getAbsolutePath();
}

DirectoryScanResult dsr = DirectoryScanner.diffFiles( oldInputFiles, inputFileNames );

try
{
FileUtils.fileWriteArray( mojoConfigFile, inputFileNames );
}
catch ( IOException e )
{
throw new MojoExecutionException( "Error while storing the mojo status", e );
}

return dsr;
}

private boolean hasInputFileTreeChanged( DirectoryScanResult dsr )
{
return ( dsr.getFilesAdded().length > 0 || dsr.getFilesRemoved().length > 0 );
}

public void setTarget( String target )
{
this.target = target;
Expand Down