Failures after V8 5.4 update on master

[MylesBorins]

Looks like three failures have we:

• level
• serialport
• node-sass

/cc @ralphtheninja @reconbot @xzyfer

[targos]

Related: #179

I'm pretty sure this is because we have to update NODE_MODULE_VERSION or force a node-gyp rebuild

[xzyfer]

Is there a Travis build I can look at?

[MylesBorins]

Here is a citgm run with a forced rebuild

https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/396/

/cc @jasnell can you do an updated release of v7 with the NODE_MODULE_VERSION bumped?

edit: lolol no node-gyp... one more time

https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/397/

[xzyfer]

https://ci.nodejs.org is currently 502'ing. I'll keep an eye on it.

[xzyfer]

@thealphanerd sorry, I'm failing to find the node-sass build log.

[MylesBorins]

bah... the rebuild job is not working in the regular CI job... made a custom CI job for rebuild, this should hopefully work

https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker-rebuild/2/

[MylesBorins]

@xzyfer

warn: npm-install:        | node: symbol lookup error:                        
warn:                     | /tmp/5c033f2c-bc4d-4dfe-8130-d8d940e9d288/node-sa…
warn:                     | ss/vendor/linux-x64-48/binding.node: undefined    
warn:                     | symbol:                                           
warn:                     | _ZN2v816FunctionTemplate3NewEPNS_7IsolateEPFvRKNS…
warn:                     | _20FunctionCallbackInfoINS_5ValueEEEENS_5LocalIS4…
warn:                     | _EENSA_INS_9SignatureEEEi                

[xzyfer]

@thealphanerd is there a link to the build?

[jasnell]

Yep. The hope is to get that updated build today.

On Monday, September 26, 2016, Myles Borins notifications@github.com
wrote:

Here is a citgm run with a forced rebuild

https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/396/

/cc @jasnell https://github.com/jasnell can you do an updated release
of v7 with the NODE_MODULE_VERSION bumped?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#187 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAa2ebfJODJGKcAE5GWdkL0nJfb2ZifSks5quL14gaJpZM4KHTBS
.

[reconbot]

Why doesn't the regular job rebuild? Seems like the right thing to do.

On Tue, Sep 27, 2016, 8:47 AM James M Snell notifications@github.com
wrote:

Yep. The hope is to get that updated build today.

On Monday, September 26, 2016, Myles Borins notifications@github.com
wrote:

Here is a citgm run with a forced rebuild

https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/396/

/cc @jasnell https://github.com/jasnell can you do an updated release
of v7 with the NODE_MODULE_VERSION bumped?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#187 (comment), or
mute
the thread
<
https://github.com/notifications/unsubscribe-auth/AAa2ebfJODJGKcAE5GWdkL0nJfb2ZifSks5quL14gaJpZM4KHTBS

.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#187 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AABlbpF3gMbOJIjntIbr_C2Z3GaKfRd0ks5quRBGgaJpZM4KHTBS
.

[xzyfer]

Looking at the node-sass failures, it appears to be because the pre-built binary isn't compatible anymore. I'm guessing this is because the ABI changed but the module version didn't?

As for why it's erring the way it is, there looks to be a problem with our install script. After pulling down a prebuilt binary we try to execute the binary with a sample input. If an exception is raised we know the binary is no good. It looks like the node: symbol lookup error: error may not be catchable?

https://github.com/sass/node-sass/blob/master/scripts/build.js#L208-L218

[reconbot]

Because of the wired state the abi gets in (eg not bumped when it has
clearly changed) we're going to see this with any prebuilt or binary
bundled package because the abi version has broken it's stable api
guarantee. I think the version should be bumped as soon as it changes
because that will allow a graceful recompile is the "right thing". But I
may have to make that argument in a different venue.

Forcing a compile will catch incompatibilities with newer versions of node
and since that's a goal of the citgm I think it should be default.

On Tue, Sep 27, 2016, 9:52 AM Michael Mifsud notifications@github.com
wrote:

Looking at the node-sass failures, it appears to be because the pre-built
binary isn't compatible anymore. I'm guessing this is because the ABI
changed but the module version didn't?

As for why it's erring the way it is, there looks to be a problem with our
install script. After pulling down a prebuilt binary we try to execute
the binary with a sample input. If an exception is raised we know the
binary is no good. It looks like the node: symbol lookup error: error may
not be catchable?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#187 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AABlbvC5MKiIuh9DiPQgkT5PJ3fStDIfks5quR-lgaJpZM4KHTBS
.

[MylesBorins]

so it seems like the rebuild job is never getting to the "rebuild" lifecycle stage on node-sass as the install lifecycle hook is running a custom script, and it never has the chance to rebuild. The new release with bumped module version number should fix this.

As for why rebuild is not the standard approach... we want to actually catch things breaking as people would normally use npm, we can then use rebuild as a follow up for deeper diving. totally open to reviewing that process.

[reconbot]

I agree with you, I think getting the ABI version bumped as soon as it's broken is a good thing. =)

I also think your current process is 👍