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

Can't compile 0.7.1 on node 4.1.0 #97

Closed
amaramth opened this Issue Sep 25, 2015 · 22 comments

Comments

Projects
None yet
6 participants
@amaramth

amaramth commented Sep 25, 2015

I reverted to node 0.12 for now. Does npm install node-spotify reproduce this for you on node 4.1.0?

(osx, vanilla setup; node 4.x installed with "brew upgrade node")

@FrontierPsychiatrist

This comment has been minimized.

Show comment
Hide comment
@FrontierPsychiatrist

FrontierPsychiatrist Sep 27, 2015

Owner

Yes, I tried that too. node-spotify is not compatible with the changes in V8 that are in NodeJS 4. Unfortunately I currently don't have the time to take a look at it.

Owner

FrontierPsychiatrist commented Sep 27, 2015

Yes, I tried that too. node-spotify is not compatible with the changes in V8 that are in NodeJS 4. Unfortunately I currently don't have the time to take a look at it.

@Joeasaurus

This comment has been minimized.

Show comment
Hide comment
@Joeasaurus

Joeasaurus Dec 2, 2015

👍 Please!

I tried having a look myself but it's certainly not the kind of thing to be diving in to for your first Node/C++ expedition!

Joeasaurus commented Dec 2, 2015

👍 Please!

I tried having a look myself but it's certainly not the kind of thing to be diving in to for your first Node/C++ expedition!

@amaramth

This comment has been minimized.

Show comment
Hide comment
@amaramth

amaramth Dec 2, 2015

I'm working on pruning it down to the subset that's not compatible with the new web api, and then that would be much easier to convert to node 4.x or whatever.

amaramth commented Dec 2, 2015

I'm working on pruning it down to the subset that's not compatible with the new web api, and then that would be much easier to convert to node 4.x or whatever.

@FrontierPsychiatrist

This comment has been minimized.

Show comment
Hide comment
@FrontierPsychiatrist

FrontierPsychiatrist Dec 7, 2015

Owner

I guess I could take a look into upgrading nan to a newer version and hope that helps.

Owner

FrontierPsychiatrist commented Dec 7, 2015

I guess I could take a look into upgrading nan to a newer version and hope that helps.

@FrontierPsychiatrist

This comment has been minimized.

Show comment
Hide comment
@FrontierPsychiatrist

FrontierPsychiatrist Dec 8, 2015

Owner

Ok, I started an upgrade to nan 2.1.x which sadly means a lot of changes (NanNew -> Nan::New, EscapableHandleScopes,...). I hope I find some time to make it compile and work again.

Owner

FrontierPsychiatrist commented Dec 8, 2015

Ok, I started an upgrade to nan 2.1.x which sadly means a lot of changes (NanNew -> Nan::New, EscapableHandleScopes,...). I hope I find some time to make it compile and work again.

@Joeasaurus

This comment has been minimized.

Show comment
Hide comment
@Joeasaurus

Joeasaurus Dec 8, 2015

@FrontierPsychiatrist That's what I was trying, I hope you have a better time of it!

Joeasaurus commented Dec 8, 2015

@FrontierPsychiatrist That's what I was trying, I hope you have a better time of it!

@FrontierPsychiatrist

This comment has been minimized.

Show comment
Hide comment
@FrontierPsychiatrist

FrontierPsychiatrist Dec 8, 2015

Owner

Ok, I progressed a bit further, this is really a lot of work to do and I don't even know if it will work when it compiles. Will take a while.

Owner

FrontierPsychiatrist commented Dec 8, 2015

Ok, I progressed a bit further, this is really a lot of work to do and I don't even know if it will work when it compiles. Will take a while.

@FrontierPsychiatrist

This comment has been minimized.

Show comment
Hide comment
@FrontierPsychiatrist

FrontierPsychiatrist Jan 6, 2016

Owner

Just a short life sign: Still not much progress. I'm still working on making it compile and I really hope when it compiles it will also run. There were so many changes to nan...

Owner

FrontierPsychiatrist commented Jan 6, 2016

Just a short life sign: Still not much progress. I'm still working on making it compile and I really hope when it compiles it will also run. There were so many changes to nan...

@Joeasaurus

This comment has been minimized.

Show comment
Hide comment
@Joeasaurus

Joeasaurus Jan 8, 2016

Stay strong brother!

Joeasaurus commented Jan 8, 2016

Stay strong brother!

@FrontierPsychiatrist

This comment has been minimized.

Show comment
Hide comment
@FrontierPsychiatrist

FrontierPsychiatrist Apr 7, 2016

Owner

Since it's been a while: I haven't found time to work on this, especially after the need to travel for my work. I'm really sorry for that and hope that soon there will be more time.

Owner

FrontierPsychiatrist commented Apr 7, 2016

Since it's been a while: I haven't found time to work on this, especially after the need to travel for my work. I'm really sorry for that and hope that soon there will be more time.

@clarkieryan

This comment has been minimized.

Show comment
Hide comment
@clarkieryan

clarkieryan Apr 26, 2016

I've had a look into this - looks as if it's the nan library - had a shot at upgrading to the latest version - but they have changed their namespacing - so really not sure what to do? I'm happy to help if you have any pointers!

clarkieryan commented Apr 26, 2016

I've had a look into this - looks as if it's the nan library - had a shot at upgrading to the latest version - but they have changed their namespacing - so really not sure what to do? I'm happy to help if you have any pointers!

@FrontierPsychiatrist

This comment has been minimized.

Show comment
Hide comment
@FrontierPsychiatrist

FrontierPsychiatrist Apr 27, 2016

Owner

@clarkieryan basically all nan usages have to be changed, sadly it's not just adding the Nan:: namespace. A lot of functions have different signatures. Maybe I can push my migration branch later.

Owner

FrontierPsychiatrist commented Apr 27, 2016

@clarkieryan basically all nan usages have to be changed, sadly it's not just adding the Nan:: namespace. A lot of functions have different signatures. Maybe I can push my migration branch later.

@crwgregory

This comment has been minimized.

Show comment
Hide comment
@crwgregory

crwgregory Apr 27, 2016

I am also willing to lend a helping hand.

crwgregory commented Apr 27, 2016

I am also willing to lend a helping hand.

@clarkieryan

This comment has been minimized.

Show comment
Hide comment
@clarkieryan

clarkieryan Apr 28, 2016

@FrontierPsychiatrist If you could push that branch that would be helpfull :)

clarkieryan commented Apr 28, 2016

@FrontierPsychiatrist If you could push that branch that would be helpfull :)

@FrontierPsychiatrist

This comment has been minimized.

Show comment
Hide comment
@FrontierPsychiatrist

FrontierPsychiatrist Apr 28, 2016

Owner

So I pushed https://github.com/FrontierPsychiatrist/node-spotify/tree/nan-2.1

As you can see, my last commit before today was start of December. I had some uncommited stuff from during December lying around.

I might want to restart clean at some point., these commits were not meant to be published, my plan was to squash them. Have a look at it and if you can manage to get more stuff to compile, feel free to open a PR (be sure to make it point to nan-2.1). Please add why you changed something to compile in the commit messages (unlike I did!).

Oh, and by the way, end of week I don't need to travel anymore for work - maybe that will result in a fresh start.

Owner

FrontierPsychiatrist commented Apr 28, 2016

So I pushed https://github.com/FrontierPsychiatrist/node-spotify/tree/nan-2.1

As you can see, my last commit before today was start of December. I had some uncommited stuff from during December lying around.

I might want to restart clean at some point., these commits were not meant to be published, my plan was to squash them. Have a look at it and if you can manage to get more stuff to compile, feel free to open a PR (be sure to make it point to nan-2.1). Please add why you changed something to compile in the commit messages (unlike I did!).

Oh, and by the way, end of week I don't need to travel anymore for work - maybe that will result in a fresh start.

@FrontierPsychiatrist

This comment has been minimized.

Show comment
Hide comment
@FrontierPsychiatrist

FrontierPsychiatrist May 27, 2016

Owner

So, https://github.com/FrontierPsychiatrist/node-spotify/tree/nan-2.3 just happened - at least it compiles. I almost don't dare to start it...

Owner

FrontierPsychiatrist commented May 27, 2016

So, https://github.com/FrontierPsychiatrist/node-spotify/tree/nan-2.3 just happened - at least it compiles. I almost don't dare to start it...

@FrontierPsychiatrist

This comment has been minimized.

Show comment
Hide comment
@FrontierPsychiatrist

FrontierPsychiatrist May 27, 2016

Owner

Well, my tests don't exactly run, but in the REPL I can load a track just fine (node 5.3.0).

Anyone interested, please check the branch out and give it a try and tell me what goes wrong, that would be very helpful.

Owner

FrontierPsychiatrist commented May 27, 2016

Well, my tests don't exactly run, but in the REPL I can load a track just fine (node 5.3.0).

Anyone interested, please check the branch out and give it a try and tell me what goes wrong, that would be very helpful.

@FrontierPsychiatrist

This comment has been minimized.

Show comment
Hide comment
@FrontierPsychiatrist

FrontierPsychiatrist May 28, 2016

Owner

On OSX I had to do a xcode-select --install, otherwise it wouldn't find the libspotify header file in /usr/local. Probably not a problem with node, but if someone gets the error that #include <libspotify/api.h> is not working on OSX even though it is installed (e.g. brew install libspotify), do that.

Source http://stackoverflow.com/questions/23905661/on-mac-g-clang-fails-to-search-usr-local-include-and-usr-local-lib-by-def

Owner

FrontierPsychiatrist commented May 28, 2016

On OSX I had to do a xcode-select --install, otherwise it wouldn't find the libspotify header file in /usr/local. Probably not a problem with node, but if someone gets the error that #include <libspotify/api.h> is not working on OSX even though it is installed (e.g. brew install libspotify), do that.

Source http://stackoverflow.com/questions/23905661/on-mac-g-clang-fails-to-search-usr-local-include-and-usr-local-lib-by-def

@FrontierPsychiatrist

This comment has been minimized.

Show comment
Hide comment
@FrontierPsychiatrist

FrontierPsychiatrist Jun 2, 2016

Owner

Yesterday I changed some more things (mostly Handle<T> to Local<T>) and node-spotify now compiles on 0.10, 0.12, 5.0, 6.0 (I guess 4.0 to). 0.8 doesn't have libuv's uv_cond_t, I'm leaving that out for now.

After I look a little bit more into why my tests don't run I will publish 0.7.2 with the updated Nan. A next good step would then be to look into #89 to make the binary distribution easier.

Owner

FrontierPsychiatrist commented Jun 2, 2016

Yesterday I changed some more things (mostly Handle<T> to Local<T>) and node-spotify now compiles on 0.10, 0.12, 5.0, 6.0 (I guess 4.0 to). 0.8 doesn't have libuv's uv_cond_t, I'm leaving that out for now.

After I look a little bit more into why my tests don't run I will publish 0.7.2 with the updated Nan. A next good step would then be to look into #89 to make the binary distribution easier.

@FrontierPsychiatrist

This comment has been minimized.

Show comment
Hide comment
@FrontierPsychiatrist

FrontierPsychiatrist Jun 5, 2016

Owner

I pushed 0.7.2 to npm with nan 2.3.5.

There is one regression, spotify.player.off does not do anything anymore (see here:

//SessionCallbacks::endOfTrackCallback = Nan::Callback();
and #100).

Owner

FrontierPsychiatrist commented Jun 5, 2016

I pushed 0.7.2 to npm with nan 2.3.5.

There is one regression, spotify.player.off does not do anything anymore (see here:

//SessionCallbacks::endOfTrackCallback = Nan::Callback();
and #100).

@philipbulley

This comment has been minimized.

Show comment
Hide comment
@philipbulley

philipbulley Jun 5, 2016

Working well for me under node 6.2.1... I can finally ditch 0.12.7 which also means that iron-node can be used to debug my app! ⚡️ Thanks for fixing!

philipbulley commented Jun 5, 2016

Working well for me under node 6.2.1... I can finally ditch 0.12.7 which also means that iron-node can be used to debug my app! ⚡️ Thanks for fixing!

@Joeasaurus

This comment has been minimized.

Show comment
Hide comment
@Joeasaurus

Joeasaurus Jun 7, 2016

Thanks guys!!!

Joeasaurus commented Jun 7, 2016

Thanks guys!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment