Spec URL: http://orbsky.homelinux.org/packages/qjson.spec SRPM URL: http://orbsky.homelinux.org/packages/qjson-0.6.3-1.fc11.src.rpm Description: JSON is a lightweight data-interchange format. It can represents integer, real number, string, an ordered sequence of value, and a collection of name/value pairs.QJson is a qt-based library that maps JSON data to QVariant objects.
Here is my review: ! All relevant doc files should be packaged. - I don't think we need the INSTALL file in %doc - There is doxygen documentation in the doc directory that can be built and packaged. * rpmlint says: qjson.x86_64: W: name-repeated-in-summary QJson qjson.x86_64: W: invalid-license LGPL qjson.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libqjson.so qjson.x86_64: W: devel-file-in-non-devel-package /usr/include/qjson/serializer.h qjson.x86_64: W: devel-file-in-non-devel-package /usr/include/qjson/parser.h qjson.x86_64: W: devel-file-in-non-devel-package /usr/include/qjson/qjson_export.h qjson.x86_64: W: devel-file-in-non-devel-package /usr/include/qjson/serializerrunnable.h qjson.x86_64: W: devel-file-in-non-devel-package /usr/lib64/pkgconfig/QJson.pc qjson.x86_64: E: library-without-ldconfig-postin /usr/lib64/libqjson.so.0.6.3 qjson.x86_64: E: library-without-ldconfig-postun /usr/lib64/libqjson.so.0.6.3 qjson.x86_64: W: devel-file-in-non-devel-package /usr/include/qjson/parserrunnable.h qjson-debuginfo.x86_64: W: invalid-license LGPL qjson.src: W: name-repeated-in-summary QJson qjson.src: W: invalid-license LGPL -The header files, .pc file and the *.so file should go to the devel package. Also the doxygen doc, if built, should go to the devel package. The devel package must require pkgconfig in addition to %{name} = %{version}-%{release} -Summary can simply be "qt-based library that maps JSON data to QVariant objects" -There is GPLv2+ code in the src/ directory, making the effective license GPLv2+. Please verify this, possibly via upstream, as they claim that the library is LGPL. ! The tests in the tests/ directory can be run in %check ! If you can, please make the description more descriptive. Not just a copy of the summary. * Every binary RPM package which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun. ? Do we need to keep those commented out lines in the specfile? * %{_includedir}/qjson/ is not owned. * What package owns the directory? %{_datadir}/apps/cmake/modules/ That package needs to be in Requires.
OK... Its gonna be obvious now that I'm no developer. I've managed to address some of the issues as detailed below. - Split off development libraries to its own package - Modified licensing in spec file to reflect GPL2 code though docs state that qjson licensed under LPGL - Uncommeted and corrected sed line in this spec file I'm open to any suggestions and comments you may have regarding the following issues: - I have no idea how to run the tests. - How do I compile the doxygen doc? Can that be done during rpmbuild? - Since this is a development package, it would probably be best if this was a multilib package. How do I accomplish that? - I thought the %defattr macro conferred ownership to files and directories being installed. Please correct me. - Where can I find an example to properly add FindQJSON.cmake as a requirment? Or better yet and quicker if you could detail the correct line. Looking very forward to your feedback and thankyou very much for your help Spec URL: http://orbsky.homelinux.org/packages/qjson.spec SRPM URL: http://orbsky.homelinux.org/packages/qjson-0.6.3-2.fc11.src.rpm
Oh... One other thing. I think that the "test" is not neccessary as it isn't mentioned at all in the INSTALL doc.
(In reply to comment #2) > OK... Its gonna be obvious now that I'm no developer. I've managed to address > some of the issues as detailed below. It's all okay. Nobody is born as a programmer. I will try to be as explicit as possible. > > - Split off development libraries to its own package > - Modified licensing in spec file to reflect GPL2 code though docs state that > qjson licensed under LPGL > - Uncommeted and corrected sed line in this spec file > > > I'm open to any suggestions and comments you may have regarding the following > issues: > > - I have no idea how to run the tests. When you are running "%{cmake} ..", you can add the flag "-DQJSON_BUILD_TESTS=1" so the tests are built. i.e. %{cmake} .. -DQJSON_BUILD_TESTS=1 Then in the %check section you will execute these tests. Something like %check LD_PRELOAD=%{_target_platform}/%{_lib}/libqjson.so %{_target_platform}/tests/testparser LD_PRELOAD=%{_target_platform}/%{_lib}/libqjson.so %{_target_platform}/tests/testserializer If you don't use LD_PRELOAD, the tests will fail because it can't access the library you just built. > - How do I compile the doxygen doc? Can that be done during rpmbuild? Add BR: doxygen. Then in the doc/ directory, just run doxygen It will create an html/ direcory inside doc/ add this directory to the %doc of the devel subpackage. > - Since this is a development package, it would probably be best if this > was a multilib package. How do I accomplish that? You don't need to do anything about multilib. Releng takes care of that. > - I thought the %defattr macro conferred ownership to files and directories > being installed. Please correct me. Think of it this way: All files and directories are some entries in the filesystem. Directories are special kind of "files". Now each RPM must either own the directory it puts a file inside, or it must require a package that owns a directory. This package puts a file inside %{_includedir}/qjson/ but this directory is not owned by any package! You must own this directory with the devel subpackage. So replace the line %{_includedir}/qjson/*.h with either %{_includedir}/qjson/ or %dir %{_includedir}/qjson %{_includedir}/qjson/*.h so that the directory is owned. I hope you got the idea. This must be satisfied by any package in Fedora. > - Where can I find an example to properly add FindQJSON.cmake as a requirment? > Or better yet and quicker if you could detail the correct line. > I don't know exactly. You might want to do some research. However, I see that cmake package put a lot of modules into /usr/share/cmake/Modules/ so you might want to move that file into that directory in %install. Also I believe that this file must go to the devel package. Note that you will need to require cmake (for the devel package) for directory ownership. But as I said, I don't know exactly. Please check this. > Looking very forward to your feedback and thankyou very much for your help > > > Spec URL: http://orbsky.homelinux.org/packages/qjson.spec > SRPM URL: http://orbsky.homelinux.org/packages/qjson-0.6.3-2.fc11.src.rpm You are welcome. One more thing (at least) need to be fixed. In Fedora the files %{_libdir}/libfoo.so.* go to the main library package, whereas the single file, which is a symlink %{_libdir}/libfoo.so goes to the devel package. Please fix this in your package accordingly.
Sorry about the delay in getting this looked after. I have had an extremely distracting couple of weeks. (In reply to comment #4) > (In reply to comment #2) > When you are running "%{cmake} ..", you can add the flag > "-DQJSON_BUILD_TESTS=1" so the tests are built. i.e. > %{cmake} .. -DQJSON_BUILD_TESTS=1 > > Then in the %check section you will execute these tests. Something like > %check > LD_PRELOAD=%{_target_platform}/%{_lib}/libqjson.so > %{_target_platform}/tests/testparser > LD_PRELOAD=%{_target_platform}/%{_lib}/libqjson.so > %{_target_platform}/tests/testserializer Done > Add BR: doxygen. Then in the doc/ directory, just run > doxygen > It will create an html/ direcory inside doc/ add this directory to the %doc of > the devel subpackage. Done. This should probably go in the base package shouldn't it? Its documentation not header files and I would assume that the docs contains how to use. > a directory. This package puts a file inside %{_includedir}/qjson/ but this > directory is not owned by any package! You must own this directory with the > devel subpackage. So replace the line > %{_includedir}/qjson/*.h > with either > %{_includedir}/qjson/ > or > %dir %{_includedir}/qjson > %{_includedir}/qjson/*.h > so that the directory is owned. I hope you got the idea. This must be satisfied > by any package in Fedora. Done > > - Where can I find an example to properly add FindQJSON.cmake as a requirment? > > Or better yet and quicker if you could detail the correct line. > > > > I don't know exactly. You might want to do some research. However, I see that > cmake package put a lot of modules into /usr/share/cmake/Modules/ so you might > want to move that file into that directory in %install. Also I believe that > this file must go to the devel package. Note that you will need to require > cmake (for the devel package) for directory ownership. > Cmake was added as Requirment in a the devel package as well and the module was moved over to the devel package. I don't believe that it should be included as a requirment because if it is required and it isn't yet built and installed then I would not be able to build the package. Please correct me if I'm wrong. > %{_libdir}/libfoo.so.* > go to the main library package, whereas the single file, which is a symlink > %{_libdir}/libfoo.so > goes to the devel package. Please fix this in your package accordingly. Done
Ooops... Forgot to add urls to the new spec and srpm Spec URL: http://orbsky.homelinux.org/packages/qjson.spec SRPM URL: http://orbsky.homelinux.org/packages/qjson-0.6.3-3.fc11.src.rpm
(In reply to comment #5) > In Fedora the files > > %{_libdir}/libfoo.so.* > go to the main library package, whereas the single file, which is a symlink > %{_libdir}/libfoo.so > goes to the devel package. Please fix this in your package accordingly. Oh. I think this must be an error because, if an app should make reference to the symlink and the development package is not installed it won't find the libarary. So I left all the libraries including he mentioned symlink in the base package. Please correct me if I'm wrong about that.
(In reply to comment #7) > (In reply to comment #5) > > In Fedora the files > > > > %{_libdir}/libfoo.so.* > > go to the main library package, whereas the single file, which is a symlink > > %{_libdir}/libfoo.so > > goes to the devel package. Please fix this in your package accordingly. > > Oh. I think this must be an error because, if an app should make reference to > the symlink and the development package is not installed it won't find the > libarary. So I left all the libraries including he mentioned symlink in the > base package. Please correct me if I'm wrong about that. I don't think I made a mistake. This is the standard way we handle shared libraries in Fedora. - .so goes to devel - .so.* go to the main package (or to the -libs subpackage if there is a further split) You can verify this by checking any shared library installed in your system. rpm -ql libogg rpm -ql libogg-devel rpm -ql libXinerama rpm -ql libXinerama-devel etc. When you are building a shared library, the compiler (or ld) needs the .so file. During runtime, on the other hand, the linker takes care of this and it will point your application to the correct .so.* file. So the .so file is only needed during building.
(In reply to comment #8) > (In reply to comment #7) > > (In reply to comment #5) > > > In Fedora the files > > > > > > %{_libdir}/libfoo.so.* > > > go to the main library package, whereas the single file, which is a symlink > > > %{_libdir}/libfoo.so > > > goes to the devel package. Please fix this in your package accordingly. > > > > Oh. I think this must be an error because, if an app should make reference to > > the symlink and the development package is not installed it won't find the > > libarary. So I left all the libraries including he mentioned symlink in the > > base package. Please correct me if I'm wrong about that. > > I don't think I made a mistake. This is the standard way we handle shared > libraries in Fedora. > - .so goes to devel > - .so.* go to the main package (or to the -libs subpackage if there is a > further split) > > You can verify this by checking any shared library installed in your system. > rpm -ql libogg > rpm -ql libogg-devel > rpm -ql libXinerama > rpm -ql libXinerama-devel > etc. > > When you are building a shared library, the compiler (or ld) needs the .so > file. During runtime, on the other hand, the linker takes care of this and it > will point your application to the correct .so.* file. So the .so file is only > needed during building. I defer to your judgement and request.... And so done :)
SPEC URL: http://orbsky.homelinux.org/packages/qjson.spec SRPM URL: http://orbsky.homelinux.org/packages/qjson-0.6.3-4.fc11.src.rpm
Thanks for the update. We are getting there.The SRPM link above is broken but I regenerated it from the SPEC file * Please don't just use the summary here %description devel %{summary}. You can use something like The %{name}-devel package contains libraries and header files for developing applications that use %{name}. * These need to be in one line each LD_PRELOAD=%{_target_platform}/%{_lib}/libqjson.so %{_target_platform}/tests/testparser LD_PRELOAD=%{_target_platform}/%{_lib}/libqjson.so %{_target_platform}/tests/testserializer so just add a \ at the end of the first lines. * This issue still needs to be addressed: > > - Where can I find an example to properly add FindQJSON.cmake as a requirment? > > Or better yet and quicker if you could detail the correct line. > > > > I don't know exactly. You might want to do some research. However, I see that > cmake package put a lot of modules into /usr/share/cmake/Modules/ I found that you can pass this flag to cmake -DCMAKE_MODULES_INSTALL_DIR=%{_datadir}/cmake/Modules/ to make it install the cmake module to the correct location. Note that you will need to update the %files section accordingly. * I fixed the \ issue myself to build the package otherwise it will fail to build. The new package has the rpmlints: - qjson.x86_64: E: description-line-too-long JSON is a lightweight data-interchange format. It can represents integer, real number, string, Please make the description fit 80 columns. - qjson.x86_64: W: incoherent-version-in-changelog 0.6.3-3 ['0.6.3-4.fc12', '0.6.3-4'] - qjson-devel.x86_64: W: summary-not-capitalized qjson Development Files These can be fixed easily - qjson.x86_64: W: spurious-executable-perm /usr/share/doc/qjson-0.6.3/html /installdox - qjson.x86_64: W: doc-file-dependency /usr/share/doc/qjson-0.6.3/html /installdox /usr/bin/perl I don't think this installdox file should be installed. - qjson-devel.x86_64: W: no-documentation There are multiple mistakes here: . The doxygen documentation you built normally belongs to the devel package. It is supposed to be API documentation. . The doxygen documentation is not built properly. Check the generated index.html file. It is blank. You need the build the doxygen documentation that is inside the doc/ directory. Please make a test build in mock and check for rpmlints before you do the next update.
I hope we're done. I think that I made all the changes requested (please double check) and ran rpmlint on the SPEC and SRPM files with zero errors on both files. The error you got with the doxygen index.html was funny. When I looked at the insides of the built RPMS, everything seemed fine. Could you please double check this with the provided files that I'm linking to here. Thanks SPEC URL: http://orbsky.homelinux.org/packages/qjson.spec SRPM URL: http://orbsky.homelinux.org/packages/qjson-0.6.3-5.fc12.src.rpm
Please change BR: qt to BR-qt-devel. Otherwise the package won't even build in mock, which means it won't build in koji. Moreover, I made a scratch build in koji with the above BR corrected. You can download the RPM's that are generated. We still have rpmlints: qjson-devel.x86_64: W: summary-not-capitalized qjson Development Files This can be fixed easily. qjson-devel.x86_64: W: spurious-executable-perm /usr/share/doc/qjson-devel-0.6.3/html/installdox qjson-devel.x86_64: W: doc-file-dependency /usr/share/doc/qjson-devel-0.6.3/html/installdox /usr/bin/perl The documentation is still blank. I see this line in the build.log: Error: file `/builddir/build/BUILD/qjson/doc/' not found Perhaps this will help you finding the problem. Again, I urge you to build packages on mock. It will make your life easier.
(In reply to comment #13) > Please change BR: qt to BR-qt-devel. Otherwise the package won't even build in > mock, which means it won't build in koji. > > Moreover, I made a scratch build in koji with the above BR corrected. You can > download the RPM's that are generated. We still have rpmlints: > qjson-devel.x86_64: W: summary-not-capitalized qjson Development Files > > This can be fixed easily. Done > qjson-devel.x86_64: W: spurious-executable-perm > /usr/share/doc/qjson-devel-0.6.3/html/installdox > qjson-devel.x86_64: W: doc-file-dependency > /usr/share/doc/qjson-devel-0.6.3/html/installdox /usr/bin/perl > Please see links below and comments regarding blank doxygen html directory: http://fedoraproject.org/wiki/MinGW/Rpmlint https://bugzilla.redhat.com/show_bug.cgi?id=467397#c4 The above warning doesn't seem to be of concern to Fedora. The warning is produced because I'm building the doxygen docs during rpmbuild and an installdox script is placed in the html folder on that build. > The documentation is still blank. I see this line in the build.log: Finally clued into this one. Sorry about the misunderstanding. Indeed in the source contained in the SRPM the html folder is indeed blank. However, if you were to examine the SPEC file on line 42 you would see that I build the doxygen during the build process. Which produces the rpmlint warning you mentioned above. Also, the doxygen docs are then successfully installed in the devel package on line 77. So if you were to look in the built devel package, you would see that the doxygen docs are built and in the correct location. > > Error: file `/builddir/build/BUILD/qjson/doc/' not found > > Perhaps this will help you finding the problem. If you were to look in both the qjson and qjson-devel package all the documentation specified in the installation goes to the location expected. > Again, I urge you to build packages on mock. It will make your life easier. Did that with the same results. SPEC URL: http://orbsky.homelinux.org/packages/qjson.spec SRPM URL: http://orbsky.homelinux.org/packages/qjson-0.6.3-6.fc12.src.rpm
1- Please install the devel package and open the index.html that it installs to your /usr/share/doc/qjson-devel-VERSION/ in your browser. 2- Next, unzip the source tarball of qjson manually, go to the doc/ directory. Do a doxygen inside that directory. It will build the docs. Now open the doc/html/index.html that you just generated in your browser. Do you see a difference between the two index.html's?
Also version 0.7.1 is up. Could you update to the latest version? Thanks.
*** Bug 544737 has been marked as a duplicate of this bug. ***
Will do. Regarding the doxygen thing. I guess I must be a bit thick. Aging will do that to you. :) Sorry about the confusion.
Finally doxygen docs fixed.... I think... I hope. Thanks for your patience on this. SPEC URL: http://orbsky.homelinux.org/packages/qjson.spec SRPM URL: http://orbsky.homelinux.org/packages/qjson-0.7.1-1.fc12.src.rpm
Thanks! I think it is good now. ---------------------------------------- This package (qjson) is APPROVED by oget ----------------------------------------
New Package CVS Request ======================= Package Name: qjson Short Description: A qt-based library that maps JSON data to QVariant objects Owners: eliwap oget Branches: F-11 F-12 InitialCC: oget
cvs done.
qjson-0.7.1-1.fc12 has been submitted as an update for Fedora 12. http://admin.fedoraproject.org/updates/qjson-0.7.1-1.fc12
qjson-0.7.1-1.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/qjson-0.7.1-1.fc11
qjson-0.7.1-1.fc12 has been pushed to the Fedora 12 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update qjson'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F12/FEDORA-2009-13633
qjson-0.7.1-1.fc11 has been pushed to the Fedora 11 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update qjson'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-13638
qjson-0.7.1-1.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report.
qjson-0.7.1-1.fc12 has been pushed to the Fedora 12 stable repository. If problems still persist, please make note of it in this bug report.
qjson-0.7.1-2.fc13 has been submitted as an update for Fedora 13. https://admin.fedoraproject.org/updates/qjson-0.7.1-2.fc13
qjson-0.7.1-2.fc14 has been submitted as an update for Fedora 14. https://admin.fedoraproject.org/updates/qjson-0.7.1-2.fc14
qjson-0.7.1-2.fc12 has been submitted as an update for Fedora 12. https://admin.fedoraproject.org/updates/qjson-0.7.1-2.fc12
qjson-0.7.1-2.fc14 has been pushed to the Fedora 14 stable repository. If problems still persist, please make note of it in this bug report.
qjson-0.7.1-2.fc12 has been pushed to the Fedora 12 stable repository. If problems still persist, please make note of it in this bug report.
qjson-0.7.1-2.fc13 has been pushed to the Fedora 13 stable repository. If problems still persist, please make note of it in this bug report.