node-v8
node-v8 copied to clipboard
nodejs
Build error on SmartOS
https://ci.nodejs.org/job/node-test-commit-smartos/45420/nodes=smartos20-64/console
10:51:08 In file included from ../deps/v8/src/base/free_deleter.h:15,
10:51:08 from ../deps/v8/src/base/debug/stack_trace_posix.cc:41:
10:51:08 ../deps/v8/src/base/platform/memory.h: In function 'std::size_t v8::base::MallocUsableSize(void*)':
10:51:08 ../deps/v8/src/base/platform/memory.h:118:10: error: 'malloc_usable_size' was not declared in this scope; did you mean 'MallocUsableSize'?
10:51:08 118 | return malloc_usable_size(ptr);
10:51:08 | ^~~~~~~~~~~~~~~~~~
10:51:08 | MallocUsableSize
10:51:08 make[2]: *** [tools/v8_gypfiles/v8_libbase.target.mk:177: /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos20-64/out/Release/obj.target/v8_libbase/deps/v8/src/base/debug/stack_trace_posix.o] Error 1
@nodejs/platform-smartos
Hi @targos, I'm struggling to find where this is coming from - GitHub search across the entire nodejs and v8 organisations show no code matches for malloc_usable_size(). It's not something we have in SmartOS, and many other platforms don't have it either, so the fix should be pretty straight-forward, I just can't figure out where it's actually being used.
I'd be more than happy to submit a fix for it if you could help me get up to speed on which repositories that test job is pulling from. Thanks!
In the V8 repo: https://github.com/v8/v8/blob/50504b168d2c364d49e3332cbd934dc60a4bfad8/src/base/platform/memory.h#L117-L118
Introduced by https://chromium-review.googlesource.com/c/v8/v8/+/3858226
In this repo (canary is updated every day so the commit is ephemeral): https://github.com/nodejs/node-v8/blob/4e97ad7ae1c6d4639f227094f3270de44214c4f1/deps/v8/src/base/platform/memory.h#L117-L118
Ah, ok, thanks! Looks like GitHub search just completely doesn't work.
The fix looks straight-forward (add V8_OS_SOLARIS to the exclusion list for V8_HAS_MALLOC_USABLE_SIZE on line 26, similar to AIX) but I assume I'll need to get this through the v8 contribution process first.
The fix looks straight-forward (add V8_OS_SOLARIS to the exclusion list for V8_HAS_MALLOC_USABLE_SIZE on line 26, similar to AIX) but I assume I'll need to get this through the v8 contribution process first.
I'm floating a patch for now in https://github.com/nodejs/node/pull/44741 but let's keep this issue open until it's upstreamed.
@bnoordhuis Ok, I'm looking into the CLA requirement. It's not clear to me that MNX is the most appropriate agent to contribute the patch, since we are not the author/copyright holder.