Bug 509856 - Review Request: qrencode - The libqrencode library and application encodes QR Code symbols (2d barcodes)
Review Request: qrencode - The libqrencode library and application encodes QR...
Status: CLOSED DUPLICATE of bug 612533
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-07-06 11:02 EDT by Bowe Strickland
Modified: 2013-10-19 10:42 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-07-08 12:36:15 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Bowe Strickland 2009-07-06 11:02:41 EDT
Spec URL: http://people.redhat.com/bowe/rpms/qrencode/qrencode.spec
SRPM URL: http://people.redhat.com/bowe/rpms/qrencode/qrencode-3.1.0-1.fc11.src.rpm
Description: 
qrencode is a C application for encoding data in a QR Code symbol, a
kind of 2D symbology that can be scanned by handy terminals such as a
mobile phone with CCD. The capacity of QR Code is up to 7000 digits or
4000 characters, and is highly robust.

IN NEED OF SPONSOR: this is my first package.
Comment 1 Steve Traylen 2009-07-10 10:10:00 EDT
Hi Bowe,

This is an unofficial review:

rpmlint of spec file:

$ rpmlint qrencode.spec 
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

Building the package:

1) checking pkg-config is at least version 0.9.0... yes
  checking for png... configure: error: Package requirements ("libpng12")
  were not met:

  No package 'libpng12' found

   After installing "libpng-devel" it completes so should be added to the
   BuildRequires of course.

2) Rpath errors. If you add 

%__arch_install_post   /usr/lib/rpm/check-rpaths   /usr/lib/rpm/check-buildroot

to your ~/.rpmmacros  which you do by running "rpmdev-setuptree" then
you get the following...


ERROR   0001: file '/usr/bin/qrencode' contains a standard rpath '/usr/lib64' in [/usr/lib64]
error: Bad exit status from /var/tmp/rpm-tmp.6ynHaY (%install)

See:

http://fedoraproject.org/wiki/Packaging/Guidelines#Beware_of_Rpath

for explanation.

Steve
Comment 2 Steve Traylen 2009-07-23 14:16:00 EDT
ping
Comment 3 Bowe Strickland 2009-07-23 16:17:27 EDT
missed comment #1 notification.  looking now...
Comment 4 Bowe Strickland 2009-07-23 16:50:50 EDT
hmmm... must be a 64 bit thing... i've got %__arch_install_post as speced, and still seeing a clean build:

>> Executing(%install): /bin/sh -e /var/tmp/rpm-tmp.odUISp
>> ....
>> extracting debug info from /home/bowe/rpmbuild/BUILDROOT/qrencode->3.1.0-1.fc11.i386/usr/lib/libqrencode.so.3.1.0
>> 230 blocks
>> + /usr/lib/rpm/check-rpaths /usr/lib/rpm/check-buildroot
>> + /usr/lib/rpm/redhat/brp-compress
>> ...

also, i'm familiar with concept but not implementation, but can't find rpath reference in executable:

[bowe@granny d]$ rpm2cpio /home/bowe/rpmbuild/RPMS/i586/qrencode-3.1.0-1.fc11.i586.rpm | cpio -id
194 blocks
[bowe@granny d]$ readelf -a usr/bin/qrencode | grep -i -e rpath -e lib64 
[bowe@granny d]$ 

at any rate, blindly added second solution in guidelines.

updated spec file at 
  http://people.redhat.com/bowe/rpms/qrencode/qrencode.spec
updated srpm at 
  http://people.redhat.com/bowe/rpms/qrencode/qrencode-3.1.0-2.fc11.src.rpm


thanks!
Comment 5 Jason Tibbitts 2009-08-05 22:02:28 EDT
rpath is indeed generally a 64-bit thing.  Libtool somehow knows not to add /usr/lib to rpath, but can't figure out not to add /usr/lib64.  That's libtool for you.  However, when you add hacks like those sed calls over libtool, it's best to also add a comment indicating why it's necessary.

You should not duplicate those doc files between the main and -devel packages.

Are you sure the license is GPLv2 only?  It looks like LGPLv2+ to me.  The COPYING file contains the LGPL and the source code and upstream web site all says:
 * [...] GNU Lesser General Public
 * License as published by the Free Software Foundation; either
 * version 2.1 of the License, or any later version.
which is what we call LGPLv2+.

Fix that up and I can go ahead and do a full review.
Comment 6 Bowe Strickland 2009-08-07 14:55:17 EDT
sed hacks commented, doc files trimmed (duped COPYING only), and obviously went too quickly when examining licenses... sorry :(.

updated spec file at 
  http://people.redhat.com/bowe/rpms/qrencode/qrencode.spec
updated srpm at 
  http://people.redhat.com/bowe/rpms/qrencode/qrencode-3.1.0-3.fc11.src.rpm

thanks!
Comment 7 Bowe Strickland 2009-08-25 10:32:36 EDT
ping... just making sure the ball's in yall's court...
Comment 8 Mamoru TASAKA 2009-08-29 14:41:35 EDT
Some random comments

? About enabling tests
  - As this package contains tests/ directory, it is usually highly recommended
    that you add %check section and execute some test program there.

    However for this package to execute test_all.sh under tests/ directory
    we have to add "--with-tests" option explicitly to configure
    - because without this option some symbols needed for test program are
      marked as "static" and these symbols are not included in libqrencode.so.
    i.e. when "--with-tests" option is added to configure, it will add
         some addtional symbols to libqrencode.so which seems to be used only
         the test program in this tarball.

    So while I think generally executing test program is preferable, for this
    package I am not sure. How do you think?

* Timestamps
  - Please consider to use
-----------------------------------------------------------------
make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"
-----------------------------------------------------------------
    to keep timestamps on installed files as much as possible (especially
    on header files).
    This method usually works for Makefiles generated from recent autotools.

* pkgconfig file
  - Currently I don't see any reason why pkgconfig file should be deleted.
    This pkgconfig file may be used by other applications.

* Documents
  - You don't have to add documents to subpackages which are already
    included in the main package.
Comment 9 Mamoru TASAKA 2009-09-12 10:54:59 EDT
ping?
Comment 10 Bowe Strickland 2009-09-14 09:33:10 EDT
re: testing:
   - to me some extra symbols doesn't seem overly burdensome, if the library
     doesn't bloat much.  never used a %check before, though, so would need
     to research it some, and currently under the gun.  am willing to add it
     in future, but want to incorporate other quick changes.

- pgkconfig kept
- install -p added
- migrated docs from -devel to base package, rpmlint now warns there's no docs. shrug. 


updated spec file at 
  http://people.redhat.com/bowe/rpms/qrencode/qrencode.spec
updated srpm at 
  http://people.redhat.com/bowe/rpms/qrencode/qrencode-3.1.0-4.fc11.src.rpm
Comment 11 Mamoru TASAKA 2009-09-15 12:20:25 EDT
Well, now this package itself is okay, so:

-------------------------------------------------------------
NOTE: Before being sponsored:

This package will be accepted with another few (or no) work. 
But before I accept this package, someone (I am a candidate) 
must sponsor you.

Once you are sponsored, you have the right to review other 
submitters' review requests and approve the packages formally. 
For this reason, the person who want to be sponsored (like you) 
are required to "show that you have an understanding 
of the process and of the packaging guidelines" as is described
on :
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

Usually there are two ways to show this.
A. submit other review requests with enough quality.
B. Do a "pre-review" of other person's review request
   (at the time you are not sponsored, you cannot do
   a formal review)

When you have submitted a new review request or have pre-reviewed other 
person's review request, please write the bug number on this bug report 
so that I can check your comments or review request.

Fedora package collection review requests which are waiting for someone to
review can be checked on my wiki page:
http://fedoraproject.org/wiki/User:Mtasaka#B._Review_request_tickets
(Check "No one is reviewing")

Review guidelines are described mainly on:
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
------------------------------------------------------------
Comment 12 Mamoru TASAKA 2009-09-30 12:11:46 EDT
ping?
Comment 13 Mamoru TASAKA 2009-10-10 12:19:29 EDT
ping again?
Comment 14 Mamoru TASAKA 2009-10-19 13:49:48 EDT
Again ping?
Comment 15 Mamoru TASAKA 2009-11-07 12:32:39 EST
I will close this bug as NOTABUG within one week if no response
is received from the reporter within ONE WEEK.
Comment 16 Bowe Strickland 2009-11-10 04:30:11 EST
yep... at this point, i'm finding myself motivated to maintain this package, but can't find the motivation to carve out some time to learn to review others.

if that's not a fit with the fedora mentality, i'd understand, and may try again later when life has less competing requirements.

--b
Comment 17 Mamoru TASAKA 2009-11-10 11:23:43 EST
Well, as written above I request all submitter who needs sponsorship
to either submit another review request or do at least one pre-review
if _I_ am going to sponsor him/her. 

So without it I am not going to sponsor you, however some other sponsor
(sponsors are not only me) may want to sponsor you. So for now I will
revoke the role of reviewing this ticket so that you can find other
sponsor and get sponsored.
Comment 18 Michael Schwendt 2010-01-11 10:40:19 EST
Re: comment 16

> if that's not a fit with the fedora mentality, [...]

Wow!

As another potential sponsor, who has run into this ticket, let me add the following:

For a lower barrier to entry, I approve some packagers' accounts without requiring them to do reviews of stuff they may not have any interest in. You can review a hundred [perhaps trivial] packages and do packaging mistakes in your own packages nevertheless. Sometimes, someone wants to maintain a single package only, but shows off sufficient motivation in becoming a Fedora package contributor. If during the review process such a person is easy to deal with, responsive and open to hints and corrections, that's a chance to get sponsored because of criteria which are unrelated to having done prior reviews. If later on, the person changes attitudes and causes trouble, sponsorship may be revoked.

If, on the contrary, a review -- like this one -- is not trouble-free already, the chance to get sponsored is reduced.

You may find somebody inside Red Hat to sponsor you nevertheless based on other criteria, but during this review your comments/answers have been counterproductive.
Comment 19 Bowe Strickland 2010-01-12 13:43:18 EST
It was not my intent to show disrespect, and I greatly appreciate the
time that Steve, Jason, and Mamoru have spent in reviewing my submission.  
With the exception of %check, I thought I had willingly implemented all of the suggestions.

By submitting packaging I was already doing for another project, I
hoped to learn to make more compliant packages, and to contribute to
the Fedora project what I think is worthwhile and relevant software,
and my efforts in supporting that software through future releases.

Through the process, I'm learning that the Fedora project greatly values
time spent reviewing the work of others.  This seems wise, and could
well be the policy which keeps the system working.  I certainly have
respect for the process of peer review.

Due to family, work, repetative stress, circadean rhythms, yadda yadda,
however, it's not something I think I can currently commit to.  In my
previous life as a grad student, in my subsequent life as a senile old
man, with hope even next month, my decisions could well be different.

I like to consider OSS and in particular Linux as one of the cathedrals of
our age, a beautiful work of craftmanship to which people devote lifetimes
of effort.  My intent wasn't to p*** off its artisans...

Thank you for all of your contributions!
Comment 20 Mamoru TASAKA 2010-07-08 12:36:15 EDT

*** This bug has been marked as a duplicate of bug 612533 ***

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