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

Make sure the last modified timestamp is set correctly #1515

Merged
merged 1 commit into from
Aug 20, 2023

Conversation

laeubi
Copy link
Member

@laeubi laeubi commented Aug 19, 2023

Linux stores modification time with nano-seconds precision, but we currently set the timestamp of the cached file only in milliseconds. This can lead to the situation that the source is considered a few nanoseconds older than the cache file and therefore the cache is regenerated every time.

@laeubi laeubi requested a review from HannesWell August 19, 2023 17:18
Copy link
Contributor

@HannesWell HannesWell left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions
Copy link

github-actions bot commented Aug 19, 2023

Test Results

106 files  ±0  106 suites  ±0   10m 5s ⏱️ - 2m 2s
660 tests ±0  650 ✔️ ±0  10 💤 ±0  0 ±0 
660 runs  ±0  649 ✔️ ±0  11 💤 ±0  0 ±0 

Results for commit d0396e3. ± Comparison against base commit df437f8.

♻️ This comment has been updated with latest results.

@laeubi
Copy link
Member Author

laeubi commented Aug 20, 2023

The test failure was that if the artifact was already wrapped once with error it might be still created (bnds principle is to never really 'fail' but only emit an error/warning), so the second time it finds the cache file and proceeds without an error.

I now choose to simply delete the result file in case of an error, a more sophisticated approach might would store the error message along with the cached file but this has some uncertainties e.g if the error was not caused by the artifact itself so this seems the safe and easy path for now as one should not optimize for the error case.

Linux stores modification time with nano-seconds precision, but we
currently set the timestamp of the cached file only in milliseconds.
This can lead to the situation that the source is considered a few
nanoseconds older than the cache file and therefore the cache is
regenerated every time.
Copy link
Contributor

@HannesWell HannesWell left a comment

Choose a reason for hiding this comment

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

Makes sense to me.

if (jar != null) {
jar.close();
}
wrap.getJar().ifPresent(jar -> jar.close());
Copy link
Contributor

Choose a reason for hiding this comment

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

You could probably use a Method reference here.

@laeubi laeubi merged commit 1d90aa9 into eclipse-m2e:master Aug 20, 2023
4 of 5 checks passed
@HannesWell HannesWell added this to the 2.4.0 milestone Aug 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants