Skip to content

Conversation

GMishx
Copy link
Member

@GMishx GMishx commented Oct 20, 2018

Description

Remove dependency from external file download for json-hpp and shift to packaged based dependency (libjsoncpp from Debian based and jsoncpp from RedHat based systems).

Changes

  1. Remove download for external json-hpp file.
  2. Add jsoncpp package dependency.

How to test

Compile the copyright package in Debian and RedHat based systems and check the JSON output.

$ sudo ./utils/fo-installdeps -ye
$ make copyright
$ cd src/copyright/agent
$ ./copyright -J ./copyright.cc

Closes #1084

@ghost ghost assigned GMishx Oct 20, 2018
@ghost ghost added the needs code review label Oct 20, 2018
@GMishx GMishx force-pushed the feat/libjson-cpp branch 3 times, most recently from 1193be1 to 007608d Compare October 20, 2018 12:47
@mcjaeger
Copy link
Member

appreciate this commit, but I did some testing on 3.4.0 rc1 already to make sure it is working. I am not so sure about adding this on short final. maybe tests fail because of timing issues of course.

@GMishx GMishx added the WIP label Oct 23, 2018
@GMishx GMishx removed the WIP label Oct 23, 2018
@GMishx
Copy link
Member Author

GMishx commented Oct 23, 2018

The tests are now fixed. Can you please check again?

@mcjaeger
Copy link
Member

(from discussion) let#s compare output of copyright with open ssl for external dependency and package dependency.

@mcjaeger mcjaeger added this to the 3.4.0 milestone Oct 26, 2018
@mcjaeger
Copy link
Member

(from discussion:) better to use the c++ lib instead of the c one

@GMishx
Copy link
Member Author

GMishx commented Oct 29, 2018

Found jsoncpp for Fedora 27.

And can be enabled in CentOS 7 using EPEL.
sudo yum install epel-release

@GMishx GMishx force-pushed the feat/libjson-cpp branch 2 times, most recently from 9d88168 to bdf28f9 Compare October 30, 2018 11:01
@GMishx
Copy link
Member Author

GMishx commented Oct 31, 2018

Tested on Ubuntu 18.04 and CentOS 7

@GMishx GMishx force-pushed the feat/libjson-cpp branch 2 times, most recently from 5ac724c to 3639e51 Compare October 31, 2018 09:42
@GMishx
Copy link
Member Author

GMishx commented Oct 31, 2018

Compilation failed on Fedora 27 and fixed accordingly.

Remove dependency from external file for json-hpp and shift
to packaged based dependency (libjsoncpp from Debian based
and jsoncpp from RedHat based systems).

(Since library is changed, no need to check GCC version)

Signed-off-by: Gaurav Mishra <[email protected]>
Since json library is not independent of C++ version,
the macro DISABLE_JSON is no longer required.

Signed-off-by: Gaurav Mishra <[email protected]>
@mcjaeger
Copy link
Member

tested again with u1604: works

Copy link
Member

@mcjaeger mcjaeger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works also for debian8 based package installaton

@mcjaeger mcjaeger merged commit 261d1a3 into fossology:master Oct 31, 2018
@ghost ghost removed the needs code review label Oct 31, 2018
@GMishx GMishx mentioned this pull request Nov 1, 2018
@GMishx GMishx deleted the feat/libjson-cpp branch April 4, 2019 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants