Hello. It is my second package for Fedora. Spec URL: https://github.com/drizt/psi-plus/blob/master/qconf.spec SRPM URL: https://github.com/drizt/psi-plus/blob/master/qconf-1.4-1.fc14.src.rpm QConf allows you to have a nice configure script for your qmake-based project. It is intended for developers who don't need (or want) to use the more complex GNU autotools. With qconf/qmake, it is easy to maintain a cross-platform project that uses a familiar configuration interface on unix. koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2793306 Expected results: Additional info: Really this package is need only for psi-plus package. I will create Psi+ review request after qconf package will be added to Fedora repo.
Package looks good. Just two small issues: - The BuildRoot tag can be dropped - Please replace %defattr(-,root,root) with %defattr(-,root,root,-)
Done https://github.com/drizt/psi-plus/blob/afc653818c65a10fd3977569878f9aa1ea7c3477/qconf.spec
"GPL" is not an acceptable value of the "License" tag. From a quick look at the license header in qconf.cpp and the COPYING file I'd guess the correct value is "GPLv2+ with exceptions". See https://fedoraproject.org/wiki/Licensing Can you use the "%configure" macro instead of passing arguments to ./configure by yourself? Please use parallel make whenever possible. See https://fedoraproject.org/wiki/Packaging/Guidelines#Parallel_make You do not need to clean the buildroot at the beginning of %install. On all current Fedora releases it is cleaned automatically. Only if you want the spec file to work on EPEL-5, you still need to clean it yourself. See https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag You may want to drop the %clean section. Again, it is only needed in EPEL-5. https://fedoraproject.org/wiki/Packaging/Guidelines#.25clean Using macros for the standard utilities (as in "%{__rm}") is discouraged. See https://fedoraproject.org/wiki/Packaging/Guidelines#Macros
Hello Michal. Thank you for review. I corrected .spec file. https://github.com/drizt/psi-plus/raw/aec116b07209ec55ed104ea56f8e9a1222e117d3/qconf.spec I must use ./configure instead macro. qconf don't use autotools configure. So number of macro arguments don't acceptable this ./configure. I trying used macro but get error. Also I wonder that it will prefer if i use .gz source tarball instead .bz2 .
(In reply to comment #4) > I must use ./configure instead macro. qconf don't use autotools configure. So > number of macro arguments don't acceptable this ./configure. I trying used > macro but get error. I see. This is a sufficient reason. Next I was going to suggest that you need to ensure %{optflags} are used in the build ( http://fedoraproject.org/wiki/PackagingGuidelines#Compiler_flags ), but apparently this somehow works already. I made a scratch build and I can see the familiar compiler flags in the g++ invocations ("-D_FORTIFY_SOURCE=2 -fstack-protector ..."). http://koji.fedoraproject.org/koji/taskinfo?taskID=2923980 I do not understand where the ./configure script gets the flags from, so it may or may not be the correct way. > Also I wonder that it will prefer if i use .gz source tarball instead .bz2 . You can choose whichever you like more. bz2 practically always achieves better compression than gz, so most packagers select bz2 when choosing between the two.
> I do not understand where the ./configure script gets the flags from, so it may or may not be the correct way. Perphars it get flags from qt. > You can choose whichever you like more. bz2 practically always achieves better compression than gz, so most packagers select bz2 when choosing between the two. I thought that gz is standard. So should use this format when it's possible. But I prefer .bz2. So I will use it.
I mistake. Have a look at new variant of .spec file https://github.com/drizt/psi-plus/blob/a796fddb7e6da8e1bf7ed17909db9e6a28c9c748/qconf.spec
Looks quite good to me. I am still worried about the optflags though. I do not think getting them from Qt is sufficient. Consider a situation when standard Fedora optflags change and a mass-rebuild is done to apply the change to all packages. I don't think in such a rebuild anything would guarantee that qconf is built after Qt. So it might end up still using the old optflags. Or a simpler case - someone might reasonably put custom optflags into his ~/.rpmmacros and then rebuild the package expecting his flags to apply. If the build used Qt's optflags instead, it would not work as expected.
I have learnt a build scripts. Really for building was used QMAKE_CXXFLAGS_RELEASE and QMAKE_CFLAGS_RELEASE from g++.conf . I can create patch for add support of opt flags for ./configure . If it really is needest. Anyway if i use qconf for creating Makefile i haven't way to specify opt flags for my project. And in that case I need to add the same feature into qconf. What say?
Also you can see into psi.spec. It don't use %{optflags} too.
I had a look at the generated Makefile after ./configure. It seems that passing the optflags is easy. Just do: make %{?_smp_mflags} CFLAGS="%{optflags}" CXXFLAGS="%{optflags}" (If there's only C++ code, then CFLAGS can be dropped, but I'd leave it there just in case.)
(In reply to comment #9) > Anyway if i use qconf for creating Makefile i haven't way to specify opt flags > for my project. And in that case I need to add the same feature into qconf. > What say? It does not allow overriding of CFLAGS and CXXFLAGS? That would be a problem indeed. But it's out of the scope of this package review. (In reply to comment #10) > Also you can see into psi.spec. It don't use %{optflags} too. You have found a bug in psi.spec then :)
I don't shure that it proper to give %{opflags} for Makefile. I have tryed to use make %{?_smp_mflags} CFLAGS="%{optflags}" CXXFLAGS="%{optflags}" and get error. + make 'CFLAGS=-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic' 'CXXFLAGS=-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic' g++ -c -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -I/usr/lib64/qt4/mkspecs/linux-g++ -I. -I/usr/include/QtCore -I/usr/include/QtXml -I/usr/include -I. -o stringhelp.o src/stringhelp.cpp g++ -c -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -I/usr/lib64/qt4/mkspecs/linux-g++ -I. -I/usr/include/QtCore -I/usr/include/QtXml -I/usr/include -I. -o qconf.o src/qconf.cpp src/qconf.cpp: In function 'int main(int, char**)': src/qconf.cpp:1327:22: error: 'DATADIR' was not declared in this scope make: *** [qconf.o] Error 1 error: Bad exit status from /var/tmp/rpm-tmp.PI7KfG (%build) Also I compared CXXFLAGS and %{opflags}. It's different. [taurus@lix qconf-1.4]$ grep CXXFLAGS Makefile CXXFLAGS = -pipe -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fstack-protect [taurus@lix qconf-1.4]$ rpm -E '%{optflags}' -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic
Oh, I see. It is unfortunate that CXXFLAGS in the Makefile contain optimization flags and important macro definitions together. So another option is to modify qconf.pro, adding a way to accept CXXFLAGS from the environment, like here: https://www.redhat.com/archives/fedora-devel-list/2007-October/msg01455.html
Ok. I have added such patch. Have a look at https://github.com/drizt/psi-plus/blob/be2dc571a94fb34f7dd4033d630a98b16e0793bb/qconf-1.4-optflags.patch https://github.com/drizt/psi-plus/blob/be2dc571a94fb34f7dd4033d630a98b16e0793bb/qconf.spec
Well, I don't think you need to copy Kevin's fragment exactly. I linked it to provide inspiration. For instance, I don't think you really need to add -Wno-non-virtual-dtor. And there's no pkg-config usage in this project, so I don't see any value in PKGCONFIG_CFLAGS either. And did you really need to switch to calling qmake-qt4 instead of ./configure? I believe that when the patch is applied, this should work fine: CXXFLAGS="%{optflags}" ./configure --prefix... But I do not really understand what difference it makes, so if you think calling qmake-qt4 is better, that's fine with me.
here's a scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2964263 There is a suspicious message in build.log: WARNING: /builddir/build/BUILD/qconf-1.4/qconf.pro:9: Unable to find file for inclusion /builddir/build/BUILD/qconf-1.4/conf.pri I'll leave it to you to decide whether it is important.
(In reply to comment #17) > here's a scratch build: > http://koji.fedoraproject.org/koji/taskinfo?taskID=2964263 > > There is a suspicious message in build.log: > WARNING: /builddir/build/BUILD/qconf-1.4/qconf.pro:9: Unable to find file for > inclusion /builddir/build/BUILD/qconf-1.4/conf.pri > > I'll leave it to you to decide whether it is important. I have made both rpm packages on a local machine. Unpacked both. And compared it. Files in both packages are identical. I consider that it prefer if I will use standard qmake instead non-standard configure. Altough if I use configure it build binary qconf to create Makefile which it will use for build binary qconf. It's weird and eat more time.
(In reply to comment #16) > Well, I don't think you need to copy Kevin's fragment exactly. I linked it to > provide inspiration. For instance, I don't think you really need to add > -Wno-non-virtual-dtor. And there's no pkg-config usage in this project, so I > don't see any value in PKGCONFIG_CFLAGS either. I have rewrited patch. Now I just override QMAKE_CXXFLAGS_RELEASE and QMAKE_CXXFLAGS_DEBUG with value from CXXFLAGS if it's possible. In negative order it's used value from Qt. Also I switched off warning. https://github.com/drizt/psi-plus/blob/e24acf78de103730848946ece8d721ddba3009ca/qconf-1.4-optflags.patch
rpmlint output: qconf.src: W: spelling-error Summary(en_US) qmake -> make, quake, q make qconf.src: W: spelling-error %description -l en_US qmake -> make, quake, q make qconf.src: W: spelling-error %description -l en_US autotools -> auto tools, auto-tools, autopilots qconf.src: W: spelling-error %description -l en_US unix -> UNIX, Unix, uni qconf.x86_64: W: spelling-error Summary(en_US) qmake -> make, quake, q make qconf.x86_64: W: spelling-error %description -l en_US qmake -> make, quake, q make qconf.x86_64: W: spelling-error %description -l en_US autotools -> auto tools, auto-tools, autopilots qconf.x86_64: W: spelling-error %description -l en_US unix -> UNIX, Unix, uni qconf.x86_64: W: devel-file-in-non-devel-package /usr/share/qconf/conf/conf4.h qconf.x86_64: W: devel-file-in-non-devel-package /usr/share/qconf/conf/conf4.cpp qconf.x86_64: W: devel-file-in-non-devel-package /usr/share/qconf/conf/conf.cpp qconf.x86_64: W: no-manual-page-for-binary qconf The warnings about spelling errors are bogus. The missing manual page is unfortunate, but not a blocker. The devel-file-in-non-devel-package warnings are false positives. If I understand qconf correctly, the files are necessary to its operation. Formal checks: "OK" means the package matches the guideline "??" means unclear status, needs explanation "--" means the guideline it not relevant "ERR" signifies a problem Checking with respect to the Packaging Guidelines (http://fedoraproject.org/wiki/Packaging/Guidelines): OK - naming OK - version and release OK - licensing "GPLv2+ with exceptions" because the sources have the usual GPLv2-or-later header and the COPYING file gives an extra permission on top of GPLv2 permissions. OK - no pre-built binaries OK - spec legible OK - no architecture excluded OK - filesystem layout OK - rpmlint, see above OK - changelog OK - tags OK - BuildRoot tag, not used, not necessary OK - %clean, not used, not necessary OK - Requires, no explicit ones OK - BuildRequires OK - summary and description OK - encoding, ASCII OK - documentation OK - compiler flags OK - debuginfo packages -- - devel packages -- - requiring base package -- - static libraries OK - no duplication of system libraries OK - rpath, none -- - configuration files -- - initscripts -- - desktop files OK - macros -- - %global preferred over %define -- - locale files OK - timestamps OK - parallel make -- - scriptlets -- - conditional deps OK - relocatable packages, not OK - code vs content OK - file and dir ownership -- - users and groups -- - web apps ERR- conflicts: - the package "gridengine" also provides /usr/bin/qconf - You'll need to resolve somehow. Follow the hints at: http://fedoraproject.org/wiki/Packaging:Conflicts#Conflicting_Files OK - no kernel modules OK - no files under /srv OK - no bundling of multiple projects ?? - patches should have an upstream bug link or comment - it is unclear to me whether qconf-1.4-optflags.patch is going to be Fedora-specific forever or if it's going to be resolved in the upstream project. OK - use of epochs, none -- - symlinks -- - man pages, would be nice to have though -- - test suites -- - tmpfiles.d -- - application-specific guidelines Steps of the Review Guidelines (http://fedoraproject.org/wiki/Packaging/ReviewGuidelines): OK - rpmlint OK - Naming Guidelines OK - spec name matches package %{name} ERR- Packaging Guidelines, see above OK - approved license OK - license tag matches OK - COPYING in %doc OK - US English OK - legible OK - source checksum, sha256sum: 212bce09a585a22cf4b9e1a881e8f79c32a82e5cb8ea7f99a056a50faf809af8 qconf-1.4.tar.bz2 OK - builds on all archs OK - BuildRequires -- - locales -- - ldconfig OK - no bundling -- - not relocatable OK - file ownership OK - no duplicate files OK - sane permissions OK - macros OK - code or content -- - large doc OK - %doc not essential to runtime OK - header files to be in -devel - the headers included in the package are needed for its operation -- - no static libs -- - dynamic libs -- - devel packages' reqs -- - *.la files -- - desktop files for GUI apps OK - no ownership of other packages' files OK - valid filenames
ERR- conflicts: - the package "gridengine" also provides /usr/bin/qconf I would use qconf-qt4 binary name by analogue with qmake-qt4. What you think?
How you found this conflict?
(In reply to comment #21) > I would use qconf-qt4 binary name by analogue with qmake-qt4. What you think? Yes, qconf-qt4 would be acceptable and the analogy with qmake-qt4 is nice. I had a look at Debian. They renamed the binary to qt-qconf: http://packages.debian.org/sid/amd64/qconf/filelist But they also use different naming of qmake than we do... I like qconf-qt4 better. (In reply to comment #22) > How you found this conflict? repoquery -f '*/qconf'
It's fine! ?? - patches should have an upstream bug link or comment - it is unclear to me whether qconf-1.4-optflags.patch is going to be Fedora-specific forever or if it's going to be resolved in the upstream project. I wrote to Justin Karneges (founder of Psi and related projects). And will wait for him answer.
I made patch to rename. Have a look at https://github.com/drizt/psi-plus/blob/9d1791dcf50ecb6980e4070c0575e8eeda192312/qconf-1.4-rename-binary.patch and https://github.com/drizt/psi-plus/blob/0b737ab7752b1d3a0b6b322a9e99e9312916f3c7/qconf.spec
Source package with both patches. https://github.com/drizt/psi-plus/raw/dce9824171a29d07100e560b128b68decc33d38e/qconf-1.4-2.fc14.src.rpm
I am satisfied with the latest package. The only small remaining issue (the applicability of qconf-1.4-optflags.patch upstream) is being discussed with the upstream author. I see no reason to block the review. I approve this package.
New Package SCM Request ======================= Package Name: qconf Short Description: Tool for generating configure script for qmake-based projects Owners: ivanromanov Branches: f15 InitialCC: ivanromanov
Git done (by process-git-requests).
qconf-1.4-2.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/qconf-1.4-2.fc15
qconf-1.4-2.fc15 has been pushed to the Fedora 15 testing repository.
qconf-1.4-2.fc15 has been pushed to the Fedora 15 stable repository.
New Package SCM Request ======================= Package Name: qconf Short Description: Tool for generating configure script for qmake-based projects Owners: ivanromanov Branches: f14 InitialCC: ivanromanov Also I need this package in Fedora 14.
You cannot submit a new package request for a package that is already in the distribution. Please submit a change request instead.
Package Change Request ======================= Package Name: qconf Owners: ivanromanov New Branches: f14 InitialCC: ivanromanov Also I need this package in Fedora 14.
qconf-1.4-2.fc14 has been submitted as an update for Fedora 14. https://admin.fedoraproject.org/updates/qconf-1.4-2.fc14
qconf-1.4-2.fc14 has been pushed to the Fedora 14 stable repository.
Package Change Request ======================= Package Name: qconf Owners: ivanromanov New Branches: el6 InitialCC: ivanromanov
qconf-1.4-3.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/qconf-1.4-3.el6
qconf-1.4-4.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/qconf-1.4-4.el6
qconf-1.4-4.el6 has been pushed to the Fedora EPEL 6 stable repository. If problems still persist, please make note of it in this bug report.
Package Change Request ======================= Package Name: qconf Owners: ivanromanov New Branches: epel7 InitialCC: ivanromanov