Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing implausibly old time stamp 1970-01-01 00:00:00 by using the timestamp from the Git revision instead of default 0 value #3883

Merged
merged 3 commits into from
Jul 13, 2022

Conversation

reta
Copy link
Collaborator

@reta reta commented Jul 13, 2022

Signed-off-by: Andriy Redko [email protected]

Description

Fixing implausibly old time stamp 1970-01-01 00:00:00 by using the timestamp from the Git revision instead of default 0 value.

Issues Resolved

Closes #2412

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

…mestamp from the Git revision instead of default 0 value

Signed-off-by: Andriy Redko <[email protected]>
@reta reta requested a review from a team as a code owner July 13, 2022 15:25
@reta
Copy link
Collaborator Author

reta commented Jul 13, 2022

@andrross what do you think of this fix? thank you!

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@codecov-commenter
Copy link

codecov-commenter commented Jul 13, 2022

Codecov Report

Merging #3883 (c190281) into main (2763e80) will increase coverage by 0.01%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##               main    #3883      +/-   ##
============================================
+ Coverage     70.47%   70.48%   +0.01%     
+ Complexity    56617    56588      -29     
============================================
  Files          4557     4557              
  Lines        272683   272687       +4     
  Branches      40038    40038              
============================================
+ Hits         192163   192211      +48     
+ Misses        64254    64209      -45     
- Partials      16266    16267       +1     
Impacted Files Coverage Δ
...ensearch/gradle/tar/SymbolicLinkPreservingTar.java 0.00% <0.00%> (ø)
...cluster/coordination/PendingClusterStateStats.java 20.00% <0.00%> (-48.00%) ⬇️
...regations/metrics/AbstractHyperLogLogPlusPlus.java 51.72% <0.00%> (-44.83%) ⬇️
...cluster/coordination/PublishClusterStateStats.java 33.33% <0.00%> (-37.51%) ⬇️
...java/org/opensearch/threadpool/ThreadPoolInfo.java 56.25% <0.00%> (-37.50%) ⬇️
...search/search/aggregations/pipeline/EwmaModel.java 24.44% <0.00%> (-33.34%) ⬇️
...nsearch/index/shard/IndexShardClosedException.java 66.66% <0.00%> (-33.34%) ⬇️
...ations/bucket/terms/heuristic/ScriptHeuristic.java 5.55% <0.00%> (-31.49%) ⬇️
...earch/client/indices/GetIndexTemplatesRequest.java 60.00% <0.00%> (-30.00%) ⬇️
...rc/main/java/org/opensearch/ingest/IngestInfo.java 51.72% <0.00%> (-27.59%) ⬇️
... and 474 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2763e80...c190281. Read the comment docs.

Signed-off-by: Andriy Redko <[email protected]>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

Copy link
Member

@andrross andrross left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessarily a blocker, but I did find this commit in the history: 6b72d42

Apparently forking a process to invoke git was problematic and this was changed to read a .git file directly. @reta Do you think this will be a problem here?

@reta
Copy link
Collaborator Author

reta commented Jul 13, 2022

Not necessarily a blocker, but I did find this commit in the history: 6b72d42

Apparently forking a process to invoke git was problematic and this was changed to read a .git file directly. @reta Do you think this will be a problem here?

@andrross That's one of the reasons I have not included revision date into BuildParams directly. Unfortunately, I have not found the way to extract the revision date from .git metadata folder (only the revision hash is available and refs). One of the wild ideas I explored is find out the way to map revision hash to date somehow (but failed to come up with tangible solution so far).

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@andrross
Copy link
Member

@andrross That's one of the reasons I have not included revision date into BuildParams directly.

Does BuildParams.getGitRevisionDate get called multiple times during a build? If so, the result could be memoized into a static variable.

@reta
Copy link
Collaborator Author

reta commented Jul 13, 2022

@andrross That's one of the reasons I have not included revision date into BuildParams directly.

Does BuildParams.getGitRevisionDate get called multiple times during a build? If so, the result could be memoized into a static variable.

Yes, it could be called up to 9 times (if I am not mistaken), this is how many tars we may have, making it static would basically mean - adding the property to BuildParams. I would prefer not doing that: BuildParams are used mostly everywhere (plugins, etc), the revision date is only applicable to OpenSeach core distributions. I could probably move it to build.gradle instead to try and fetch the revision date there once.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@reta reta added the backport 2.x Backport to 2.x branch label Jul 13, 2022
@andrross andrross merged commit f165845 into opensearch-project:main Jul 13, 2022
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jul 13, 2022
…mestamp from the Git revision instead of default 0 value (#3883)

* Fixing implausibly old time stamp 1970-01-01 00:00:00 by using the timestamp from the Git revision instead of default 0 value

Signed-off-by: Andriy Redko <[email protected]>

* Address code review comments

Signed-off-by: Andriy Redko <[email protected]>

* Address code review comments (encapsulating the Git origin date inside build script

Signed-off-by: Andriy Redko <[email protected]>
(cherry picked from commit f165845)
andrross pushed a commit that referenced this pull request Jul 13, 2022
…mestamp from the Git revision instead of default 0 value (#3883) (#3891)

* Fixing implausibly old time stamp 1970-01-01 00:00:00 by using the timestamp from the Git revision instead of default 0 value

Signed-off-by: Andriy Redko <[email protected]>

* Address code review comments

Signed-off-by: Andriy Redko <[email protected]>

* Address code review comments (encapsulating the Git origin date inside build script

Signed-off-by: Andriy Redko <[email protected]>
(cherry picked from commit f165845)

Co-authored-by: Andriy Redko <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] tar: opensearch-1.3.0/jdk/...: implausibly old time stamp 1970-01-01 00:00:00 (JDK 11) in 1.3.0
3 participants