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: v5.0.4 #1893

Closed
wants to merge 21 commits into from
Closed

Release proposal: v5.0.4 #1893

wants to merge 21 commits into from

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Sep 26, 2019

This is the same as the #1892 but without the lone apparent semver-major: #1818. See thread in #1791 (comment) for more discussion on this approach.

Will take advice on what else this needs, I don't have any strong opinions other than "let's get this stuff out".


  • [f753c167c5] - gyp: decode stdout on Python 3 (cclauss) #1890
  • [60a4083523] - doc: update xcode install instructions to match Node's BUILDING (Nhan Khong) #1884
  • [19dbc9ac32] - deps: update tar to 4.4.12 (Matheus Marchini) #1889
  • [5f3ed92181] - bin: fix the usage instructions (Halit Ogunc) #1888
  • [aab118edf1] - lib: adding keep-alive header to download requests (Milad Farazmand) #1863
  • [1186e89326] - lib: ignore non-critical os.userInfo() failures (Rod Vagg) #1835
  • [785e527c3d] - doc: fix missing argument for setting python path (lagorsse) #1802
  • [a97615196c] - gyp: rm semicolons (Python != JavaScript) (MattIPv4) #1858
  • [06019bac24] - gyp: assorted typo fixes (XhmikosR) #1853
  • [3f4972c1ca] - gyp: use "is" when comparing to None (Vladyslav Burzakovskyy) #1860
  • [1cb4708073] - src,win: improve unmanaged handling (Peter Sabath) #1852
  • [5553cd910e] - gyp: improve Windows+Cygwin compatibility (Jose Quijada) #1817
  • [8bcb1fbb43] - gyp: Python 3 Windows fixes (João Reis) #1843
  • [2e24d0a326] - test: accept Python 3 in test-find-python.js (João Reis) #1843
  • [1267b4dc1c] - build: add test run Python 3.7 on macOS (Christian Clauss) #1843
  • [da1b031aa3] - build: import StringIO on Python 2 and Python 3 (Christian Clauss) #1836
  • [fa0ed4aa42] - build: more Python 3 compat, replace compile with ast (cclauss) #1820
  • [18d5c7c9d0] - win,src: update win_delay_load_hook.cc to work with /clr (Ivan Petrovic) #1819

ipetrovic11 and others added 18 commits September 26, 2019 13:39
Added "#pragma unmanaged" in win_delay_load_hook.cc to support clr

PR-URL: #1819
Reviewed-By: João Reis <reis@janeasystems.com>
Make Python 3 compatiblity changes so the code works in both Python 2
and Python 3.  Especially, make changes required because the compiler
module was removed in Python 3 in favor of the ast module that exists
in both Python 2 and Python 3.

PR-URL: #1820
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Rod Vagg <r@va.gg>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
This change supports access to StringIO on both Python 2 and Python 3

PR-URL: #1836
Reviewed-By: Rod Vagg <r@va.gg>
Reviewed-By: João Reis <reis@janeasystems.com>
Refs: #1846
PR-URL: #1843
Reviewed-By: João Reis <reis@janeasystems.com>
Fixes: #1826
PR-URL: #1843
Reviewed-By: Christian Clauss <cclauss@me.com>
Reviewed-By: Rod Vagg <r@va.gg>
Reviewed-By: Matt Cowley <me@mattcowley.co.uk>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
PR-URL: #1843
Reviewed-By: Christian Clauss <cclauss@me.com>
Reviewed-By: Rod Vagg <r@va.gg>
Reviewed-By: Matt Cowley <me@mattcowley.co.uk>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Fixes: #1782
PR-URL: #1817
Reviewed-By: Christian Clauss <cclauss@me.com>
Reviewed-By: Rod Vagg <r@va.gg>
Used scoped disabling of managed code handling to ensure no other
files get affected.

PR-URL: #1852
Reviewed-By: João Reis <reis@janeasystems.com>
PR-URL: #1860
Reviewed-By: Christian Clauss <cclauss@me.com>
Reviewed-By: Rod Vagg <r@va.gg>
PR-URL: #1853
Reviewed-By: Christian Clauss <cclauss@me.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: #1858
Reviewed-By: Christian Clauss <cclauss@me.com>
Reviewed-By: Rod Vagg <r@va.gg>
(accepted with minor modifications by rvagg)

PR-URL: #1802
Reviewed-By: Rod Vagg <rod@vagg.org>
PR-URL: #1863
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
PR-URL: #1888
Reviewed-By: Rod Vagg <rod@vagg.org>
Fixes: #1887
PR-URL: #1889
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
PR-URL: #1890
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
@rvagg
Copy link
Member Author

rvagg commented Sep 26, 2019

This PR isn't meant to be merged into master, it will establish a v5.x branch that we'll maintain like we do in nodejs/node for as long as we need a v5.x release line to live (as yet undetermined).

Copy link
Contributor

@cclauss cclauss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few suggestions... Implemented on #1894

@@ -44,6 +45,12 @@ matrix:
python: 3.7
env: NODE_GYP_FORCE_PYTHON=python3 EXPERIMENTAL_NODE_GYP_PYTHON3=1
before_install: nvm install 12
- name: "Python 3.7 on macOS"
os: osx
#osx_image: xcode11
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This # should be removed so that we use Py37 instead of Py36.

stdout=subprocess.PIPE,
stderr=subprocess.STDOUT)
stdout,stderr = out.communicate()
return "CYGWIN" in str(stdout)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Insert the if PY3: stdout = stdout.decode("utf-8") code from #1890.

stdout=subprocess.PIPE,
stderr=subprocess.STDOUT)
stdout,stderr = out.communicate()
return str(stdout)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Insert the if PY3: stdout = stdout.decode("utf-8") code from #1890.

stdout=subprocess.PIPE,
stderr=subprocess.STDOUT)
stdout,stderr = out.communicate()
return "CYGWIN" in str(stdout)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Insert the if PY3: stdout = stdout.decode("utf-8") code from #1890.

@rvagg
Copy link
Member Author

rvagg commented Sep 26, 2019

hm, I'm not sure why this isn't showing properly, I've only removed that one commit which makes this the only meaningful diff when doing a git diff v5.x master locally:

diff --git a/lib/find-python.js b/lib/find-python.js
index 30bb25f..1a4390d 100644
--- a/lib/find-python.js
+++ b/lib/find-python.js
@@ -19,7 +19,7 @@ PythonFinder.prototype = {
   argsExecutable: [ '-c', 'import sys; print(sys.executable);' ],
   argsVersion: [ '-c', 'import sys; print("%s.%s.%s" % sys.version_info[:3]);' ],
   semverRange: process.env.EXPERIMENTAL_NODE_GYP_PYTHON3 ? '2.7.x || >=3.5.0'
-    : '>=2.7.0 <3.0.0',
+    : '>=2.6.0 <3.0.0',

   // These can be overridden for testing:
   execFile: cp.execFile,

The "files changed" isn't correct, at least according to my diff. I suppose it's because this PR is adding one HEAD commit (changelog and version bump) but removing a commit way back in history, and for some reason it's getting confused?

@cclauss the changes you're commenting on are on master so maybe they need a PR to address them?

@cclauss cclauss mentioned this pull request Sep 26, 2019
4 tasks
@sam-github
Copy link
Contributor

@rvagg This LGTM. @cclauss's comments are addressed in #1894 -- could you land that, and then cherry-pick it into 5.x? That will fix the known py3 issues (and as for unknown issues, we don't know :-).

PR-URL: #1894
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
@rvagg
Copy link
Member Author

rvagg commented Sep 27, 2019

done, I suppose I'll push this out then!

@rvagg
Copy link
Member Author

rvagg commented Sep 27, 2019

Included #1895 too and published as v5.0.4. Closing this PR but this branch, v5.x, will live on as our branch to maintain the v5.x series while we also move on to 6.0.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Node Sass --> Dart Sass https://github.com/sass/node-sass/issues/2952
Projects
None yet
Development

Successfully merging this pull request may close these issues.