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: v1.7.0 #1396

Closed
rvagg opened this issue Apr 11, 2015 · 42 comments
Closed

Release proposal: v1.7.0 #1396

rvagg opened this issue Apr 11, 2015 · 42 comments
Labels
meta Issues and PRs related to the general management of the project.

Comments

@rvagg
Copy link
Member

rvagg commented Apr 11, 2015

Let's get some commits off our books, this will be a fairly minor release unless something makes it in within the next couple of days

  • [d2b62a4973] - benchmark: don't check wrk in non-http benchmark (Jackson Tian) #1368
  • [fd90b33b94] - build: validate options passed to configure (Johan Bergström) #1335
  • [04b02f5e34] - build: Remove deprecated flags (Johan Bergström) #1407
  • [39d395c966] - build: minor changes to fix rpm build (Dan Varga) #1408
  • [f9a2d31b32] - build: Simplify fetching release version (Johan Bergström) #1405
  • [cd38a4af8f] - build: support building io.js as a static library (Marat Abdullin) #1341
  • [d726a177ed] - build: Remove building against a shared V8 (Johan Bergström) #1331
  • [a5244d3a39] - (SEMVER-MINOR) deps: backport 1f8555 from v8's upstream (Fedor Indutny) #1395
  • [09d4a286ea] - deps: make node-gyp work with io.js (cjihrig) #990
  • [cc8376ae67] - deps: upgrade npm to 2.7.6 (Forrest L Norvell) #1390
  • [5b0e5755a0] - deps: generate opensslconf.h for architectures (Shigeki Ohtsu) #1377
  • [7d14aa0222] - deps: add Makefile to generate opensslconf.h (Shigeki Ohtsu) #1377
  • [29a3301461] - deps: make opensslconf.h include each target arch (Shigeki Ohtsu) #1377
  • [93a1a07ef4] - doc: remove keepAlive options from http.request (Jeremiah Senkpiel) #1392
  • [3ad6ea7c38] - doc: remove redundant parameter in end listener. (Alex Yursha) #1387
  • [2bc3532461] - doc: document Console class (Jackson Tian) #1388
  • [69bc1382b7] - doc: properly indent http.Agent keepAlive options (Jeremiah Senkpiel) #1384
  • [b464d467a2] - doc: update curl usage in COLLABORATOR_GUIDE (Roman Reiss) #1382
  • [61c0e7b70f] - doc: update CONTRIBUTING links. (Andrew Crites) #1380
  • [8d467e521c] - doc: add TC meeting 2015-03-18 minutes (Rod Vagg) #1370
  • [8ba9c4a7c2] - doc: add TC meeting 2015-04-01 minutes (Rod Vagg) #1371
  • [48facf93ad] - doc: update AUTHORS list (Rod Vagg) #1372
  • [1219e7466c] - lib: reduce process.binding() calls (Brendan Ashworth) #1367
  • [264a8f3a1b] - linux: fix epoll_pwait() fallback on arm64 (Ben Noordhuis) #1365
  • [f0bf6bb024] - readline: fix calling constructor without new (Alex Kocharin) #1385
  • [ff74931107] - smalloc: do not track external memory (Fedor Indutny) #1375
  • [a07c69113a] - (SEMVER-MINOR) src: use global SealHandleScope (Fedor Indutny) #1395
  • [a4d88475fa] - src: disable fast math only on armv6 (Ben Noordhuis) #1398
  • [e306c78f83] - src: disable fast math on arm (Ben Noordhuis) #1398
  • [7049d7b474] - test: increase timeouts on ARM (Roman Reiss) #1366
  • [3066f2c0c3] - test: double test timeout on arm machines (Ben Noordhuis) #1357
  • [66db9241cb] - tools: Remove unused files (Johan Bergström) #1406
  • [8bc8bd4bc2] - tools: add to install deps/openssl/config/archs (Shigeki Ohtsu) #1377
  • [907aaf325a] - win,node-gyp: optionally allow node.exe/iojs.exe to be renamed (Bert Belder) #1266
  • [372bf83818] - zlib: make constants keep readonly (Jackson Tian) #1361
@jbergstroem
Copy link
Member

This only has half of the commits @shigeki made as part of the openssl 1.0.2 bump. I don't think that is a problem, but the full patchset would be more tested (or no patchset).

@shigeki
Copy link
Contributor

shigeki commented Apr 11, 2015

Yes, I agree. It is better that v1.6.5 is the last final version which supports openssl-1.0.1. Go 1.0.2 after that.

@jbergstroem
Copy link
Member

@shigeki that puts us in a bad seat as well since we'd have to revert your commits or explore creating more branches for managing releases (please correct me if I'm wrong here).

@shigeki
Copy link
Contributor

shigeki commented Apr 11, 2015

@jbergstroem No. My commits included here to be in 1.6.5 are kinds of refactoring for just preparing upgrade, in which there is nothing to do with 1.0.2. If something trouble would happen in specific 1.0.2 in future, these commits here are not affected.

@jbergstroem
Copy link
Member

I see what you mean now. Ok, just disregard my comment.

@mscdex mscdex added the meta Issues and PRs related to the general management of the project. label Apr 11, 2015
@Fishrock123
Copy link
Contributor

👍

of note: arm fixes, npm, reduced process.binding calls

@Fishrock123
Copy link
Contributor

Would be nice if we could get #1341 in, so as to remove that "static libs don't build" bit. :)

@silverwind
Copy link
Contributor

We should put a known issue for #1376 if we can't get a patch ready in time.

Edit: The workaround here would make that note unnecessary: #1398
Edit2: Workaround merged.

@Fishrock123
Copy link
Contributor

Now 1.7.0 - #1395

@silverwind
Copy link
Contributor

In that case, we might want to get some of these in: https://github.com/iojs/io.js/labels/semver-minor

@rvagg rvagg changed the title Release proposal: v1.6.5 Release proposal: v1.7.0 Apr 11, 2015
@rvagg
Copy link
Member Author

rvagg commented Apr 12, 2015

can we talk about openssl 1.0.2 a bit more please? is there enough uncertainty about its stability to hold off for another release or two or is it relatively straight forward and therefore could just be part of 1.7.0?

@bnoordhuis
Copy link
Member

The build was refactored quite drastically so it's possible people with unusual configurations may see build breakage. I have no reason to believe that run-time stability is any worse though.

@Fishrock123
Copy link
Contributor

The build was refactored quite drastically so it's possible people with unusual configurations may see build breakage.

Isn't lots of that already in 1.6.4?

@bnoordhuis
Copy link
Member

Yes, but there is still more to come. I don't expect major issues though.

@shigeki
Copy link
Contributor

shigeki commented Apr 13, 2015

I would like to wait upgrading 1.0.2a to the next version because #1377 here has a possibility to break some compatibilities of openssl configuration especially on android or mips/mipsel that was listed in not implemented yet of #1377. And upgrading of #1389 has changes for assembler that also depend on cpu types.
It would be helpful for us to identify issues if theses two PRs are deployed in different releases.

@shigeki
Copy link
Contributor

shigeki commented Apr 13, 2015

@Gioyik Could you please make a build and run tests in android for this release ?

@Gioyik
Copy link
Contributor

Gioyik commented Apr 13, 2015

@shigeki sure, I am going to make a build early tomorrow. So, should I make a build from v1.x branch?

@shigeki
Copy link
Contributor

shigeki commented Apr 13, 2015

@Gioyik Thanks. Yes, the current HEAD of v1.x includes #1377. If errors occurs, please give me full error logs.

@benjamingr
Copy link
Member

Runs fine for me 👍

@jbergstroem
Copy link
Member

@Gioyik could you also mention if you're testing this on device or crossdev? thanks.

@rvagg
Copy link
Member Author

rvagg commented Apr 13, 2015

going to punt on this till tomorrow

@Gioyik
Copy link
Contributor

Gioyik commented Apr 13, 2015

I compiled it and is not problems during compilation. When I tested on Android devices I got the following error:

root@android:/data/local/tmp # ./iojs                                          
assertion "(err) == (0)" failed: file "../src/node.cc", line 3481, function "void node::PlatformInit()"
[1] + Stopped (signal)     ./iojs 

@jbergstroem I compiled it using Android NDK for cross compiling and the final binary is tested on a real Android phone.

@Gioyik
Copy link
Contributor

Gioyik commented Apr 13, 2015

Forget about the error, it's the phone where I am testing on. I tested in other and works as expected.

@shigeki
Copy link
Contributor

shigeki commented Apr 14, 2015

I made tests on my Nexus4 (armv7l, Android5.1) and found that there are several issues on Android.
The following patches are applied but there still remains several test failures. It needs more works to fix them. Fortunately, there are no failures caused by my commits on openssl.

I think it better to left them as known issues on Android and fix them later version.

diff --git a/configure b/configure
index 16d71a8..099bd2a 100755
--- a/configure
+++ b/configure
@@ -514,6 +514,10 @@ def configure_mips(o):
 def configure_node(o):
   if options.dest_os == 'android':
     o['variables']['OS'] = 'android'
+    # Probably they are needed above Android 5.x
+    o['cflags'] = ['-pie', '-fPIE']
+    o['ldflags'] = ['-pie', '-fPIE']
+
   o['variables']['node_prefix'] = os.path.expanduser(options.prefix or '')
   o['variables']['node_install_npm'] = b(not options.without_npm)
   o['default_configuration'] = 'Debug' if options.debug else 'Release'
diff --git a/lib/child_process.js b/lib/child_process.js
index 0fdbcf4..6716455 100644
--- a/lib/child_process.js
+++ b/lib/child_process.js
@@ -568,6 +568,9 @@ function normalizeExecArgs(command /*, options, callback */) {
     // options object.
     options = util._extend({}, options);
     options.windowsVerbatimArguments = true;
+  } else if (process.platform === 'android') {
+    file = '/system/bin/sh';
+    args = ['-c', command];
   } else {
     file = '/bin/sh';
     args = ['-c', command];
diff --git a/lib/dns.js b/lib/dns.js
index d39eec8..90cf493 100644
--- a/lib/dns.js
+++ b/lib/dns.js
@@ -129,7 +129,7 @@ exports.lookup = function lookup(hostname, options, callback) {

     // FIXME(indutny): V4MAPPED on FreeBSD results in EAI_BADFLAGS, because
     // the libc does not support it
-    if (process.platform === 'freebsd' && family !== 6)
+    if ((process.platform === 'freebsd' || process.platform === 'android') && family !== 6)
       hints &= ~exports.V4MAPPED;
   } else {
     family = options >>> 0;

@rvagg
Copy link
Member Author

rvagg commented Apr 14, 2015

2015-04-14, Version 1.7.0, @rvagg

Notable changes

  • C++ API: Fedor Indutny contributed a feature to V8 which has been backported to the V8 bundled in io.js. SealHandleScope allows a C++ add-on author to seal a HandleScope to prevent further, unintended allocations within it. Currently only enabled for debug builds of io.js. This feature helped detect the leak in #1075 and is now activated on the root HandleScope in io.js. (Fedor Indutny) #1395.
  • ARM: This release includes significant work to improve the state of ARM support for builds and tests. The io.js CI cluster's ARMv6, ARMv7 and ARMv8 build servers are now all (mostly) reporting passing builds and tests.
    • ARMv8 64-bit (AARCH64) is now properly supported, including a backported fix in libuv that was mistakenly detecting the existence of epoll_wait(). (Ben Noordhuis) #1365.
    • ARMv6: #1376 reported a problem with Math.exp() on ARMv6 (incl Raspberry Pi). The culprit is erroneous codegen for ARMv6 when using the "fast math" feature of V8. --nofast_math has been turned on for all ARMv6 variants by default to avoid this, it can be turned back on with --fast_math. (Ben Noordhuis) #1398.
    • Tests: timeouts have been tuned specifically for slower platforms, detected as ARMv6 and ARMv7. (Roman Reiss) #1366.
  • npm: Upgrade npm to 2.7.6. See the release notes for details. Summary:
    • b747593#7630 Don't automatically log all git failures as errors. maybeGithub needs to be able to fail without logging to support its fallback logic. (@othiym23)
    • 78005eb#7743 Always quote arguments passed to npm run-script. This allows build systems and the like to safely escape glob patterns passed as arguments to run-scripts with npm run-script <script> -- <arguments>. This is a tricky change to test, and may be reverted or moved to npm@3 if it turns out it breaks things for users. (@mantoni)
    • da015ee#7074 read-package-json@1.3.3: read-package-json no longer caches package.json files, which trades a very small performance loss for the elimination of a large class of really annoying race conditions. See #7074 for the grisly details. (@othiym23)

@rvagg
Copy link
Member Author

rvagg commented Apr 14, 2015

https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/502/

the usual Jenkins-specific TIMEOUTs on Windows slaves, the rest looks pretty good. So happy about the state of ARM in this one, great work everyone!

@rvagg
Copy link
Member Author

rvagg commented Apr 14, 2015

v1.7.0 tagged, building @ https://jenkins-iojs.nodesource.com/job/iojs+release/54/

@rvagg
Copy link
Member Author

rvagg commented Apr 14, 2015

2015-04-14, Version 1.7.1, @rvagg

Notable changes

  • build: A syntax error in the Makefile for release builds caused 1.7.0 to be DOA and unreleased. (Rod Vagg) #1421.

Commits

@rvagg
Copy link
Member Author

rvagg commented Apr 14, 2015

tagged v1.7.1 building @ https://jenkins-iojs.nodesource.com/job/iojs+release/55/ and looking better. I'm deleting 1.7.0 from staging, it'll be unreleased.

@targos
Copy link
Member

targos commented Apr 14, 2015

@rvagg Shouldn't there be a "Working on v1.7.2" commit ?

@rvagg
Copy link
Member Author

rvagg commented Apr 14, 2015

thanks @targos, rectified

@rvagg rvagg closed this as completed Apr 14, 2015
@rvagg
Copy link
Member Author

rvagg commented Apr 14, 2015

v1.7.1 has landed, will promote armv6 tomorrow

@Gioyik
Copy link
Contributor

Gioyik commented Apr 14, 2015

@shigeki did you create a PR with changes in that diff patch? Added to it, which test you run to detect issues on Android build?

@shigeki
Copy link
Contributor

shigeki commented Apr 14, 2015

@Gioyik Yes, I will work it tomorrow. The problem is that there are no python for Android build in PIE to run tests. I made an test shell script for an alternative but it does not have a timeout feature so that it is very hard to finish all tests.

@Gioyik
Copy link
Contributor

Gioyik commented Apr 14, 2015

@shigeki what about if you use the python inside Android NDK? when it uncompress the NDK in android-toolchain folder you have a python binary in android-toolchain/bin. Not sure if it's what you are looking for (I am not sure what is PIE), so let me know if it's useful.

@rvagg
Copy link
Member Author

rvagg commented Apr 15, 2015

FYI I found a problem with a couple of raspberry pis in the cluster, the one that prepared the 1.7.1 binary was naming it "armv7l" when it should have been "armv6l" so I've started a build again to get the right binary for release. There was also a problem compressing the binary with xz due to memory limitations and xz being a memory hog. If I encounter this again we'll have to reassess how we're dealing with xz files.

@silverwind
Copy link
Contributor

Are you using a swap? If not, that should probably fix that issue.

@rvagg
Copy link
Member Author

rvagg commented Apr 15, 2015

yeah, there's a 100M swapfile atm, @jbergstroem is telling me that -9 is requiring ~750M for our files so that's not quite enough

I'm torn between increasing swap or adjusting the compression level .. input welcome

@silverwind
Copy link
Contributor

If compression time is no concern, i'd just increase the swap file to like 1GB. As I see it, it totally makes sense to optimize xz for size, as people who don't care about that will get the gz.

@jbergstroem
Copy link
Member

Since newer versions of xz is supposed to thread over multiple cores I fear memory consumption might go up more. I think @silverwind is on point though; xz should be used for maximum size decrease and nothing else. FWIW, here's the difference between compression levels 6-9 (xz -$level):

level size (bytes) size increase
6 11887124 2.824%
7 11652552 0.930%
8 11594984 0.431%
9 11545128 -

A suggested patch that would expose control would look like this:

diff --git Makefile Makefile
index 141cee2..28fd263 100644
--- Makefile
+++ Makefile
@@ -223,6 +223,7 @@ TARBALL=$(TARNAME).tar
 BINARYNAME=$(TARNAME)-$(PLATFORM)-$(ARCH)
 BINARYTAR=$(BINARYNAME).tar
 XZ=$(shell which xz > /dev/null 2>&1; echo $$?)
+XZ_COMPRESSION ?= 9
 PKG=out/$(TARNAME).pkg
 PACKAGEMAKER ?= /Developer/Applications/Utilities/PackageMaker.app/Contents/MacOS/PackageMaker

@@ -296,7 +297,7 @@ $(TARBALL): release-only $(NODE_EXE) doc
        rm -rf $(TARNAME)
        gzip -c -f -9 $(TARNAME).tar > $(TARNAME).tar.gz
 ifeq ($(XZ), 0)
-       xz -c -f -9 $(TARNAME).tar > $(TARNAME).tar.xz
+       xz -c -f -$(XZ_COMPRESSION) $(TARNAME).tar > $(TARNAME).tar.xz
 endif
        rm $(TARNAME).tar

@@ -314,7 +315,7 @@ $(BINARYTAR): release-only
        rm -rf $(BINARYNAME)
        gzip -c -f -9 $(BINARYNAME).tar > $(BINARYNAME).tar.gz
 ifeq ($(XZ), 0)
-       xz -c -f -9 $(BINARYNAME).tar > $(BINARYNAME).tar.xz
+       xz -c -f -$(XZ_COMPRESSION) $(BINARYNAME).tar > $(BINARYNAME).tar.xz
 endif
        rm $(BINARYNAME).tar

..and invoked:

$ XZ_COMPRESSION=6 make tar

@rvagg
Copy link
Member Author

rvagg commented Apr 15, 2015

that -7 size tradeoff is totally worth it for speed and memory consumption, I'm +1 on that patch @jbergstroem, thanks for running the numbers

@shigeki
Copy link
Contributor

shigeki commented Apr 15, 2015

@Gioyik I opened a new issue #143 for test failures on Android. Let's move to it. The python in the toolchain/bin seems to be a x86-64 binary not for arm.

$ file arm-android-8/toolchain/bin/python2.7
arm-android-8/toolchain/bin/python2.7: ELF 64-bit LSB  executable, x86-64, version 1 (SYSV), dynamically linked (uses shared libs), for GNU/Linux 2.6.8, not stripped

PIE stands for "Position-Independent Executable" which is introduced in Android5.x for security protection see http://source.android.com/devices/tech/security/enhancements/enhancements50.html . It is

non-PIE linker support removed. Android now requires all dynamically linked executables to support PIE (position-independent executables). This enhances Android’s address space layout randomization (ASLR) implementation.

I made tests by disabling PIE by changing linker binary and test results are shown in #1430 .

Starefossen pushed a commit to Starefossen/docker-iojs that referenced this issue Apr 15, 2015
PR-URL: nodejs#50
Related: nodejs/node#1396

Signed-off-by: Hans Kristian Flaatten <hans.kristian.flaatten@turistforeningen.no>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

No branches or pull requests

10 participants