Bug 676129 - Review Request: qconf - Allows you to have a nice configure script for your qmake-based project
Summary: Review Request: qconf - Allows you to have a nice configure script for your q...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Linux
unspecified
unspecified
Target Milestone: ---
Assignee: Michal Schmidt
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-02-08 21:50 UTC by Ivan Romanov
Modified: 2017-01-07 09:09 UTC (History)
6 users (show)

Fixed In Version: qconf-1.4-4.el6
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-04-25 02:42:09 UTC
Type: ---
mschmidt: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)


Links
System ID Priority Status Summary Last Updated
Red Hat Bugzilla 1408580 None None None Never

Internal Links: 1408580

Description Ivan Romanov 2011-02-08 21:50:03 UTC
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.

Comment 1 Erik van Pienbroek 2011-02-09 22:16:16 UTC
Package looks good.
Just two small issues:
- The BuildRoot tag can be dropped
- Please replace %defattr(-,root,root) with %defattr(-,root,root,-)

Comment 3 Michal Schmidt 2011-03-18 12:05:46 UTC
"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

Comment 4 Ivan Romanov 2011-03-18 17:05:20 UTC
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 .

Comment 5 Michal Schmidt 2011-03-18 19:25:54 UTC
(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.

Comment 6 Ivan Romanov 2011-03-18 20:04:58 UTC
> 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.

Comment 7 Ivan Romanov 2011-03-29 19:29:15 UTC
I mistake. 
Have a look at new variant of .spec file
https://github.com/drizt/psi-plus/blob/a796fddb7e6da8e1bf7ed17909db9e6a28c9c748/qconf.spec

Comment 8 Michal Schmidt 2011-03-30 15:09:48 UTC
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.

Comment 9 Ivan Romanov 2011-03-30 17:48:52 UTC
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?

Comment 10 Ivan Romanov 2011-03-30 19:49:28 UTC
Also you can see into psi.spec. It don't use %{optflags} too.

Comment 11 Michal Schmidt 2011-03-30 20:16:22 UTC
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.)

Comment 12 Michal Schmidt 2011-03-30 20:20:17 UTC
(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 :)

Comment 13 Ivan Romanov 2011-03-31 01:29:44 UTC
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

Comment 14 Michal Schmidt 2011-03-31 07:44:57 UTC
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

Comment 16 Michal Schmidt 2011-03-31 18:01:20 UTC
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.

Comment 17 Michal Schmidt 2011-03-31 18:09:54 UTC
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.

Comment 18 Ivan Romanov 2011-03-31 18:45:01 UTC
(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.

Comment 19 Ivan Romanov 2011-03-31 19:37:35 UTC
(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

Comment 20 Michal Schmidt 2011-04-08 13:03:21 UTC
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

Comment 21 Ivan Romanov 2011-04-09 04:25:36 UTC
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?

Comment 22 Ivan Romanov 2011-04-09 08:32:26 UTC
How you found this conflict?

Comment 23 Michal Schmidt 2011-04-09 13:00:30 UTC
(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'

Comment 24 Ivan Romanov 2011-04-09 13:23:39 UTC
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.

Comment 27 Michal Schmidt 2011-04-18 15:08:04 UTC
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.

Comment 28 Ivan Romanov 2011-04-18 17:11:28 UTC
New Package SCM Request
=======================
Package Name: qconf
Short Description: Tool for generating configure script for qmake-based projects
Owners: ivanromanov
Branches: f15
InitialCC: ivanromanov

Comment 29 Jason Tibbitts 2011-04-19 18:11:13 UTC
Git done (by process-git-requests).

Comment 30 Fedora Update System 2011-04-21 01:15:09 UTC
qconf-1.4-2.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/qconf-1.4-2.fc15

Comment 31 Fedora Update System 2011-04-21 03:01:05 UTC
qconf-1.4-2.fc15 has been pushed to the Fedora 15 testing repository.

Comment 32 Fedora Update System 2011-04-25 02:42:04 UTC
qconf-1.4-2.fc15 has been pushed to the Fedora 15 stable repository.

Comment 33 Ivan Romanov 2011-05-20 07:09:29 UTC
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.

Comment 34 Jason Tibbitts 2011-05-23 17:40:40 UTC
You cannot submit a new package request for a package that is already in the
distribution.  Please submit a change request instead.

Comment 35 Ivan Romanov 2011-05-23 17:53:19 UTC
Package Change Request
=======================
Package Name: qconf
Owners: ivanromanov
New Branches: f14
InitialCC: ivanromanov

Also I need this package in Fedora 14.

Comment 36 Jason Tibbitts 2011-05-24 14:44:31 UTC
Git done (by process-git-requests).

Comment 37 Fedora Update System 2011-05-24 17:34:45 UTC
qconf-1.4-2.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/qconf-1.4-2.fc14

Comment 38 Fedora Update System 2011-05-27 20:28:58 UTC
qconf-1.4-2.fc14 has been pushed to the Fedora 14 stable repository.

Comment 39 Ivan Romanov 2011-10-03 15:18:35 UTC
Package Change Request
=======================
Package Name: qconf
Owners: ivanromanov
New Branches: el6
InitialCC: ivanromanov

Comment 40 Gwyn Ciesla 2011-10-06 20:09:25 UTC
Git done (by process-git-requests).

Comment 41 Fedora Update System 2012-01-01 02:45:54 UTC
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

Comment 42 Fedora Update System 2012-01-03 10:11:13 UTC
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

Comment 43 Fedora Update System 2012-01-18 19:56:38 UTC
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.

Comment 44 Ivan Romanov 2015-04-30 22:54:30 UTC

Package Change Request
=======================
Package Name: qconf
Owners: ivanromanov
New Branches: epel7
InitialCC: ivanromanov

Comment 45 Gwyn Ciesla 2015-05-01 11:08:47 UTC
Git done (by process-git-requests).


Note You need to log in before you can comment on or make changes to this bug.