dedupe the numerous jsoncpp files in third_party |
||||
Issue descriptionThere is a jsoncpp.cpp in third_party\openvr, and one under protobuf\conformance. There is also the jsoncpp code itself under third_party\jsoncpp. We could patch openvr to use one of the other jsoncpp's.
,
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
The openvr instance that we filed the bug for is now removed. I don't plan to update protobuf\conformance to debupe with third_party\jsoncpp, but I believe the others are deduped already.
,
Oct 25
After the change in comment#2 my Windows build of a Chromium-based application is failing with:
lld-link: error: undefined symbol: "class Json::Value & __thiscall Json::Value::operator=(class Json::Value)" (??4Value@Json@@QAEAAV01@V01@@Z)
>>> referenced by C:\code\chromium_git3\chromium\src\third_party\openvr\src\src\vrcommon\vrpathregistry_public.cpp:258
>>> openvr_api.lib(vrpathregistry_public.obj):("bool __thiscall CVRPathRegistry_Public::BSaveToFile(void) const" (?BSaveToFile@CVRPathRegistry_Public@@QBE_NXZ))
>>> referenced by C:\code\chromium_git3\chromium\src\third_party\openvr\src\src\vrcommon\vrpathregistry_public.cpp:259
>>> openvr_api.lib(vrpathregistry_public.obj):("bool __thiscall CVRPathRegistry_Public::BSaveToFile(void) const" (?BSaveToFile@CVRPathRegistry_Public@@QBE_NXZ))
This is because third_party\jsoncpp\source\include\json\value.h defines:
Value &operator=( const Value &other );
And third_party\openvr\src\src\json\json.h defines:
Value& operator=(Value other);
I was able to fix the error by changing the includes in third_party\openvr\src\src\vrcommon\vrpathregistry_public.cpp from:
#include "json/json.h"
to:
#include <json/reader.h>
#include <json/value.h>
#include <json/writer.h>
So that it uses the jsoncpp headers instead of the bundled header.
,
Oct 25
I think we probably need to merge the changes from this issue to M71 as well. M71 builds at version 71.0.3578.20 (which does not include your changes) are failing with: 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) I was able to fix those errors by cherry-picking your change from comment#2 and also applying the change that I describe in comment#4.
,
Oct 25
What was your gn args for the build issue you hit with json.h in #4? I haven't seen that issue, but third_party\openvr\src\src\json\json.h should be deleted after #2.
,
Oct 25
Good point about the deleted file. It was still in my local checkout, which explains the problem (otherwise it would have picked up third_party/jsoncpp/source/include/json/json.h). Here are my GN defines: is_component_build=false is_debug=true target_cpu="x86" use_jumbo_build=true
,
Oct 25
ok, so I think there is no new build issues, but we may want to merge to M71. I'll request a merge on the bug about the build failure.
,
Oct 25
Thanks! :)
,
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 billorr@chromium.org
, Dec 16 2017