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

Don't use deprecated v8::Template::Set(). #128

Merged
merged 1 commit into from Apr 29, 2016
Merged

Don't use deprecated v8::Template::Set(). #128

merged 1 commit into from Apr 29, 2016

Conversation

bnoordhuis
Copy link
Contributor

See [0] and [1]: starting with node.js v6, setting non-primitive values
on FunctionTemplate and ObjectTemplate instances is discouraged; v7 will
downright disallow it. Update the code base.

[0] nodejs/node#6216
[1] nodejs/node#6228

Refs: #127

@MylesBorins
Copy link

I'm still seeing the warnings with these changes applied

@bnoordhuis
Copy link
Contributor Author

Are you sure you're testing the changes? Could it be node-pre-gyp is interfering?

@MylesBorins
Copy link

@bnoordhuis it looks like nan needs to be updated too

@bnoordhuis
Copy link
Contributor Author

On hold until nodejs/nan#560 is released.

@spalger
Copy link

spalger commented Apr 27, 2016

@bnoordhuis, nodejs/nan#560 is out

See [0] and [1]: starting with node.js v6, setting non-primitive values
on FunctionTemplate and ObjectTemplate instances is discouraged; v7 will
downright disallow it.  Update the code base.  Also update to nan@2.3.0.

[0] nodejs/node#6216
[1] nodejs/node#6228
@bnoordhuis
Copy link
Contributor Author

Updated. @thealphanerd If you can run citgm again? Can I start it somehow?

@MylesBorins
Copy link

MylesBorins commented Apr 27, 2016

@bnoordhuis I was never running citgm on this change, technically the test always passed and it is platform specific to osx so I just was manually testing on my own machine.

it is worth mentioning that i was still seeing the noise with the latest patch, but I'm not sure if nan was properly updated

@zakdances
Copy link

It looks like everything but iojs-v3 is passing. Is there reason for the node@<4 support? Could package.json be updated with something like

"engines": {
    "node": ">=4.4.3"
  }

instead? Or is that too drastic?

@MylesBorins
Copy link

MylesBorins commented Apr 28, 2016 via email

@olalonde
Copy link

@thealphanerd sorry to be pushy, but could you release a new version with this patch and work on fixing the iojs-v3 build in another release? Looking at the travis logs, it seems that an "unknown" event is fired... but it's hard to see why exactly. Might help if you added console.log(flags) here: https://github.com/strongloop/fsevents/blob/master/fsevents.js#L84 and see what comes up

@olalonde
Copy link

olalonde commented Apr 29, 2016

If that's any help, I tried to reproduce the bug locally using iojs-v3 (nvm install iojs-v3). Stangely, the first npm test passes but subsequent calls to npm test have errors. I'm also not getting that 'unknown' event like in travis logs. Not sure if that helps...

~/code/fsevents$ ./node_modules/.bin/node-pre-gyp install --update-binary            bnoordhuis-v6
node-pre-gyp info it worked if it ends with ok
node-pre-gyp info using node-pre-gyp@0.6.26
node-pre-gyp info using node@3.3.1 | darwin | x64
node-pre-gyp http GET https://fsevents-binaries.s3-us-west-2.amazonaws.com/v1.0.11/fse-v1.0.11-node-v45-darwin-x64.tar.gz
node-pre-gyp http 200 https://fsevents-binaries.s3-us-west-2.amazonaws.com/v1.0.11/fse-v1.0.11-node-v45-darwin-x64.tar.gz
node-pre-gyp info install unpacking fse.node
node-pre-gyp info tarball done parsing tarball
node-pre-gyp info validate Running test command: '/Users/olalonde/.nvm/versions/io.js/v3.3.1/bin/iojs --eval 'require(\'/Users/olalonde/code/fsevents/lib/binding/Release/node-v45-darwin-x64/fse.node\')''
[fsevents] Success: "/Users/olalonde/code/fsevents/lib/binding/Release/node-v45-darwin-x64/fse.node" is installed via remote
node-pre-gyp info ok
~/code/fsevents$ npm test                                                            bnoordhuis-v6

> fsevents@1.0.11 test /Users/olalonde/code/fsevents
> tap ./test

ok test/fsevents.js ..................................... 6/6
===========================================================================
        writeFileSync(__dirname + '/temp/created-fsevent', 'created-fsevent');
===========================================================================
id:     100044905
flags:  69888
name:   /Users/olalonde/code/fsevents/test/temp/created-fsevent
base:   created-fsevent
event:  modified
info:   {"path":"/Users/olalonde/code/fsevents/test/temp/created-fsevent","event":"modified","type":"file","changes":{"inode":false,"finder":false,"access":false,"xattrs":false},"flags":69888,"id":100044905}
===========================================================================
        renameSync(__dirname + '/temp/created-fsevent', __dirname + '/temp/moved-fsevent');
===========================================================================
id:     100044908
flags:  71936
id:     100044909
flags:  67584
name:   /Users/olalonde/code/fsevents/test/temp/created-fsevent
base:   created-fsevent
event:  moved-out
info:   {"path":"/Users/olalonde/code/fsevents/test/temp/created-fsevent","event":"moved-out","type":"file","changes":{"inode":false,"finder":false,"access":false,"xattrs":false},"flags":71936,"id":100044908}
name:   /Users/olalonde/code/fsevents/test/temp/moved-fsevent
base:   moved-fsevent
event:  moved-in
info:   {"path":"/Users/olalonde/code/fsevents/test/temp/moved-fsevent","event":"moved-in","type":"file","changes":{"inode":false,"finder":false,"access":false,"xattrs":false},"flags":67584,"id":100044909}
===========================================================================
        unlinkSync(__dirname + '/temp/moved-fsevent');
===========================================================================
id:     100044912
flags:  68096
name:   /Users/olalonde/code/fsevents/test/temp/moved-fsevent
base:   moved-fsevent
event:  deleted
info:   {"path":"/Users/olalonde/code/fsevents/test/temp/moved-fsevent","event":"deleted","type":"file","changes":{"inode":false,"finder":false,"access":false,"xattrs":false},"flags":68096,"id":100044912}
===========================================================================
        rmdirSync(__dirname + '/temp');
===========================================================================
ok test/function.js ................................... 17/17
total ................................................. 23/23

ok
~/code/fsevents$ npm test                                                            bnoordhuis-v6

> fsevents@1.0.11 test /Users/olalonde/code/fsevents
> tap ./test

ok test/fsevents.js ..................................... 6/6
===========================================================================
        writeFileSync(__dirname + '/temp/created-fsevent', 'created-fsevent');
===========================================================================
id:     100045003
flags:  71936
name:   /Users/olalonde/code/fsevents/test/temp/created-fsevent
base:   created-fsevent
event:  moved-in
info:   {"path":"/Users/olalonde/code/fsevents/test/temp/created-fsevent","event":"moved-in","type":"file","changes":{"inode":false,"finder":false,"access":false,"xattrs":false},"flags":71936,"id":100045003}
===========================================================================
        renameSync(__dirname + '/temp/created-fsevent', __dirname + '/temp/moved-fsevent');
===========================================================================
id:     100045006
flags:  71936
id:     100045007
flags:  68096
name:   /Users/olalonde/code/fsevents/test/temp/moved-fsevent
base:   moved-fsevent
event:  deleted
info:   {"path":"/Users/olalonde/code/fsevents/test/temp/moved-fsevent","event":"deleted","type":"file","changes":{"inode":false,"finder":false,"access":false,"xattrs":false},"flags":68096,"id":100045007}
name:   /Users/olalonde/code/fsevents/test/temp/created-fsevent
base:   created-fsevent
event:  moved-out
info:   {"path":"/Users/olalonde/code/fsevents/test/temp/created-fsevent","event":"moved-out","type":"file","changes":{"inode":false,"finder":false,"access":false,"xattrs":false},"flags":71936,"id":100045006}
===========================================================================
        unlinkSync(__dirname + '/temp/moved-fsevent');
===========================================================================
id:     100045010
flags:  68096
name:   /Users/olalonde/code/fsevents/test/temp/moved-fsevent
base:   moved-fsevent
event:  deleted
info:   {"path":"/Users/olalonde/code/fsevents/test/temp/moved-fsevent","event":"deleted","type":"file","changes":{"inode":false,"finder":false,"access":false,"xattrs":false},"flags":68096,"id":100045010}
===========================================================================
        rmdirSync(__dirname + '/temp');
===========================================================================
not ok test/function.js ............................... 16/17
    Command: "/Users/olalonde/.nvm/versions/io.js/v3.3.1/bin/iojs function.js"
    TAP version 13
    ok 1 created file was caught with flags:71936
    ok 2 id is a number 100045003
    ok 3 matched path
    not ok 4 file moved in: created-fsevent
      ---
        file:   /Users/olalonde/code/fsevents/test/function.js
        line:   55
        column: 11
        stack:
          - |
            getCaller (/Users/olalonde/code/fsevents/node_modules/tap/lib/tap-assert.js:418:17)
          - |
            Function.assert (/Users/olalonde/code/fsevents/node_modules/tap/lib/tap-assert.js:21:16)
          - |
            Test._testAssert [as ok] (/Users/olalonde/code/fsevents/node_modules/tap/lib/tap-test.js:87:16)
          - |
            FSEvents.<anonymous> (/Users/olalonde/code/fsevents/test/function.js:55:11)
          - |
            emitTwo (events.js:87:13)
          - |
            FSEvents.emit (events.js:172:7)
          - |
            /Users/olalonde/code/fsevents/fsevents.js:50:15
          - |
            FSReqWrap.oncomplete (fs.js:82:15)
      ...
    ok 5 created file was caught with flags:71936
    ok 6 id is a number 100045006
    ok 7 renamed file was caught with flags:68096
    ok 8 id is a number 100045007
    ok 9 matched path
    ok 10 file deleted: moved-fsevent
    ok 11 matched path
    ok 12 file moved out: created-fsevent
    ok 13 renamed file was caught with flags:68096
    ok 14 id is a number 100045010
    ok 15 matched path
    ok 16 file deleted: moved-fsevent
    ok 17 test/function.js

    1..17
    # tests 17
    # pass  16
    # fail  1

total ................................................. 22/23

not ok
npm ERR! Test failed.  See above for more details.
~/code/fsevents$ node --version                                                                        bnoordhuis-v6
v3.3.1
~/code/fsevents$ node --help                                                                           bnoordhuis-v6
Usage: iojs [options] [ -e script | script.js ] [arguments]
       iojs debug script.js [arguments]

@es128
Copy link
Contributor

es128 commented Apr 29, 2016

I re-ran the job on Travis and it passed. I think it's good enough to proceed.

@es128 es128 merged commit 09ccf80 into fsevents:master Apr 29, 2016
@bnoordhuis bnoordhuis deleted the v6 branch April 29, 2016 14:24
@shreeve
Copy link

shreeve commented May 10, 2016

Elan - Any idea on an ETA for releasing this? Thanks!

@es128
Copy link
Contributor

es128 commented May 10, 2016

@shreeve this was published in 1.0.12

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

Successfully merging this pull request may close these issues.

None yet

7 participants