Bug 693493 - Review Request: thunderbird-lightning - The calendar extension to Thunderbird
Summary: Review Request: thunderbird-lightning - The calendar extension to Thunderbird
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Matěj Cepl
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-04-04 19:47 UTC by Orion Poplawski
Modified: 2018-04-11 08:54 UTC (History)
8 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-05-09 22:45:57 UTC
Type: ---
mcepl: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Orion Poplawski 2011-04-04 19:47:06 UTC
Spec URL: http://www.cora.nwra.com/~orion/fedora/thunderbird-lightning.spec
SRPM URL: http://www.cora.nwra.com/~orion/fedora/thunderbird-lightning-1.0-0.39.b3pre.fc14.src.rpm
Description:
Lightning brings the Sunbird calendar to the popular email client,
Mozilla Thunderbird. Since it's an extension, Lightning is tightly
integrated with Thunderbird, allowing it to easily perform email-related
calendaring tasks.

Comment 1 Randy Berry 2011-04-04 20:24:09 UTC
Thunderbird-lightning is already in Fedora.

Comment 2 Orion Poplawski 2011-04-04 20:58:17 UTC
Not in F15+

Comment 3 Levente Farkas 2011-04-04 21:03:56 UTC
any chance for an el6/epel6 branch? thanks.

Comment 4 Orion Poplawski 2011-04-04 21:19:02 UTC
lightning for EL6 already comes from the sunbird package.

Comment 5 Christopher Aillon 2011-04-04 22:36:46 UTC
(In reply to comment #1)
> Thunderbird-lightning is already in Fedora.

Also, when this gets approved, I'd like to split out lightning from the F13 and F14 thunderbird packages, so we'll need it there too.

Comment 6 Matěj Cepl 2011-04-06 13:53:04 UTC
+ BAD : rpmlint is silent on both source and binary package.
It isn't.
bradford:build $ rpmlint -i thunderbird-lightning-1.0-0.39.b3pre.fc14.src.rpm RPMS/x86_64/thunderbird-lightning-*
thunderbird-lightning.src: W: strange-permission thunderbird-mozconfig-branded 0755L
A file that you listed to include in your package has strange permissions.
Usually, a file should have 0644 permissions.

thunderbird-lightning.src: W: strange-permission find-external-requires 0755L
A file that you listed to include in your package has strange permissions.
Usually, a file should have 0644 permissions.

thunderbird-lightning.src:19: W: macro-in-comment %define
There is a unescaped macro after a shell style comment in the specfile. Macros
are expanded everywhere, so check if it can cause a problem in this case and
escape the macro with another leading % if appropriate.

thunderbird-lightning.src: W: patch-not-applied Patch0: thunderbird-version.patch
A patch is included in your package but was not applied. Refer to the patches
documentation to see what's wrong.

thunderbird-lightning.src: W: %ifarch-applied-patch Patch6: mozilla-build-s390.patch
A patch is applied inside an %ifarch block. Patches must be applied on all
architectures and may contain necessary configure and/or code patch to be
effective only on a given arch.

thunderbird-lightning-debuginfo.x86_64: E: empty-debuginfo-package
This debuginfo package contains no files.  This is often a sign of binaries
being unexpectedly stripped too early during the build, rpmbuild not being
able to strip the binaries, the package actually being a noarch one but
erratically packaged as arch dependent, or something else.  Verify what the
case is, and if there's no way to produce useful debuginfo out of it, disable
creation of the debuginfo package.

thunderbird-lightning.x86_64: E: script-without-shebang /usr/lib64/mozilla/extensions/{3550f703-e582-4d05-9a08-453d09bdfdc6}/{e2fda1a4-762b-4020-b5ad-a41df1933103}/calendar-js/calFreeBusyService.js
This text file has executable bits set or is located in a path dedicated for
executables, but lacks a shebang and cannot thus be executed.  If the file is
meant to be an executable script, add the shebang, otherwise remove the
executable bits or move the file elsewhere.

thunderbird-lightning.x86_64: E: script-without-shebang /usr/lib64/mozilla/extensions/{3550f703-e582-4d05-9a08-453d09bdfdc6}/{e2fda1a4-762b-4020-b5ad-a41df1933103}/calendar-js/calAlarmMonitor.js
This text file has executable bits set or is located in a path dedicated for
executables, but lacks a shebang and cannot thus be executed.  If the file is
meant to be an executable script, add the shebang, otherwise remove the
executable bits or move the file elsewhere.

3 packages and 0 specfiles checked; 3 errors, 5 warnings.
bradford:build $ 

Errors must be fixed, warnings should be avoided as well if possible (and I don't see any which couldn't).

+ GOOD: The package is named according to the Package Naming Guidelines .
+ GOOD: The spec file name matches the base package %{name}, in the format
  %{name}.spec.
+ GOOD: The package meets the Packaging Guidelines .
+ GOOD: The package is licensed with a Fedora approved license and meet the
Licensing Guidelines.
+ GOOD: The License field in the package spec file matches the actual license.
+ GOOD: LICENSE file is in %doc.
+ GOOD: The spec file is written in American English.
+ GOOD: The spec file for the package is legible.
- GOOD: The sources used to build the package matches the upstream source,
as provided in the spec URL.
SHA256: 8b499ec3d81d3242b0cc2de27effb1891a07259adf7e5e4c06150f8c9f5254c2
+ GOOD: The package successfully compiles and build into binary rpms on at
least one supported architecture.
Built on my Fedora15/x86_64
+ GOOD: builds on all architectures
http://koji.fedoraproject.org/koji/taskinfo?taskID=2978202
+ GOOD: All build dependencies are listed in BuildRequires. (builds in koji)
+ GOOD: The spec file MUST handle locales properly.
  No locales provided.
+ GOOD: %post and %postun scripts
no scripts
+ GOOD: not relocatable
+ GOOD: A package owns all directories that it creates.
+ GOOD: A package must not contain any duplicate files in the %files listing.
+ GOOD: Permissions on files must be set properly.
+ GOOD: Each package have a %clean section.
+ GOOD: Each package consistently use macros.
+ GOOD: The package contains code, or permissable content.
+ GOOD: -doc subpackage provided. No documentation
+ GOOD: Files registered in %doc does not affect the runtime of the
application.
+ GOOD: No header files.
+ GOOD: No static libraries.
+ GOOD: No pkgconfig(.pc) files.
+ GOOD: No .so file
+ GOOD: No -devel subpackage.
+ GOOD: No .la libtool archives.
+ GOOD: Packages does not contain separate GUI applications.
+ GOOD: Packages does not own files or directories owned by other packages.
+ BAD : Runs rm -rf $RPM_BUILD_ROOT in %install
It doesn't.
+ GOOD: All filenames in rpm packages are valid UTF-8.
+ GOOD: Includes license text.

Please fix the above indicated problems.

Comment 7 Orion Poplawski 2011-04-06 18:05:38 UTC
(In reply to comment #6)
> + BAD : rpmlint is silent on both source and binary package.
> It isn't.
> bradford:build $ rpmlint -i thunderbird-lightning-1.0-0.39.b3pre.fc14.src.rpm
> RPMS/x86_64/thunderbird-lightning-*
> thunderbird-lightning.src: W: strange-permission thunderbird-mozconfig-branded
> 0755L

Fixed.  Note that this was copied from the current thunderbird package so is an issue there.

> thunderbird-lightning.src: W: strange-permission find-external-requires 0755L
> A file that you listed to include in your package has strange permissions.
> Usually, a file should have 0644 permissions.

I think this has to be executable to work.
 
> thunderbird-lightning.src:19: W: macro-in-comment %define
> There is a unescaped macro after a shell style comment in the specfile. Macros
> are expanded everywhere, so check if it can cause a problem in this case and
> escape the macro with another leading % if appropriate.

Fixed.

> thunderbird-lightning.src: W: patch-not-applied Patch0:
> thunderbird-version.patch
> A patch is included in your package but was not applied. Refer to the patches
> documentation to see what's wrong.

False positive.  Again, same issue in thunderbird package.  We modify the patch before applying it.
 
> thunderbird-lightning.src: W: %ifarch-applied-patch Patch6:
> mozilla-build-s390.patch
> A patch is applied inside an %ifarch block. Patches must be applied on all
> architectures and may contain necessary configure and/or code patch to be
> effective only on a given arch.

Fixed.

> thunderbird-lightning-debuginfo.x86_64: E: empty-debuginfo-package

I'm a bit at a loss.  This appears to be because libcalbasecomps.so is getting stripped by the mozilla build/install process, but I can't figure out where.  Any help would be appreciated.

> thunderbird-lightning.x86_64: E: script-without-shebang
> /usr/lib64/mozilla/extensions/{3550f703-e582-4d05-9a08-453d09bdfdc6}/{e2fda1a4-762b-4020-b5ad-a41df1933103}/calendar-js/calFreeBusyService.js
> thunderbird-lightning.x86_64: E: script-without-shebang
> /usr/lib64/mozilla/extensions/{3550f703-e582-4d05-9a08-453d09bdfdc6}/{e2fda1a4-762b-4020-b5ad-a41df1933103}/calendar-js/calAlarmMonitor.js

Fixed.

> + BAD : Runs rm -rf $RPM_BUILD_ROOT in %install
> It doesn't.

Not needed in F15, but since we have requests for F13/F14 I added it.

http://www.cora.nwra.com/~orion/fedora/thunderbird-lightning.spec
http://www.cora.nwra.com/~orion/fedora/thunderbird-lightning-1.0-0.40.b3pre.fc14.src.rpm

Comment 8 Christopher Aillon 2011-04-09 23:43:05 UTC
(In reply to comment #7)
> > thunderbird-lightning-debuginfo.x86_64: E: empty-debuginfo-package
> 
> I'm a bit at a loss.  This appears to be because libcalbasecomps.so is getting
> stripped by the mozilla build/install process, but I can't figure out where. 
> Any help would be appreciated.

-make -f client.mk build
+make -f client.mk build STRIP=/bin/true

Comment 9 Christopher Aillon 2011-04-09 23:57:48 UTC
A few more comments:

* I don't think you need to worry about official branding vs not for this package, so can probably take out that macro and related blocks.

* You may wish to consider installing the .xpi instead of unpacking it.  It should lead to a slightly faster startup time for Thunderbird.

Comment 10 Orion Poplawski 2011-04-11 16:01:54 UTC
Thanks.  Unpacking the xpi is necessary for proper debuginfo generation.

* Mon Apr 11 2011 Orion Poplawski <orion.com> 1.0-0.41.b3pre
- Fix debuginfo builds
- Remove official branding sections

http://www.cora.nwra.com/~orion/fedora/thunderbird-lightning.spec
http://www.cora.nwra.com/~orion/fedora/thunderbird-lightning-1.0-0.41.b3pre.fc14.src.rpm

Comment 11 Orion Poplawski 2011-04-19 17:36:31 UTC
Ping?

Comment 12 Matěj Cepl 2011-04-20 21:24:30 UTC
(In reply to comment #7)
> Fixed.  Note that this was copied from the current thunderbird package so is
> an issue there.

Thanks, I have never said that our packages are the pinnacle of purity and quality. Unfortunately.

> > thunderbird-lightning.src: W: strange-permission find-external-requires 0755L
> > A file that you listed to include in your package has strange permissions.
> > Usually, a file should have 0644 permissions.
> 
> I think this has to be executable to work.

Agree.

> > thunderbird-lightning-debuginfo.x86_64: E: empty-debuginfo-package
> 
> I'm a bit at a loss.  This appears to be because libcalbasecomps.so is getting
> stripped by the mozilla build/install process, but I can't figure out where. 
> Any help would be appreciated.

Did that STRIP=/bin/true help? I am afraid, until we fix this, I won't let this package to go.

> > + BAD : Runs rm -rf $RPM_BUILD_ROOT in %install
> > It doesn't.
> 
> Not needed in F15, but since we have requests for F13/F14 I added it.

I would just note that this may end in EPEL as well, actually may I ask to make sure there will be EPEL-6 branch, please? (I would use it for for my wife's notebook ;)).

With exception of the debuginfo the rest is OK. Waiting on reply about that.

Comment 13 Orion Poplawski 2011-04-20 21:47:08 UTC
Yes, STRIP=/bin/true fixed the debuginfo package generation.

Comment 14 Matěj Cepl 2011-04-20 22:14:27 UTC
APPROVED.

Comment 15 Orion Poplawski 2011-04-20 22:26:54 UTC
New Package SCM Request
=======================
Package Name: thunderbird-lightning
Short Description: The calendar extension to Thunderbird
Owners: orion
Branches: f14 f15 el6
InitialCC: 

Co-maintainers welcome!

Comment 16 Christopher Aillon 2011-04-20 22:35:36 UTC
f13 too please?

Comment 17 Orion Poplawski 2011-04-20 22:42:50 UTC
New Package SCM Request
=======================
Package Name: thunderbird-lightning
Short Description: The calendar extension to Thunderbird
Owners: orion
Branches: f13 f14 f15 el6
InitialCC: 

Okay, I've added f13.

Comment 18 Jason Tibbitts 2011-04-21 03:35:52 UTC
Git done (by process-git-requests).

Comment 19 Christopher Aillon 2011-04-21 05:32:25 UTC
FWIW, I built thunderbird-3.1.9-2.fc13 and thunderbird-3.1.9-2.fc14 which drop the lightning subpackage.

They should probably be submitted in the same update with this package for 13 and 14.

Comment 20 Vít Ondruch 2011-04-21 10:42:45 UTC
Also Sunbird for F15 should be probably retired if we have lightning and Sunbird does not build anyway.

Comment 21 Orion Poplawski 2011-04-21 15:51:07 UTC
Okay, I have built:

thunderbird-lightning-1.0-0.41.b3pre.fc15
thunderbird-lightning-1.0-0.41.b3pre.fc14
thunderbird-lightning-1.0-0.41.b3pre.fc13
thunderbird-lightning-1.0-0.41.b3pre.el6

I'll submit and push the f15 update.  Chris - can you add lighting to your thunderbird updates for 13 and 14?  Looks like there is already a thunderbird-lightning package in EPEL6 that is part of sunbird, so will need to coordinate.  I'm adding jhorak to the CC here.

Comment 22 Orion Poplawski 2012-03-14 21:30:08 UTC
Package Change Request
======================
Package Name: thunderbird-lightning
New Branches: el5
Owners: orion
InitialCC:

Comment 23 Gwyn Ciesla 2012-03-15 12:38:42 UTC
Git done (by process-git-requests).


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