[Bug 1210754] Review Request: json - JSON for Modern C++

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1210754



--- Comment #5 from Petr Pisar <ppisar@xxxxxxxxxx> ---
Spec file changes:

--- json.spec.old       2015-04-10 15:38:34.000000000 +0200
+++ json.spec   2015-04-14 13:37:12.000000000 +0200
@@ -1,16 +1,19 @@
-%global gitdate                20150410
-%global gittag         d7d05091617d8173923df14cf46bc74311dc176e
-%global shorttag       %(cut -b -7 <<< %{gittag})
-%global user           nlohmann
-
-Name:          json
-Version:       0
-Release:       1.%{gitdate}git.%{shorttag}%{?dist}
-Summary:       JSON for Modern C++
-Group:         Development/Libraries
-License:       MIT
-URL:           https://github.com/%{user}/%{name}/
-Source0:      
https://github.com/%{user}/%{name}/tarball/%{gittag}/%{user}-%{name}-%{shorttag}.tar.gz
+%global gitdate         20150410
+%global gittag          d7d05091617d8173923df14cf46bc74311dc176e
+%global shorttag        %(c=%{gittag}; echo ${c:0:7})
+%global user            nlohmann
+%global debug_package   %{nil}
+
+Name:           json
+Version:        0
+Release:        2.%{gitdate}git.%{shorttag}%{?dist}
+Summary:        JSON for Modern C++
+Group:          Development/Libraries
+License:        MIT
+## Not installed
+# src/json.hpp: Boost Software License, Version 1.0
+URL:            https://github.com/%{user}/%{name}/
+Source0:       
https://github.com/%{user}/%{name}/tarball/%{gittag}/%{user}-%{name}-%{shorttag}.tar.gz

 BuildRequires: cppcheck
 BuildRequires: /usr/bin/make
@@ -26,6 +29,7 @@
 Provides:       %{name}-static = %{version}-%{release}
 Requires:       %{name} = %{version}-%{release}
 Requires:       pkgconfig
+Requires:       libstdc++-devel

 %description devel
 The %{name}-devel package contains libraries and header files for
@@ -39,27 +43,12 @@

 %check
 make cppcheck
+make json_unit

 %install
 mkdir -p %{buildroot}%{_includedir}
 install -p -m 0644 src/json.hpp %{buildroot}%{_includedir}/json.hpp

-mkdir -p %{buildroot}%{_libdir}/pkgconfig
-cat > %{buildroot}%{_libdir}/pkgconfig/json.pc << EOF
-prefix=%{_prefix}
-exec_prefix=%{_exec_prefix}
-libdir=%{_libdir}
-includedir=%{_includedir}
-
-Name: json
-Description: JSON for Modern C++
-URL: https://github.com/%{user}/%{name}
-Version: %{version}-%{release}
-Requires:
-Libs:
-Cflags: -I${includedir}
-EOF
-
 %files
 %doc README.md
 %license LICENSE.MIT
@@ -67,8 +56,13 @@
 %files devel
 %doc html/
 %{_includedir}/json.hpp
-%{_libdir}/pkgconfig/json.pc

 %changelog
+* Tue Apr 14 2015 Daniel Kopecek <dkopecek@xxxxxxxxxx> -
0-2.20150410git.d7d0509
+- run json_unit target from the check section
+- document catch.hpp license
+- don't build the debuginfo subpackage
+- don't generate a distribution specific pkg-config file
+
 * Fri Apr 10 2015 Daniel Kopecek <dkopecek@xxxxxxxxxx> -
0-1.20150410git.d7d0509
 - Initial package


> TODO: Please note there has already been a `json' package in Fedora (bug
> #495801). The name of this package is not very inventive and it can lead to
> confusions. Please consider more designating name.
Not addressed. Ok.

> TODO: Use the github syntax for computing shorttag recommended by guidelines.
> The recommended code does not depend on coreutils.
-%global shorttag       %(cut -b -7 <<< %{gittag})
+%global shorttag        %(c=%{gittag}; echo ${c:0:7})
Ok.

> TODO: Document Boost license (test/catch.hpp) in a spec file comment.
+## Not installed
+# src/json.hpp: Boost Software License, Version 1.0
TODO: The file is test/catch.hpp, not src/json.hpp.

> TODO: I recommend not creating non-upstream pkg-config module. Such
> distribution-specific extensions give to developers an opportunity to write
> non-portable code.
-mkdir -p %{buildroot}%{_libdir}/pkgconfig
-cat > %{buildroot}%{_libdir}/pkgconfig/json.pc << EOF
-prefix=%{_prefix}
-exec_prefix=%{_exec_prefix}
-libdir=%{_libdir}
-includedir=%{_includedir}
-
-Name: json
-Description: JSON for Modern C++
-URL: https://github.com/%{user}/%{name}
-Version: %{version}-%{release}
-Requires:
-Libs:
-Cflags: -I${includedir}
-EOF
Ok.

> TODO: The src/json.hpp is generated file. Regenerate it from src/json.hpp.re2c.
Not addressed. Ok.

> TODO: Run the tests (make && ./json_unit).
 %check
 make cppcheck
+make json_unit
TODO: This compiles the test, but does not execute it. You have to execute the
resulting ./json_unit.

> TODO: Normalize spaces in the spec file.
Ok.

> FIX: Disable generating debuginfo package (%global debug_package %{nil}).
+%global debug_package   %{nil}
Ok.

> FIX: Run-require `libstdc++-devel' by json-devel because /usr/include/json.hpp
> includes its header files.
+Requires:       libstdc++-devel
ok.

FIX: The package does not build in F23
(http://koji.fedoraproject.org/koji/taskinfo?taskID=9476433):

+ make json_unit
g++ -std=c++11  -Wall -Wextra -pedantic -Weffc++ -Wcast-align -Wcast-qual
-Wctor-dtor-privacy -Wdisabled-optimization -Wformat=2 -Winit-self
-Wmissing-declarations -Wmissing-include-di
rs -Wold-style-cast -Woverloaded-virtual -Wredundant-decls -Wshadow
-Wsign-conversion -Wsign-promo -Wstrict-overflow=5 -Wswitch -Wundef -Wno-unused
-Wnon-virtual-dtor -Wreorder -Wdepre
cated -Wfloat-equal  -I src -I test -Dprivate=public test/unit.cpp  -o
json_unit
In file included from test/catch.hpp:60:0,
                 from test/unit.cpp:11:
/usr/include/c++/5.0.0/sstream:300:7: error: 'struct
std::__cxx11::basic_stringbuf<_CharT, _Traits, _Alloc>::__xfer_bufptrs'
redeclared with different access
       struct __xfer_bufptrs
       ^

TODO: You should unbundle the catch.hpp. It could be fixed in the upstream
<https://github.com/philsquared/Catch>.


Please correct all `FIX' items, consider fixing `TODO' items, and provide new
spec file.
Resolution: Package NOT approved.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]