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

Add pre compiled binaries for windows, mac and linux #188

Merged
merged 2 commits into from Feb 11, 2015

Conversation

edgarsilva
Copy link
Contributor

Adds pre compiled binaries for all 3 major platforms using node-pre-gyp. This is the same thing we did and are using in node-serialport, has worked great so far.

@peterbraden after merging I do need your help to generate the secure keys in travis and app-veyor (since they are repo and account bound respectively). We also need to create a new branch for OSX pre-compiled binaries called osx-binaries, since travis does not support multiple OS builds, so we need a different branch to compile for OSX.

I'm creating a pre-compiled-binaries.md file to describe the process, I'll also add a make task to make it easier to generate the binaries with each release.

BTW we at The Hybrid Group (hybridgroup.com) are sponsoring the AS3 buckets, we also do it for node-serialport (as mentioned above) and we'll be adding them for node-gamepad.

Regards,

@edgarsilva
Copy link
Contributor Author

Hi @peterbraden,

Let me know if you have any doubts or there is anything I can do to help this moving fwd.

Regards,

@peterbraden
Copy link
Owner

Hi, sorry for the delay, this looks awesome but I haven't had a chance to check it out yet. I'll try and find some time at the weekend to take a look.

Thanks!

@edgarsilva
Copy link
Contributor Author

Hey that sounds great!

Just let me know the time, so we can coordinate, because you won't be able to publish until the secure env variables for AS3 have been setup, that way I can pass them along so you can encrypt for travis and appveyor.

You should be able to compile just fine though, in travis and appveyor.

@dkniffin
Copy link

I may be completely misunderstanding this pull request, but will this provide an easy way to do an npm install opencv on Windows machines, without requiring all the trouble of installing pkg-config, etc?

@edgarsilva
Copy link
Contributor Author

Yes, this will create pre-compiled binaries for all 3 major platforms (both x86 and amd64) using node-pre-gyp.

Once this is merged in doing:

$ npm install opencv
/
> opencv@1.0.0 install /home/workspace/testNpm/node_modules/opencv
> node-pre-gyp install --fallback-to-build

[opencv] Success: "/home/workspace/testNpm/node_modules/opencv/build/opencv/v1.0.0/Release/node-v11-linux-x64/opencv.node" is installed via remote
opencv@1.0.0 node_modules/opencv
├── buffers@0.1.1
├── nan@1.4.1
└── node-pre-gyp@0.5.31

As you can see it will install the binaries from the remote AS3 bucket instead of compiling them.

Although you still need to install opencv and add it to the PATH. But Chocolatey makes that pretty easy.

@dkniffin
Copy link

Awesome! 👍 I hope this gets merged soon

@peterbraden
Copy link
Owner

Sorry I've been slow with this - @oddityoverseer13 if you could pull this branch and tell me your experience on windows, it'd be great - I don't have access to a windows machine to test this on.

Still hoping to get to this soon

@dkniffin
Copy link

Yea, I can do that. I'm working on installing opencv via chocolatey now (@edgarsilva - thanks for mentioning choco. Hadn't heard of it before) After that, I'll install that branch and let you know how it goes.

@dkniffin
Copy link

Looks like it works:

out

All I had to do was install opencv via chocolatey, and then do npm install opencv.

@peterbraden does this still rely on Visual Studio being installed? I have it installed, but I'm wondering if it's necessary. My guess is no, since you're distributing binaries.

@edgarsilva
Copy link
Contributor Author

@oddityoverseer13 nope, since you are not compiling anything, you just need to install opencv and add it to the PATH, as you did, npm install will pull from remote and install node-opencv, sweet!

@peterbraden
Copy link
Owner

@edgarsilva one question I have - how does this handle fallback if a binary for the platform is unavailable ? Is it possible to override the binaries?

@edgarsilva
Copy link
Contributor Author

@peterbraden if there is no binary available for the platform it just fallbacks to try to compile. Binaries are stored by version, if we want to override one that has already been published for the same version, we need to either node-pre-gyp unpublish or go to the bucket and manually delete it.

@peterbraden
Copy link
Owner

Hi @edgarsilva - sorry for the delay on this - What do I need to do to get the travis build working again?

@edgarsilva
Copy link
Contributor Author

This is happening because the latest version of opencv in chocolatey for windows is a newer one than the one in in the ubuntu repos, let me add ad a validation to the bindings file to check for different versions in linux and windows.

@edgarsilva
Copy link
Contributor Author

@peterbraden does node-opencv still works with opencv 4.3.1? or what is the minimum opencv version?

@edgarsilva
Copy link
Contributor Author

Seems like I have fixed most of the issues, but tests are failing, do you have any idea what this is about:

$ npm test

> opencv@1.0.0 test /home/travis/build/hybridgroup/node-opencv

> node test/unit.js

TAP version 13

# Smoke tests / Can Import

libdc1394 error: Failed to initialize libdc1394

node: symbol lookup error: /home/travis/build/hybridgroup/node-opencv/build/opencv/v1.0.0/Release/node-v11-linux-x64/opencv.node: undefined symbol: _ZN7Calib3D4InitEN2v86HandleINS0_6ObjectEEE

npm ERR! Test failed. See above for more details.

npm ERR! not ok code 0

The command "npm test" failed and exited with 1 during .

Your build has been stopped.

Basically that means the pre-compiled is being generated and can be published, but tests fail.

@Queuecumber
Copy link
Collaborator

@edgarsilva Just quickly looking it over it might be something from the Calib3D module I added. Did you merge the most recent commits to master?

@edgarsilva
Copy link
Contributor Author

I did merge master like 30 mins ago, compilation was failing bfore that, now it works but tests fail, same behavior in my local if I build on my pc and run npm test.

I'm running opencv version 2.4.9.

@Queuecumber
Copy link
Collaborator

@edgarsilva Pretty weird stuff, I just ran npm test against my most recent pull request (#221) and I get all tests passed. It looks like I am on version 2.4.8, is it possible that one minor version change is causing the failure?

@edgarsilva
Copy link
Contributor Author

@Queuecumber I pulled your latest changes deom commit #221 and npm test still fails with the same error on my computer:

$ npm test

> opencv@1.0.0 test /home/edgar/workspace/node-opencv
> node test/unit.js

TAP version 13
# Smoke tests / Can Import
node: symbol lookup error: /home/edgar/workspace/node-opencv/build/opencv/v1.0.0/Release/node-v11-linux-x64/opencv.node: undefined symbol: _ZN7Calib3D4InitEN2v86HandleINS0_6ObjectEEE
npm ERR! Test failed.  See above for more details.
npm ERR! not ok code 0

Trying with older version next.

@Queuecumber
Copy link
Collaborator

@edgarsilva When the compilation is running, do you see the Calib3D.cc file being compiled? Is it listed in your bindings.gyp?

@edgarsilva
Copy link
Contributor Author

@Queuecumber nice catch! tests are now working on local, it seems I overwrote the file when merging master, and both Calib3D and ImgProc were missing from the targets list. Let me push to check in the CI tools.

@Queuecumber
Copy link
Collaborator

@edgarsilva Fantastic, that was a close one

@edgarsilva
Copy link
Contributor Author

Build works now but I need to fix an issue with windows pre-compiled binary for the new sources, I will merge a PR to our master branch in our fork soon. It should be reflected here.

BTW @Queuecumber I also added the changes included in your PR #221 to avoid any conflicts as you suggested above.

@Queuecumber
Copy link
Collaborator

@edgarsilva Ok so what's the best path forward in this situation, wait for #221 to be merged first and then have yours merged?

@edgarsilva
Copy link
Contributor Author

Once I fix the issue with the windows pre-compiled binaries and we merge this PR, #221 will change nothing, since those changes will be already included in here.

If we merge #221 first then this PR will not change that file, since they are the same, so both ways work, no biggie.

Custom os cflags, fixes pre compiled binaries
@edgarsilva
Copy link
Contributor Author

There was also an issue with node-pre-gyp in some windows envs (windows 7) for which I have pushed a fix. This is ready to be merged, @peterbraden let me know when you have time to coordinate so Ic an give you the AS3 credentials and help you setup the secret keys in both .travis and .appveyor.

This secret keys are repo bound in travis and account bound in appveyor, so I cannot do it myself. We will also need to create a new osx-binaries branch to generate the pre-compiled binaries for OSX.

@peterbraden
Copy link
Owner

Yep, how about tomorrow evening CET? I should be able to grab some time then. Thanks!

@edgarsilva
Copy link
Contributor Author

@peterbraden I can make some time from 5:00pm onwards CET(which is arounf 10am my time, Central US Time), any day. Just let me know to schedule it. Tomorrow sounds good for me.

@peterbraden
Copy link
Owner

Great, I'll be around for the next few hours, gimme a ping when is good for you.

@edgarsilva
Copy link
Contributor Author

Hey @peterbraden I'm here, just saw your email you can contact me here on hangouts to set this up, using my github email edgar.osc@gmail.com.

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

4 participants