Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

Better support for io.js rapid release cycle #694

Closed
xzyfer opened this issue Feb 22, 2015 · 25 comments
Closed

Better support for io.js rapid release cycle #694

xzyfer opened this issue Feb 22, 2015 · 25 comments
Milestone

Comments

@xzyfer
Copy link
Contributor

xzyfer commented Feb 22, 2015

Io.js 1.3 has been released and is now failing to download the binary.

@am11 I think we should update the install script to be less strict on version mathing for io.js. Their releases more frequently than ours, and are unlikely to have broken older binaries.

I think in the long run it's better for us to assume binaries are good until proven broken (at least for io.js) rather than trying to keep releases in sync.

@xzyfer xzyfer changed the title Support io.js 1.3 Better support for io.js rapid release cycle Feb 22, 2015
@Globegitter
Copy link

Yep I think that sounds good. Keeping support for all 1.x.x versions sounds imo reasonable.

@am11
Copy link
Contributor

am11 commented Feb 22, 2015

That was fast! Its like yesterday they released v1.2.. 😜

See the comment by @kkoopa #627 (comment). As soon as the minor version changes, it signifies a breaking change and requires a binary rebuild.

However, I can confirm that our iojs-v1.2 binary for win-x64 is working with iojs-v1.3.

For the resolution, I agree that instead of changing the binary names, we should change the install script to fallback to first available version. Here is one way to go about it: where we call the download function, retry with one lesser version in case of 404 and repeat it thrice before giving up.

@Globegitter
Copy link

@am11 Doesn't a change in a minor version mean that the API has been changed in a backwards-compatible manner? Or is iojs not follwoing http://semver.org/? That was my reasoning behind the comment - that 1.x changes should not break things in most cases.

This more fundamental discussion aside, I think this sounds like a good solution 👍

@am11
Copy link
Contributor

am11 commented Feb 22, 2015

The current stable version of node.js is v0.12.0 and the major version hasn't been changed yet. It could be that all changes ever made in node.js are backward compatible but those do not account for the breaking changes presented by v8 and libuv (on which the C++ add-on binaries are dependent upon).

@Globegitter
Copy link

Oh yeah that is a good point @am11, especially when getting into the land of C/C++ :)

@xzyfer
Copy link
Contributor Author

xzyfer commented Feb 22, 2015

An alternate suggestion would be to change the version string in the binary path from the compiled version to the lowest supported version i.e.

- darwin-x64-iojs-1.0
- darwin-x64-iojs-1.2
- darwin-x64-iojs-1.7

This essentially translates to

- darwin-x64-iojs-1.0 < 1.2
- darwin-x64-iojs-1.2 < 1.7
- darwin-x64-iojs-1.7+

Then use the github contents API to query the directory listing.

This makes downloading a compatible binary easy, and makes us future proof for minor changes, compatible changes.

@am11
Copy link
Contributor

am11 commented Feb 22, 2015

@xzyfer, I think with that approach, we will be kind of assuring the user that the downloaded binary is compatible. That approach also means extra work: finding lowest compatible version for each io.js binary with all previous versions up to the available tag: in other words O(n^ln(n)) (all this to detect whether it is choking due to the binary mismatch). 😄

I guess it is different than saying "couldn't find the matching binary, lets see if the previous version works". When it stops working due to some breaking change (after say 3 minor iterations), users will complain, and we will release the newer version.

Nonetheless, we may use GitHub APIs at some point when addressing #56 with node-pre-gyp, unless we decide to go with (node-pre-gyp recommended) Amazon cloud storage bucket. :)

@xzyfer
Copy link
Contributor Author

xzyfer commented Feb 22, 2015

That approach also means extra work: finding lowest compatible version for each io.js binary with all previous versions up to the available tag

This is rather simple since we have the version number, and list of available versions. We simple loop decremting the minor digit. It's exactly the same approach you mentioned retry with one lesser version in case of 404 and repeat it thrice before giving up except that we don't need to go over the network.

@am11
Copy link
Contributor

am11 commented Feb 22, 2015

except that we don't need to go over the network

Then we will implement GH API and on install, query the available releases and download the closest lowest version for forward compatibility. So that will be two network requests per install. With the other approach, there is one network request in best case scenario (versions matched) and two more in worse-case scenario. The point is implementing GH API in node-sass build process to avoid three/four network requests sounds overkill.

@am11 am11 added the npm build label Feb 22, 2015
@nschonni
Copy link
Contributor

@am11 you may want to look at the original binary naming approach which just used platform + v8 version.

@am11
Copy link
Contributor

am11 commented Feb 22, 2015

@nschonni, awesome! It seems like process.versions.v8 is the key criterion.

I just tested, the binary that was built with iojs v1.2.0, works with v1.3.0 as well as v1.1.0, as all three have same v8 version. Also, the one that is built with v0.10.31, works with v0.10.36 (but not v0.12 because v8 differs).

am11 added a commit to am11/node-sass that referenced this issue Feb 22, 2015
am11 added a commit to am11/node-sass that referenced this issue Feb 22, 2015
The new binary path format is:
`<platform>-<arch>-<v8 version>/binding.node`

Issue URL: sass#694.
PR URL: sass#695.
am11 added a commit to am11/node-sass that referenced this issue Feb 22, 2015
The new binary path format is:
`<platform>-<arch>-<v8 version>/binding.node`

Issue URL: sass#694.
PR URL: sass#695.
@xzyfer
Copy link
Contributor Author

xzyfer commented Feb 22, 2015

👍 to v8 version

@am11
Copy link
Contributor

am11 commented Feb 23, 2015

👍

Thais is fixed by 19b5e3c via #695.

@saper
Copy link
Member

saper commented Mar 8, 2015

Request reopen:

Why are we using v8 version number instead of NODE_MODULE_VERSION?

Engine Version v8 NODE_MODULE_VERSION
iojs 1.4.3 4.1.0.21 43
node 0.12.0 3.28.73 14
node 0.10.36 3.14.5.9 11

I think NODE_MODULE_VERSION is exactly what we need; covers not only v8 version but also other libraries. It is changed whenever ABI incompatibility is introduced, for example see discussion nodejs/node#952 whether it needs to be raised or not (not all v8 upgrades cause incompatibility).

So I think we should use NODE_MODULE_VERSION, not v8 version. Thoughts?

@am11
Copy link
Contributor

am11 commented Mar 8, 2015

@saper,

c:\Users\Adeel\Source\Repos\node-sass>nvmw use iojs-v1.4.1
Now using iojs v1.4.1

c:\Users\Adeel\Source\Repos\node-sass>iojs -p process.versions
{ http_parser: '2.3.0',
  node: '1.4.1',
  v8: '4.1.0.21',
  uv: '1.4.2',
  zlib: '1.2.8',
  ares: '1.10.0-DEV',
  modules: '43',
  openssl: '1.0.1k' }

c:\Users\Adeel\Source\Repos\node-sass>nvmw use iojs-v1.5.0
Now using iojs v1.5.0

c:\Users\Adeel\Source\Repos\node-sass>iojs -p process.versions
{ http_parser: '2.3.0',
  node: '1.5.0',
  v8: '4.1.0.21',
  uv: '1.4.2',
  zlib: '1.2.8',
  ares: '1.10.0-DEV',
  modules: '43',
  openssl: '1.0.1k' }

modules versions remained same. I have tested it and v8 version is most reliable and give us flexibility to maneuver. If you find a real bug in future version, we can reconsider it or just release a new version of node-sass. Note that we already are binding the binaries with node-sass tag release, so duplicate names will not get mixed up across the releases.

@saper
Copy link
Member

saper commented Mar 8, 2015

This is correct, iojs 1.4.1 and 1.5.0 should be binary compatible from our point of view. They didn't bump modules because ABI didn't change. You can sometimes upgrade v8 and not break compatibility. They can upgrade libuv and break compatibility. Of course we need to track process.versions.modules separately for iojs and nodejs since I don't think they coordinate this.

@xzyfer
Copy link
Contributor Author

xzyfer commented Mar 9, 2015

@am11 I think modules is worth testing. Seems sane to use a public flag that's designed for our exact purpose.

@Globegitter
Copy link

I didn't know about that either. It does sound like the modules version number is something at least worth looking into.

am11 added a commit to am11/node-sass that referenced this issue Mar 9, 2015
@am11
Copy link
Contributor

am11 commented Mar 9, 2015

Fixed by c5b4fa6. The new format is <runtime>-<arch>-<module>.node.

am11 added a commit to am11/node-sass that referenced this issue Mar 9, 2015
am11 added a commit to am11/node-sass that referenced this issue Mar 9, 2015
am11 added a commit to am11/node-sass that referenced this issue Mar 9, 2015
am11 added a commit to am11/node-sass that referenced this issue Mar 9, 2015
am11 added a commit to am11/node-sass that referenced this issue Mar 9, 2015
am11 added a commit to am11/node-sass that referenced this issue Mar 10, 2015
@akre54
Copy link

akre54 commented Mar 10, 2015

Any news on a release for this? Thanks.

am11 added a commit to am11/node-sass that referenced this issue Mar 11, 2015
am11 added a commit to am11/node-sass that referenced this issue Mar 11, 2015
@MattiSG
Copy link

MattiSG commented Mar 26, 2015

+1 :)

@saper
Copy link
Member

saper commented Apr 2, 2015

What is interesting: FreeBSD binaries for io.js 1.0.4 (module version 42) and io.js 1.1.0+ (module version 43) differ only by one byte: 42 is changed to 43 in one location... no real ABI change?

@am11
Copy link
Contributor

am11 commented Apr 3, 2015

@saper, yes but that bit change require recompilation of node module whenever NODE_MODULE_VERSION is incremented. I think it throws a runtime error if there is a variance in installed vs. compiled v8 version.

@kikar
Copy link

kikar commented Oct 18, 2015

Hi, I'm using node v4.2.1 and i'm getting the Can not download file from https://raw.githubusercontent.com/sass/node-sass-binaries/v2.1.1/darwin-x64-node-4.2/binding.node Error.
Is there a quick fix without downgrading node?

@saper
Copy link
Member

saper commented Oct 18, 2015

Hi, I'm using node v4.2.1 and i'm getting the Can not download file from https://raw.githubusercontent.com/sass/node-sass-binaries/v2.1.1/darwin-x64-node-4.2/binding.node Error.
Is there a quick fix without downgrading node?

Use current version of node-sass, v2.1.1 is quite old.

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

No branches or pull requests

8 participants