Bug 226445 - Merge Review: symlinks
Summary: Merge Review: symlinks
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: manuel wolfshant
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-01-31 21:03 UTC by Nobody's working on this, feel free to take it
Modified: 2007-11-30 22:11 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-03-06 11:28:38 UTC
Type: ---
Embargoed:
manuel.wolfshant: fedora-review+


Attachments (Terms of Use)

Description Nobody's working on this, feel free to take it 2007-01-31 21:03:25 UTC
Fedora Merge Review: symlinks

http://cvs.fedora.redhat.com/viewcvs/devel/symlinks/
Initial Owner: twaugh

Comment 1 Ralf Corsepius 2007-02-06 16:57:40 UTC
A very interesting case ;)

1. rpmlint:
W: symlinks summary-ended-with-dot A utility which maintains a system's symbolic
links.
Stylishness - Should be fixed.

W: symlinks invalid-license distributable
More on this below.

W: symlinks no-url-tag
Doesn't make much sense, but to satisfy the burecrats, I'd propose to use
URL: ftp://metalab.unc.edu/pub/Linux/utils/file/

2. License
Could not find an explict license, but a terse "freely distributably" inside of
the *.lsm. tsx-11 origin => Very old, widely used and known to be distributable
package - IMO "distributable" is the correct term for this.

3. *.spec:
Would you explain the getconf-call in:
make CFLAGS="$RPM_OPT_FLAGS $(getconf LFS_CFLAGS)"

getconf causes the package to use the flags it receives from system the package
is being built on
=> 
- Potential (I am inclined to think almost zero) risk of non-deterministic build
results when users rebuild the package
- Not much of an issue when building the package inside of a build system as
part of a distro, except that it might tie this package to the specific
environment it is being built on - I am not sure what to do about it. Hardcoding?


Comment 2 Tim Waugh 2007-02-06 17:20:23 UTC
Thanks.

The purpose of the getconf call is to build with large file support.  The exact
flags to do this vary from platform to platform.  It basically comes down to
'-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64' for 32-bit hosts and nothing at all
for 64-bit hosts as far as I am aware.

Tagged and built 1.2-27.fc7.

Comment 3 Ruben Kerkhof 2007-02-06 19:44:06 UTC
Hi Tim,

I've assigned the ticket back to the original reviewer, that way he can see your comments (he is not on the 
CC list).

Yes, I know, the process is clear as mudd at the moment, but have a look at http://
www.fedoraproject.org/wiki/WarrenTogami/ReviewWithFlags

Ralf, back to you :-)

Comment 4 Ralf Corsepius 2007-02-07 02:11:07 UTC
(In reply to comment #3)
> Ralf, back to you :-)
I never reviewed this package, I just commented.

BTW: 
- BuildRoot doesn't comply to Fedora standards


Comment 5 Tim Waugh 2007-02-07 10:05:04 UTC
Tagged and built as 1.2-28.fc7.

Comment 6 manuel wolfshant 2007-02-23 11:20:38 UTC
 - Package meets naming and packaging guidelines
 - Spec file matches base package name.
 - Spec has consistant macro usage.
 - Meets Packaging Guidelines.
 - License is "distributable", already discussed above
 - License field in spec matches
 - Spec is legible, in American English
 - Sources match upstream, sha1sum:
a3dafe4b55206dcf19a8b4c67252628c2ad3fab4 symlinks-1.2.tar.gz
 - No BuildRequires
 - No locales/find_lang
 - Package is not relocatable
 - Permissions are sane [*]
 - Package has a correct %clean section.
 - Package has correct buildroot
      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 - Package is code or permissible content.
 - Doc subpackage not needed/used (no %doc files at all)
 - no headers/static/.pc/.la libs
 - no need for ldconfig or scriptlets
 - not a GUI
 - Package builds fine in mock/devel/x86_64
 - Package has no duplicate files in %files.
 - Package doesn't own any directories other packages own.
 - Package owns all files it creates; it does not create any directories
 - rpmlint output:
Source RPM:
W: symlinks invalid-license distributable
- discussed above
W: symlinks setup-not-quiet
- please consider using setup -q
Binary RPM:
E: symlinks no-cleaning-of-buildroot %install
that;s a MUSTFIX: Package lacks cleaning the buildroot in the %install section
rpmlint of symlinks:
W: symlinks invalid-license distributable
- see above

SHOULD Items:

 - Should build in mock. - OK for devel/x86_64 and i386
 - Should build on all supported archs - tested on x86_64 and i386, OK
 - Should function as described - OK
 - Should have sane scriptlets - OK (no scriptlets)
 - Should have subpackages require base package with fully versioned depend. -
not needed
 - Should have dist tag OK
 - Should package latest version OK
 - check for outstanding bugs on package. (For core merge reviews) - OK (none)

Summary:
mostly OK, with one MUSTFIX and a couple of cosmetic fixes:
cosmetic:
- please consider using the newer preferred value in %files, (-,root,root,-)
- please add -q to setup in order to silence it
- it would be nice to add usage of smp_flags to make (not that it matters for a
5K source, but the rules are the rules)

MUSTFIX
- %install should contain rm -fR $RPM_BUILD_ROOT

Please fix the above and the package is APPROVED

Comment 7 Tim Waugh 2007-02-23 12:58:42 UTC
Tagged and built as 1.2-29.fc7.

Comment 8 manuel wolfshant 2007-02-23 14:12:36 UTC
All problems fixed. Packaged is APPROVED

Comment 9 manuel wolfshant 2007-03-06 09:25:59 UTC
Tim,

How about importing this one to CVS and closing the ticket ? :)


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