problems with llhttp 9.x upgrades in v20.x

[UlisesGascon]

I created this issue for visibility, based on team slack discussion

When I was working on the release 20.11.0 proposal (step 10) I was checking the dependencies and I got an error with the llhttp version.

I got the version 8.1.3, but the expected is 9.1.3 based on nodejs/node#50080 as was included in the proposal.

The thing is that version 8.1.3 does not exist at all, as the current version is 8.1.0 since [email protected]. So, the issue is that LLHTTP_VERSION_MAJOR didn't got updated when cherry picking this commit, only the LLHTTP_VERSION_PATCH.

I added the label dont-land-on-v20 for now. But this error will occur again when landing new deps upgrades for llhttp in [email protected]. So maybe we can work on a backport for the next release

20.11.0 Proposal dependencies

20.0.0 dependencies

[targos]

I wonder how nodejs/node#50080 could be cherry-picked cleanly to v20.x-staging.

This change at least should have generated a conflict:

[RafaelGSS]

cc: @ShogunPanda

We should flag all v9.x updates as dont-land-on-v20.x and dont-land-on-v18.x. There's just one update for v8.x (v8.1.1) and we didn't receive an automatic update for that.

@UlisesGascon I think just dropping the commit from v20.x-staging and rebasing again should be enough.

[UlisesGascon]

I wonder how nodejs/node#50080 could be cherry-picked cleanly to v20.x-staging.

I was wondering the same, but now that I tried to cherry pick it again in staging.. I realised that I introduced the regression when I was solving the conflict in the llhttp.h (before the holidays). Seems like I forgot to manually patch LLHTTP_VERSION_MAJOR value to match the current release version.

@UlisesGascon I think just dropping the commit from v20.x-staging and rebasing again should be enough.

Yes, I dropped the commit and the versions in the binaries are as expected. So, as soon as the CI/CITGM is clear I will release the proposal.

Next steps

I will work in a backport for this, so it can be landed easier in the next release 👍

[RafaelGSS]

I will work in a backport for this, so it can be landed easier in the next release 👍

There's no need for a backport for that. It's a change that should exist only on v21.x and main since it's semver-major

[ShogunPanda]

I agree. llhttp 9.1.2 was a breaking change which should not belong to v20.x.
Can you please clarify how 8.1.3 was created? Is any of the commit broken or what?

[UlisesGascon]

Can you please clarify how 8.1.3 was created? Is any of the commit broken or what?

Seems like the issue was due a manual regression that I introduced (see). As the [email protected] was included in [email protected] when I was working in the release I didn't update the major version reference value to 9 when solving the commit conflict, so the llhttp library version was wrong reflected as 8.1.3 instate of 9.1.3 as expected.