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

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

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