Bug 507106 - Review Request: msp430-libc - C library for use with GCC on Texas Instruments MSP430 microcontrollers
Review Request: msp430-libc - C library for use with GCC on Texas Instruments...
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Steve Whitehouse
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-06-20 17:41 EDT by Rob Spanton
Modified: 2009-12-04 19:02 EST (History)
4 users (show)

See Also:
Fixed In Version: 0-3.20090726cvs.fc12
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-12-04 19:02:58 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
swhiteho: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Rob Spanton 2009-06-20 17:41:35 EDT
Spec URL: http://users.ecs.soton.ac.uk/rds/rpm/mspgcc/msp430-libc.spec
SRPM URL: http://users.ecs.soton.ac.uk/rds/rpm/mspgcc/msp430-libc-0-1.20090620cvs.fc11.src.rpm
Description: I've just finished packaging msp430-libc, which is a C library for use on Texas Instruments MSP430 microcontrollers with the mspgcc toolchain.

Please review :-)
Comment 1 Jason Tibbitts 2009-07-05 20:26:18 EDT
About the only comments I can make, since I really know nothing of cross-toolchain packages:

Nothing seems to own /usr/msp430/include.

msp430-binutils owns /usr/msp430, and /usr/msp430/lib, but this package doesn't have a dependency on msp430-binutils so those directories will be unowned.  If this package really isn't supposed to depend on msp430-binutils then you need to rethink which packages need to own which directories.
Comment 2 Rob Spanton 2009-07-05 22:04:57 EDT
Hi Jason,

> Nothing seems to own /usr/msp430/include.

Yep, I think you're right.  It looks like this package should own that.  I'll resolve this soon.

> ... If this package really isn't supposed to depend on msp430-binutils then you need to rethink which packages need to own which directories. ...

This package depends on msp430-gcc, which depends on msp430-binutils.

R
Comment 3 Rob Spanton 2009-07-20 10:10:04 EDT
Hey,

I've just updated this to own /usr/msp430/include:

Spec URL: http://users.ecs.soton.ac.uk/rds/rpm/mspgcc/msp430-libc.spec
SRPM URL: http://users.ecs.soton.ac.uk/rds/rpm/mspgcc/msp430-libc-0-2.20090620cvs.fc11.src.rpm

Rob
Comment 4 Rob Spanton 2009-07-26 12:52:18 EDT
Patches are now upstream.  Reflected in this specfile:

Spec URL: http://users.ecs.soton.ac.uk/rds/rpm/mspgcc/msp430-libc.spec
SRPM URL: http://users.ecs.soton.ac.uk/rds/rpm/mspgcc/msp430-libc-0-3.20090726cvs.fc11.src.rpm

Rob
Comment 5 Steve Whitehouse 2009-08-14 03:53:53 EDT
I'll try and review this today - sorry for the delay.
Comment 6 Steve Whitehouse 2009-08-14 05:17:17 EDT
rpmlint output:

[steve@quoit ~]$ rpmlint ./msp430-libc.spec 
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

[steve@quoit ~]$ rpmlint ./msp430-libc-0-3.20090726cvs.fc11.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

Package name: OK
Spec file name: OK
Packaging Guidelines:
Licensing Guidelines: OK
License matches spec file: OK
License not in upstream source: OK (but should request that upstream adds it)
Spec file in US English: OK
Spec file legible: OK
Sources match upstream: OK
Must build on one arch: OK
BuildRequires: OK
Locales: N/A
Dynamic Lib: N/A
Owns all created directories: OK
Files only listed once: OK
File permissions: OK
Consistent Macro use: OK
Contains code and permissible content: OK
Large doc files: OK (there are none)
Nothing in %doc is runtime: OK

Header files must be in a -devel package:
Static libraries must be in a -static package:
  - I assume these two only apply if the package is targetted at the installed platform and that this doesn't apply to cross-libraries & tools. It makes no sense to separate the headers from the library since both are always required to make use of this package. It makes no sense to name the library -static when msp430 only supports static libraries anyway.

pkgconfig: OK (No .pc files included)
Library files with .so suffix: OK (None included)
Must not contain .la files: OK (None included)
GUI Applications: N/A
Must not own files/directories owned by other packages: OK
Install removes build root: OK
Filenames are UTF-8: OK

... and now for the SHOULD items...
Comment 7 Steve Whitehouse 2009-08-14 05:29:32 EDT
Package review SHOULD items:

License in upstream source: As above, that should be requested to be added
Translations of description & summary: No translations available - can we get those via Transifex I wonder? Not sure how we can arrange that for the spec file itself.
Mock build: Robert, have you already tried that? If so I'll take your word for it that it works.
Package functions: It is impossible to test all package functions without a huge test suite. I can see that the code looks sane and the only reason for non-functioning would be a broken compiler which is not an issue relating to this particular package. I have used certain functions from this library before and they have worked as expected.
Scriptlets: N/A
Subpackages: N/A
.pc files: N/A
File deps outside of certain directories: N/A

So I think we are most of the way there with this.
Comment 8 Till Maas 2009-10-22 11:04:35 EDT
FYI: it builds on the Fedora buildsystem:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1762830

You can test this yourself with "koji build dist-f13 --scratch msp430-libc-0-3.20090726cvs.fc11.src.rpm"
Comment 9 Rob Spanton 2009-11-04 21:26:34 EST
Hi,

Sorry about the delay.  Yes, as Till says, it does build OK on koji.

So I think this means that this package has passed its review.

Cheers,

Rob
Comment 10 Steve Whitehouse 2009-11-05 03:46:26 EST
Yes, I agree.
Comment 11 Rob Spanton 2009-11-05 05:18:46 EST
New Package CVS Request
=======================
Package Name: msp430-libc
Short Description: C library for use with GCC on Texas Instruments MSP430 microcontrollers
Owners: rspanton
Branches: F-10 F-11 F-12
InitialCC:
Comment 12 Kevin Fenzi 2009-11-06 15:36:32 EST
cvs done.
Comment 13 Fedora Update System 2009-11-20 00:22:24 EST
msp430-libc-0-3.20090726cvs.fc12 has been pushed to the Fedora 12 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update msp430-libc'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F12/FEDORA-2009-11662
Comment 14 Fedora Update System 2009-12-04 19:02:52 EST
msp430-libc-0-3.20090726cvs.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.

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