Allow build info goal's build.time to be disabled so that its output is repeatable

[nosan]

see gh-17294

[wilkinsona]

Our Gradle plugin allows the automatic generation of the build time to be disabled by either setting it to null (which switches it off, removing the property entirely) or to a fixed value. We should consider if we want to do the same for Maven rather than just allowing it to be switched off.

[nosan]

Thanks,
From the very beginning, I wanted to do:

	@Parameter(defaultValue = "${maven.build.timestamp}")
	private Date buildTime;

to disable buildTime the following configuration needs to be used:

<plugin>
				<groupId>@project.groupId@</groupId>
				<artifactId>@project.artifactId@</artifactId>
				<version>@project.version@</version>
				<executions>
					<execution>
						<configuration>
							<buildTime/>
						<goals>
							<goal>build-info</goal>
						</goals>
					</execution>
				</executions>
			</plugin>

[wilkinsona]

@nosan That looks like it would give us the behaviour that we would like. Was there a reason that you didn’t propose that approach in the end?

[nosan]

@wilkinsona
I don't have any reason for that. I have not known which approach is better and suitable.
Sorry, that I've not mentioned the second approach at the beginning.

[nosan]

Btw, Maybe it makes sense to use time instead of buildTime for consistency with a Gradle plugin.

[wilkinsona]

Thanks again. There's no need for an apology. I just wanted to check that we hadn't overlooked something that meant the approach would not work.

Yeah, I think that time makes sense too. It's consistent with the Gradle plugin and I think there's sufficient context to make it clear that it's the build time that's being configured.

[nosan]

@wilkinsona
The second approach does not work. It is weird as it worked yesterday at least I thought so.

Background

If the value of a property is empty, Maven treats that as null.

Considering the above, the following would not work:

<plugin>
    <groupId>@project.groupId@</groupId>
    <artifactId>@project.artifactId@</artifactId>
    <version>@project.version@</version>
    <executions>
        <execution>
            <configuration>
                <time/>
            </configuration>
            <goals>
                <goal>build-info</goal>
            </goals>
        </execution>
    </executions>
</plugin>

@Parameter
private Date time = new Date();  // as is, <time/> -> empty -> null -> ignored

@Parameter(defaultValue = "...")
private Date time; // default value,  <time/> -> empty -> null -> ignored

I confused myself and you as well. 😕 🤦‍♂

Possible solution

This is how to achieve the same behavior as Gradle plugin has:

Click to expand

@Mojo(name = "build-info", defaultPhase = LifecyclePhase.GENERATE_RESOURCES, threadSafe = true)
public class BuildInfoMojo extends AbstractMojo {

	@Component
	private BuildContext buildContext;

	/**
	 * The Maven project.
	 */
	@Parameter(defaultValue = "${project}", readonly = true, required = true)
	private MavenProject project;

	/**
	 * The location of the generated build-info.properties.
	 */
	@Parameter(defaultValue = "${project.build.outputDirectory}/META-INF/build-info.properties")
	private File outputFile;

	/**
	 * Additional properties to store in the build-info.properties. Each entry is prefixed
	 * by {@code build.} in the generated build-info.properties.
	 * @deprecated since 2.2.0
	 */
	@Parameter
	@Deprecated
	private Map<String, String> additionalProperties;

	/**
	 * The properties that are written into the {@code build-info.properties} file.
	 */
	@Parameter
	private Map<String, Object> properties;

	@Override
	public void execute() throws MojoExecutionException, MojoFailureException {
		try {
			new BuildPropertiesWriter(this.outputFile).writeBuildProperties(new ProjectDetails(getGroup(),
					getArtifact(), getVersion(), getName(), getTime(), getAdditionalProperties()));
			this.buildContext.refresh(this.outputFile);
		}
		catch (NullAdditionalPropertyValueException ex) {
			throw new MojoFailureException("Failed to generate build-info.properties. " + ex.getMessage(), ex);
		}
		catch (Exception ex) {
			throw new MojoExecutionException(ex.getMessage(), ex);
		}
	}

	private String getGroup() {
		return Objects.toString(getProperties().getOrDefault("group", this.project.getGroupId()));
	}

	private String getArtifact() {
		return Objects.toString(getProperties().getOrDefault("artifact", this.project.getArtifactId()));
	}

	private String getVersion() {
		return Objects.toString(getProperties().getOrDefault("version", this.project.getVersion()));
	}

	private String getName() {
		return Objects.toString(getProperties().getOrDefault("name", this.project.getName()));
	}

	private Instant getTime() {
		if (getProperties().containsKey("time")) {
			String time = Objects.toString(getProperties().get("time"), null);
			return (time != null) ? Instant.parse(time) : null;
		}
		return Instant.now();
	}

	@SuppressWarnings("unchecked")
	private Map<String, String> getAdditionalProperties() {
		return (Map<String, String>) getProperties().getOrDefault("additionalProperties", this.additionalProperties);
	}

	private Map<String, Object> getProperties() {
		return (this.properties != null) ? this.properties : Collections.emptyMap();
	}

}

[wilkinsona]

That's a shame. Maven's handling of null and empty properties is rather frustrating.

Thanks for the alternative solution. Another option, inspired by it, would be to special-case the handling of an additional property named time. Unlike other additional properties, we could allow it to be null and, if it is, disable the build time. If it's non-null, we'd then parse the value into an Instant and use that.

Yet another option would be to provide a String time property that can either be configured with off to disable the build.time property or to a value that's parsed into an Instant and then used to set the build time.

[nosan]

@wilkinsona
PR has been updated. I have added a way to configure build-info properties through the plugin.
To disable build.time , off value can be used.

[wilkinsona]

I've just discussed this with @snicoll and he is in favour of the <time>off<time> approach.

[wilkinsona]

Thanks for the latest updates, @nosan. I'd prefer to keep this change focussed on the build time. Do you have time to revert the changes that allow the artifact, group, name, and version to be customized?

[nosan]

@wilkinsona sure, I let you know

[nosan]

@wilkinsona PR has been updated.

[wilkinsona]

That was quick! Thanks very much, @nosan. The proposed changes are now in master.

[nosan]

Thanks once again, @wilkinsona