Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 199402 - Review Request: chrpath - Modify rpath of compiled programs
Summary: Review Request: chrpath - Modify rpath of compiled programs
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Dan Horák
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT 199405
TreeView+ depends on / blocked
 
Reported: 2006-07-19 11:15 UTC by Axel Thimm
Modified: 2007-11-30 22:11 UTC (History)
0 users

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-07-23 10:59:00 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Axel Thimm 2006-07-19 11:15:26 UTC
Spec URL: http://people.atrpms.net/~athimm/fedorasubmit/chrpath/chrpath.spec
SRPM URL: http://people.atrpms.net/~athimm/fedorasubmit/chrpath/chrpath-0.13-1.at.src.rpm
Description:
chrpath allows you to modify the dynamic library load path (rpath) of
compiled programs.  Currently, only removing and modifying the rpath
is supported.

Comment 1 Dan Horák 2006-07-19 12:01:45 UTC
You should add '-%(%{__id_u} -n)' at the end of BuildRoot definition.

Comment 2 Michał Bentkowski 2006-07-19 12:26:18 UTC
I haven't been sponsored yet, so this is not official review.

MUST items:
	* rpmlint doesn't show anything.
	* package is named according to the Package Naming Guidelines.
	* spec file is named correctly.
	* package is licensed with an open-source GPL license.
	* the License field in spec matches the actual license.
	* license file is included in %doc.
	* spec file is legible.
	* package succesfully compiles on i386.
	* there is no need to any build dependencies - package successfully 
compile on
mock.
	* there is no locales.
	* there is no shared library files.
	* there is no duplicate files in %files section.
	* %files section includes %defattr line.
	* package has %clean section.
	* macros are used properly.
	* there is no need to -doc subpackage.
	* files in %doc don't affect the runtime of the application.
	* there is no GUI applications.
COMMENTS:
	* I cannot check if sources match md5sum because I cannot connect to 
ftp.hungry.com server.
	* BuildRoot should be: %{_tmppath}/%{name}-%{version}-%{release}-
root-%(%{__id_u} -n)
	* why do you use 'make' instead of 'make %{?_smp_mflags}'? According to 
Parallel
make chapter of Packaging Guidelines you should use the second option.

Comment 3 Dan Horák 2006-07-19 12:44:53 UTC
I agree with my pre-reviewer :-) Only the using of %{_smp_flags} is not much
useful when compiling 4 small C files.

Review:
- no rpmlint output on any package
- package name OK
- spec file name OK, is in English and is legible
- package meets the Packaging Guidelines
- license OK (GPL) and is included
- ?source matches upstream? - couldn't be checked due the problems of ftp.hungry.com
- compiles and builds at least on i386 (FC4 and devel)
- no BuildRequires needed
- no localized files
- has no shared lib
- do not create any directory
- no duplicates files, permissions are set properly, uses %defattr
- has %clean section
- consistent use of macros
- contains code
- no large docs, %doc is not required during runtime
- no devel subpackage required, no pkgconfig file
- no .la libtool archives
- not a GUI application
- it works

APPROVED, when you fix the BuildRoot


Comment 4 Axel Thimm 2006-07-19 13:17:57 UTC
Michael, you don't need to be sponsored to review a package, you only need to
open up an account on admin.fedora.redhat.com/accounts/

On the comments by both (thanks for the fast replies!):
o ftp.hungry.com has been often bad for me, too. Have you tried accessing the file 
  (not the folder) directly?
o BuildRoot as quoted is the "preferred buildroot" which doesn't really make
  sense. I submitted a request to review the guidelines on this. But as a
  _preferred_ entry this is considered a SHOULD, not MUST. 
o %{?_smp_mflags} should be used IMO if one really knows the package builds as 
  such. I have been bitten by too many Makefiles that didn't build in parallel.
  As the ordering is non-deterministic there is noooo way to find out other then
  reviewing the Makefiles themselves. That may make sense on large packages of
  the size of openoffice, but for a tiny package the review and risk of Makefile
  bugs isn't worth the few CPU cycles.


Comment 5 Michał Bentkowski 2006-07-19 14:44:16 UTC
(In reply to comment #4)
> Michael, you don't need to be sponsored to review a package, you only need to
> open up an account on admin.fedora.redhat.com/accounts/

I have written it to take note of I'm not sponsored ;-) maybe someone will have 
a look to one of my packages.

> o ftp.hungry.com has been often bad for me, too. Have you tried accessing the 
file 
>   (not the folder) directly?

Yes, wget output:

Resolving ftp.hungry.com... 199.181.107.40
Connecting to ftp.hungry.com|199.181.107.40|:21... connected.
Logging in as anonymous ... Logged in!
==> SYST ... done.    ==> PWD ... done.
==> TYPE I ... done.  ==> CWD /pub/hungry/chrpath ... done.
==> PASV ... couldn't connect to 199.181.107.40 port 58316: No route to host

Comment 6 Dan Horák 2006-07-19 15:34:59 UTC
I have tried both wget and ftp and I have the same problems. Also have played
with passive/no passive. No success yet.

Because the BuildRoot is only suggested, the package is APPROVED even when you
disagree with me/us.

Comment 7 Axel Thimm 2006-07-20 08:31:59 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > Michael, you don't need to be sponsored to review a package, you only need >
> > to open up an account on admin.fedora.redhat.com/accounts/
> 
> I have written it to take note of I'm not sponsored ;-) maybe someone will
> have a look to one of my packages.

OK, I understand now. I'd take a look, but I'm not a sponsor. :/
Ping me for the second package to get a review for.

(In reply to comment #6)
> Because the BuildRoot is only suggested, the package is APPROVED even when you
> disagree with me/us.

Thanks!

Comment 8 Michał Bentkowski 2006-07-20 08:55:00 UTC
(In reply to comment #7)
> OK, I understand now. I'd take a look, but I'm not a sponsor. :/
> Ping me for the second package to get a review for.

My packages are Bug 199192 and Bug 198878. More important to me is the first 
one.

Comment 9 Axel Thimm 2006-07-23 10:59:00 UTC
Thanks, packages for FC4-FC6 have been built and will appear soon.

I saw you got sponsored on the second bug, so I can look into the first one.


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