Re: pkg/44163: misc/gkrellm-weather compiles in incorrect path for GrabWeather (+FIX)

[David Holland <dholland-pbugs%netbsd.org@localhost>]

The following reply was made to PR pkg/44163; it has been noted by GNATS.

From: David Holland <dholland-pbugs%netbsd.org@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: pkg-manager%netbsd.org@localhost, gnats-admin%netbsd.org@localhost, 
pkgsrc-bugs%netbsd.org@localhost
Subject: Re: pkg/44163: misc/gkrellm-weather compiles in incorrect path for
 GrabWeather (+FIX)
Date: Sun, 28 Nov 2010 20:40:18 +0000

 On Sun, Nov 28, 2010 at 01:35:00PM +0000, kre%munnari.OZ.AU@localhost wrote:
  >     misc/gkrellm-weather references the (included) GrabWeather script
  >     twice, and in two completely different ways.   One of those is
  >     patched (by patches/patch-aa) to remove the path, and so rely
  >     upon the user's PATH setting (that's OK, if not ideal).  The other
  >     uses
  >             PREFIX "/bin/GrabWeather"
  >     which if PREFIX were correct would be perfect (GrabWeather gets
  >     installed in ${PREFIX}/bin).
 
 In an ideal world packages should not have PREFIX compiled into them
 when they don't need to. However that's a long way from reality, so I
 think I'll use PREFIX for both.
 
  >     However, while gkrellm-weather's pkgsrc Makefile includes
  >             MAKE_ENV+=      PREFIX=${PREFIX:Q}
  > 
  >     it uses gmake, and for that (it seems) the explicit setting of
  >     PREFIX in the Makefile
  >             PREFIX = /usr/local
  > 
  >     overrides the one that pkgsrc is putting in the environment.
 
 That's odd because the makefile also uses PREFIX for installing and I
 don't see how it could possibly have worked at all the way it is.
 
 ...oh, I see, the pkgsrc makefile has its own install rules. bleh.
 
 Anyway, clearly PREFIX should not be getting set explicitly to the
 wrong thing, so we'll fix that.
 
  >     There must be a dozen different ways to fix this one, I chose the
  >     version with minimal impact on pkgsrc itself (ie: modify the
  >     Makefile and only the Makefile - this change requires a revbump,
  >     so the Makefile is going to get modified, keeping the entire
  >     fix there means only one modified cvs file...
 
 That's not the primary consideration :-) but I guess it does make
 sending the patch easier. Dealing with patches of patches is always a
 pain.
 
  >     Alternatives would be to simply modify patch-aa so it corrects
  >     this problem as well as the other (more obviously incorrect)
  >     reference to GrabWeather (it would probably make sense for both
  >     references to be implemented the same way, but ...)
  > 
  >     Or, the Makefile (the gkrellm-weather Makefile, rather than pkgsrc's)
  >     could be modified (patch-ab is already touching it) by simply
  >     removing the setting of PREFIX from there, then the one from the
  >     environment would be used (I assume.)
 
 I'm doing the latter. Having that in there is clearly wrong, so I may
 as well fix it.
 
  >     Also, I have no way to test it, as I have no idea what it is
  >     used for, but this stuff claims to use nls (locales) and the
  >     setting of the locale directory is based upon the (I think)
  >     incorrect version fo PREFIX - again, pkgsrc attempts to
  >     override that by putting a setting for LOCALEDIR in gmake's
  >     environment, but if that doesn't work for PREFIX, I doubt it
  >     works for LOCALEDIR either.
 
 This should be fixed now also.
 
  > +SUBST_CLASSES+=            pfx
  > +SUBST_MESSAGE.pfx= PREFIX replacement
  > +SUBST_STAGE.pfx=   post-patch
  > +SUBST_FILES.pfx=   gkrellweather.c
  > +SUBST_SED.pfx=             -e 's,PREFIX,"${PREFIX}",'
 
 It's usually not a good idea to do SUBST on a file that's also
 patched, because sooner or later someone accidentally rolls the subst
 into the patch and that eventually causes at least confusion if not
 overt problems. :-/
 
 -- 
 David A. Holland
 dholland%netbsd.org@localhost