Allow individual build info properties to be excluded

[vpavic]

At present, BuildPropertiesWriter can fail with a NPE if build properties (group, artifact, name, version) are set to null.

This is specifically problematic with the Gradle plugin, since its DSL can be used to set group, name and version properties to null in an attempt to remove them from the generated build-info.properties. Additionally, setting artifact property to null results in the value unspecified.

This PR updates BuildPropertiesWriter to write the properties only if they are not null and have non-whitespace characters.

This closes #27092

[wilkinsona]

Thanks for the PR, @vpavic. This looks good on the Gradle side. For Maven, I can't see how group, artifact, and version can be omitted. Have I missed something?

[vpavic]

Thanks for the quick feedback @wilkinsona - please see #27092 (comment).

Things don't seem to be not quite consistent between Gradle and Maven plugins. To be more precise, I doesn't seem that the Maven plugin allows customization of build info properties (group, artifact, name, version) in the same manner as Gradle plugin does.

This is because Gradle plugin has BuildInfoProperties sitting in between the project's properties (that are the default source of build info props) and the BuildPropertiesWriter (or more precisely, its ProjectDetails) whereas Maven plugin populates those directly from the project. See:

spring-boot/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/main/java/org/springframework/boot/gradle/tasks/buildinfo/BuildInfo.java

Lines 64 to 68 in 9f001ef

	new ProjectDetails(this.properties.getGroup(),
	(this.properties.getArtifact() != null) ? this.properties.getArtifact()
	: "unspecified",
	this.properties.getVersion(), this.properties.getName(), this.properties.getTime(),
	coerceToStringValues(this.properties.getAdditional())));

spring-boot/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/BuildInfoMojo.java

Lines 91 to 92 in 9f001ef

	ProjectDetails details = new ProjectDetails(this.project.getGroupId(), this.project.getArtifactId(),
	this.project.getVersion(), this.project.getName(), getBuildTime(), this.additionalProperties);

It seems to me that it's a separate enhancement to get Maven plugin the capability to pull the core build info properties (i.e. group, artifact, name, version) from some other place than the project itself.

[wilkinsona]

Sorry, Vedran, #27092 (comment) got lost in my notifications. I agree with your analysis there. To fully implement #27092, I think we need to add artifact, group, and name parameters to BuildInfoMojo that default to their respective values on the project. Their values can then be customized in the same way the existing time parameter can be today. Would you like to tackle that as part of this PR or would you prefer to split things into two, one piece for Gradle and one for Maven?

[vpavic]

I can do this as a part of this PR if that works for you.

It's just that it seemed to me that from the project's perspective it would be better to track the new capabilities of Maven plugin separately, rather than having them hidden under something called Allow build info properties to be disabled.

[wilkinsona]

Thanks.

The primary, perhaps even only at this point, reason for making the properties configurable with Maven is to allow them to be disabled so I think it's fine to roll it all under a single issue/PR.

[vpavic]

OK then. I'll update this PR accordingly in the following days.

[vpavic]

I've updated the PR with needed changes on the Maven side. I also updated the docs.

Note that the empty tag approach didn't work as such configuration was ignored and resulted in the default values being written to the build-info.properties. So I resorted to using off as the signal to remove properties, which is IMO a good thing since it's consistent with the existing support for removing build.time.

[snicoll]

I've looked at the end result and I am wondering if that's the right call. While I don't deny that being able to disable certain properties is useful, I find the addition of new top-level properties for the only purpose of being able to set them to off wrong. If you want to expose, let's say the groupId information of the project, the value is already in the POM so there's really no need to be able to configure it. Worse, the plugin now offers to expose something we meant to be standard to a completely different value.

I think we need to take a step back and rather offer a way for users to take full control. The plugin has a additionalProperties argument. How about offering a boolean that doesn't add defaults at all? This way you can add any entry you like and the configuration in the POM represents what you'd get in the POM.

[vpavic]

@snicoll I agree that having a full control would be ideal - that was discussed on #27092 and #27413 was opened to consider something like that for 3.x.

[vpavic]

@philwebb Sorry for the ping, but is this on track to be included in 2.6?

I'm asking because it's a bit unclear to me - there's no milestone assigned here, the issue is labeled as for: merge-with-amendments and was apparently a subject of team meeting. Meanwhile the original issue (#27092) is still open and assigned to 2.6.x.

[philwebb]

Sorry for the confusion. We discussed it on the call and wanted to experiment some more with the Maven plugin configuration. The git info maven plugin has an <includeOnlyProperties>:

<configuration>
    <generateGitPropertiesFile>true</generateGitPropertiesFile>
    <generateGitPropertiesFilename>${project.build.outputDirectory}/git.properties</generateGitPropertiesFilename>
    <includeOnlyProperties>
        <includeOnlyProperty>^git.build.(time|version)$</includeOnlyProperty>
        <includeOnlyProperty>^git.commit.id.(abbrev|full)$</includeOnlyProperty>
    </includeOnlyProperties>
    <commitIdGenerationMode>full</commitIdGenerationMode>
</configuration>

We wondered if something similar might work here but we didn't get as far as really designing anything.

I'll mark the other issue as superseded and put this one in 2.6.x.

[philwebb]

Thanks very much for PR @vpavic (and for your patience)!

I've merged this into main and updated the Maven plugin so that it can accept a list of excludeInfoProperties values rather than having individual group, artifact,version and name properties. Although it's not quite as flexible, it feels more Maven like.