[Debian GNUstep maintainers] preview.app uploaded to mentors

[Yavor Doganov]

Federico Gimenez Nieto wrote:
> Hi, i've uploaded preview.app to mentors,

Thanks very much for your work.

> The directory structure is modified to comply with the FHS and with
> the policy, now the Resources directory contents are installed under
> the directory /usr/share/GNUstep/Preview.app, and a symlink points
> to it from
> /usr/lib/GNUstep/Applications/Preview.app/Resources.

This approach will not work; dpkg will not replace a real directory
with a symlink so all upgrades will be broken (the app will fail to
find the Info-gnustep.plist and the .gorm files and will complain with
"Bad application class").  More importantly, it wont work anymore at
all (you can reproduce this by installing preview.app from the
official archive, and then your package).

You need to add a preinst script to remove the old Resources dir.  See
the gnumail package from Etch for an example.  You can then drop the
preinst entirely for Squeeze+1.

> There are other minor changes

Here are some comments, in decreasing order of importance (the last
ones are entirely cosmetic):

1) The removal of the binary-arch target is very wrong.  This is
   certainly an architecture-dependent package (see the output of
   `file /usr/lib/GNUstep/Applications/Preview.app/Preview').

2) Please export GNUSTEP_MAKEFILES and replace every occurrence of
   `gs_make' with `$(MAKE)'.  Sub-make invocations should always use
   the $(MAKE) variable so that `make' passes all options and
   variables via its internal $(MAKEFLAGS) variable.  Currently it
   works as it is, but probably won't work for parallel builds and may
   lead to weird bugs in the future that are hard to debug.

3) GNUstep Make has a bug [1] (fixed in SVN trunk) that by default
   builds unoptimized binaries, which is a Debian Policy violation.
   It is trivial to make it do the right thing, for example:

ifneq (,$(findstring noopt,$(DEB_BUILD_OPTIONS)))
OPTFLAG := -O0
else
OPTFLAG := -O2
endif

build-stamp:
	...
	$(MAKE) OPTFLAG=$(OPTFLAG)

4) Please consider making the build log verbose by adding `messages=yes'
   to the $(MAKE) invocation.  We've been bitten before by
   incompatible changes that were hidden in silent builds.  (Like, for
   example, the cited gnustep-make bug).

5) The second `cp' after `mv' in debian/rules where you move the
   Resources dir seems redundant to me.  `mv' will copy the whole
   contents recursively.  It is better not to hardcode the path; it
   changed before and may change in the future.  When it happens, your
   package will need amendments.  Here's an example from agenda.app:


ifeq ($(GS_USE_FHS),yes)
        dh_installdirs usr/share/GNUstep
        mv $(d_app)$(GNUSTEP_SYSTEM_APPS)/SimpleAgenda.app/Resources \
          $(d_app)/usr/share/GNUstep/SimpleAgenda.app
        dh_link usr/share/GNUstep/SimpleAgenda.app \
          $(GNUSTEP_SYSTEM_APPS)/SimpleAgenda.app/Resources
endif

   The conditional is to be nice to the GNUstep LiveCD maintainer: he
   doesn't like the FHS very much :-) and prefers the Resources to be
   in the app bundle.  It is not a requirement to conditionalize the
   move, of course.

   There's also a typo in the comment: s/FSH/FHS/.

6) clean target: `$(MAKE) clean distclean' was necessary to workaround
   an ancient gnustep-make bug; you can simply do `$(MAKE) distclean'
   these days.

7) Please consider adding an icon for debian/menu, for those people
   who use window managers that support icons in the menus.  See
   agenda.app for one (easy) way to do it.

8) debian/Preview.desktop is not valid according to
   `desktop-file-validate'.  The Version field is the version of the
   spec (1.0, I think).  The MimeType field should be removed
   entirely, because Preview.app doesn't support command-line options
   and right-clicking on an image file in environments that support it
   (e.g. GNOME) does not actually open it.  IOW, if
   
     $ Preview foo.png

   doesn't work from the console, it shouldn't have MimeType defined
   in its .desktop file as it just clutters the context menu.

9) You can remove the versioned build-dependency on libgnustep-gui-dev
   (0.14 is in Lenny) and gnustep-make entirely (pulled in by the
   dependency chain).

10) Add ${gnustep:Depends} to the Depends field, and invoke
    gsdh_gnustep in the binary-arch target to populate it.  (This is
    again wishlist item -- to prevent dpkg from installing the package
    on systems that have Debian GNUstep packages configured not to
    follow the FHS.)

11) You can get rid of the dpkg-shlibdeps warnings (if they annoy you)
    with LDFLAGS, see agenda.app for example.  This is mostly
    cosmetic, because the extra linking imposed by gnustep-make
    doesn't bring in new dependencies via ${shlibs:Depends}, except
    libgcc1.

> Just one question regarding the package, there are two empty
> directories in the installation,
> Resources/English.lproj/Preview.help/ and
> Resources/French.lproj/Preview.help/, is this intentional?

Apparently they were supposed to contain the unwritten
documentation...

> Should we get rid of them?

Yes, you can safely delete them; they're useless. 

As a final comment, I'm not sure you're aware: This is an abandoned
app with dead upstream, unfortunately like a lot of GNUstep packages.
PRICE (price.app) and LaternaMagica (not in Debian) are considered
replacements, although personally I still use Preview.app (old habits
die hard, and I've translated it :-)).  So you're pretty much on your
own to fix all bugs.  (Of course, we'll gladly help if needed.)

Thanks again for your efforts.

[1] https://savannah.gnu.org/bugs/index.php?27222