[Bug 985] Review request: ProjectX-0.90.4.00-2

Tue Aug 3 17:15:50 CEST 2010

http://bugzilla.rpmfusion.org/show_bug.cgi?id=985

--- Comment #18 from David Timms <dtimms at iinet.net.au>  2010-08-03 17:15:46 ---
(In reply to comment #16)
====
> A script is included as you suggested that does the download and tar packaging.
>  (And excludes empty directories, I was fooled by my .cvsrc there.)  This
> script makes the date part of the name.
OK.

====
> The start script in /usr/bin is a separate source file.  That change also made
> the separate %attr() no longer needed.
OK, much easier to understand.

=====
> The tag name is adjusted to exactly match the wiki page.
OK.

=====
> I'm not quite sure what other gcj bits you find missing.  There are some
> additional comments on the wiki page on how to do with packages that have
> subpackages, but that doesn't apply to this package.  Or do you mean you want
> an explicit %attr() although there is a %defattr()?  I'm also no expert on
> Java, and try to follow the instructions.  But I'm not sure what you see
> that I miss.
Changes are OK, meets the guidelines.

===== 1. patch file naming
> I'm really confused why your build fails.  It builds fine for me.  Also with
> mock, which should rule out environment influences like .cvsrc above. 
> Furthermore, your error message says it saves the rejects to MANIFEST.MF.rej,
> but ProjectX.sysjava.patch only patches build.sh.  Is the
> ProjectX.sysjava.patch in your build environment not the one from the latest
> SRPM?
Correctamundo. I guess that's another of the not so obvious things: 
if a patch is named projectx-1.3.4-fix-libdir.patch, then when the patch needs
to change to suit projectx-1.3.5, then the old patch would be removed, and the
new one added with it's name indicating the reference upstream version. This
aids clarity, and makes it easier to see the difference between branches, eg:
http://cvs.fedoraproject.org/viewvc/rpms/audacity/F-12/

Keeping the same name while the patch changes could allow issue (like mine -
where I manually extract and review all the files, but since there was no
mention of a change or obvious name change for the patch I didn't copy it into
rpmbuild/SOURCES). In spec/packages I have come across, it seems fairly normal
to to name patches referencing the base file version, so I think we should do
the same here.

=====
> Updated version available at:
> ftp://ftp.uddeborg.se/pub/ProjectX/ProjectX.spec
> ftp://ftp.uddeborg.se/pub/ProjectX/ProjectX-0.90.4.00-5.20100801cvs.fc14.src.rpm
- Builds OK.

===== Many build warnings:
...
Type safety: The method add(Object) belongs to the raw type ArrayList.
References to generic type ArrayList<E> should be parameterized
----------
1369 problems (1369 warnings)+ cp 

OK. Not a direct packaging issues, although as you find time it would be good
to submit patches to the upstream to take care of them.

===== later in build:
/src/net/sourceforge/dvb/projectx/parser/Scan.java:1980,
                 from <built-in>:4:
/home/davidt/rpmbuild/BUILD/ProjectX-0.90.4.00-20100801cvs/src/net/sourceforge/dvb/projectx/video/MpvDecoder.java:0:
note: variable tracking size limit exceeded with -fvar-tracking-assignments,
retrying without

- not sure if this is significant ?

- snapshot script works.

- 2. snapshot includes some bits we might as well nuke before adding to
look-aside cache, mainly the /lib/os2, and /lib windows dll and precompiled
included jars.
further removals for archive create:
lib]# file *
commons-net-1.3.0.jar: Zip archive data, at least v2.0 to extract
idctref.dll:           PE32 executable for MS Windows (DLL) (GUI) Intel 80386
32-bit
idctsse.dll:           PE32 executable for MS Windows (DLL) (GUI) Intel 80386
32-bit
jakarta-oro-2.0.8.jar: Zip archive data, at least v2.0 to extract
OS2:                   directory

- source archive: included archive and local archive created using the snapshot
script create identical archives (expanded both, and diff -ur).

- might like to see if there is much size reduction by using the xz
compression.

- rpmlint:
=====rpmlint src
$ rpmlint
/home/davidt/rpmbuild/SRPMS/ProjectX-0.90.4.00-5.20100801cvs.fc12.src.rpm
ProjectX.src: W: spelling-error Summary(en_US) demultiplexing -> de
multiplexing, de-multiplexing, multiplexing
ProjectX.src: I: enchant-dictionary-not-found sv
ProjectX.src: W: spelling-error %description -l en_US analyse -> analyst,
analyze, ana lyse
ProjectX.src: W: spelling-error %description -l en_US demultiplex -> de
multiplex, de-multiplex, multiplexer
ProjectX.src: W: strange-permission ProjectX-snapshot.sh 0775
ProjectX.src:113: W: libdir-macro-in-noarch-package (main package)
%_libdir/gcj/%name
ProjectX.src: W: no-cleaning-of-buildroot %install
ProjectX.src: W: no-cleaning-of-buildroot %clean
ProjectX.src: W: no-buildroot-tag
ProjectX.src: W: no-%clean-section
ProjectX.src: W: invalid-url Source0: ProjectX-0.90.4.00-20100801cvs.tar.bz2
1 packages and 0 specfiles checked; 0 errors, 10 warnings.

=====rpmlint bin
rpmlint
/home/davidt/rpmbuild/RPMS/x86_64/ProjectX-0.90.4.00-5.20100801cvs.fc12.x86_64.rpm
ProjectX.x86_64: W: spelling-error Summary(en_US) demultiplexing -> de
multiplexing, de-multiplexing, multiplexing
ProjectX.x86_64: I: enchant-dictionary-not-found sv
ProjectX.x86_64: W: spelling-error %description -l en_US analyse -> analyst,
analyze, ana lyse
ProjectX.x86_64: W: spelling-error %description -l en_US demultiplex -> de
multiplex, de-multiplex, multiplexer
ProjectX.x86_64: W: no-manual-page-for-binary projectx
1 packages and 0 specfiles checked; 0 errors, 4 warnings.

=====rpmlint bin -debuginfo
rpmlint
/home/davidt/rpmbuild/RPMS/x86_64/ProjectX-debuginfo-0.90.4.00-5.20100801cvs.fc12.x86_64.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

===== 3. libdir-macro-in-noarch-package
- not sure about this one. I thought gcj made native arch libraries /
executable, and hence on an x86_64, this would need to point to lib64.

- also, why does it think it's a no-arch package when we are building native
stuff ?

===== 4. missing help file:
Help|Help
missing resource ! :
htmls/Index.html

- wonder if that is a capitalization problem needing a patch.
- or need to patch source to look for the help in the right place.

> --- Comment #17 from GÃ¶ran Uddeborg <goeran at uddeborg.se>  2010-08-03
> Yes, I am sponsored and maintain packages in Fedora already.  (Just a couple,
> but still.)
> 
> My FAS name is "goeran".
OK, confirmed. Hence I could approve package.
https://admin.fedoraproject.org/community/?username=goeran&_csrf_token=90de19743069dfb67ab1512f5701d1a28cab0690#people/package_maintenance

===== application bugs:
- select file open
- change to detail view
- click a folder
  -> the dir list is not updated to the sub folder
- click the sort view button
  -> detail list headers stay in the dialog
  -> the entries move left, overwriting what was previously the icons

===== 5. Requires:
Can you check whether rpm can autodetect these Requires, eg by leaving them out
of the spec, and seeing whether you get useable executable ?

===== 6. I also realize that the inline desktop patching is not very
understandable (even if it works). I would suggest placing this as a patch to
the included .desktop file

===== Secondary review:
I'll ask anyone who has good java skills to cast there eyes over GÃ¶ran'a
next spec/srpm ...

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