[Bug 370561] Review Request: bmpx - Beep Media Player eXperimental

[bugzilla at redhat.com]

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

Summary: Review Request: bmpx - Beep Media Player eXperimental


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





------- Additional Comments From mtasaka at ioa.s.u-tokyo.ac.jp  2007-11-15 09:37 EST -------
Well,

! License is actually strict GPLv2.

* First triggerscripts
  - Personally, I don't like triggerscripts.
    However, even if we ignore what I feel, current triggerscripts
    contains some problems.

  1. /usr/share/doc/rpm-4.4.2.2/triggers says the order of the scriptlets
     when one package is upgrade is:
--------------------------------------------------------
  new-%pre      for new version of package being installed
  ...           (all new files are installed)
  new-%post     for new version of package being installed

  any-%triggerin (%triggerin from other packages set off by new install)
  new-%triggerin
  old-%triggerun
  any-%triggerun (%triggerun from other packages set off by old uninstall)

  old-%preun    for old version of package being removed
  ...           (all old files are removed)
  old-%postun   for old version of package being removed

  old-%triggerpostun
  any-%triggerpostun (%triggerpostun from other packages set off by old un
                install)
--------------------------------------------------------
    So, (if I understand this correctly) when firefox is upgraded,
    - First (any)-%triggerin of bmpx against "new firefox" is executed.
      When this %triggerin ends, the contents of 
      %{_libdir}/%{name}/%{name}-plugin-path points to new
      firefox directory.
    - Next (any)-%triggerun of bmpx against "old firefox" is executed.
      At this stage, the files under the directory written in
      %{name}-plugin-path is removed, so as the result bmpx extension
      for "new" firefox is removed.

    ! By the way, if I am correct new install of bmpx should call
      %triggerin scripts of bmpx itself (new-%triggerin).

  2. Usually these method leaves unowned files.
--------------------------------------------------------
$ LANG=C rpm -qf
/usr/lib/firefox-2.0.0.9/extensions/{bc3572da-daf9-435d-a8a6-33cc20fe4533}
file /usr/lib/firefox-2.0.0.9/extensions/{bc3572da-daf9-435d-a8a6-33cc20fe4533}
is not owned by any package
--------------------------------------------------------
     Moreover, %{_libdir}/%{name}/%{name}-plugin-path is not owned
     either.

     I think the proper way is to rebuild bmpx every time firefox
     is upgraded, or to separate firefox extension related files
     from bmpx (and submit a new review request for bmpx-mozextension,
     for example) if you don't want to rebuild whole bmpx.

- For generic issues:
* koji build
  - -2 does not build.
    http://koji.fedoraproject.org/koji/taskinfo?taskID=236212
    Note: basically, %_iconsdir is not defined.
----------------------------------------------------------
[tasaka1 at localhost ~]$ grep iconsdir /etc/rpm/macros.*
.........
/etc/rpm/macros.jpackage:%_iconsdir     %{_datadir}/icons
.........
[tasaka1 at localhost ~]$ rpm -qf /etc/rpm/macros.jpackage
jpackage-utils-1.7.3-1jpp.3.fc8
----------------------------------------------------------

* Timestamps
  - Please use "-p" option when you use "cp" or "install" commands
  - Also, try to add 'INSTALL="install -p"' option to "make install"
    if this keeps timestamps on name png file, header files or so
    (I guess yes).

* Exclude
  - It seems many .la files are installed under %_libdir/bmpx/plugins/taglib.
    Check if this is correct.
  ! Note:
    I prefer not to use %exclude but really to "remove" unneeded files
    by the time %install finishes.

* %post scriptlet
----------------------------------------------------------
if [ -x %{_bindir}/gtk-update-icon-cache ]; then
	update-desktop-database &>/dev/null || :
fi
----------------------------------------------------------
  - Perhaps this is some copy/paste mistake.

* Files entry
  - Are Makefile.* under %_defaultdocdir really needed?

* -devel package?
  - Well, why is this -devel package needed?
    bmpx does not contain any libraries in ldconfig default paths,
    and -devel package does not contain any symlinks for libraries.

    As far as I see this package, -devel package is completely unneeded.

-- 
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, or are watching someone who is.