Skip to content

v8: add date/time change notification #38638

New issue

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

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

Already on GitHub? # to your account

Closed
wants to merge 1 commit into from

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented May 11, 2021

This is an attempt at providing a workable solution for #4230 ....

V8 caches the timezone details. On windows, when process.env.TZ is changed, V8 is not picking up the changes. This PR introduces an API that can be used to tell V8 to clear it's cached timezone parameters. Whether this is going to work or not, I'm not yet sure as I'm having difficulty building on windows locally at the moment. We'll see what the CI has to say

Fixes: #4230
Signed-off-by: James M Snell jasnell@gmail.com

@github-actions github-actions bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency. labels May 11, 2021
Fixes: nodejs#4230
Signed-off-by: James M Snell <jasnell@gmail.com>
@nodejs-github-bot
Copy link
Collaborator

@jasnell
Copy link
Member Author

jasnell commented May 11, 2021

Hmm ok, looks like clearing the cache doesn't always help...

https://ci.nodejs.org/job/node-test-commit-linux-containered/26975/nodes=ubuntu1804_sharedlibs_withoutssl_x64/testReport/junit/(root)/test/parallel_test_datetime_change_notify/

AssertionError [ERR_ASSERTION]: The input did not match the regular expression /GMT-05:00/. Input:

'Tue May 11 2021 18:42:16 GMT+0000 (GMT)'

    at Object.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux-containered/test/parallel/test-datetime-change-notify.js:16:8)
    at Module._compile (node:internal/modules/cjs/loader:1109:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1138:10)
    at Module.load (node:internal/modules/cjs/loader:989:32)
    at Function.Module._load (node:internal/modules/cjs/loader:829:14)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:76:12)
    at node:internal/main/run_main_module:17:47 {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: 'Tue May 11 2021 18:42:16 GMT+0000 (GMT)',
  expected: /GMT-05:00/,
  operator: 'match'
}

Interestingly, the test works fine on my local ubuntu image...
image

The failures appear to be limited to the CI jobs that run in containers... at least so far.

@jasnell
Copy link
Member Author

jasnell commented May 11, 2021

Actually, it doesn't look like this is going to do the trick. I had missed that the datetime change notification was already being sent but it definitely does not seem to always be having any effect on Windows or some of the other machines. Hmmm.... Closing but will keep digging.

@jasnell jasnell closed this May 11, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

I can't set a default timezone on windows.
3 participants