Bug 458784 - Review Request: xcb-util - The xcb-util module provides a number of libraries which sit on top of libxcb
Summary: Review Request: xcb-util - The xcb-util module provides a number of libraries...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Patrice Dumas
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: awesome
TreeView+ depends on / blocked
 
Reported: 2008-08-12 09:50 UTC by Michal Nowak
Modified: 2013-03-08 02:04 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-12-19 00:40:58 UTC
Type: ---
Embargoed:
pertusus: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Michal Nowak 2008-08-12 09:50:10 UTC
Spec URL: http://mnowak.fedorapeople.org/xcb-util/xcb-util.spec
SRPM URL: http://mnowak.fedorapeople.org/xcb-util/xcb-util-0.2.1-1.fc9.src.rpm
Description:

The xcb-util module provides a number of libraries which sit on top of
libxcb, the core X protocol library, and some of the extension
libraries. These experimental libraries provide convenience functions
and interfaces which make the raw X protocol more usable. Some of the
libraries also provide client-side code which is not strictly part of
the X protocol but which have traditionally been provided by Xlib.

This is dependency of bug 452427

Comment 1 Michal Nowak 2008-08-12 10:04:41 UTC
There are some sort of legal issues because of source files do not have proper license statements. It's being solved. (However it's in Debian already.)

Comment 2 Michal Nowak 2008-08-12 10:05:02 UTC
[root@dhcp-lab-192 SPECS]# rpmlint /usr/src/redhat/SRPMS/xcb-util-0.2.1-1.fc9.src.rpm /usr/src/redhat/RPMS/i386/xcb-util-0.2.1-1.fc9.i386.rpm /usr/src/redhat/RPMS/i386/xcb-util-devel-0.2.1-1.fc9.i386.rpm /usr/src/redhat/RPMS/i386/xcb-util-debuginfo-0.2.1-1.fc9.i386.rpm xcb-util.spec 
xcb-util-devel.i386: W: no-documentation
4 packages and 1 specfiles checked; 0 errors, 1 warnings.

Comment 3 Jason Tibbitts 2008-08-16 17:16:04 UTC
Fails to build for me in rawhide:
  checking for gperf... no
  configure: error: Can't find gperf, please install it and try again
fixing that with a build dep on gperf, I get to:
  checking for XCB...
  configure: error: Package requirements (xcb >= 1.0) were not met:
  No package 'xcb' found
Adding libxcb-devel gets up to:

Making all in atom
make[1]: Entering directory `/builddir/build/BUILD/xcb-util-0.2.1/atom'
I. atoms.gperf.m4 >atoms.gperf
/bin/sh: I.: command not found
make[1]:
[atoms.gperf] Error 127 (ignored)
gperf --output-file atoms.c atoms.gperf
atoms.gperf: The input file is empty!
make[1]:
*** [atoms.c] Error 1

and at that point I don't know what to do.

Please make sure your packages build in mock or koji before submitting them.

Comment 4 Michal Nowak 2008-08-18 18:47:58 UTC
Thanks for noting that, Jason. I completely forgot on BuildRequires.


* Sun Aug 17 2008 Michal Nowak <mnowak> - 0.2.1-2
- new build deps: gperf, pkgconfig, libxcb, m4, xorg-x11-proto-devel
- not installing *.a files anymore
- configure with --with-pic


* Spec URL: http://www.stud.fit.vutbr.cz/~xnowak01/Fedora/xcb-util/xcb-util.spec
* SRPM URL: http://www.stud.fit.vutbr.cz/~xnowak01/Fedora/xcb-util/xcb-util-0.2.1-2.fc9.src.rpm


Builds in mock.

Comment 6 Patrice Dumas 2008-10-03 18:52:33 UTC
You could use 
make install DESTDIR=$RPM_BUILD_ROOT INSTALL='install -p' to keep 
header files timestamps.

You could do make check in %check.

rpmlint output is ignorable:
xcb-util-devel.i386: W: no-documentation

I'd propose a shorter summary:

  Convenience libraries sitting on top of libxcb

And for the devel summary I propose:
 
  Development and header files for xcb-util

There is a license issue, wm/reply_formats.c notice refers to COPYING
which isn't present.

In rm -rf %{buildroot}%{_libdir}/*.la, the -r is not needed. And the
-f isn't needeed too, I prefer personnally being warned when a file
I had to remove isn't present anymore.

Comment 7 Michal Nowak 2008-10-06 09:08:08 UTC
(In reply to comment #6)
> You could use 
> make install DESTDIR=$RPM_BUILD_ROOT INSTALL='install -p' to keep 
> header files timestamps.

Improved.

> 
> You could do make check in %check.
> 

Done.

> rpmlint output is ignorable:
> xcb-util-devel.i386: W: no-documentation
> 
> I'd propose a shorter summary:
> 
>   Convenience libraries sitting on top of libxcb
> 
> And for the devel summary I propose:
> 
>   Development and header files for xcb-util
> 

Both done.

> There is a license issue, wm/reply_formats.c notice refers to COPYING
> which isn't present.
> 

Probably somewhat more of such problems, see https://bugs.freedesktop.org/show_bug.cgi?id=17078 ...

> In rm -rf %{buildroot}%{_libdir}/*.la, the -r is not needed. And the
> -f isn't needeed too, I prefer personnally being warned when a file
> I had to remove isn't present anymore.

Good idea, why not. Done.



http://www.stud.fit.vutbr.cz/~xnowak01/Fedora/xcb-util/xcb-util.spec

http://www.stud.fit.vutbr.cz/~xnowak01/Fedora/xcb-util/xcb-util-0.3.0-2.el5.src.rpm

Comment 8 Patrice Dumas 2008-10-10 22:18:37 UTC
The license issue is unfortunately a blocker. 

Another comment is that, in my opinion, it is better in general 
to have make variables passed as make arguments and not as 
shell env variables (though both work with gnu make), like:

make install DESTDIR="$RPM_BUILD_ROOT" INSTALL="install -p"

Not a blocker.

Otherwise I haven't seen anything obviously wrong by glancing at the
spec file.

Comment 9 Michal Nowak 2008-10-13 08:37:53 UTC
(In reply to comment #8)
> The license issue is unfortunately a blocker. 
> 

Sure. Pinged on upstream BZ.

> Another comment is that, in my opinion, it is better in general 
> to have make variables passed as make arguments and not as 
> shell env variables (though both work with gnu make), like:
> 
> make install DESTDIR="$RPM_BUILD_ROOT" INSTALL="install -p"

That sounds reasonable.


* Mon Oct 13 2008 Michal Nowak <mnowak> - 0.3.0-3
- move from system env vars to local GNU make ones

http://www.stud.fit.vutbr.cz/~xnowak01/Fedora/xcb-util/xcb-util.spec

http://www.stud.fit.vutbr.cz/~xnowak01/Fedora/xcb-util/xcb-util-0.3.0-2.el5.src.rpm

Comment 10 Michal Nowak 2008-11-24 17:24:10 UTC
The license problem is now resolved upstream. Will publish xcb-util-0.3.1 with the patch (http://cgit.freedesktop.org/xcb/util/commit/?id=7ba0d2c98a6fc033dc1edfd791cfacdace4eab51) soon.

Comment 11 Michal Nowak 2008-11-28 23:35:02 UTC
License issue fixed, patch included in package & upstream git repo.

* http://mnowak.fedorapeople.org/xcb-util/xcb-util-0.3.1-1.fc10.src.rpm
* http://mnowak.fedorapeople.org/xcb-util/xcb-util.spec

Known problem:

xcb-util.i386: W: shared-lib-calls-exit /usr/lib/libxcb-aux.so.0.0.0 exit

see: https://bugs.freedesktop.org/show_bug.cgi?id=18808

Runs fine with awesome.

Ready for review.

Comment 12 Patrice Dumas 2008-12-04 11:49:29 UTC
You didn't picked the summaries from Comment #6?

Otherwise rpmlint is right, given that known problem is fixed upstream:
xcb-util.i386: W: shared-lib-calls-exit /usr/lib/libxcb-aux.so.0.0.0 exit
xcb-util-devel.i386: W: no-documentation

* follow guidelines
* free software, license corrected, not included, but not included upstream either
* library rightly packaged
* match upstream
4b06006e438c3926d077439b31d290d6  xcb-util-0.3.1.tar.bz2
* %files section right
* sane provides

2 issues:
URL should be http://xcb.freedesktop.org/
source tarball timestamp not kept:
-rw-rw-r-- 1 dumas dumas 265979 nov. 24 00:33 ../SOURCES/xcb-util-0.3.1.tar.bz2
-rw-rw-r-- 1 dumas dumas 265979 nov. 20 15:36 xcb-util-0.3.1.tar.bz2

APPROVED if you fix the 2 issues.

Comment 13 Michal Nowak 2008-12-04 12:31:09 UTC
(In reply to comment #12)
> You didn't picked the summaries from Comment #6?

Now I do. Thx for them.

> Otherwise rpmlint is right, given that known problem is fixed upstream:
> xcb-util.i386: W: shared-lib-calls-exit /usr/lib/libxcb-aux.so.0.0.0
> exit
> xcb-util-devel.i386: W: no-documentation

In -2 I have a patch for this from upstream git.

> * follow guidelines
> * free software, license corrected, not included, but not included upstream
> either

It is included. (The copyright patch.)

> * library rightly packaged
> * match upstream
> 4b06006e438c3926d077439b31d290d6  xcb-util-0.3.1.tar.bz2
> * %files section right
> * sane provides
> 
> 2 issues:
> URL should be http://xcb.freedesktop.org/

Fixed.

> source tarball timestamp not kept:
> -rw-rw-r-- 1 dumas dumas 265979 nov. 24 00:33 ../SOURCES/xcb-util-0.3.1.tar.bz2
> -rw-rw-r-- 1 dumas dumas 265979 nov. 20 15:36 xcb-util-0.3.1.tar.bz2

Should be in -2.

> APPROVED if you fix the 2 issues.

Please, do so.

--
http://www.stud.fit.vutbr.cz/~xnowak01/Fedora/xcb-util/xcb-util.spec

http://www.stud.fit.vutbr.cz/~xnowak01/Fedora/xcb-util/xcb-util-0.3.1-2.el5.src.rpm

Comment 14 Patrice Dumas 2008-12-04 14:35:23 UTC
Everything is alrigth, I confirm my approval.

Comment 15 Michal Nowak 2008-12-04 14:50:10 UTC
Patrice, thank you for your review.

Comment 16 Kevin Fenzi 2008-12-05 05:13:32 UTC
Please add a cvs template here for what you want and reset the fedora-cvs flag to ?

See: http://fedoraproject.org/wiki/PackageMaintainers/CVSAdminProcedure

Comment 17 Michal Nowak 2008-12-05 09:59:18 UTC
New Package CVS Request
=======================
Package Name: xcb-util
Short Description: Convenience libraries sitting on top of libxcb
Owners: mnowak
Branches: F-9 F-10

Comment 18 Kevin Fenzi 2008-12-07 03:28:51 UTC
cvs done.

Comment 19 Milos Jakubicek 2008-12-16 00:11:23 UTC
Hm, is it just me seeing rpaths here?

>rpmbuild --rebuild xcb-util-0.3.1-2.fc11.src.rpm
...
+ /usr/lib/rpm/check-rpaths /usr/lib/rpm/check-buildroot                                                                                                     
*******************************************************************************                                                                              
*                                                                                                                                                            
* WARNING: 'check-rpaths' detected a broken RPATH and will cause 'rpmbuild'                                                                                  
*          to fail. To ignore these errors, you can set the '$QA_RPATHS'                                                                                     
*          environment variable which is a bitmask allowing the values                                                                                       
*          below. The current value of QA_RPATHS is 0x0000.                                                                                                  
*                                                                                                                                                            
*    0x0001 ... standard RPATHs (e.g. /usr/lib); such RPATHs are a minor                                                                                     
*               issue but are introducing redundant searchpaths without                                                                                      
*               providing a benefit. They can also cause errors in multilib                                                                                  
*               environments.                                                                                                                                
*    0x0002 ... invalid RPATHs; these are RPATHs which are neither absolute                                                                                  
*               nor relative filenames and can therefore be a SECURITY risk
*    0x0004 ... insecure RPATHs; these are relative RPATHs which are a
*               SECURITY risk
*    0x0008 ... the special '$ORIGIN' RPATHs are appearing after other
*               RPATHs; this is just a minor issue but usually unwanted
*    0x0010 ... the RPATH is empty; there is no reason for such RPATHs
*               and they cause unneeded work while loading libraries
*    0x0020 ... an RPATH references '..' of an absolute path; this will break
*               the functionality when the path before '..' is a symlink
*
*
* Examples:
* - to ignore standard and empty RPATHs, execute 'rpmbuild' like
*   $ QA_RPATHS=$[ 0x0001|0x0010 ] rpmbuild my-package.src.rpm
* - to check existing files, set $RPM_BUILD_ROOT and execute check-rpaths like
*   $ RPM_BUILD_ROOT=<top-dir> /usr/lib/rpm/check-rpaths
*
*******************************************************************************
ERROR   0001: file '/usr/lib64/libxcb-wm.so.0.0.0' contains a standard rpath '/usr/lib64' in [/usr/lib64]
ERROR   0001: file '/usr/lib64/libxcb-image.so.0.0.0' contains a standard rpath '/usr/lib64' in [/usr/lib64]
ERROR   0001: file '/usr/lib64/libxcb-icccm.so.1.0.0' contains a standard rpath '/usr/lib64' in [/usr/lib64]
ERROR   0001: file '/usr/lib64/libxcb-property.so.1.0.0' contains a standard rpath '/usr/lib64' in [/usr/lib64]

Please fix them accordingly to https://fedoraproject.org/wiki/Packaging/Guidelines#Beware_of_Rpath

Comment 20 Michal Nowak 2008-12-18 14:40:53 UTC
Confirmed, contacted upstream on this, will be properly resolved until this hits fedora-X-testing. Temporary fix in place now.

* Thu Dec 18 2008 Michal Nowak <mnowak> - 0.3.2-1
- 0.3.2
- remove rpath (x86-64)
- xcb_keysyms: remove xcb_lookup_t
- Revert "keysyms: use xcb_key_lookup_t type for col paramter"
- temporary disabled %%check due to RPATH regression

Soon in rawhide.

Thanks for report.

Comment 21 Michal Nowak 2008-12-18 21:09:00 UTC
xcb-util-0.3.2-1.fc11 successfully built in Koji

Comment 22 Milos Jakubicek 2008-12-19 00:43:24 UTC
Ops, sorry for noise, NEXTRELEASE is proper resolution of course.


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