New issue
Advanced search Search tips

Issue 780987 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 23
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

dedupe the numerous jsoncpp files in third_party

Project Member Reported by billorr@chromium.org, Nov 2 2017

Issue description

There 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.
 
Status: Assigned (was: Untriaged)
Project Member

Comment 2 by bugdroid1@chromium.org, 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

Status: Fixed (was: Assigned)
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.
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.

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.
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.
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
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.
Thanks! :)
Labels: Merge-Merged-71-3578
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}
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 26

Labels: merge-merged-3578
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