Bugzilla will be upgraded to version 5.0 on a still to be determined date in the near future. The original upgrade date has been delayed.
Bug 599097 - Review Request: libgexiv2 - Gexiv2 is a GObject-based wrapper around the Exiv2 library
Review Request: libgexiv2 - Gexiv2 is a GObject-based wrapper around the Exiv...
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Martin Gieseking
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2010-06-02 13:11 EDT by Ankur Sinha (FranciscoD)
Modified: 2010-09-02 16:48 EDT (History)
6 users (show)

See Also:
Fixed In Version: libgexiv2-0.2.0-1.fc13
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-09-02 00:00:15 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
martin.gieseking: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Ankur Sinha (FranciscoD) 2010-06-02 13:11:40 EDT
Spec URL: http://ankursinha.fedorapeople.org/libgexiv2/libgexiv2.spec
SRPM URL: http://ankursinha.fedorapeople.org/libgexiv2/libgexiv2-0.0.90-1.fc13.src.rpm

Other results from a mock build are all here:

http://ankursinha.fedorapeople.org/libgexiv2/

Description: ligexiv2 is a GObject-based wrapper around the Exiv2 library.It makes the basic features of Exiv2 available to GNOME applications.

rpmlint output:

[Ankur@localhost SPECS]$ rpmlint libgexiv2.spec ../SRPMS/libgexiv2-0.0.90-1.fc13.src.rpm /var/lib/mock/fedora-rawhide-i386/result/*.rpmlibgexiv2.spec:36: W: configure-without-libdir-spec
libgexiv2.src: W: spelling-error Summary(en_US) Gexiv -> Ge xiv, Ge-xiv, Gelid
libgexiv2.src: W: spelling-error %description -l en_US ligexiv -> lighten, lighted, lighter
libgexiv2.src:36: W: configure-without-libdir-spec
libgexiv2.i686: W: spelling-error Summary(en_US) Gexiv -> Ge xiv, Ge-xiv, Gelid
libgexiv2.i686: W: spelling-error %description -l en_US ligexiv -> lighten, lighted, lighter
libgexiv2.src: W: spelling-error Summary(en_US) Gexiv -> Ge xiv, Ge-xiv, Gelid
libgexiv2.src: W: spelling-error %description -l en_US ligexiv -> lighten, lighted, lighter
libgexiv2.src:36: W: configure-without-libdir-spec
libgexiv2-debuginfo.i686: W: spelling-error Summary(en_US) libgexiv -> Libreville, Liberian, Liberia
libgexiv2-debuginfo.i686: W: spelling-error %description -l en_US libgexiv -> Libreville, Liberian, Liberia
libgexiv2-devel.i686: W: spelling-error Summary(en_US) libgexiv -> Libreville, Liberian, Liberia
libgexiv2-devel.i686: W: spelling-error %description -l en_US libgexiv -> Libreville, Liberian, Liberia
5 packages and 1 specfiles checked; 0 errors, 13 warnings.


NOTE : Line 36 is a comment.
Comment 1 Ankur Sinha (FranciscoD) 2010-06-02 13:13:17 EDT
hi,

typo in description noted and changed

ligexiv2 -> libgexiv2.

regards,
Ankur
Comment 2 Ankur Sinha (FranciscoD) 2010-06-03 03:50:10 EDT
Ticket at the yorba trac regarding the configure script:

http://trac.yorba.org:8000/ticket/2001

Ankur
Comment 3 Ankur Sinha (FranciscoD) 2010-06-03 04:23:42 EDT
hi,

updated:

* Thu Jun 03 2010 Ankur Sinha <ankursinha AT fedoraproject DOT org> - 0.0.90-2
- some fixes in spec
- moved *.vapi to devel
- removed INSTALL from doc
- added comment to yorba ticket link
- corrected typo in description

http://ankursinha.fedorapeople.org/libgexiv2/libgexiv2.spec

http://ankursinha.fedorapeople.org/libgexiv2/libgexiv2-0.0.90-2.fc13.src.rpm

mock build results at 

http://ankursinha.fedorapeople.org/libgexiv2/

regards,
Ankur
Comment 4 Aleš Koval 2010-06-03 14:56:35 EDT
Hi,
provided source rpm cannot be build on x86_64 arch due this error:

error: File not found by glob: /home/als/rpmbuild/BUILDROOT libgexiv2-0.0.90-2.fc13.x86_64/usr/lib64/*.so.*

You using correct %{_libdir} macro in spec file in files section, but there is hardcoded $(DESTDIR)$(PREFIX)/lib path in source Makefile in install section.

Regards Als
Comment 5 Ankur Sinha (FranciscoD) 2010-06-03 16:01:03 EDT
(In reply to comment #4)
> Hi,
> provided source rpm cannot be build on x86_64 arch due this error:
> 
> error: File not found by glob: /home/als/rpmbuild/BUILDROOT
> libgexiv2-0.0.90-2.fc13.x86_64/usr/lib64/*.so.*
> 
> You using correct %{_libdir} macro in spec file in files section, but there is
> hardcoded $(DESTDIR)$(PREFIX)/lib path in source Makefile in install section.
> 
> Regards Als    

hey,

I patched the Makefile

spec: http://ankursinha.fedorapeople.org/libgexiv2/libgexiv2.spec
srpm: http://ankursinha.fedorapeople.org/libgexiv2/libgexiv2-0.0.90-3.fc13.src.rpm

rest of the mock logs : http://ankursinha.fedorapeople.org/libgexiv2/

regards,
Ankur
Comment 6 Aleš Koval 2010-06-03 16:21:49 EDT
(In reply to comment #5)
> srpm:
> http://ankursinha.fedorapeople.org/libgexiv2/libgexiv2-0.0.90-3.fc13.src.rpm

Well, this version now build correct. Good job man :)

Regards Als
Comment 7 Ankur Sinha (FranciscoD) 2010-06-03 23:46:00 EDT
patch sent upstream as per guidelines:

http://trac.yorba.org:8000/ticket/2001#comment:7
Comment 8 Martin Gieseking 2010-06-04 15:17:03 EDT
Hi Ankur, here are some more remarks about your spec file:

- add a short comment above Patch0 telling what the patch does

- you can simplify calling the configure configure script as follows:
    echo '%{configure}' | sed '/--program-prefix=/d' >configure.new
    sh configure.new
  This also fixes the rpmlint warning about a missing libdir specification

- it's not necessary to remove the empty files as they are not listed in %doc and thus not packaged

- drop the %doc files from the -devel package (the files should only be added once)

- %{_includedir}/* is a bit too generic. Replace it with %{_includedir}/gexiv2/
Comment 9 Susi Lehtola 2010-06-04 15:33:17 EDT
(In reply to comment #8)
> - %{_includedir}/* is a bit too generic. Replace it with %{_includedir}/gexiv2/    

I would say the same thing about
 %{_libdir}/*.so
and
 %{_libdir}/*.so.*
since I'd guess there is only one or a couple of libraries that are installed, so I usually recommend filling in these in more precision, e.g.
 %{_libdir}/libfoo.so
and
 %{_libdir}/libfoo.so.*

Also, I think -devel should Requires: vala for dir ownership, as there is a file put in %{_datadir}/vala/vapi/.
Comment 10 Ankur Sinha (FranciscoD) 2010-06-05 01:21:47 EDT
(In reply to comment #9)
> (In reply to comment #8)
> > - %{_includedir}/* is a bit too generic. Replace it with %{_includedir}/gexiv2/    
> 
> I would say the same thing about
>  %{_libdir}/*.so
> and
>  %{_libdir}/*.so.*
> since I'd guess there is only one or a couple of libraries that are installed,
> so I usually recommend filling in these in more precision, e.g.
>  %{_libdir}/libfoo.so
> and
>  %{_libdir}/libfoo.so.*
> 
> Also, I think -devel should Requires: vala for dir ownership, as there is a
> file put in %{_datadir}/vala/vapi/.    

(In reply to comment #8)
> Hi Ankur, here are some more remarks about your spec file:
> 
> - add a short comment above Patch0 telling what the patch does
> 
> - you can simplify calling the configure configure script as follows:
>     echo '%{configure}' | sed '/--program-prefix=/d' >configure.new
>     sh configure.new
>   This also fixes the rpmlint warning about a missing libdir specification
> 
> - it's not necessary to remove the empty files as they are not listed in %doc
> and thus not packaged
> 
> - drop the %doc files from the -devel package (the files should only be added
> once)
> 
> - %{_includedir}/* is a bit too generic. Replace it with %{_includedir}/gexiv2/    

hey,

I've fixed these (at least I think so)

* Sat Jun 05 2010 Ankur Sinha <ankursinha AT fedoraproject DOT org> - 0.0.90-4
- changed configure portion
- added Requires:  vala for devel
- made the file section more precise
- bugzilla #599097 
- changed patch to include a default LIB setting

http://ankursinha.fedorapeople.org/libgexiv2/libgexiv2.spec

http://ankursinha.fedorapeople.org/libgexiv2/libgexiv2-0.0.90-4.fc13.src.rpm

rest of the mock build stuff at:

http://ankursinha.fedorapeople.org/libgexiv2/

regards,
Ankur
Comment 11 Martin Gieseking 2010-06-05 02:23:16 EDT
You can also remove the variable definitions (CFLAGS, CXXFLAGS, FFLAGS) from the %build section. They are part of the %configure macro. 
However, since I obviously was too tired yesterday, I gave you a wrong configure replacement (sorry for that!). The Makefile doesn't recognize the locally set variables. It should work with a %build section that looks like this:

echo '%{configure}' | sed '/--program-prefix=/d' >build.tmp
echo 'make %{?_smp_mflags}' >>build.tmp
sh build.tmp


Since there are no empty files in the buildroot, you should really remove the redundant for loop. :)
Comment 12 Ankur Sinha (FranciscoD) 2010-06-05 02:45:36 EDT
(In reply to comment #11)
> You can also remove the variable definitions (CFLAGS, CXXFLAGS, FFLAGS) from
> the %build section. They are part of the %configure macro. 
> However, since I obviously was too tired yesterday, I gave you a wrong
> configure replacement (sorry for that!). The Makefile doesn't recognize the
> locally set variables. It should work with a %build section that looks like
> this:
> 
> echo '%{configure}' | sed '/--program-prefix=/d' >build.tmp
> echo 'make %{?_smp_mflags}' >>build.tmp
> sh build.tmp
> 
> 
> Since there are no empty files in the buildroot, you should really remove the
> redundant for loop. :)    

hey,

updated spec:

http://ankursinha.fedorapeople.org/libgexiv2/libgexiv2.spec

srpm:

http://ankursinha.fedorapeople.org/libgexiv2/libgexiv2-0.0.90-5.fc13.src.rpm

mock results:

http://ankursinha.fedorapeople.org/libgexiv2/

regards,
Ankur

PS : I dont understand how including the make portion(the new hack) works any different from the earlier hack you had mentioned.
Comment 13 Martin Gieseking 2010-06-05 03:39:58 EDT
(In reply to comment #12)
> PS : I dont understand how including the make portion(the new hack) works any
> different from the earlier hack you had mentioned.    

It has to do with the way the shell handles variables. Each shell gets its own environment so that variables defined there are usually local and unknown outside. Adding "export" makes a variable accessible in the current shell S and also in all its sub-shells, but after returning to the parent of S they are removed. 
So if I, for instance, define CFLAGS in a shell script "build.tmp" and use it in a spec file, CFLAGS is known inside the shell calling "build.tmp" but not in the parent (rpm) environment, i.e. all variable assignments are lost when returning to the parent (rpm) shell. Thus, the following "make" process doesn't know anything about the previously set CFLAGS variable. But when placing the make statement inside "build.tmp", the variable is still accessible. 
I hope that's understandable. :)
Comment 14 Martin Gieseking 2010-06-11 11:54:03 EDT
A new version has been released upstream:
http://yorba.org/download/gexiv2/0.0/unstable/

Would you like to update the package before the formal review?
Comment 15 Ankur Sinha (FranciscoD) 2010-06-11 14:19:28 EDT
(In reply to comment #13)
> (In reply to comment #12)
> > PS : I dont understand how including the make portion(the new hack) works any
> > different from the earlier hack you had mentioned.    
> 
> It has to do with the way the shell handles variables. Each shell gets its own
> environment so that variables defined there are usually local and unknown
> outside. Adding "export" makes a variable accessible in the current shell S and
> also in all its sub-shells, but after returning to the parent of S they are
> removed. 
> So if I, for instance, define CFLAGS in a shell script "build.tmp" and use it
> in a spec file, CFLAGS is known inside the shell calling "build.tmp" but not in
> the parent (rpm) environment, i.e. all variable assignments are lost when
> returning to the parent (rpm) shell. Thus, the following "make" process doesn't
> know anything about the previously set CFLAGS variable. But when placing the
> make statement inside "build.tmp", the variable is still accessible. 
> I hope that's understandable. :)    


hey,

thanks, that does teach me a little more :)

(In reply to comment #14)
> A new version has been released upstream:
> http://yorba.org/download/gexiv2/0.0/unstable/
> 
> Would you like to update the package before the formal review?    

updated:

http://ankursinha.fedorapeople.org/libgexiv2/libgexiv2.spec

http://ankursinha.fedorapeople.org/libgexiv2/libgexiv2-0.0.91-1.fc13.src.rpm

mock results at 

http://ankursinha.fedorapeople.org/libgexiv2/

regards,
Ankur
Comment 16 Martin Gieseking 2010-06-13 14:18:03 EDT
Here's the formal review. The package looks almost fine to me, except one remaining aspect:
- replace %{_includedir}/gexiv2/* with %{_includedir}/gexiv2/
  to make the package own the directory too (and not only the header files)

$ rpmlint /var/lib/mock/fedora-13-x86_64/result/*.rpm
libgexiv2.src: W: spelling-error Summary(en_US) Gexiv -> Ge xiv, Ge-xiv, Gelid
libgexiv2.src: W: spelling-error %description -l en_US libgexiv -> Libreville, Liberian, Liberia
libgexiv2.x86_64: W: spelling-error Summary(en_US) Gexiv -> Ge xiv, Ge-xiv, Gelid
libgexiv2.x86_64: W: spelling-error %description -l en_US libgexiv -> Libreville, Liberian, Liberia
libgexiv2-debuginfo.x86_64: W: spelling-error Summary(en_US) libgexiv -> Libreville, Liberian, Liberia
libgexiv2-debuginfo.x86_64: W: spelling-error %description -l en_US libgexiv -> Libreville, Liberian, Liberia
libgexiv2-devel.x86_64: W: spelling-error Summary(en_US) libgexiv -> Libreville, Liberian, Liberia
libgexiv2-devel.x86_64: W: spelling-error %description -l en_US libgexiv -> Libreville, Liberian, Liberia
libgexiv2-devel.x86_64: W: no-documentation
4 packages and 0 specfiles checked; 0 errors, 9 warnings.

The above spelling errors can be ignored.

---------------------------------
keys used in following checklist:

[+] OK
[.] OK, not applicable
[X] needs work
---------------------------------

[+] MUST: The package must be named according to the Package Naming Guidelines.
[+] MUST: The spec file name must match the base package %{name}.
[+] MUST: The package must meet the Packaging Guidelines.
[+] MUST: The package must be licensed with a Fedora approved license.
[+] MUST: The License field in the package spec file must match the actual license.
[+] MUST: The file containing the text of the license(s) for the package must be included in %doc.
[+] MUST: The spec file must be written in American English.
[+] MUST: The spec file for the package MUST be legible.
[+] MUST: The sources used to build the package must match the upstream source.
    $ md5sum libgexiv2-0.0.91.tar.gz*
    16b6252efabb196ae2bf799104caa0cc  libgexiv2-0.0.91.tar.gz
    16b6252efabb196ae2bf799104caa0cc  libgexiv2-0.0.91.tar.gz.1

[+] MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture.
[.] MUST: If the package does not successfully compile, build or work on an architecture, ...
[+] MUST: All build dependencies must be listed in BuildRequires.
[.] MUST: The spec file MUST handle locales properly.
[+] MUST: Packages storing shared library files (not just symlinks) must call ldconfig in %post and %postun.
[+] MUST: Packages must NOT bundle copies of system libraries.
[.] MUST: If the package is designed to be relocatable, ...
[X] MUST: A package must own all directories that it creates.
    - directory %{_includedir}/gexiv2/ must be owned by the -devel package

[+] MUST: A Fedora package must not list a file more than once in %files.
[+] MUST: Permissions on files must be set properly.
[+] MUST: Each package must consistently use macros.
[+] MUST: The package must contain code, or permissable content.
[.] MUST: Large documentation files must go in a -doc subpackage.
[+] MUST: Files in %doc must not affect the runtime of the application.
[+] MUST: Header files must be in a -devel package.
[.] MUST: Static libraries must be in a -static package.
[+] MUST: Library files that end in .so (without suffix) must go in a -devel package.
[+] MUST: devel packages must require the base package using a fully versioned dependency.
[+] MUST: Packages must NOT contain any .la libtool archives, these must be removed in the spec if they are built.
[.] MUST: Packages containing GUI applications ...
[+] MUST: Packages must not own files or directories already owned by other packages.
[+] MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot}.
[+] MUST: All filenames in rpm packages must be valid UTF-8.

[+] SHOULD: The reviewer should test that the package builds in mock.
[+] SHOULD: The package should compile and build into binary rpms on all supported architectures.
[+] SHOULD: The reviewer should test that the package functions as described. 
[+] SHOULD: If scriptlets are used, those scriptlets must be sane.
[.] SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency.
[+] SHOULD: pkgconfig(.pc) files should be placed in a -devel pkg.
[+] SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself.
Comment 17 Ankur Sinha (FranciscoD) 2010-06-14 11:55:01 EDT
(In reply to comment #16)
hey, 

Thank you for reviewing this :)

> Here's the formal review. The package looks almost fine to me, except one
> remaining aspect:
> - replace %{_includedir}/gexiv2/* with %{_includedir}/gexiv2/
>   to make the package own the directory too (and not only the header files)
> ......
> .......
> [X] MUST: A package must own all directories that it creates.
>     - directory %{_includedir}/gexiv2/ must be owned by the -devel package
> 

Fixed it. 

new srpm :

http://ankursinha.fedorapeople.org/libgexiv2/libgexiv2-0.0.91-2.fc13.src.rpm

new spec

http://ankursinha.fedorapeople.org/libgexiv2/libgexiv2.spec

rest of mock logs at 

http://ankursinha.fedorapeople.org/libgexiv2/

regards,
Ankur
Comment 18 Martin Gieseking 2010-06-14 12:49:14 EDT
(In reply to comment #17)
> Thank you for reviewing this :)

You're welcome.


> > [X] MUST: A package must own all directories that it creates.
> >     - directory %{_includedir}/gexiv2/ must be owned by the -devel package
> > 
> Fixed it. 

OK, the package is ready now, and we can finish here. :)

----------------
Package APPROVED
----------------
Comment 19 Ankur Sinha (FranciscoD) 2010-06-14 13:06:00 EDT
New Package CVS Request
=======================
Package Name: libgexiv2
Short Description: ligexiv2 is a GObject-based wrapper around the Exiv2 library
Owners: ankursinha
Branches: F-12 F-13
InitialCC:
Comment 20 Kevin Fenzi 2010-06-20 22:14:42 EDT
CVS done (by process-cvs-requests.py).
Comment 21 Fedora Update System 2010-06-21 00:15:37 EDT
libgexiv2-0.0.91-2.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/libgexiv2-0.0.91-2.fc13
Comment 22 Fedora Update System 2010-06-21 17:44:22 EDT
libgexiv2-0.0.91-2.fc13 has been pushed to the Fedora 13 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 libgexiv2'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/libgexiv2-0.0.91-2.fc13
Comment 23 Fedora Update System 2010-08-08 06:26:37 EDT
libgexiv2-0.1.90-2.fc14 has been submitted as an update for Fedora 14.
http://admin.fedoraproject.org/updates/libgexiv2-0.1.90-2.fc14
Comment 24 Fedora Update System 2010-08-08 06:26:43 EDT
libgexiv2-0.1.90-1.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/libgexiv2-0.1.90-1.fc13
Comment 25 Fedora Update System 2010-08-09 21:31:05 EDT
libgexiv2-0.1.90-2.fc14 has been pushed to the Fedora 14 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 libgexiv2'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/libgexiv2-0.1.90-2.fc14
Comment 26 Fedora Update System 2010-08-10 17:43:36 EDT
libgexiv2-0.1.90-1.fc13 has been pushed to the Fedora 13 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 libgexiv2'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/libgexiv2-0.1.90-1.fc13
Comment 27 Fedora Update System 2010-08-24 09:27:33 EDT
libgexiv2-0.2.0-1.fc14 has been submitted as an update for Fedora 14.
http://admin.fedoraproject.org/updates/libgexiv2-0.2.0-1.fc14
Comment 28 Fedora Update System 2010-08-24 09:37:40 EDT
libgexiv2-0.2.0-1.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/libgexiv2-0.2.0-1.fc13
Comment 29 Fedora Update System 2010-09-02 00:00:06 EDT
libgexiv2-0.2.0-1.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 30 Fedora Update System 2010-09-02 16:48:07 EDT
libgexiv2-0.2.0-1.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

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