Pierre Pronchery <khorben%defora.org@localhost> writes: > On 07/23/15 07:23, David Holland wrote: >> On Wed, Jul 22, 2015 at 07:25:08PM -0700, Alistair Crooks wrote: >> > It might be better, if this is just for pkgsrc, to call the definition >> > PKGSRC_USE_SSP. I had the same reaction as agc (but without a specific name). > I do not mind, but: > - there are only 6 options in all of mk/defaults/mk.conf that are > prefixed this way; That can be viewed as a bug; namespacing for config variables feels like collection of historical artifacts.. I have found it confusing meaning, had to actually go look things up) about which options are or base and which are for pkgsrc. Generally, we declare each variable to be user-settable in mk.conf or pkg-settable in Makefile, and never both. See the FETCH_USING flamage... > - it can easily be applied to pkgsrc only, in mk.conf: > .ifdef BSD_PKG_MK > USE_SSP=yes > .endif > - unprivileged builds will use a different mk.conf anyway. That's true, but a) a person with USE_SSP in base will now get SSP in pgksrc, which may break a lot of things, and b) I think it's better in general to have separate names. On the patch, I agree with the concept, but have a few questions I wonder if PKGSRC_USE_SSP should provoke an error if used with a compiler that doesn't support it, rather than silently failing. Or perhaps a warning; an error seems too much. But overall I think I agree with how you handled it. The hunks in gcc.mk that add CFLAGS and remove LDFLAGS aren't immediately understandable, but you didn't include the commit message. If this is fixing gcc.mk for an issue that's separate from adding SSP (and it seems it must be), I (with pmc hat OFF) would tend to want to see a separate commit for that. If it's just that nothing sets _GCC_LDFLAGS now, I don't see why the LDFLAGS+= line should be removed.
Attachment:
pgpD1mqP9Lahc.pgp
Description: PGP signature