[Bug 522920] Review Request: tnef - Extract files from email attachments like winmail.dat

Wed Sep 30 14:27:26 UTC 2009

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=522920

--- Comment #7 from David Timms <dtimms at iinet.net.au>  2009-09-30 10:27:23 EDT ---
(In reply to comment #6)
> This is my first pre-review so I might be wrong. And please note that this is
> an informal review.
> Since I want to become a packager, I'm pre-reviewing this package. I also
> think this package is useful.
Hi Naoki, thanks for your detailed and helpful pre-review.

> # MUST: rpmlint must be run on every package. The output should be posted in
> the review.
> => FAIL(1 warning)
>   $ rpmlint -i SPECS/tnef-1.4.6-2.fc11.src.rpm
>   tnef.src: W: strange-permission tnef.sh 0444
>   A file that you listed to include in your package has strange permissions.
>   Usually, a file should have 0644 permissions.
I had tried to fix this, and found that it made no difference. I realize now
that I was running rpmlint on the -1 version of my package, doh.
So fixed that.

> # MUST: The package must be licensed with a Fedora approved license and meet
> the  Licensing Guidelines .
> => seems OK (GPLv2+ and UCD)

Actually, without raising it, perhaps the mixed X + Y license would better
describe it, I'm not really sure.

> # MUST: If the package does not successfully compile, build or work on an
> architecture, then those architectures should be listed in the spec in
> ExcludeArch. Each architecture listed in ExcludeArch MUST have a bug filed in
> bugzilla, describing the reason that the package does not compile/build/work on
> that architecture. The bug number MUST be placed in a comment, next to the
> corresponding ExcludeArch line.
> => FAIL(build error on ppc and ppc64)
I had not tried build on non intel arches, thanks for triggering a mock build.

>   koji build result(ppc):
>   http://koji.fedoraproject.org/koji/taskinfo?taskID=1716349
=====
mv -f .deps/mapi_types.Tpo .deps/mapi_types.Po
mv -f .deps/tnef_names.Tpo .deps/tnef_names.Po
mapi_names.c:1307: fatal error: error writing to -: Broken pipe
compilation terminated.
The bug is not reproducible, so it is likely a hardware or OS problem.
gcc: Internal error: Interrupt (program as)
Please submit a full bug report.
See <http://bugzilla.redhat.com/bugzilla> for instructions.
=====
No idea what is happening here.

>   koji build result(ppc64) (a test failed):
>   http://koji.fedoraproject.org/koji/taskinfo?taskID=1716351
=====
PASS: directory.test
PASS: maxsize.test
\'diff ./body.output ./body.baseline > ./body.diff\' -- Test Failed!
FAIL: body.test
PASS: mime-types.test
cat: write error: Broken pipe
PASS: stdin.test
=====
again broken pipe.

I tried to queue some new scratch builds, but I don't remember the proper
commands to set it up. (certificate error).

> # MUST: All build dependencies must be listed in BuildRequires, except for any
> that are listed in the exceptions section of the Packaging Guidelines ;
> inclusion of those as BuildRequires is optional. Apply common sense.
> => FAIL
> 
> 'kde-filesystem' seems to be required to build.
> %{_kde4_datadir} macro is defined in /etc/rpm/macros.kde4 which is included in
> kde-filesystem package.
And, since the installed .desktop file wouldn't be owned without out this
package, I am including it now as both Requires and BR.

> # MUST: Packages containing GUI applications must include a %{name}.desktop
Hmmn, that is an unfortunate side effect. The app is not a gui app, and it
doesn't make sense for an icon to appear in the normal Application (or other)
menu, because the app needs an input tnef file to extract to be useful.

It does make sense to have the right click (open with) menu in nautilus/doplhin
though. It seems you get one when you want the other, unless there is another
way to achieve this file mime-type association ?

I have asked on fedora-packaging list about this item. Perhaps I'm doing
something that isn't allowed for other reasons ?

> # MUST: Packages must not own files or directories already owned by other
> packages. The rule of thumb here is that the first package to be installed
> should own the files or directories that other packages may rely upon. This
> means, for example, that no package in Fedora should ever share ownership with
> any of the files or directories owned by the filesystem or man package. If you
> feel that you have a good reason to own a file or directory that another
> package
> owns, then please present that at package review time.
> => I can't judge.
> 
> Because I don't know who owns '%{_datadir}/mimelnk/application/' directory.
rpm -qf /usr/share/mimelnk/application/
kde-filesystem-4-25.fc11.noarch
, but I think other non kde items (eg gnome desktop uses that folder) live
there as well.

> # SHOULD: The reviewer should test that the package builds in mock.
> => FAIL
> 
> RPM build errors:
>     File must begin with "/":
> %{_kde4_datadir}/kde4/services/tnefextract.desktop
As you mention above, that is probably because the macro is not defined, so
adding the build requires fixes that.

> # SHOULD: The package should compile and build into binary rpms on all
> supported
> architectures.
> => FAIL(build error on ppc and ppc64)

> # SHOULD: The reviewer should test that the package functions as described. A
> package should not segfault instead of running, for example.
> => OK(tested with some files in tests/files/datafiles)
Did you try the menu integration from either nautilus or dolphin ?
I would appreciate your feedback on testing from both if you are able ;-)

Updated package:
http://members.iinet.net.au/~timmsy/tnef/tnef.spec
http://members.iinet.net.au/~timmsy/tnef/tnef-1.4.6-3.fc11.src.rpm

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.