spring-projects / spring-boot

Spring Boot
https://spring.io/projects/spring-boot
Apache License 2.0
74.95k stars 40.65k forks source link

IncrementalBuildMetadataGenerationTests#incrementalBuildTypeRenamed does not test what it's supposed to #38119

Open wilkinsona opened 11 months ago

wilkinsona commented 11 months ago

26271 has been fixed in 2.7.x. A different fix is required in 3.0.x and later due to the move to Spring Framework's test compiler. The tests for incremental build appear to be broken in a different way as the previous metadata is now never found. This means that no merging happens at all.

wilkinsona commented 11 months ago

I don't think we can fix this without some changes in Framework. For incremental compilation to work, the second compilation needs to be able to see the metadata json that was produced by the first compilation. This can be done by adding it as a resource to the compiler:

if (previousMetadata != null) {
    ByteArrayOutputStream output = new ByteArrayOutputStream();
    try {
        new JsonMarshaller().write(previousMetadata, output);
    }
    catch (IOException ex) {
        throw new UncheckedIOException(ex);
    }
    compiler = compiler
        .withResources(ResourceFile.of("META-INF/spring-configuration-metadata.json", output.toByteArray()));
    }
}

This works in so far as the annotation processor sees the old metadata and merges it with the new. Unfortunately, it does not work when this merged metadata is then being written. When DynamicFileManager has a resource – as is the case here due to the compiler being configured with the resource – any changes that are made to it are lost due to this code:

@Override
public FileObject getFileForOutput(Location location, String packageName,
    String relativeName, FileObject sibling) {
    ResourceFile resourceFile = this.resourceFiles.get(relativeName);
    if (resourceFile != null) {
        return new DynamicResourceFileObject(relativeName, resourceFile.getContent());
    }
    return this.dynamicResourceFiles.computeIfAbsent(relativeName, DynamicResourceFileObject::new);
}

resourceFile is not null so a new DynamicResourceFileObject is returned that contains the existing resource's content. Any changes that are made through the returned DynamicResourceFileObject are lost so when the tests try to retrieve the updated metadata they see the original resource file instead. This will have to be fixed in Framework before we can proceed here.