Proposal: JDK-8231640 - (prop) Canonical property storage

[Jaikiran Pai]

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