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

Release proposal: v6.0.0 #1892

Closed
wants to merge 1 commit into from
Closed

Release proposal: v6.0.0 #1892

wants to merge 1 commit into from

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Sep 26, 2019

Just a proposal at this stage, I expect there's more to go into this, but this is what it might look like:

Due out on Oct 4th, UTC.

  • [dd0e97ef0b] - (SEMVER-MAJOR) lib: try to find python after python3 (Sam Roberts) #1907
  • [f60ed47d14] - travis: add Python 3.5 and 3.6 tests on Linux (Christian Clauss) #1903
  • [c763ca1838] - (SEMVER-MAJOR) doc: Declare that node-gyp is Python 3 compatible (cclauss) #1811
  • [3d1c60ab81] - (SEMVER-MAJOR) lib: accept Python 3 by default (João Reis) #1844
  • [c6e3b65a23] - (SEMVER-MAJOR) lib: raise the minimum Python version from 2.6 to 2.7 (cclauss) #1818

(EDIT: pulled in v5.0.4 as a foundation and removed the commits that are common on both)
(EDIT: pulled in the v5.0.5 proposal from #1905 as a foundation and removed the commits that are common on both, branch-diff rvagg/v5.0.5-proposal rvagg/v6.0.0-proposal)

@cclauss
Copy link
Contributor

cclauss commented Sep 26, 2019

#1843 is in the list twice. Is that intentional?

#1844 is not in the list. Will that need to wait until the next semver-major?

@rvagg
Copy link
Member Author

rvagg commented Sep 27, 2019

#1843 is in the list 3 times actually, it got landed as 3 separate commits so this is how it should show up in the changelog. 👌

#1844 isn't in the list because it hasn't landed. I don't follow enough of what's in there to jump in and land it myself. Happy for @joaocgreis to land it if he feels that you have both reached consensus on it, then it can be included here.

@joaocgreis
Copy link
Member

#1844 has landed. @rvagg can you include it here? Thanks!

@cclauss
Copy link
Contributor

cclauss commented Sep 30, 2019

We need #1811 in here too.

@cclauss cclauss added the Python label Sep 30, 2019
@joaocgreis
Copy link
Member

It would be great if #1875 could make it into the release as well. Thanks!

cclauss added a commit that referenced this pull request Oct 1, 2019
@rvagg This would be good to land before #1892

The current code would raise NameError at runtime.
@cclauss cclauss mentioned this pull request Oct 1, 2019
4 tasks
@cclauss
Copy link
Contributor

cclauss commented Oct 2, 2019

What else needs to land here? On the Python side, I see nothing that needs to be in 6.0.0.
https://github.com/nodejs/node-gyp/labels/python

@rvagg
Copy link
Member Author

rvagg commented Oct 2, 2019

I've pulled in the v5.0.4 release commit to master so the CHANGELOG has it in there and rebased this branch on master. Running branch-diff rather than changelog-maker because there's one commit that's mid-history rather than on top of the stack, the new changelog is in OP, only 5 commits, 2 of which will go into the 5.x branch (and if a 5.x release happens before this one then they'll be removed from here because this release should sit on top of whatever is done on the 5.x just before we start a 6.x release line).

If the Python items are done, let's get in #1902 (VS Express) and #1899 (standard@14) (ed: done) both here and the 5.x branch and push out a 6.0.0 release eh?

What's going to be the recommendation for npm on the jump to 6.0.0 (and nodejs/node following npm)? I don't really understand that piece. /cc @sam-github

@rvagg rvagg force-pushed the rvagg/v6.0.0-proposal branch 2 times, most recently from 51f69c5 to f4de705 Compare October 2, 2019 03:55
rvagg added a commit that referenced this pull request Oct 2, 2019
@cclauss
Copy link
Contributor

cclauss commented Oct 2, 2019

What's going to be the recommendation for npm on the jump to 6.0.0 (and nodejs/node following npm)? I don't really understand that piece. /cc @sam-github

npm/cli#260 should land our current node-gyp v5.0.4 in npm/cli in the coming days. Once node-gyp v6.0.0 has released, we can ask @isaacs to repeat that process with v6.0.0. Each time that those PRs land, we (Sam ;-) ) will refresh nodejs/node /deps/npm/node_modules/node-gyp from the current npm/cli. We will also rebase nodejs/node#29451 until it is only contains changes to .travis.yml.

@rvagg
Copy link
Member Author

rvagg commented Oct 2, 2019

npm/cli@dcff367

npm 6.12 has been pushed back to the 8th by the look of it.

So there's a choice here, do we roll ahead with a node-gyp@6 and see if we can get that into npm 6.12? Or wait until 6.12 so there's distinct releases of npm available with different python functionality?

@cclauss
Copy link
Contributor

cclauss commented Oct 2, 2019

For me the Python delta between node-gyp v5.0.4 and v6.0.0 is small so it would be better to use the delay to get npm/cli to jump directly to v6.0.0. Perhaps @sam-github and @isaacs have a different impression.

In any case, nodejs/node is dependent on npm/cli but nodejs/node-gyp is not so the node-gyp v6.0.0 release (this PR) is not blocked by npm/cli@dcff367

@rvagg
Copy link
Member Author

rvagg commented Oct 2, 2019

@nodejs/node-gyp anything else you want to get into a 6.0.0 before we push it out? May as well do it in the next day or two.

@sam-github
Copy link
Contributor

Its not clear to me whether 6.x merely allows py3 to be chosen, or whether it prefers py3: #1844 (comment)

If it merely allows, then I think npm should update to it, it won't break anything that wasn't already broken.

If it prefers py3, that's a big change. I guess we'll find out how good the test coverage is, and if it turns out node-gyp doesn't have good py3 support, npm can roll back to node-gyp@5, or we can fix and update node-gyp@6. Most of the problems seen seem to be easy to fix.

@joaocgreis
Copy link
Member

joaocgreis commented Oct 2, 2019

We should be very careful pushing v6 out, I don't think our test coverage is good enough to just trust it.

FWIW, CitGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/2024/ https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/2025/ (to compare with https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/2019/)

@sam-github it prefers Python 3, the logic going forward is to use the latest. It looks for python, then python3, then python2 and uses the first one it finds.

We could easily change the order for a few releases. But I'm not sure it makes sense, the users who would get the errors would be the ones without Python 2 installed, more likely to be new users clueless about what's going on.

@sam-github
Copy link
Contributor

sam-github commented Oct 2, 2019

it prefers Python 3, the logic going forward is to use the latest. It looks for python, then python3

If this is the case, it prefers Python 2, because python is almost universally going to be Python 2 for backwards compatibility.

If the intention is to prefer Python 3, then the search order should be python3, python, python2.

This latter order would align better with official Python recommendations (@cclauss link?) which are to always use either python3 (if that's what is wanted) or python2 -- but still allow fallbacks for systems which are missing Python 3 or unadvisedly installed it as python. EDIT: "unadvisedly" should be "optionally", but recommendation remains the same.

@cclauss
Copy link
Contributor

cclauss commented Oct 2, 2019

Python Enhancement Proposal (PEP) 394:
https://www.python.org/dev/peps/pep-0394/#for-python-runtime-distributors

On Windows... https://devblogs.microsoft.com/python/python-in-the-windows-10-may-2019-update/

On macOS... https://developer.apple.com/documentation/macos_release_notes

Use of Python 2.7 isn’t recommended as this version is included in macOS for compatibility with legacy software. Future versions of macOS won’t include Python 2.7. Instead, it’s recommended that you run python3 from within Terminal.

@joaocgreis
Copy link
Member

#1904 needs to be included here as well, there are known failures without it.

@rvagg
Copy link
Member Author

rvagg commented Oct 3, 2019

OK, to be clear, we're good to go with v6? @sam-github your reservation is settled by knowing python is first in the search list? So we're not technically preferring Python 3 on many platforms where that resolves to Python 2.
I might push out another small v5 release to flush out these non-semver-major commits before we so a 6. I'll put up a proposal shortly and we can do one after the other.

rvagg added a commit that referenced this pull request Oct 3, 2019
@rvagg
Copy link
Member Author

rvagg commented Oct 3, 2019

Merged the outstanding PRs in master, rebased this. I have a proposal up for a v5.0.5 in #1905, it includes everything that's not labelled semver-major and also excludes #1903 (see note there about that). I've updated the changelog which is now just semver-major commits and #1903 and I've dated this one the same as v5.0.5. The plan is to push out a v5.0.5 tomorrow and follow straight up with a v6.0.0 (after pulling in the v5.0.5 changelog entry into this branch).

@sam-github
Copy link
Contributor

@rvagg I don't have really strong feelings, we can take small steps or large steps, but at this point @cclauss and I've talked ourselves into thinking that we should take only two steps, "allowing py 3" (5.x) and "preferring py 3" (6.x), see nodejs/node#25789

In which case, my concern is the opposite of what you said: that by having python earlier in the preferences 6.x doesn't go far enough yet -- it should change to have a path list of python3, python, and then python2.

If instead we go with 6.x as it is now, then we'll have to follow up later with a 7.x that changes the search order. I've no big objections to that, but I'm thinking its too many "small semver-major" changes.

Sorry, maybe you want a stronger opinion, but that's what I've got.

@rvagg
Copy link
Member Author

rvagg commented Oct 3, 2019

I have even less strong feelings about that @sam-github! I'd be fine either way with finding python3 or python first. I don't see any proposals to do that so maybe that's not what @cclauss or @joaocgreis prefer? I defer to them and I'll just ship what I'm given.
Will hold off on this until we have something that (a) looks more like consensus and (b) have the code changes if any are required. In the meantime I may as well ship the 5.0.5 tomorrow anyway.

rvagg added a commit that referenced this pull request Oct 3, 2019
@sam-github
Copy link
Contributor

👍 for the 5.x ship, and I will PR the rearrangement of search order for master/6.x

@joaocgreis
Copy link
Member

I also don't have strong feelings. I'd like this to go as smoothly as possible to the users, but I'm not sure how to accomplish that.

For v5.x, I think we should keep allowing only Python 2 to have a safe version to come back to if needed, otherwise I don't see much of a need to maintain a v5.x branch.

For v6.x, I'm ok with both search orders. I don't think we'll need to change the order quickly in a v7.x, because we'll keep testing on Python 2 for a while.

@sam-github
Copy link
Contributor

@joaocgreis

For v5.x, I think we should keep allowing only Python 2 to have a safe version to come back to if needed, otherwise I don't see much of a need to maintain a v5.x branch.

5.x must allow Python 3, otherwise the npm CLI won't allow Python 3, and people (us) are blocked from trying Python 3 out. I don't think its unsafe, becaue it will still try to find Python 2 first. That means anyone with an existing, working, Python 2 setup should see no changes.

Anyone who has only Python 3 would previously have failed to get a build, but with the latest 5.x, they will now get a build.

For v6.x, I'm ok with both search orders. I don't think we'll need to change the order quickly in a v7.x, because we'll keep testing on Python 2 for a while.

If #1907 lands and is in 6.0.0 then I'm not aware of any further (7.x) semver-majors that will be required, at least, required because of Python!

@sam-github
Copy link
Contributor

@joaocgreis Sorry, I just looked more closely at find-python in the 5.x branch... it still requires EXPERIMENTAL_NODE_GYP_PYTHON3 to consider Python 3 to be compatible. I thought that'd been removed already!

I think that's OK for the next couple weeks, but I would like to remove that ASAP from 5.x.

@sam-github sam-github mentioned this pull request Oct 3, 2019
2 tasks
@joaocgreis
Copy link
Member

@sam-github I'm ok with that. If I understand correctly, it's landing #1844 on v5.x changing the order to look for python2 then python then python3. In this case, it also probably makes sense to pick some of the other semver major changes (update the README?).

@sam-github
Copy link
Contributor

@joaocgreis What specific semver-major changes would you suggest on 5.x?

I don't think adding Python 3 to the end of the search path is semver-major, so it should be good for 5.x. This is it, #1844 cherry picked to the v5.x branch, with the search order put back to Py3-after-Py2: #1910

The README change is here: #1908 (comment) and I agree it should go on 5.x.

@rvagg ^---- and sorry, this is a 5.x discussion in a 6.x release proposal :-(

@joaocgreis
Copy link
Member

@sam-github looking at it, it's only #1811, but it's not essential. No strong opinion about it.

rvagg pushed a commit that referenced this pull request Oct 4, 2019
Unadorned `python` can be either Python 2 or Python 3, but it is likely
to be Python 2 for quite a while.

To find Python3, it is recommended to use the explicit name `python3`.

See:
- https://www.python.org/dev/peps/pep-0394/#for-python-runtime-distributors
- #1892 (comment)

PR-URL: #1907
Reviewed-By: Christian Clauss <cclauss@me.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: João Reis <reis@janeasystems.com>
@rvagg
Copy link
Member Author

rvagg commented Oct 4, 2019

waiting for CI, if it doesn't take too long I'll push this out after 5.0.5, otherwise I'm going to have to go to bed and do this in the morning

@rvagg rvagg closed this Oct 4, 2019
@rvagg rvagg deleted the rvagg/v6.0.0-proposal branch October 4, 2019 13:42
rvagg added a commit that referenced this pull request Oct 4, 2019
@rvagg
Copy link
Member Author

rvagg commented Oct 4, 2019

6.0.0 is out, good job all!

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

Successfully merging this pull request may close these issues.

4 participants