OpenVR triggers duplicate symbol errors in json code |
|||||||||
Issue descriptionOne of my standard test-build configurations is a debug non-component build, and apparently this is a lightly tested configuration because it recently broke. Specifically, I get these errors: FAILED: chrome_child.dll chrome_child.dll.lib chrome_child.dll.pdb ninja -t msvc -e environment.x86 -- ../../third_party/llvm-build/Release+Asserts/bin/lld-link.exe /nologo /IMPLIB:./chrome_child.dll.lib /DLL /OUT:./chrome_child.dll /PDB:./chrome_child.dll.pdb @./chrome_child.dll.rsp lld-link: error: duplicate symbol: "static __int64 const Json::Value::maxLargestInt" (?maxLargestInt@Value@Json@@2_JB) in obj/third_party/jsoncpp/jsoncpp/json_value.obj and in openvr_api.lib(jsoncpp.obj) lld-link: error: duplicate symbol: "static unsigned __int64 const Json::Value::maxLargestUInt" (?maxLargestUInt@Value@Json@@2_KB) in obj/third_party/jsoncpp/jsoncpp/json_value.obj and in openvr_api.lib(jsoncpp.obj) lld-link: error: duplicate symbol: "static __int64 const Json::Value::minLargestInt" (?minLargestInt@Value@Json@@2_JB) in obj/third_party/jsoncpp/jsoncpp/json_value.obj and in openvr_api.lib(jsoncpp.obj) lld-link: error: duplicate symbol: "static int const Json::Value::maxInt" (?maxInt@Value@Json@@2HB) in obj/third_party/jsoncpp/jsoncpp/json_value.obj and in openvr_api.lib(jsoncpp.obj) lld-link: error: duplicate symbol: "static class Json::Value const &Json::Value::null" (?null@Value@Json@@2ABV12@B) in obj/third_party/jsoncpp/jsoncpp/json_value.obj and in openvr_api.lib(jsoncpp.obj) lld-link: error: duplicate symbol: "static int const Json::Value::minInt" (?minInt@Value@Json@@2HB) in obj/third_party/jsoncpp/jsoncpp/json_value.obj and in openvr_api.lib(jsoncpp.obj) lld-link: error: duplicate symbol: "static unsigned int const Json::Value::maxUInt" (?maxUInt@Value@Json@@2IB) in obj/third_party/jsoncpp/jsoncpp/json_value.obj and in openvr_api.lib(jsoncpp.obj) lld-link: error: duplicate symbol: "static __int64 const Json::Value::minInt64" (?minInt64@Value@Json@@2_JB) in obj/third_party/jsoncpp/jsoncpp/json_value.obj and in openvr_api.lib(jsoncpp.obj) lld-link: error: duplicate symbol: "static __int64 const Json::Value::maxInt64" (?maxInt64@Value@Json@@2_JB) in obj/third_party/jsoncpp/jsoncpp/json_value.obj and in openvr_api.lib(jsoncpp.obj) lld-link: error: duplicate symbol: "static unsigned __int64 const Json::Value::maxUInt64" (?maxUInt64@Value@Json@@2_KB) in obj/third_party/jsoncpp/jsoncpp/json_value.obj and in openvr_api.lib(jsoncpp.obj) ninja: build stopped: subcommand failed. These are my gn args: use_goma = true is_component_build = false is_debug = true enable_nacl = false remove_webcore_debug_symbols = true target_cpu = "x86" This is at 722649c8bc057a1601d94efb57ffa09270063730, from Oct 16 2018. The duplicate definitions of these class constants come from these two source files: third_party\jsoncpp\overrides\src\lib_json\json_value.cpp third_party\openvr\src\src\jsoncpp.cpp I could not find which recent changes triggered this behavior, but I know that this worked a few weeks ago. Clearly jsoncpp.cpp and json_value.cpp cannot be linked together. I don't know which one is supposed to be pulled in to chrome_child.dll, so assigning to an openvr developer to adjudicate.
,
Oct 22
Have a patch that builds - doing some testing before reviewing.
,
Oct 22
,
Oct 23
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2f969bff5532233afff5564f947b453b5dac6be6 commit 2f969bff5532233afff5564f947b453b5dac6be6 Author: Bill Orr <billorr@chromium.org> Date: Tue Oct 23 18:33:04 2018 Remove duplicate jsoncpp from OpenVR OpenVR by default includes its own jsoncpp. However, we have one in third_party already, so we don't need OpenVR's copy. BUG= 780987 , 790814 , 896087 Change-Id: Iac57998856ac72b36b6b731b3860ca67ae7c3f45 Reviewed-on: https://chromium-review.googlesource.com/c/1294669 Reviewed-by: Klaus Weidner <klausw@chromium.org> Commit-Queue: Bill Orr <billorr@chromium.org> Cr-Commit-Position: refs/heads/master@{#602033} [modify] https://crrev.com/2f969bff5532233afff5564f947b453b5dac6be6/device/vr/BUILD.gn [modify] https://crrev.com/2f969bff5532233afff5564f947b453b5dac6be6/third_party/openvr/BUILD.gn [modify] https://crrev.com/2f969bff5532233afff5564f947b453b5dac6be6/third_party/openvr/README.chromium [delete] https://crrev.com/af38308b7112bdfbc0cfcc27f966314accc471d0/third_party/openvr/src/headers/openvr_api.cs [delete] https://crrev.com/af38308b7112bdfbc0cfcc27f966314accc471d0/third_party/openvr/src/headers/openvr_api.json [delete] https://crrev.com/af38308b7112bdfbc0cfcc27f966314accc471d0/third_party/openvr/src/src/json/json-forwards.h [delete] https://crrev.com/af38308b7112bdfbc0cfcc27f966314accc471d0/third_party/openvr/src/src/json/json.h [delete] https://crrev.com/af38308b7112bdfbc0cfcc27f966314accc471d0/third_party/openvr/src/src/jsoncpp.cpp
,
Oct 23
switched to use \\third_party\jsoncpp for openvr.
,
Oct 25
There appears to be an additional build error related to this, which I've commented on here: https://bugs.chromium.org/p/chromium/issues/detail?id=780987#c4
,
Oct 25
It was reported that there are failures on M71 without this, so we may want to merge. This should hit issues on non-component debug builds from my understanding.
,
Oct 25
This bug requires manual review: M71 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 26
How is the change looking in canary? Is it full safe to merge? And why it is critical to merge to M71 beta?
,
Oct 26
Change looks good in canary and should be safe to merge. As for why its critical to fix in beta - I'm a bit on the fence and wanted guidance. This bug causes build failures in some setups (duplicate symbols on debug non-component builds for example). If we aren't hitting build failures in anything critical to ship Chrome then we may not need the fix merged for Chrome. However, if people start with stable Chrome, then modify the source to ship their own browser / apps, they may hit issues and want the fix.
,
Oct 26
Approving merge to M71 branch 3578 based on comments #7 and #10. Please merge ASAP so we can pick it up for next week beta release. Thank you.
,
Oct 26
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5651b7e8d2688f718831d5282f910b70ab0f9b9b Commit: 5651b7e8d2688f718831d5282f910b70ab0f9b9b Author: billorr@chromium.org Commiter: billorr@chromium.org Date: 2018-10-26 20:27:38 +0000 UTC Remove duplicate jsoncpp from OpenVR OpenVR by default includes its own jsoncpp. However, we have one in third_party already, so we don't need OpenVR's copy. BUG= 780987 , 790814 , 896087 Change-Id: Iac57998856ac72b36b6b731b3860ca67ae7c3f45 Reviewed-on: https://chromium-review.googlesource.com/c/1294669 Reviewed-by: Klaus Weidner <klausw@chromium.org> Commit-Queue: Bill Orr <billorr@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#602033}(cherry picked from commit 2f969bff5532233afff5564f947b453b5dac6be6) Reviewed-on: https://chromium-review.googlesource.com/c/1302859 Reviewed-by: Bill Orr <billorr@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#351} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
,
Oct 26
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5651b7e8d2688f718831d5282f910b70ab0f9b9b commit 5651b7e8d2688f718831d5282f910b70ab0f9b9b Author: Bill Orr <billorr@chromium.org> Date: Fri Oct 26 20:27:38 2018 Remove duplicate jsoncpp from OpenVR OpenVR by default includes its own jsoncpp. However, we have one in third_party already, so we don't need OpenVR's copy. BUG= 780987 , 790814 , 896087 Change-Id: Iac57998856ac72b36b6b731b3860ca67ae7c3f45 Reviewed-on: https://chromium-review.googlesource.com/c/1294669 Reviewed-by: Klaus Weidner <klausw@chromium.org> Commit-Queue: Bill Orr <billorr@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#602033}(cherry picked from commit 2f969bff5532233afff5564f947b453b5dac6be6) Reviewed-on: https://chromium-review.googlesource.com/c/1302859 Reviewed-by: Bill Orr <billorr@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#351} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/5651b7e8d2688f718831d5282f910b70ab0f9b9b/device/vr/BUILD.gn [modify] https://crrev.com/5651b7e8d2688f718831d5282f910b70ab0f9b9b/third_party/openvr/BUILD.gn [modify] https://crrev.com/5651b7e8d2688f718831d5282f910b70ab0f9b9b/third_party/openvr/README.chromium [delete] https://crrev.com/fd47f329fe335046fcf28660866a8ca4ff2328dc/third_party/openvr/src/headers/openvr_api.cs [delete] https://crrev.com/fd47f329fe335046fcf28660866a8ca4ff2328dc/third_party/openvr/src/headers/openvr_api.json [delete] https://crrev.com/fd47f329fe335046fcf28660866a8ca4ff2328dc/third_party/openvr/src/src/json/json-forwards.h [delete] https://crrev.com/fd47f329fe335046fcf28660866a8ca4ff2328dc/third_party/openvr/src/src/json/json.h [delete] https://crrev.com/fd47f329fe335046fcf28660866a8ca4ff2328dc/third_party/openvr/src/src/jsoncpp.cpp |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by brucedaw...@chromium.org
, Oct 17