libc++.gyp makes (wrong) assumption about directory depth. |
|||||
Issue description
I use a custom workflow where I compile V8 with
GYP_DEFINES="v8_enable_disassembler=1 v8_object_print=1 release_extra_cflags='-ggdb3' use_goma=1 clang=1 msan=1 target_arch=x64 v8_target_arch=arm64" GYP_GENERATOR_FLAGS="output_dir=out/msan" gclient runhooks
ninja -j1024 -C out/msan/Release d8
Compiling with msan has a dependency on the custom libc++ in v8/buildtools. Compiling libc++ fails. The reason is that the variable for the directory root is set like this:
'libcxx_root': '../../buildtools/third_party/libc++',
However, given my output dir structure (out/msan/Release instead of out/Release), the directory depth is 3 instead of the assumed 2.
Changing the variable to
'libcxx_root': '<(DEPTH)/buildtools/third_party/libc++',
fixes the issue.
I have no idea how to upstream this change though... help?
,
Apr 30 2016
I simply have no idea how to upstream the change, which is why I filed this issue :)
,
May 2 2016
I _think_ there should have been a reason for having that '../..' instead of '<(DEPTH)', but I don't remember for sure. Maybe there were problems with different build systems. Maybe there weren't. I'll give this a shot.
,
May 2 2016
Wait. <(DEPTH) is a path relative to GYP file, not the build directory. Unless we're in a gypi file, there's no point in using <(DEPTH), I even heard it was discouraged. I have also noticed that Chrome recently got GYP disabled by default. This may have not happened for MSan yet, but it will happen sooner or later.
,
May 2 2016
V8 still uses and relies on GYP, and this will not change until node.js has moved away from GYP. I'm not too familiar with GYP myself, but changing ../.. to <(DEPTH) indeed fixes the issue for me.
,
May 2 2016
Ah, I see now. That's because buildtools/ resides under v8/ now, i.e. libc++ descends one level deeper in the directory tree. It should not actually have anything to do with your output dir structure.
,
May 2 2016
+Michael, who touched the code in question. I've tried to replace the dots with '<(DEPTH)' (see https://codereview.chromium.org/1934993003/), but it broke MSan build for me.
,
May 2 2016
,
May 2 2016
Your chromium trybot is not (yet) usable for buildtools patches. Lemme fix that.
,
May 2 2016
Sorry, to be clear, that change also breaks MSan for me locally, so this does not seem to be an option. (thanks for fixing the bots anyway)
,
May 2 2016
Right, but just as a showcase that these bots function now infra-wise: Could you rebase and reup the patch and trigger the bot again?
,
May 2 2016
Done, will be watching the bots.
,
May 2 2016
Can I see which CL it is?
,
May 2 2016
,
May 2 2016
I don't think youv'e rebased the CL - at least I don't see PROJECT field.
,
May 2 2016
I can try to reup the CL myself and see...
,
May 3 2017
This issue has been available for more than 365 days, and should be re-evaluated. Please re-triage this issue. The Hotlist-Recharge-Cold label is applied for tracking purposes, and should not be removed after re-triaging the issue. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
May 3 2017
This'll be obsolete with gyp deprecation... |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by euge...@chromium.org
, Apr 29 2016