autobuild flags in package.json

[wouldgo]

Hi,

thanks for making opencv compilation process easy to handle. For a personal need (pkg-config file creation), I have to enable some cmake flags. I already saw the autobuild flags way but setting an environment variable before the install script was run did not propagate the variables to the install.js script.
What I did was something like:

{
  "name": "my-project-where-i-ll-using-opencv",
  "scripts": {
    "preinstall": "export OPENCV4NODEJS_AUTOBUILD_FLAGS='...'"
}

I also tried this way:

{
  "name": "my-project-where-i-ll-using-opencv",
  "scripts": {
    "install": "export OPENCV4NODEJS_AUTOBUILD_FLAGS='...'"
}

In the first scenario, nothing was read by the build process; in the latter "install" stage is before the installation of dependencies so it's useless.

In the end, I choose for setting the flags into dependent package.json like this:

{
  "name": "my-project-where-i-ll-using-opencv",
  "opencv4nodejs": {
      "flags": "-DOPENCV_GENERATE_PKGCONFIG=ON -DOPENCV_PC_FILE_NAME=opencv.pc"
  }
}

This PR bring this logic into the module setting the environment variable following the "flags" property.

Hope this will help as helped me.
Have a great day

Dario

[justadudewhohacks]

Thanks for the PR, this looks like a reasonable feature for me, although I do not quite understand, why setting the proper env doesn't work for you.

Two things:

1. Could you rename the variable from flags to autoBuildFlags or something more descriptive.
2. You wanna make the changes in install.ts as your changes in the install.js file will get overwritten when compiling this with typescript.

[wouldgo]

Using this module as a dependency unless I set the environment variable before running the npm install command the values are not set when here are read.

Agree with the consistency issue you raise on flags property, and sorry for did not spot the install.ts file :|.

[justadudewhohacks]

Sorry for the delay. PR looks good to me!

[wouldgo]

Any plan to publish on registry?

[justadudewhohacks]

I will publish it in the coming days along with the PR that's currently open at opencv4nodejs.

[justadudewhohacks]

Your feature is now published to npm.

[zaquest]

Hello. It would be nice check if there's package.json at all before trying to require. It is legal to npm install package in any random directory, which might not be a package directory containing package.json. And I'd say even common for packages that have exectable scripts, when you dont want to npm install -g. In particular this hit me in docker where I preinstall opencv4nodejs to avoid rebuilding OpenCV on every docker image build.
A workaround is to create package.json containing {}, in case anyone else has the same issue.

[wouldgo]

@zaquest I completely forgot that a package can be installed even outside a package.json containing folder. Imho checking the existance of package.json it's trivial and the right way. If it's absent the overall feature that I've implement makes no sense so has to be skipped.

@justadudewhohacks seems fair to you?

[justadudewhohacks]

@justadudewhohacks seems fair to you?

Definitely. I mean it doesn't hurt to include that check.

[justadudewhohacks]

@wouldgo I am currently working on modifying the script, such that we can also set other environments via package.json.

I think the behaviour should be such, that when an env is set via package.json, it should override the process environment variable and not merge both definitions as done with process.env.OPENCV4NODEJS_AUTOBUILD_FLAGS currently.

Does that conflict with your initial incentives / did you implement it on purpose that way?

[wouldgo]

@justadudewhohacks I merged because the defaults were ok for what I need. But If it's possible to declare back the same cmake configurations I have no problems in what your're doing.