Based on these inputs, I have now updated the PR to use the value of
this new system property verbatim while writing out the comment. I
realize you mentioned that we should use this value if it is non-null
and non-empty. In my updated PR, I instead use this value only if it is
non-null and non-blank. I based it on my opinion that whitespace-only
comment through this system property, shouldn't be allowed. If however
you and others think that using non-empty check instead of non-blank
check is more appropriate, feel free to let me know and I'll update the
PR accordingly.
The new tests have been updated accordingly and they continue to pass
along with the existing ones in test/jdk/java/util/Properties/.
-Jaikiran
On 10/09/21 8:30 pm, Roger Riggs wrote:
Hi,
It does move the responsibility for that specific compatibility to the
person defining the system property.
I would document it as conventionally having the date in the format
expected.
In the context of a reproducible build, where the property is expected
to be used, the builder would be
able to track down tools that were expecting a specific format and
correct the input/property setting.
I think the incorrectly formatted string would be more use for
tracking down a problem
than having the date be in the correct format but quietly different
from what was expected.
(As is, an unparseable property value is replaced by the current date).
Roger
On 9/10/21 10:40 AM, Jaikiran Pai wrote:
On 10/09/21 7:59 pm, Roger Riggs wrote:
On Fri, 10 Sep 2021 10:15:45 GMT, Jaikiran Pai <j...@openjdk.org>
wrote:
I'm inclined to agree with Magnus, supplying the property as a
string, avoids hardcoding details that are not really in the domain
of Properties. It opens up the option to omit the date and
simplifies the code and spec.
Since the tools exist at build time or in the setting of the
property to format it as needed, Properties does not need to.
If non-null and non-empty it would be inserted as a comment. The
string would not contain the "# " prefix.
However, that goes against one of the goals this PR is trying to
achieve of backward compatibility of these existing store() APIs.
Allowing a free form text will mean that someone can feed a "foo bar"
and we will end up writing that as a comment, which essentially means
that the stored properties file no longer has any date comment.
Another example is, someone feeds a valid formatted text date to this
system property but that format doesn't match the format that is
advertized till date (the one which Date.toString() uses).
Are we willing to allow that?
-Jaikiran
