Skip to content

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

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.

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

Closed
wilkinsona opened this issue Oct 30, 2023 · 2 comments
Assignees
Labels
type: task A general task
Milestone

Comments

@wilkinsona
Copy link
Member

wilkinsona commented Oct 30, 2023

#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 wilkinsona added status: forward-port An issue tracking the forward-port of a change made in an earlier branch type: task A general task labels Oct 30, 2023
@wilkinsona wilkinsona modified the milestones: 3.0.13, 3.0.x Oct 30, 2023
@wilkinsona wilkinsona removed the status: forward-port An issue tracking the forward-port of a change made in an earlier branch label Oct 30, 2023
@wilkinsona
Copy link
Member Author

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.

@philwebb philwebb modified the milestones: 3.0.x, 3.1.x Nov 8, 2023
@scottfrederick scottfrederick added the status: blocked An issue that's blocked on an external project change label Nov 16, 2023
@wilkinsona wilkinsona modified the milestones: 3.1.x, 3.2.x May 20, 2024
@wilkinsona
Copy link
Member Author

spring-projects/spring-framework#33850 contains the necessary changes to Framework. https://github.com/wilkinsona/spring-boot/tree/gh-38119 contains the changes that should fix the test in Boot once the Framework changes are in place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: task A general task
Projects
None yet
Development

No branches or pull requests

3 participants