RFR: 8231640: (prop) Canonical property storage [v12]

Jaikiran Pai jpai at openjdk.java.net
Tue Sep 14 02:26:03 UTC 2021


> The commit in this PR implements the proposal for enhancement that was discussed in the core-libs-dev mailing list recently[1], for https://bugs.openjdk.java.net/browse/JDK-8231640
> 
> At a high level - the `store()` APIs in `Properties` have been modified to now look for the `SOURCE_DATE_EPOCH` environment variable[2]. If that env variable is set, then instead of writing out the current date time as a date comment, the `store()` APIs instead will use the value set for this env variable to parse it to a `Date` and write out the string form of such a date. The implementation here uses the `d MMM yyyy HH:mm:ss 'GMT'` date format and `Locale.ROOT` to format and write out such a date. This should provide reproducibility whenever the `SOURCE_DATE_EPOCH` is set. Furthermore, intentionally, no changes in the date format of the "current date" have been done.
> 
> These  modified `store()` APIs work in the presence of the `SecurityManager` too. The caller is expected to have a read permission on the `SOURCE_DATE_EPOCH` environment variable. If the caller doesn't have that permission, then the implementation of these `store()` APIs will write out the "current date" and will ignore any value that has been set for the `SOURCE_DATE_EPOCH` env variable. This should allow for backward compatibility of existing applications, where, when they run under a `SecurityManager` and perhaps with an existing restrictive policy file, the presence of `SOURCE_DATE_EPOCH` shouldn't impact their calls to the `store()` APIs.
> 
> The modified `store()` APIs will also ignore any value for `SOURCE_DATE_EPOCH` that  cannot be parsed to an `long` value. In such cases, the `store()` APIs will write out the "current date" and ignore the value set for this environment variable. No exceptions will be thrown for such invalid values. This is an additional backward compatibility precaution to prevent any rogue value for `SOURCE_DATE_EPOCH` from breaking applications.
> 
> An additional change in the implementation of these `store()` APIs and unrelated to the date comment, is that these APIs will now write out the property keys in a deterministic order. The keys will be written out in the natural ordering as specified by `java.lang.String#compareTo()` API.
> 
> The combination of the ordering of the property keys when written out and the usage of `SOURCE_DATE_EPOCH` environment value to determine the date comment should together allow for reproducibility of the output generated by these `store()` APIs.
> 
> New jtreg test classes have been introduced to verify these changes. The primary focus of `PropertiesStoreTest` is the ordering aspects of the property keys that are written out. On the other hand `StoreReproducibilityTest` focuses on the reproducibility of the output generated by these APIs.  The `StoreReproducibilityTest` runs these tests both in the presence and absence of `SecurityManager`. Plus, in the presence of SecurityManager, it tests both the scenarios where the caller is granted the requisite permission and in other case not granted that permission.
> 
> These new tests and existing tests under `test/jdk/java/util/Properties/` pass with these changes.
> 
> [1] https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-August/080758.html
> [2] https://reproducible-builds.org/specs/source-date-epoch/

Jaikiran Pai has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 16 additional commits since the last revision:

 - Merge latest from master branch
 - Implement Roger's and the agreed upon decision to allow free-form text for the system property. Plus updates to test to match the expectations
 - Update javadoc based on Stuart's inputs
 - allow free-form text for java.util.Properties.storeDate system property
 - jcheck fix - trailing whitespace in test
 - Implement Roger's suggestions:
    - Rely on java.util.Properties.storeDate system property instead of SOURCE_DATE_EPOCH environment variable
    - No need for caching the parsed date
    - Update the javadoc to match new expectations
    - Update tests to match the new expectations
 - dummy commit to trigger GitHub actions job to try and reproduce an unexplained failure with the new tests that happened around 24 hours back, this time yesterday (rule out any time/date/timezone specific issues)
 - update javadoc to match latest changes
 - Implement Stuart's suggestion to use Collections instead of Arrays for ordering property keys
 - update the new tests to match the new date format expectations and also print out some log messages for better debuggability of the tests
 - ... and 6 more: https://git.openjdk.java.net/jdk/compare/dae096b0...6447f9bb

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5372/files
  - new: https://git.openjdk.java.net/jdk/pull/5372/files/c1dfb18f..6447f9bb

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5372&range=11
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5372&range=10-11

  Stats: 10450 lines in 483 files changed: 6216 ins; 2542 del; 1692 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5372.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5372/head:pull/5372

PR: https://git.openjdk.java.net/jdk/pull/5372


More information about the core-libs-dev mailing list