tools: fix timezone update tool

[RaisinTen]

tools: fix timezone update tool

The spawnSync() call was previously silently failing with this error:

icupkg: unable to open input file "icudt*.dat"

because spawnSync() doesn't support globbing. This change replaces the spawnSync() call with execSync() because that supports globbing.

I have tested this workflow with some minor modifications in my fork and I can confirm that it works as expected now. The bot opened this PR - RaisinTen#2 which updates deps/icu-small/source/data/in/icudt71l.dat.bz2 in RaisinTen@44c2400.

Fixes: #44865
Signed-off-by: Darshan Sen [email protected]

---

tools: remove faulty early termination logic from update-timezone.mjs

We do not build Node.js in the workflow so

node/tools/update-timezone.mjs

Line 18 in f4815fc

const currentVersion = process.versions.tz;

is actually the version of tzdata in the Node.js in the runner instead
of what's in main.

The script is pretty fast even when the versions differ and there is an
update, so this optimization doesn't seem to be worth having given the
problem.

Signed-off-by: Darshan Sen [email protected]

[richardlau]

Do you think we should be checking the sync calls for errors?

Also I've spotted another problem -- we do not build Node.js in the workflow so

node/tools/update-timezone.mjs

Line 18 in f4815fc

const currentVersion = process.versions.tz;

is actually the version of tzdata in the Node.js in the runner instead of what's in main. I'm not even sure what version of Node.js that is as the workflow doesn't appear to specify.

[RaisinTen]

Do you think we should be checking the sync calls for errors?

execSync() already throws when there is an error unlike spawnSync() which puts it on the returned object, so I don't think it's strictly necessary.

Also I've spotted another problem -- we do not build Node.js in the workflow so

Good find! I've removed that optimization because the script is pretty fast anyways, PTAL.

[aduh95]

Wouldn't this be an issue if file contains "weird" characters?

[RaisinTen]

Maybe but that's never supposed to happen because file is always gonna be one of

node/tools/update-timezone.mjs

Lines 7 to 12 in 0d80e73

	const fileNames = [
	'zoneinfo64.res',
	'windowsZones.res',
	'timezoneTypes.res',
	'metaZones.res',
	];

and I think this list is unlikely to change.

[nodejs-github-bot]

Commit Queue failed

- Loading data for nodejs/node/pull/44870
✔  Done loading data for nodejs/node/pull/44870
----------------------------------- PR info ------------------------------------
Title      tools: fix timezone update tool (#44870)
Author     Darshan Sen  (@RaisinTen)
Branch     RaisinTen:fix-timezone-update-workflow-tool -> nodejs:main
Labels     tools, author ready
Commits    2
 - tools: fix timezone update tool
 - tools: remove faulty early termination logic from update-timezone.mjs
Committers 1
 - Darshan Sen 
PR-URL: https://github.com/nodejs/node/pull/44870
Fixes: https://github.com/nodejs/node/issues/44865
Reviewed-By: Richard Lau 
Reviewed-By: Antoine du Hamel 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/44870
Fixes: https://github.com/nodejs/node/issues/44865
Reviewed-By: Richard Lau 
Reviewed-By: Antoine du Hamel 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Mon, 03 Oct 2022 06:31:26 GMT
   ✔  Approvals: 2
   ✔  - Richard Lau (@richardlau) (TSC): https://github.com/nodejs/node/pull/44870#pullrequestreview-1128150857
   ✔  - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/44870#pullrequestreview-1136162346
   ✔  Last GitHub CI successful
   ℹ  Green GitHub CI is sufficient
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 44870
From https://github.com/nodejs/node
 * branch                  refs/pull/44870/merge -> FETCH_HEAD
✔  Fetched commits as 78d280a76821..33d4ff6c52fb
--------------------------------------------------------------------------------
[main bf715e2db2] tools: fix timezone update tool
 Author: Darshan Sen 
 Date: Mon Oct 3 10:46:48 2022 +0530
 1 file changed, 2 insertions(+), 8 deletions(-)
[main cf8eca8e4f] tools: remove faulty early termination logic from update-timezone.mjs
 Author: Darshan Sen 
 Date: Mon Oct 3 15:48:06 2022 +0530
 1 file changed, 7 deletions(-)
   ✔  Patches applied
There are 2 commits in the PR. Attempting autorebase.
Rebasing (2/4)
Executing: git node land --amend --yes

⚠  Found Fixes: #44865, skipping..

--------------------------------- New Message ----------------------------------

tools: fix timezone update tool
The spawnSync call was previously silently failing with this error:
icupkg: unable to open input file "icudt*.dat"
because spawnSync doesn't support globbing. This change replaces the

spawnSync call with execSync because that supports globbing.
I have tested this workflow with some minor modifications in my fork and

I can confirm that it works as expected now. The bot opened this PR -

RaisinTen#2 which updates

deps/icu-small/source/data/in/icudt71l.dat.bz2.
Fixes: #44865

Signed-off-by: Darshan Sen [email protected]

PR-URL: #44870

Reviewed-By: Richard Lau [email protected]

Reviewed-By: Antoine du Hamel [email protected]
[detached HEAD 374c459989] tools: fix timezone update tool

Author: Darshan Sen [email protected]

Date: Mon Oct 3 10:46:48 2022 +0530

1 file changed, 2 insertions(+), 8 deletions(-)

Rebasing (3/4)

Rebasing (4/4)
Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

tools: remove faulty early termination logic from update-timezone.mjs
We do not build Node.js in the workflow so


  

    

      node/tools/update-timezone.mjs
    
    

         Line 18
      in
      f4815fc
    
  
  

    


        

          

          
 const currentVersion = process.versions.tz; 
        
    
  



is actually the version of tzdata in the Node.js in the runner instead

of what's in main.

The script is pretty fast even when the versions differ and there is an

update, so this optimization doesn't seem to be worth having given the

problem.
Signed-off-by: Darshan Sen [email protected]

PR-URL: #44870

Fixes: #44865

Reviewed-By: Richard Lau [email protected]

Reviewed-By: Antoine du Hamel [email protected]
[detached HEAD bab7e83e62] tools: remove faulty early termination logic from update-timezone.mjs

Author: Darshan Sen [email protected]

Date: Mon Oct 3 15:48:06 2022 +0530

1 file changed, 7 deletions(-)
Successfully rebased and updated refs/heads/main.
ℹ  Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/3220375855

[nodejs-github-bot]

Landed in 22c39b1...b5add97