Bug 226445 - Merge Review: symlinks
Merge Review: symlinks
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: manuel wolfshant
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-31 16:03 EST by Nobody's working on this, feel free to take it
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-03-06 06:28:38 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
wolfy: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 16:03:25 EST
Fedora Merge Review: symlinks

http://cvs.fedora.redhat.com/viewcvs/devel/symlinks/
Initial Owner: twaugh@redhat.com
Comment 1 Ralf Corsepius 2007-02-06 11:57:40 EST
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 12:20:23 EST
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 14:44:06 EST
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-06 21:11:07 EST
(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 05:05:04 EST
Tagged and built as 1.2-28.fc7.
Comment 6 manuel wolfshant 2007-02-23 06:20:38 EST
 - 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 07:58:42 EST
Tagged and built as 1.2-29.fc7.
Comment 8 manuel wolfshant 2007-02-23 09:12:36 EST
All problems fixed. Packaged is APPROVED
Comment 9 manuel wolfshant 2007-03-06 04:25:59 EST
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.