drop buildnumber:create already executed by jetty-util

[hboutemy]

re-running buildnumber-maven-plugin a second time creates a new timestamp property value, which breaks Reproducible Builds https://github.com/jvm-repo-rebuild/reproducible-central/blob/master/content/org/eclipse/jetty/jetty-project/README.md

AFAIK, the second run is not really necessary, just some cleanup forgotten that until now was not identified as causing any harm

[olamy]

Thanks for this.
But this might break a bit jetty start. because of this code

jetty.project/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java

Line 83 in 2584eb0

Props buildProps = Props.load(classLoader, "org/eclipse/jetty/start/build.properties");

maybe we could repackage the builld.properties file generated by jetty-util (as jetty-util artifact is already expand here in jetty-start build)

[joakime]

jetty-start's build.properties and jetty-util's build.properties are different.

jetty-start is for the bootstrap, which is loosely tied to a jetty version.
that jetty version is only use for defaulting the jetty-home/jetty-base version.
it can be overridden to be a different version at bootstrap (some folks do this!).

jetty-util build.properties is for runtime.

completely eliminating the build.properties in jetty-start is a no-go, it's necessary and required, but for different reasons than the jetty-util build.properties.

just deleting it from jetty-start like this is not appropriate.
finding a way to copy the jetty-util version to jetty-start i would be ok with.

[hboutemy]

sorry, the goal documentation was not clear it was generating the file in addition to calculating new property values

yes, copying the output from jetty-util is a solution
given the properties have already been generated, another option would be to inject them into a filtered resource may give expected results from both ends: the file is still done, but reusing properties from first run

I'll drop this PR, as it is not the right approach to solving the issue

[hboutemy]

@joakime @olamy looking more in depth, I noticed that this PR did not remove https://github.com/jetty/jetty.project/blob/jetty-12.0.x/jetty-core/jetty-start/src/main/resources/org/eclipse/jetty/start/build.properties

then technically, by dropping the plugin execution but not this resource, this PR did not remove the file, which is finally not done by the plugin: its content is just fully in sync with the one in jetty-util

this PR as it is seems to match expectations, finally?
I just reworded the description, because it was misleading

[olamy]

@hboutemy nah this will not work. look at the code I pointed in a previous comment.
we can avoid running again buildnumber plugin BUT we need to reuse the build.properties from jetty-util.
After build with your PR see the content of

olamy@pop-os:~/dev/sources/jetty/jetty.project$ cat jetty-core/jetty-start/target/classes/org/eclipse/jetty/start/build.properties 
buildNumber=${buildNumber}
timestamp=${timestamp}
version=12.0.7-SNAPSHOT

compared to

olamy@pop-os:~/dev/sources/jetty/jetty.project$ cat jetty-core/jetty-start/target/jetty-util/org/eclipse/jetty/version/build.properties 
buildNumber=671cab97b6ba01295f2e1caf062f4952273b53c3
timestamp=1706767565042
version=12.0.7-SNAPSHOT

so we need to include the second one in the jetty-start jar and read it. (can be moved as resource in the path org/eclipse/jetty/start/build.properties to keep backward compact (but I don't really think something else than jetty-start read it with such path) or change StartArgs class to read somewhere else (another path) in the classloader

[joakime]

@hboutemy don't forget that jetty-start has a normal artifact, and a shaded artifact.
Both show up on maven central, and different groups of people use each.
We use the shaded artifact in jetty-home/start.jar (to avoid the requirement for a classpath).

[joakim@hyperion jetty-home-12.0.6]$ jar -tvf start.jar | grep -E "(util|properties)"
   172 Mon Jun 05 23:12:48 CDT 2023 org/eclipse/jetty/start/build.properties
    64 Mon Jun 05 23:12:48 CDT 2023 META-INF/maven/org.eclipse.jetty/jetty-start/pom.properties
     0 Mon Jun 05 23:12:48 CDT 2023 org/eclipse/jetty/start/shaded/util/
  7335 Mon Jun 05 23:12:48 CDT 2023 org/eclipse/jetty/start/shaded/util/FileID.class
  2312 Mon Jun 05 23:12:48 CDT 2023 org/eclipse/jetty/start/shaded/util/JavaVersion.class
  3635 Mon Jun 05 23:12:48 CDT 2023 org/eclipse/jetty/start/shaded/util/ManifestUtils.class
  1371 Mon Jun 05 23:12:48 CDT 2023 org/eclipse/jetty/start/shaded/util/TopologicalSort$CyclicException.class
  2364 Mon Jun 05 23:12:48 CDT 2023 org/eclipse/jetty/start/shaded/util/TopologicalSort$InitialOrderComparator.class
  4987 Mon Jun 05 23:12:48 CDT 2023 org/eclipse/jetty/start/shaded/util/TopologicalSort.class

That means we should copy the jetty-util build.properties, not just reference it.
That would satisfy both artifact usages.

[hboutemy]

@hboutemy nah this will not work. look at the code I pointed in a previous comment. we can avoid running again buildnumber plugin BUT we need to reuse the build.properties from jetty-util. After build with your PR see the content of

olamy@pop-os:~/dev/sources/jetty/jetty.project$ cat jetty-core/jetty-start/target/classes/org/eclipse/jetty/start/build.properties 
buildNumber=${buildNumber}
timestamp=${timestamp}
version=12.0.7-SNAPSHOT

uh, I did not get that the property defined by the first run of the goal was not available in other modules: good catch

[hboutemy]

@olamy @joakime copy of build.properties from jetty-util to proper location done and tested in cb02948

sorry for not checking sufficiently well previously

[joakime]

Neat. I didn't know this fileMapper existed.

[hboutemy]

yes, not so easy to find from the plugin documentation: I need to improve it
https://codehaus-plexus.github.io/plexus-io/filemappers.html

[joakime]

I'm good with this.

[hboutemy]

good news, with this PR merged, Jetty 12.0.7 is now confirmed reproducible https://github.com/jvm-repo-rebuild/reproducible-central/blob/master/content/org/eclipse/jetty/jetty-project/README.md
(I just ignored html documentation)