(nodejs)(nodejs.install) rework update script to be more future-proof (#2452)

[michha]

Description

Instead of parsing a markdown file, the script uses official JSON files to find the latest versions of each support channel.

Motivation and Context

NodeJS website fundamentaly changed some weeks ago. I reached out to the NodeJS team (Discussion 52407) and got the hints to json files. Fixes #2452

How Has this Been Tested?

I tested the content of the changed au_GetLatest method and also ran the update.ps1 script (which downloaded the correct files but failed locally because of missing choco command [we dont use choco on our dev systems])

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Migrated package (a package has been migrated from another repository)

Checklist:

• My code follows the code style of this repository.
• My change requires a change to documentation (this usually means the notes in the description of a package).
• I have updated the documentation accordingly (this usually means the notes in the description of a package).
• I have updated the package description and it is less than 4000 characters.
• All files are up to date with the latest Contributing Guidelines
• The added/modified package passed install/uninstall in the Chocolatey Test Environment(https://github.com/chocolatey-community/chocolatey-test-environment/). Note that we don't support the use of any other environment.
• The changes only affect a single package (not including meta package).

[michha]

I want to highlight the following points:

• The old code downloaded just the current and the latest LTS version. New code also downloads the old (but still supported) v18 LTS version.
• the new code should be tested by someone else, before merging!

[AppVeyorBot]

❌ Package verification failed, please review the Appveyor Logs and the provided Artifacts before requesting a human reviewer to take a look.

[michha]

I am clueless about the build problem or how to fix it.
Any hint is appreciated.

[pauby]

Did you check the AppVeyor logs for the error?

[michha]

Yes, I did.

Although text in the green box states an error, about missing NuspecVersion, this seems more like a warning to me. Other packages dont specify NuspecVersion either.

About the text in the red box: I have no clue what code is executed and what it should do and what fails.

Exit code was 0

ok? this should be good (but why yellow?)

Having absolutly zero knowledge about AppVeyor and considering that this is my first choco comunity PR I wouldnt see the cause of the failing pipeline in my code changes.

[pauby]

Did you test the update.ps1 locally? Did it work?

[pauby]

I haven't ran the code, but it's not necessary to specify, or cast all variables in PowerShell. This doesn't follow the code style used here. Can you remove that and only do it if necessary?

Is there a reason you're using parseexact rather then Get-Date?

[michha]

I haven't ran the code, but it's not necessary to specify, or cast all variables in PowerShell. This doesn't follow the code style used here. Can you remove that and only do it if necessary?

Of course I can remove it. Because our internal coding guideline is to explicitly set variable types (so you cannot store unwanted types) I set them here also.

Is there a reason you're using parseexact rather then Get-Date?

Get-Date is to get the current date but doesnt allow conversion from string into datetime

[AppVeyorBot]

❌ Package verification failed, please review the Appveyor Logs and the provided Artifacts before requesting a human reviewer to take a look.

[pauby]

Of course I can remove it. Because our internal coding guideline is to explicitly set variable types (so you cannot store unwanted types) I set them here also.

Appreciate that. However, you ticked the box to say that your code follows the style of the repository which it doesn't.

Get-Date is to get the current date but doesnt allow conversion from string into datetime.

That's not the only thing it does and it does.

[michha]

How will we address the failing build?
NodeJS will release another security patch tomorrow.

[pauby]

@AdmiringWorm can you take a look at this?

[AdmiringWorm]

@AdmiringWorm can you take a look at this?

I have never seen this before, and I ran the updater locally as well (to ensure it wasn't just an AppVeyor PR build error), and the same thing is happening locally as well.

I can only assume that the version we end up with is actually null or empty that causes this issue.

EDIT: Found the reason

[AppVeyorBot]

✅ Package verification completed without issues. PR is now pending human review

[michha]

@pauby Is there something I have to do to get this PR completed? Or is just the review open?

[pauby]

See the comments above from @AdmiringWorm as there may be more to do.

[michha]

@pauby
I am sorry to bother you again, but could you please clarify what is the next step here and who has to do it.

[pauby]

@michha You've requested a review from @AdmiringWorm. They next step is for that review to take place.

[michha]

@AdmiringWorm
May I ask you for a review?

As long as the script isnt fixed, the choco package still misses the NodeJS security releases from 3rd and 10th of April, see https://nodejs.org/en/blog/vulnerability

[AdmiringWorm]

I had not added this PR to my list of reviews as it had been reviewed by someone else originally.

From my view, though, there is only one change that is absolutely needed before we can have this merged in.

[AdmiringWorm]

Also, if you can. Please update your commit messages to have the package name affected as the prefix, similar to the title of the PR.

[michha]

Is it ok to squash my commits?

[AppVeyorBot]

✅ Package verification completed without issues. PR is now pending human review

[AdmiringWorm]

Is it ok to squash my commits?

That would have been okay yes.

[AdmiringWorm]

LGTM

Stale

[AdmiringWorm]

@michha your changes have been merged. Thanks for your contribution 👍

We apologize for the time it took to get this merged, and thank you for your patience.

[ovflowd]

(Node.js here) thank you all for working on this and fixing distribution updates of Chocolatey for Node.js 🙇