opencv4nodejs icon indicating copy to clipboard operation
opencv4nodejs copied to clipboard

`build-opencv` does not handle correctly directory paths whose absolute paths contain a space.

Open issuefiler opened this issue 2 years ago • 6 comments

If the absolute path to the .hpp header directory (the absolute path resolved from npx build-opencv’s --incDir) have a space (“ ”) in it, it is not written wholly into the <AdditionalIncludeDirectories> of the file (The project directory)/node_modules/@u4/opencv4nodejs/build/opencv4nodejs.vcxproj (It’s cut off halfway.) and the build, automatic or not, fails.

issuefiler avatar Nov 14 '22 17:11 issuefiler

You see dozens of issues along the lines of “.hpp not found; I can’t build,” right? I suppose this is the very cause.

issuefiler avatar Nov 14 '22 17:11 issuefiler

Try to provide me with more context, check the dockerfiles and the GitHub actions files, they contain lots of build samples.

UrielCh avatar Nov 15 '22 08:11 UrielCh

It’s not that I’m saying I cannot build OpenCV to get this package work. It works now. I’m just reporting a bug in build-opencv that has been causing trouble to many people.

Context

I downloaded OpenCV 4.6.0 from the OpenCV website, unzipped, and placed it in the project directory B:/a/b b/p/.

Then I disabled the automatic build with the package.json settings,

	"opencv4nodejs": {
		"disableAutoBuild": true
	}

installed this package with npm install @u4/opencv4nodejs,

and manually built OpenCV for this package to work, with the following command on Windows’ command prompt.

npx build-opencv rebuild
	--version 4.6.0
	--cuda
	--nocontrib
	--nobuild
	--binDir "opencv-4.6.0/build/x64/vc15/bin/"
	--libDir "opencv-4.6.0/build/x64/vc15/lib/"
	--incDir "opencv-4.6.0/build/include/"
	--keepsources

build-opencv couldn’t locate the .hpp files for building, emitting the 'opencv2/core.hpp' file not found message.

I checked out the GitHub issues, noticed many people suffering this issue (https://github.com/UrielCh/opencv4nodejs/issues/16, https://github.com/UrielCh/opencv4nodejs/issues/30, https://github.com/UrielCh/opencv4nodejs/issues/42, https://github.com/justadudewhohacks/opencv4nodejs/issues/396, https://github.com/justadudewhohacks/opencv4nodejs/issues/415, https://github.com/justadudewhohacks/opencv4nodejs/issues/591, https://github.com/justadudewhohacks/opencv4nodejs/issues/618, https://github.com/justadudewhohacks/opencv4nodejs/issues/645, https://github.com/justadudewhohacks/opencv4nodejs/issues/818, et cetera.).

I looked into the B:/a/b b/p/node_modules/@u4/opencv4nodejs/build/opencv4nodejs.vcxproj file that build-opencv generated during the build, as I wondered why it couldn’t find the .hpp files. It turned out to be because, in the <AdditionalIncludeDirectories> section of the .vcxproj file, it had B:/a/b, which is a malformed part of the complete path broken by a space character (“ ”), for the include directory, when there should’ve been the complete path, B:/a/b b/p/opencv-4.6.0/build/include/ written.

I renamed the ancestor directories of the specified include directory to get rid of the spaces (“ ”), so that the absolute path of the include directory had no space in it (e.g. renaming the directory b b to b_b), executed the command to build OpenCV above again, and only after that, it successfully built OpenCV.

Bug

build-opencv does not take an include directory whose absolute path contains a space (“ ”).

issuefiler avatar Nov 15 '22 16:11 issuefiler

I’ve figured out the reason why this happens.

The cause of this bug

When you run the command above (npx build-opencv…),

https://github.com/UrielCh/opencv4nodejs/blob/66d7d42abb17ae5c5f8b5cfe6bf7cb5409baf24a/package.json#L31-L33 https://github.com/UrielCh/opencv4nodejs/blob/66d7d42abb17ae5c5f8b5cfe6bf7cb5409baf24a/bin/install.js#L1-L4 https://github.com/UrielCh/opencv4nodejs/blob/66d7d42abb17ae5c5f8b5cfe6bf7cb5409baf24a/install/compileLib.js#L195-L197 https://github.com/UrielCh/opencv4nodejs/blob/66d7d42abb17ae5c5f8b5cfe6bf7cb5409baf24a/install/compileLib.js#L303

The opencv4nodejs/install/compileLib.js executes shell commands that are similar to this:

$Env:OPENCV4NODEJS_DEFINES="OPENCV4NODEJS_FOUND_LIBRARY_WORLD"
$Env:OPENCV4NODEJS_INCLUDES="B:/a/b b/p/opencv-4.6.0/build/include"
$Env:OPENCV4NODEJS_LIBRARIES="B:/a/b b/p/opencv-4.6.0/build/x64/vc15/lib/opencv_world460.lib"

cd B:\a\b b\p\node_modules\@u4\opencv4nodejs
node-gyp rebuild --jobs max --release

. Now node-gyp refers to the opencv4nodejs/binding.gyp file,

https://github.com/UrielCh/opencv4nodejs/blob/66d7d42abb17ae5c5f8b5cfe6bf7cb5409baf24a/binding.gyp#L4-L16

and performs the GYP command expansions, taking spaces as separators, for the <!@(…) parts.

Command Expansions (<!, <!@)

…In a command expansion, the entire string contained within the parentheses is passed to the system’s shell. The command’s output is assigned to a string value that may subsequently be expanded in list context in the same way as variable expansions if an @ character is used.

Variable Expansions (<, >, <@, >@)

…The conversion into a list is generator-specific, but generally, spaces in the string are taken as separators between list items.

The opencv4nodejs/install/parseEnv.js simply prints the string in the specified environmental variable.

https://github.com/UrielCh/opencv4nodejs/blob/66d7d42abb17ae5c5f8b5cfe6bf7cb5409baf24a/install/parseEnv.js#L1-L6

Which means, the output strings B:/a/b b/p/opencv-4.6.0/build/include (from the OPENCV4NODEJS_INCLUDES variable) and B:/a/b b/p/opencv-4.6.0/build/x64/vc15/lib/opencv_world460.lib (from the OPENCV4NODEJS_LIBRARIES variable), which are containing spaces, are expanded into lists, with the spaces being the list item separators.

This results in

"include_dirs" : [
	"B:/a/b",
	"b/p/opencv-4.6.0/build/include",
	"cc",
	"cc/core",
	…

when it’s supposed to be

"include_dirs" : [
	"B:/a/b b/p/opencv-4.6.0/build/include",
	"cc",
	"cc/core",
	…

.

issuefiler avatar Nov 20 '22 07:11 issuefiler

Fix suggestion

Variable Expansions (<, >, <@, >@)

If VAR is a string, the string is converted to a list which is inserted into the list in which expansion is taking place as above. The conversion into a list is generator-specific, but generally, spaces in the string are taken as separators between list items. The specific method of converting the string to a list should be the inverse of the encoding method used to expand list variables in string context, above.

It’s saying that GYP’s string-to-list expansion is the inverse of its list-to-string encoding. How would GYP encode list items that contain spaces? I expect that it would enclose such list items in double quotes ("…").

I tried modifying the opencv4nodejs/install/parseEnv.js file so that it prints strings enclosed in double quotes. This way, the GYP command expansions (<!@(…)) don’t break each of the output lines into multiple list items separating it by spaces ( ).

const envName = process.argv[2];
if (!envName) {
    throw new Error('no env name passed to parseEnv');
}
const outputs = (process.env[envName] || '').split(/[\n;]/);
outputs.forEach(o => console.log(`"${o}"`)); // <-- The modified line

The GYP command expansions were performed as expected, it succeeded to build regardless of the presence of spaces in the OPENCV4NODEJS_INCLUDES & OPENCV4NODEJS_LIBRARIES.

issuefiler avatar Nov 20 '22 08:11 issuefiler

TL; DR

A slight modification

https://github.com/UrielCh/opencv4nodejs/blob/66d7d42abb17ae5c5f8b5cfe6bf7cb5409baf24a/install/parseEnv.js#L6

to

outputs.forEach(o => console.log(`"${o}"`));

will correct node-gyp’s "include_dirs" and "libraries" list generation,

and resolve the following issues:

  • https://github.com/UrielCh/opencv4nodejs/issues/16,
  • https://github.com/UrielCh/opencv4nodejs/issues/30,
  • https://github.com/UrielCh/opencv4nodejs/issues/42,
  • https://github.com/justadudewhohacks/opencv4nodejs/issues/396,
  • https://github.com/justadudewhohacks/opencv4nodejs/issues/415,
  • https://github.com/justadudewhohacks/opencv4nodejs/issues/591,
  • https://github.com/justadudewhohacks/opencv4nodejs/issues/618,
  • https://github.com/justadudewhohacks/opencv4nodejs/issues/645,
  • https://github.com/justadudewhohacks/opencv4nodejs/issues/818,
  • et cetera.

issuefiler avatar Nov 20 '22 18:11 issuefiler