Bug 439453 - Review Request: sublib - a subtitle library
Summary: Review Request: sublib - a subtitle library
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Alexander Kahl
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 429006
TreeView+ depends on / blocked
 
Reported: 2008-03-28 18:33 UTC by Julian Sikorski
Modified: 2008-05-19 17:22 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-05-19 17:22:23 UTC
Type: ---
Embargoed:
fedora: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Julian Sikorski 2008-03-28 18:33:11 UTC
Spec URL: http://belegdol.fedorapeople.org/sublib.spec
SRPM URL: http://belegdol.fedorapeople.org/sublib-0.8-1.fc8.src.rpm
Description: SubLib is a library that eases the development of subtitling applications. It supports the most common text-based subtitle formats and allows for subtitle editing, conversion and synchronization.
SubLib is written in C# and can be used in platforms like Mono or .NET Framework.

This package is required by gnome-subtitles. This is a partially complete spec which currently bails out at %%install stage:
+ gacutil -i build/sublib.dll -f -package sublib -root /var/tmp/sublib-0.8-1.fc8-root-jsikorski/usr/lib64
Failure adding assembly to the cache: Attempt to install an assembly without a strong name.

Any help in getting this further would be really appreciated.

Comment 1 Alexander Kahl 2008-03-28 19:21:26 UTC
Also taking this over.

Comment 2 Alexander Kahl 2008-03-29 10:05:36 UTC
(In reply to comment #0)
> + gacutil -i build/sublib.dll -f -package sublib -root
/var/tmp/sublib-0.8-1.fc8-root-jsikorski/usr/lib64
> Failure adding assembly to the cache: Attempt to install an assembly without a
strong name.

Upstream didn't bother to add a keyfile for strong naming
(http://www.mono-project.com/Assemblies_and_the_GAC#Assembly_Names), you can
either choose to inform upstream (sign a bug) and wait for them or do it yourself. 
For the latter, you need to create a new keyfile with 'sn -k sublib.snk' and -
this is important! - provide it as part of the SRPM (Source1) instead of
creating it dynamically as the key should be common amongst all builds, no
matter which architecture. Copy the resulting keyfile into sublib-0.8/src/ in
%prep and patch sublib-0.8/src/SubLib/AssemblyInfo.cs to have the following line
added:
[assembly: AssemblyKeyFile ("sublib.snk")]
Thus the assembly will be signed with the key file after the build and the
gacutil command above will succeed. If upstream ever chooses to provide their
own key for signing however, all builds depending on sublib must be rebuild
against the version with the new key IIRC. You could also opt to send them the
key you've created to circumvent this.

After finishing either one of the options given, I'll continue this review.

Comment 3 Julian Sikorski 2008-03-29 10:25:43 UTC
How about this: I'll add a temporary key myself, but I'll file an upstream bug
anyway. Will it be a problem if the key will change later?

Comment 4 Julian Sikorski 2008-03-29 10:34:24 UTC
OK, nevermind. I'll just send them the key.

Comment 5 Julian Sikorski 2008-03-29 10:53:14 UTC
All right, files have been re-uploaded. This version at least builds correctly.

Comment 6 Alexander Kahl 2008-03-29 15:29:29 UTC
(In reply to comment #4)
> OK, nevermind. I'll just send them the key.
Great, thanks.

(In reply to comment #5)
> All right, files have been re-uploaded. This version at least builds correctly.
"Re-uploaded"..? Please raise the release number (-2) and report all changes in
a new changelog entry first.

Comment 7 Julian Sikorski 2008-03-29 15:36:12 UTC
(In reply to comment #6)
> (In reply to comment #5)
> > All right, files have been re-uploaded. This version at least builds correctly.
> "Re-uploaded"..? Please raise the release number (-2) and report all changes in
> a new changelog entry first.
I normally do that, but given that the first version of spec was not even
building correctly, I decided to re-upload. Please continue as if it were the
first version. Of course, next fixes will feature a release bump and a proper
changelog entry.


Comment 8 Alexander Kahl 2008-04-01 22:11:05 UTC
(In reply to comment #7)
> I normally do that, but given that the first version of spec was not even
> building correctly, I decided to re-upload. Please continue as if it were the
> first version. Of course, next fixes will feature a release bump and a proper
> changelog entry.
You're right, it's completely reasonable to stick with the release number here.
I'll start the package review as soon as I can.

Comment 9 Alexander Kahl 2008-05-09 15:57:47 UTC
Good news: I've been in a communication loop with Pedro for some time now and
sublib 0.9 is about to be released this weekend that installs just the way we
need. As sublib's API isn't considered stable enough for GAC registration yet,
we can skip that for now. GNOME Subtitles' next version will also use the
system-wide sublib installation by default.

Comment 10 Matthias Saou 2008-05-17 20:12:52 UTC
According to the web page, sublib 0.9 is out indeed ;-)

Comment 11 Julian Sikorski 2008-05-17 20:29:46 UTC
New release:
Spec URL: http://belegdol.fedorapeople.org/sublib.spec
SRPM URL: http://belegdol.fedorapeople.org/sublib-0.9-1.fc9.src.rpm

Changes:
- Updated to 0.9
- Switched to use make install
- Added -devel subpackage
- Skip GAC registration for now

Comment 12 Alexander Kahl 2008-05-18 17:55:10 UTC
Looks much better now :)

Package Review
==============
* rpmlint output:
  sublib.x86_64: E: no-binary
    OK, rpmlint cannot handle mono properly yet
  sublib.x86_64: E: only-non-binary-in-usr-lib
    Same
  sublib-debuginfo.x86_64: E: empty-debuginfo-package
    OK according to
   
http://fedoraproject.org/wiki/Packaging/Mono#head-a86970c53975635088edc78a0e579b524531d571
  sublib-devel.x86_64: W: no-documentation
    OK, no documentation provided for development
--------------------------------------------------------------------------------
* The package is named according to the Package Naming Guidelines
* The spec file name matches the base package
* Fedora approved license: GPLv2+
* License field in the package spec file matches the actual license
* COPYING included
* The spec file is written in American English
* The spec file is legible
* The sources used to build the package matches the upstream source:
SHA1 4c7194666e9991efc7793ab300f60f95c7b56d85  sublib-0.9.zip
* Builds locally on dist-f9 x86_64
* BuildRequires look sane
* No locales
* No libraries in dynamic linker's default paths
* No relocatable libraries
* Package owns all directories it creates
* No duplicates in %files
* File permissions are sane
* %clean starts with build root clean
--------------------------------------------------------------------------------
E Consistent use of macros
Please replace:
- sed with %{__sed}
--------------------------------------------------------------------------------
* The package contains code
* Documentation not large, no -doc subpackage needed
* No header files
* No static libs
* pkgconfig files handled properly
* No suffixed library files
* -devel package requires base package
* No libtool archives
* No desktop file
* Package does not own files or directories already owned by other packages
* %install starts with build root clean
* All filenames are valid UTF-8

Additional Checks
=================
* Latest version is being packaged: 0.9
* Dist tag is present
* Group tag is valid
* Build root is correct:
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
* Scriptlets look sane
--------------------------------------------------------------------------------
E Package does not build in mock for ppc64:
http://koji.fedoraproject.org/koji/taskinfo?taskID=616191
mono-devel is not available for ppc64. 
- Please add an ExcludeArch: ppc64
- After this package has been approved, please open a new report (with component
  gnome-subtitles) which tells that gnome-subtitles does not support ppc64 and
  make the bug block FE-ExcludeArch-ppc64.
--------------------------------------------------------------------------------
* No GConf schema files
* No scrollkeeper documentation
* Requires is complete, all dependencies resolved automatically

Mono-specific Checks
====================
* Assembly location is correct:
%{_libdir}/%{name}
* No global assemblies are registered with gacutil
* Package builds without source version of mono
* pkgconfig file in -devel
* No prebuilt assemblies distributed
* Library too immature for registration with the GAC
* Not distributing .DLLs from other projects
* _libdir not redefined
* Package not noarch


After resolving the two remaining issues, I'll approve the package.

Comment 13 Julian Sikorski 2008-05-18 18:12:41 UTC
New release:
Spec URL: http://belegdol.fedorapeople.org/sublib.spec
SRPM URL: http://belegdol.fedorapeople.org/sublib-0.9-2.fc9.src.rpm

Changes:
- Replaced sed invocation with macro
- Added ExcludeArch: ppc64

Comment 14 Alexander Kahl 2008-05-18 19:45:21 UTC
==============================
This package is APPROVED by me
==============================

Comment 15 Julian Sikorski 2008-05-18 21:48:32 UTC
New Package CVS Request
=======================
Package Name: sublib
Short Description: A subtitle library
Owners: belegdol
Branches: F-8 F-9
InitialCC: 
Cvsextras Commits: yes

Comment 16 Kevin Fenzi 2008-05-19 16:04:57 UTC
cvs done.


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