Bug 193480 - Review Request: sunifdef
Review Request: sunifdef
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Ralf Corsepius
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-05-29 10:29 EDT by Jonathan Underwood
Modified: 2010-04-07 10:45 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-06-07 12:01:05 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Jonathan Underwood 2006-05-29 10:29:55 EDT
Spec URL: http://physics.open.ac.uk/~ju83/sunifdef.spec
SRPM URL: http://physics.open.ac.uk/~ju83/sunifdef-1.0-1.src.rpm
Description: 
Sunifdef is a commandline tool for simplifying the preprocessor conditionals in
source code (\#if and related directives) based on the the user's chosen
interpretation of the preprocessor symbols. It is a more powerful successor to
the FreeBSD 'unifdef' tool.
Comment 1 Parag AN(पराग) 2006-06-01 02:03:51 EDT
SPEC looks OK but SOURCE compilations gives some warnings
Comment 2 Ralf Corsepius 2006-06-01 03:03:17 EDT
# rpmlint sunifdef-*
E: sunifdef description-line-too-long Sunifdef is most useful to developers of
constantly evolving products with large
E: sunifdef-debuginfo script-without-shellbang
/usr/src/debug/sunifdef/src/ptr_vector.c
E: sunifdef-debuginfo wrong-script-end-of-line-encoding
/usr/src/debug/sunifdef/src/ptr_vector.c
E: sunifdef-debuginfo script-without-shellbang
/usr/src/debug/sunifdef/src/ptr_vector.h
E: sunifdef-debuginfo wrong-script-end-of-line-encoding
/usr/src/debug/sunifdef/src/ptr_vector.h
E: sunifdef-debuginfo script-without-shellbang
/usr/src/debug/sunifdef/src/state_utils.c
E: sunifdef-debuginfo wrong-script-end-of-line-encoding
/usr/src/debug/sunifdef/src/state_utils.c

=> Shorten your description

=> Wrong permissions on source files.
Running
find \( -name '*.c' -o -name '*.h' \) -exec chmod -x {} \;
inside of %prep fixes this.

Further issues:
* The "source-tarball" ships an autom4te.cache, i.e. it is rather carelessly
packaged. 
I recommend to remove this autom4te.cache in %prep, in advance to running
configure. I.e. you might consider to
rm -rf autom4te.cache
in %prep

* The "source-tarball" ships a precompiled sunifdef linux binary.
I recommend to remove it in %prep, i.e. consider to add
rm -rf build-bin
to %prep

* The sources contain a test suite. I recommend to add a %check section to the
spec to run it:
%check
make check

* Many of the warnings are "punned pointer" warnings. These should not be taken
lightly. They are an indication that a package contains bad code that can causem
"random" problems caused by side-effects of compiler optimization.
Comment 3 Jonathan Underwood 2006-06-01 07:10:35 EDT
Thanks Ralf and Parag for your comments - I will implement all your suggestions
at the weekend.

Regarding the compilation warnings - I have already been in touch with the
author about these, and he's taking a look. I'm in the process of looking
through them myself too, to see if I can offer fixes.
Comment 4 Ralf Corsepius 2006-06-01 09:03:03 EDT
(In reply to comment #3)
> Regarding the compilation warnings - I have already been in touch with the
> author about these, and he's taking a look. I'm in the process of looking
> through them myself too, to see if I can offer fixes.
Note: These warnings aren't blockers. I am willing to approve the package
without them being fixed, once the testsuite is enabled.
Comment 5 Jonathan Underwood 2006-06-02 16:02:02 EDT
Updated spec and SRPM reflecting all of Ralf's comments:

Spec URL: http://physics.open.ac.uk/~ju83/sunifdef.spec
SRPM URL: http://physics.open.ac.uk/~ju83/sunifdef-1.0-2.src.rpm

Regarding the warnings - the package author has got back to me saying he's fixed
them and will make a release soon, at which point I'll update the source.
Comment 6 Ralf Corsepius 2006-06-02 21:43:11 EDT
Please put the "make check" into a %check section

--- sunifdef.spec~      2006-06-03 03:30:41.000000000 +0200
+++ sunifdef.spec       2006-06-03 03:30:41.000000000 +0200
@@ -33,6 +33,8 @@
 %build
 %configure
 make %{?_smp_mflags}
+
+%check
 make check

 %install
Comment 7 Jonathan Underwood 2006-06-03 11:12:22 EDT
Done. I had missed the point about %check before, sorry. I didn't actually know
%check existed. Live and learn.

Spec URL: http://physics.open.ac.uk/~ju83/sunifdef.spec
SRPM URL: http://physics.open.ac.uk/~ju83/sunifdef-1.0-3.src.rpm
Comment 8 Ralf Corsepius 2006-06-04 22:09:32 EDT
APPROVED
Comment 9 Ralf Corsepius 2006-06-07 11:58:10 EDT
Jonathan, would you please close this PR, once the package has been pushed to
the repos (Which AFAIS, already happened), TIA.
Comment 10 Eric Smith 2010-04-06 22:29:40 EDT
Package Change Request
======================
Package Name: unifdef
New Branches: F-13
Owners: brouhaha
Comment 11 Jonathan Underwood 2010-04-07 06:03:18 EDT
Eric - what?
Comment 12 Eric Smith 2010-04-07 10:45:21 EDT
Sorry, meant to attach that to the unifdef review request, not sunifdef.  Please disregard.

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