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

(nodejs.install) Unblock auto-updates by temporarily disabling NodeJS 23 #2562

Closed
wants to merge 1 commit into from
Closed

(nodejs.install) Unblock auto-updates by temporarily disabling NodeJS 23 #2562

wants to merge 1 commit into from

Conversation

chadlwilson
Copy link
Contributor

Description

Unblocks auto-updates for NodeJS <= v22 by disabling the v23+ streams while the v23 package is separately fixed.

Motivation and Context

Currently auto-updates are broken for all NodeJS versions (with this error) as Node v23 dropped 32-bit support, breaking the auto-update script for all versions - as noted in the primary issue #2556.

My earlier PR was rejected as noted in #2557 since the package needs to be updated to properly support 64-bit only packages which that PR didn't do. My earlier PR wasn't deemed complete enough as there are more changes needed to VERIFICATION.txt and other places to properly handle a package without a 32-bit variant.

Since I do not have necessary time/context/energy/knowledge/testing environment to fully validate such package changes, I instead propose to decouple the two issues; allow auto-updates to flow through for for v22, v20 etc until the 64-bit-only package can be addressed for v23+.

How Has this Been Tested?

Ran ./update.ps1 locally to ensure it only tries to update versions < 23.

Screenshot (if appropriate, usually isn't needed):

PS /Users/chad/Projects/community/community-chocolatey-packages/automatic/nodejs.install> ./update.ps1
nodejs.install - checking updates using Chocolatey-AU version 1.0.0

*** Stream: 18 ***
URL check
  https://nodejs.org/dist/v18.20.4/node-v18.20.4-x64.msi
  https://nodejs.org/dist/v18.20.4/node-v18.20.4-x86.msi
nuspec version: 18.20.4
remote version: 18.20.4
No new version found

*** Stream: 20 ***
URL check
  https://nodejs.org/dist/v20.18.0/node-v20.18.0-x64.msi
  https://nodejs.org/dist/v20.18.0/node-v20.18.0-x86.msi
nuspec version: 20.18.0
remote version: 20.18.0
No new version found

*** Stream: 22 ***
URL check
  https://nodejs.org/dist/v22.10.0/node-v22.10.0-x64.msi
  https://nodejs.org/dist/v22.10.0/node-v22.10.0-x86.msi
nuspec version: 22.9.0
remote version: 22.10.0
New version is available
Automatic checksum skipped
Running au_BeforeUpdate
Purging msi
Downloading to node-v22.10.0-x86.msi - https://nodejs.org/dist/v22.10.0/node-v22.10.0-x86.msi
Downloading to node-v22.10.0-x64.msi - https://nodejs.org/dist/v22.10.0/node-v22.10.0-x64.msi
Setting package description from README.md
Updating files
  $Latest data:
    Checksum32                (String)     DACA38D494C4D6D023EA0CC4D5F7974173256B80A5E485BF7FCAACE62C36DF85
    Checksum64                (String)     9D8FAD0DC2DA2C57E6FDF38FC85A23DC5EBCD5C414D8DD2948B3C45BD2398895
    ChecksumType32            (String)     sha256
    ChecksumType64            (String)     sha256
    FileName32                (String)     node-v22.10.0-x86.msi
    FileName64                (String)     node-v22.10.0-x64.msi
    FileType                  (String)     msi
    NuspecVersion             (String)     22.9.0
    PackageName               (String)     nodejs.install
    Stream                    (String)     22
    Streams                   (Hashtable)  System.Collections.Hashtable
    URL32                     (String)     https://nodejs.org/dist/v22.10.0/node-v22.10.0-x86.msi
    URL64                     (String)     https://nodejs.org/dist/v22.10.0/node-v22.10.0-x64.msi
    Version                   (String)     22.10.0
  nodejs.install.nuspec
    setting id: nodejs.install
    updating version: 22.9.0 -> 22.10.0
  .\legal\verification.txt
    (?i)(checksum64:\s+).*              = ${1}9D8FAD0DC2DA2C57E6FDF38FC85A23DC5EBCD5C414D8DD2948B3C45BD2398895
    (?i)(64-Bit.+)\<.*\>                = ${1}<https://nodejs.org/dist/v22.10.0/node-v22.10.0-x64.msi>
    (?i)(checksum32:\s+).*              = ${1}DACA38D494C4D6D023EA0CC4D5F7974173256B80A5E485BF7FCAACE62C36DF85
    (?i)(checksum type:\s+).*           = ${1}sha256
    (?i)(32-Bit.+)\<.*\>                = ${1}<https://nodejs.org/dist/v22.10.0/node-v22.10.0-x86.msi>
  .\tools\chocolateyInstall.ps1
    (?i)(^\s*SilentArgs\s*=\s*)'.*'     = ${1}'/quiet ADDLOCAL=ALL'
    (^[$]filePath32\s*=\s*"[$]toolsPath\\)(.*)" = $1node-v22.10.0-x86.msi"
    (^[$]filePath64\s*=\s*"[$]toolsPath\\)(.*)" = $1node-v22.10.0-x64.msi"

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

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).

… v23 onwards

The package needs to be updated to properly support 64-bit only packages, since 32-bit support has been dropped for NodeJS 23+.

Signed-off-by: Chad Wilson <chadw@thoughtworks.com>
@AppVeyorBot
Copy link

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

@chadlwilson
Copy link
Contributor Author

@AdmiringWorm @JPRuskin Is this better for now? NodeJS 22 has just transitioned to formal LTS with 22.11.0 so it'd be good to get these rolling again.

@michha
Copy link
Contributor

michha commented Nov 4, 2024

We need this hotfix to keep the elder (but still supported) node channels up to date. 🤕

@AdmiringWorm
Copy link
Member

Apologies for being silent on this, I have not had the time to work on this repository.

As we only prioritize the current version in this package, disabling the latest current version would not be acceptable.

I have however created #2567 that will supersede this PR to add a proper fix to the updater. Due to this, I will be closing this PR now.

@chadlwilson
Copy link
Contributor Author

Honestly, I can't understand your logic given you have multiple channels configured for the autoupdater including old versions. If you only care about current/latest you should remove tracking multiple channels?

Your practices seem entirely arbitrary, and honestly hostile to outsiders. It's your project to do what you like like, however one can only assume that you're not actually interested in community collaborators or the community side of OSS if your goal is to let 'perfect be the enemy of good' all the time. (You'd rather have left all versions broken than fix the non-latest ones,.you'd rather have all versions broken than to have misleading 32-bit support messages for latest)

@chadlwilson chadlwilson deleted the fix-node-below-23-autoupdates branch November 11, 2024 11:42
@pauby
Copy link
Member

pauby commented Nov 15, 2024

Honestly, I can't understand your logic given

The reason was given in the comment above:

As we only prioritize the current version in this package

Your practices seem entirely arbitrary, and honestly hostile to outsiders.

Why are they arbitrary and hostile?

however one can only assume that you're not actually interested in community collaborators or the community side of OSS if your goal is to let 'perfect be the enemy of good' all the time

I don't understand your reasoning here.

(You'd rather have left all versions broken than fix the non-latest ones,.you'd rather have all versions broken than to have misleading 32-bit support messages for latest)

There have been 3 versions pushed since your message was posted:

  • 23.2.0
  • 23.1.0
  • 22.11.0

So I'm confused by your comment.

@chadlwilson
Copy link
Contributor Author

(You'd rather have left all versions broken than fix the non-latest ones,.you'd rather have all versions broken than to have misleading 32-bit support messages for latest)

There have been 3 versions pushed since your message was posted:

* 23.2.0

* 23.1.0

* 22.11.0

So I'm confused by your comment.

Yes, because you're not following history/chronology correctly.

Those releases were made subsequent to this being rejected, since work was done separately by a Chocolatey team member to resolve the issues in the fashion preferred by the team.

This is after some weeks of rejecting my attempts to resolve the problem during which there were no updates coming through; after the good faith effort I spent to identify the issue (#2555 (comment), #2453 (comment) summarise the problem by raising #2556 and raising #2557 and this attempt to fix various aspects of it.

Why are they arbitrary and hostile?

Arbitrary because rejected due to "only prioritize the current version" despite the auto-updates tracking multiple prior branches (as I specifically explained as why I didn't understand the logic). If the project didn't care about prior major release channels, you wouldn't bother tracking them and publishing them and would only track the latest major/current version.

Hostile because there is no indication of any appreciation of any earlier work I'd done to identify the issue nor attempt to solve the problem in multiple different ways. PR summarily closed.

To me this is a pretty bad experience for someone trying on best efforts to contribute and meet your already elaborate guidelines. It's not pleasant to be made to feel like one is wasting their time when they have obviously put thought into something. It's not like I was a complete noob here, I fixed the Ruby auto-updates in #2393 which were broken for (I believe) nearly a year. But as I said, you can run your community how you like, and ignore feedback if you don't agree.

@pauby
Copy link
Member

pauby commented Nov 15, 2024

Yes, because you're not following history/chronology correctly.
Those releases were made subsequent to this being rejected, since work was done separately by a Chocolatey team member to resolve the issues in the fashion preferred by the team.

You said:

(You'd rather have left all versions broken than fix the non-latest ones

Unless my timezone fu is wrong (and it well could be!), PR #2567 was merged at 12:19pm UTC and the 22.11.0 package was pushed at 12:27pm UTC. 8 minutes later. That says to me that the PR change, kicked off the updates which pushed 22.11.0 (and 23.1.0 at the same time).

So that means the script updated that "non-current" one.

But, anyway, that's not essential.

This is after some weeks of rejecting my attempts to resolve the problem during which there were no updates coming through; after the good faith effort I spent to identify the issue (#2555 (comment), #2453 (comment) summarise the problem by raising #2556 and raising #2557 and this attempt to fix various aspects of it.
Arbitrary because rejected due to "only prioritize the current version"

Reading between the lines, it sounds like you are suggesting that the practices are "hostile" because your PRs were not merged?

We absolutely appreciate anybody who submits PR's, issues or tries to help in any way. But we cannot guarantee that PRs will be merged.

Just to be clear, this is what I see with what you raised:

despite the auto-updates tracking multiple prior branches (as I specifically explained as why I didn't understand the logic).

As has been explained, the current version is prioritised. That doesn't mean non-current versions are not added or supported. It means that when changes are being made, the current one is the one that is focused on. That is what happened here.

There was also an alternative provided if you wanted LTS, so there are options. I appreciate it doesn't fit what you need, but it is there. If it is not being released quickly enough for you, perhaps you can offer to help that maintainer do so.

If the project didn't care about prior major release channels, you wouldn't bother tracking them and publishing them and would only track the latest major/current version.

Nowhere did anybody say that they didn't "care" about non-current versions. Please read what has been written several times now.

Hostile because there is no indication of any appreciation of any earlier work I'd done to identify the issue nor attempt to solve the problem in multiple different ways. PR summarily closed.

As I outlined above:

I want to pick up each bit of this:

  • "no indication of appreciation": We cannot merge PR's that are detrimental to packages that are maintained. What would you have us do? How would we be able to show our appreciation?
  • "nor attempt to solve the problem in multiple different ways": one PR was rejected, and one PR was abandoned. The PR that was rejected was commented on before it was rejected.
  • "PR summarily closed.": I reject that. As I've said several times, one PR was rejected, with a comment, and the other you abandoned.

To me this is a pretty bad experience for someone trying on best efforts to contribute and meet your already elaborate guidelines.

That is clear in your posts that you are unhappy. I am genuinely sorry you are unhappy. AU is not an easy module, and nor is the way it works a simple thing to understand. It is daunting for me (I don't touch packages with "streams", for example) and I can imagine it being oven more daunting for others. That is why there are comprehensive guidelines and help from the team.

It's not pleasant to be made to feel like one is wasting their time when they have obviously put thought into something.

Surely, you are not suggesting every pull request submitted is merged, so people feel appreciated and not wasting their time? So, genuine question, what would you have us do?

It's not like I was a complete noob here, I fixed the Ruby auto-updates in #2393 which were broken for (I believe) nearly a year.

That makes it ever more surprising that you do not understand that every repository follows different processes and guidelines that all contributors are respectfully asked to follow. The life of an open-source maintainer, and contributor, is a thankless one.

@AdmiringWorm fixed the package that you raised the issue for but instead of thanking them, or learning from the PR that was raised and merged, you accused him of being "hostile to outsiders", "not interested in community collaborators or the community side of OSS", working with "arbitrary processes". Do you think this is an acceptable way to deal with somebody who has been a repository maintainer here for years? Somebody who gives up their own time to do this? A repository that has over 350 well-maintained packages and 1B package downloads.

We absolutely do want community involvement. And we know how hard it is to get that. But we want the right kind of contributor. Somebody who will work with the team. Somebody who will respectfully deal with the team, respect the processes in the repository (however much they disagree with them) and work with us.

I absolutely understand you are annoyed, frustrated and upset. But you have not shown yourself in the best light here, and you absolutely have not earned the respect of the team in your dealings with them, nor respect for that they do, thanklessly.

We have both had our say here, so I'm going to go ahead and lock this. I hope you take on board what happened here, and I have taken your remarks about how things are laid out for new contributors. I'm genuinely unsure if anything can be done, but it won't be ignored or forgotten about.

@chocolatey-community chocolatey-community locked as off-topic and limited conversation to collaborators Nov 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants