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

Node.js 12 SemVer Major Cut-off #26844

Closed
BethGriggs opened this issue Mar 21, 2019 · 33 comments
Closed

Node.js 12 SemVer Major Cut-off #26844

BethGriggs opened this issue Mar 21, 2019 · 33 comments

Comments

@BethGriggs
Copy link
Member

/cc @nodejs/tsc @nodejs/release

Refs: nodejs/Release#417

PSA: SemVer Major Cut-off for Node.js 12 is 23rd March. After this time landing SemVer Major changes in v12.x will require no objections from the TSC.

(This issue will be closed after 23rd March - it was opened as an easy way to publicise.)

@Trott
Copy link
Member

Trott commented Mar 21, 2019

After this time landing SemVer Major changes in v12.x will require no objections from the TSC.

Feel free to ignore this question as a low-value can of worms that you would like to not open, but if you're into worms like I am:

Should that be the Release WG rather than TSC? Or is Release WG saying "TSC approval is the process for post-cutoff semver majors in this release"? Or something else?

I'm asking because, if I'm not mistaken, TSC delegated release contents to the Release WG in the Release WG charter. Only way TSC can have a say is if they revoke the charter or if the Release WG grants it to them.

@mcollina
Copy link
Member

If I recall correctly, lwe have landed semver-major commits with TSC approval in this phase for the last few releases.

cc @jasnell

@jasnell
Copy link
Member

jasnell commented Mar 21, 2019

@mcollina and @BethGriggs are correct, after the cut off it's always been semver-major only with no objections from TSC. The release wg can certainly weigh in on the discussion, as can anyone else.

@targos ... Where are things at with regard to v8 updates and necessary abi stability patches?

@targos
Copy link
Member

targos commented Mar 21, 2019

@jasnell
The V8 version that we would like to have in Node.js 12 is 7.4.
PR: #26685
The PR includes what's needed to make it ABI compatible with 7.5 (@addaleax can you confirm?).
We have issues building on macOS (only in CI apparently, I think @ryzokuken could build locally) and AIX.

@addaleax
Copy link
Member

The PR includes what's needed to make it ABI compatible with 7.5 (@addaleax can you confirm?).

It provides ABI compatible with V8 lkgr at the time of those patches – that goes further than V8 7.5, and we would need to re-float some of the patches after upgrading to V8 7.5 to keep ABI compatibility, with the goal of possibly enabling another V8 bump. If we decide to stick with V8 7.5, we can probably omit a number of patches.

@richardlau
Copy link
Member

The plan was to stop at V8 7.6: #25082

@Trott
Copy link
Member

Trott commented Mar 21, 2019

@mcollina and @BethGriggs are correct, after the cut off it's always been semver-major only with no objections from TSC. The release wg can certainly weigh in on the discussion, as can anyone else.

Can of worms opened!

This practice, however long-standing, contradicts the Release WG charter which says the Release WG, not the TSC, has final authority over what goes in a release. (The TSC determines what goes in the master branch.)

As it says in the TSC repo material about working groups:

Once formed the work defined in the Working Group charter is the responsibility of the WG rather than the TSC.

So, TSC does not have authority over what ends up in a release. Release WG does. Which is why I asked:

Or is Release WG saying "TSC approval is the process for post-cutoff semver majors in this release"?

That's probably the easiest solution to aligning what's actually happening with the designation of responsibilities in the WG charter.

@ofrobots
Copy link
Contributor

ofrobots commented Mar 22, 2019 via email

@refack
Copy link
Contributor

refack commented Mar 22, 2019

We have issues building on macOS (only in CI apparently, I think @ryzokuken could build locally) and AIX.

Seems like it builds with XCode9 on macOS 10.12
https://ci.nodejs.org/job/temp-node-test-commit-osx/1/nodes=osx1012/ (just 1 failing test)

@Trott
Copy link
Member

Trott commented Mar 22, 2019

This discussion is in fact about what goes into the master branch.

@ofrobots Maybe I'm misunderstanding you, but it seems to me that it's pretty definitely about what goes in the release branch, not master:

...landing SemVer Major changes in v12.x will require no objections from the TSC.

That says that after the cut-off date, TSC can block semver majors from landing in the release branch, even though they've landed in master. But only the Release WG can decide what does and doesn't land in 12.x. (If they're voluntarily deferring to the TSC, cool. But there's no need for that and it would have to be something the Release WG as a whole decided.)

@Trott
Copy link
Member

Trott commented Mar 22, 2019

I'll stop now before I burn bridges that can't be rebuilt. I'm sure at least some people are beyond exasperated with me at this point on this topic.

This all does bring up one important issues for consideration elsewhere: The project as a whole often engages in practices that diverge from documented policies. We should not do that. Policies and practices should be updated to align.

And now I'll do this...

homer

@gireeshpunathil
Copy link
Member

I agree with @Trott :

  • documented policies MUST be followed
  • if policies are found to be stale / needing update, we can amend those, but through formal process

So in this context, it should be release WG's discretion to define the content, between the cutoff date and the actual release - I think this discretion should also be a major role of that working group .

However, if the matter at hand has implications that cannot be evaluated fully within the release WG's capacity, they can always ask for advise from TSC? (but should be fully at their discretion)

@richardlau
Copy link
Member

This is the first major release of Node.js post-convergence that isn't being done by James. As such we are making an effort to document the steps so it can be repeated, and this is currently being PR'ed over in #25497 and would probably be a better place for discussing given that in its current state it is proposing:

One month or less before the release date, commits must be cherry-picked in to
the two branches. To land SEMVER-MAJOR at this time requires no objections
from the TSC.

@Trott
Copy link
Member

Trott commented Mar 22, 2019

On a totally different note: OMG, thanks for all the hard Release work! The activities of Release WG (and Build WG for that matter) can't be appreciated enough and it's not my intention to make things more work than they already are. (But since that's the likely short-term effect, it's another reason for me to stop already.)

@richardlau richardlau pinned this issue Mar 22, 2019
@jasnell
Copy link
Member

jasnell commented Mar 22, 2019

Fwiw, the fact that release wg has never raised an issue on this in 8 major releases where we've followed this approach is relevant. I think there's generally consensus that asking the TSC for objections is an ok thing to do. And, to be honest, doing so doesn't violate any existing documented policies given that it's members of the release wg that are asking the TSC for any objections.

@ofrobots
Copy link
Contributor

v12.x has already been created? (* checks date, gasps *).

I've always interpreted things to be that the TSC defines the semver major content for a new major, and that the Release WG takes ownership of the contents from thereon. I'm happy to stand corrected if the documented policies are difference.

@vitaly-t
Copy link
Contributor

vitaly-t commented Mar 26, 2019

Are we going to see V8 7.2 within Node 11 any time soon? Native support for Private Properties is going to change the landscape for the JavaScript developers. Can't wait!!!

@richardlau
Copy link
Member

Are we going to see V8 7.2 within Node 11 any time soon? Native support for Private Properties is going to change the landscape for the JavaScript developers. Can't wait!!!

There are no plans to update the version of V8 in Node.js 11. V8 version increments are usually ABI incompatible with previous versions without floating patches on them and we normally only do that for the even numbered releases that will eventually become Long Term Support (LTS) releases. So to get a newer version of V8 than is currently in Node.js 11 you'll need to pick up Node.js 12 when it is released in April.

Re. V8 7.2 we didn't even land it in master (we skipped straight from 7.1 to 7.3, see #24875 and #25852).

@BethGriggs
Copy link
Member Author

@nodejs/tsc - could you review the following Semver Majors and let me know if there are any objections to any of them landing? Please 👍 if you have seen the list and have no objections.

Semver-Major's that have landed since 23rd March:

  • crypto: decode missing passphrase errors #25208
  • dns: remove dns.promises experimental warning #26592
  • deps: update V8 to 7.4 #26685
  • util: improve callbackify #26893
  • modules: throw an error for invalid package.json main entries #26823
  • Bootstrap: make global.process, global.Buffer getters #26882
  • repl: fix terminal default setting and improve color check #26518
  • net: do not manipulate potential user code #26751

@rvagg
Copy link
Member

rvagg commented Apr 2, 2019

I can't give a 👍 to "modules: throw an error for invalid package.json main entries #26823", it looks risky, but I also don't have time right now to dig into details so I can only abstain for now. 👍 to the rest though. Thanks for the cat herding @BethGriggs.

@BethGriggs
Copy link
Member Author

BethGriggs commented Apr 4, 2019

Has someone turned on protection on the v12.x branch?

@devsnek
Copy link
Member

devsnek commented Apr 4, 2019

15c0947 got a lot of tsc signoff but isn't listed here, is it possible for it to be in 12.x?

@mhdawson
Copy link
Member

mhdawson commented Apr 4, 2019

I'm in agreement with @rvagg they all look good to me except #26823 which could use more time in master. Is there any rush/need to get that into 12.x?

@BridgeAR
Copy link
Member

BridgeAR commented Apr 4, 2019

There is no rush but I would like to include it in v12. Is there any benefit in having it on master longer? I can't think of any way that this would be beneficial for this case as it's more like a bug fix for quite a rare case (it's not about trying out a new feature or something like that).

@targos
Copy link
Member

targos commented Apr 4, 2019

@BethGriggs

Has someone turned on protection on the v12.x branch?

It is turned on for the wild card v*.x.

@BethGriggs
Copy link
Member Author

Ah! - that makes sense. I was thinking it'd be nice to pull these majors over in the same order in which they landed on master. I can cherry-pick them on top - is that what used to happen @jasnell?

@sam-github
Copy link
Contributor

Maybe there shouldn't be a v12.x until its actually released? just -rc branches cut from v12.x-staging?

@jasnell
Copy link
Member

jasnell commented Apr 4, 2019

@BethGriggs.. yeah, prior to the actual release, after the semver-major cutoff, I would start cherry-picking into the vN.x-staging branch in the order they landed as much as possible. There were times when the order couldn't be perfectly preserved. Then I would sync the vN.x branch with the vN.x-staging then rebase the vN.0.0-proposal branch on the vN.x. Cut your test builds and release candidates from the vN.0.0-proposal branch as you. When you're ready to cut the release, you simply reverse the flow... merge vN0.0.0-proposal into vN.x, then rebase vN.x-staging on vN.x, and you're good to go. The flow is fairly straightforward.

@BethGriggs
Copy link
Member Author

Another round of majors that have landed on master:

  • [eb8a51a35c] - (SEMVER-MAJOR) child_process: use non-infinite maxBuffer defaults (kohta ito) #23027
  • [2f1ed5c063] - (SEMVER-MAJOR) crypto: remove legacy native handles (Tobias Nießen) #27011
  • [bd9109c241] - (SEMVER-MAJOR) lib: move DEP0021 to end of life (cjihrig) #27127
  • [15c0947fee] - (SEMVER-MAJOR) lib: remove Atomics.wake (Gus Caplan) #27033
  • [d11c4beb4b] - (SEMVER-MAJOR) module: remove dead code (Ruben Bridgewater) #26983
  • [75007d64c0] - (SEMVER-MAJOR) module: mark DEP0019 as End-of-Life (Ruben Bridgewater) #26973
  • [bf766c1b44] - (SEMVER-MAJOR) src: remove unused INT_MAX constant (Sam Roberts) #27078
  • [c9fece38c8] - (SEMVER-MAJOR) util: change inspect compact and breakLength default (Ruben Bridgewater) #27109
  • [892c51f330] - (SEMVER-MAJOR) util: improve inspect edge cases (Ruben Bridgewater) #27109

@nodejs/tsc please let me know if you have objections to any of these landing (or add a 👍so we know that you've seen the list). I expect to pull in the final set next week.

@mhdawson
Copy link
Member

On #23027 I wonder if we should wait for #27179 which is related and a follow on due to concerns over the smaller size introduced in #23027. @mcollina what do you think?

I'm ok with the rest.

@mcollina
Copy link
Member

Maybe it’s better to wait, true.

@MylesBorins
Copy link
Contributor

MylesBorins commented Apr 12, 2019 via email

@tniessen
Copy link
Member

Node.js 12 has been released. Closing and unpinning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests