Skip to content

opencv-build as a peer dependency#617

Closed
hypersolution1 wants to merge 1 commit into
justadudewhohacks:masterfrom
hypersolution1:peerdependency_opencv-build
Closed

opencv-build as a peer dependency#617
hypersolution1 wants to merge 1 commit into
justadudewhohacks:masterfrom
hypersolution1:peerdependency_opencv-build

Conversation

@hypersolution1

Copy link
Copy Markdown

Instead of relying on the OPENCV4NODEJS_DISABLE_AUTOBUILD environment variable, a user could choose to auto build by installing opencv-build along with opencv4nodejs:

npm i opencv-build
npm i opencv4nodejs

@justadudewhohacks

Copy link
Copy Markdown
Owner

Hmm, now this duplicates a lot of code from the opencv-build package into the install scripts of opencv4nodejs.

We could read the OPENCV4NODEJS_DISABLE_AUTOBUILD variable from a projects package.json, similar to what @wouldgo has done in this PR: justadudewhohacks/npm-opencv-build#35. This way one does not have to keep an additional environment variable on the system.

@hypersolution1

Copy link
Copy Markdown
Author

Yes, a setting in package.json would be better than the environment variable. It is still cumbersome a bit. An ideal way would be to avoid any configuration when adding this package into a project.

I just thought of another approach. How about detecting if OpenCV is installed and perform the auto build if it doesn't find it?

I avoided having to modify opencv-build but I could refactor to remove duplicate code if you are interested by this pull request.

@justadudewhohacks

Copy link
Copy Markdown
Owner

Yes, a setting in package.json would be better than the environment variable. It is still cumbersome a bit. An ideal way would be to avoid any configuration when adding this package into a project.

On the one hand I agree, on the other hand people who do not have OpenCV installed and configured on their system can come up with the same argument. So I think I would rather allow to set this env in the package.json, rather than completely removing it.

I just thought of another approach. How about detecting if OpenCV is installed and perform the auto build if it doesn't find it?

Actually that's not possible. On Linux and Mac one can look for the OpenCV libraries in the default directories of your system and that's what it is already doing when the auto build is disabled and the environments pointing to the opencv include, library and binary dirs are not set. On windows it is simply not possible.

Now that I think of it, it would be nice to also be able to set the environments pointing to your opencv include, library and binary dirs via package.json.

@hypersolution1

Copy link
Copy Markdown
Author

If it is not possible on windows, regardless of an existing OpenCV installed, we could always auto-build by default.

But if this could work on Linux/OSX, then I don't see why this couldn't be the default behaviour on these platforms. I would prefer the same behaviour for all platform but I don't see any problem having it different.

On windows: the library behaves as actually.
On Linux/OSX: the library looks for an installed version and fallback to autobuild if not.

@goulash1971

goulash1971 commented Aug 31, 2019

Copy link
Copy Markdown

@justadudewhohacks for me that solution (controlling behaviour with env vars in the package.json (overriding anything in the environment or performing a substitution aka. ${OTHER_ENV_VAR}) is perfect ...

Our solution is built on OSX (multiple developers with multiple different installed OpenCV configs) and installed on RPi’s ... whilst Docker is used to give “container level consistency” because of the nature of the solution there can be no guaruntee that a) OpenCV will be installed, b) it will be installed at a specific location, c) it will have been built with specific flags etc. ...

So having a specific auto-build (or discovery) behaviour set as env vars in the package.json removes the need for various wrapper scripts (currently we do it all in gulp) or makes it simpler.

One other option, which I’m happy to contribute to, is to allow a customer “installer” to be specified that can be invoked as an alternative/wrapper for the existing env variable driven installer.

That maybe overkill though 😬

@justadudewhohacks

Copy link
Copy Markdown
Owner

On Linux/OSX: the library looks for an installed version and fallback to autobuild if not.

The problem is, that people having OpenCV libraries sitting around on their machine will not be able to use the auto build anymore. There are cases, where you might want to have different local OpenCV installations for each project.

I avoided having to modify opencv-build but I could refactor to remove duplicate code if you are interested by this pull request.

I think the only way to avoid duplicate code when making opencv-build a peer dependency is to move all the shared code between opencv-build and opencv4nodejs to a seperate package, which both packages depend on.

@hypersolution1

Copy link
Copy Markdown
Author

For sure there is no solution that will address every uses cases. Does someone that have already opencv installed want to use a separate build is a common case? My concern is to prioritize the general use case first, then address edge cases.

Install OpenCV with a package system (apt, yum) seems straightforward to me and fast.

If OpenCV libs are already installed, why not using it? If not, what are the possible solution for this case: uninstall OpenCV or a FORCE_AUTOBUILD flag somewhere (env or package.json)

Anyway, I'll close this pull request and if I have time down the road I'll might try for a autodetect version eventually.

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.

3 participants