Hello Magnus,
On 30/08/21 5:29 pm, Magnus Ihse Bursie wrote:
On 2021-08-28 17:16, Alan Bateman wrote:
On 28/08/2021 05:45, Jaikiran Pai wrote:
I hadn't considered the system property approach to switch to old
behaviour in my proposals, so this is a very good input and I
personally think the most logical proposals so far.
Roger may be right that few would care but it would be changing
behavior that goes back to JDK 1.0. In your list (and thanks for
writing down the options with pros/cons) then your 1a where
SOURCE_DATE_EPOCH changes to write the epoch date rather than the
current date seems to be most consistent with other tools and the
wider eco system. It seems the least disruptive in that it doesn't
change existing behavior and would be opt-in when reproducibility is
required. Previous discussion was leading towards your option 2 (or
variants of) but isn't option doesn't help the cases where build
tools use libraries that rely on the older methods.
If I understood the numbering and alternatives correctly, I don't
think there is a conflict between 1a and 2, and I think both should be
implemented, for different reasons.
This is my understanding of the options, with the rationale I see for
choosing them:
* 1a says that we change the store() method to write the date from
SOURCE_DATE_EPOCH, if it is set -- otherwise it keeps the old
behavior. This allows users who want to use old Java code (or code
they can't modify) to produce output in a reproducible way to do so
without changing the source code of the product.
* 2 says that we add a new override to store() which also takes an
Instant. This way programmers who write new code and want to make sure
it is always reproducible can send in Instant.EPOCH, and not rely on
the user to configure SOURCE_DATE_EPOCH.
(In fact, when thinking of it, this seems overly complex. There is no
real need to send in a generic Instant, just an override with a
boolean skipTimestamp, or something like that. Basically, any solution
which allows programmers who write new code to get property files
without timestamps easily, is what's needed to fulfill this spot.
I do agree that it would be good to just get rid of the date comment and
let callers control what exact comments (if any) get written out.
However, I think that having both - a new method (overloaded or named
differently) and at the same time changing the implementation of the
existing store(...) methods to make the date comment reproducible is a
bit of a overkill for a comment that doesn't even play a functional role
in that API. I listed that option of a new overloaded API and would have
been in favour of it, if that was the only addition/change that we did
to support the date comment disabling/reproducibility.
Having said that, I will gladly go ahead and include/work on that
additional new API, if there is some general agreement on doing so.
-Jaikiran
