Re: PKG_SKIP/FAIL_REASON

To: tech-pkg%netbsd.org@localhost
Subject: Re: PKG_SKIP/FAIL_REASON
From: David Holland <dholland-pkgtech%netbsd.org@localhost>
Date: Tue, 30 Dec 2014 06:22:58 +0000
On Tue, Dec 30, 2014 at 05:23:39AM +0000, David Holland wrote:
 > After some discussion a couple weeks ago, I'm working on adding
 > BROKEN_ON_PLATFORM (and BROKEN_ON_COMPILER) to supplement
 > NOT_FOR_PLATFORM and NOT_FOR_COMPILER. The idea is that we restrict
 > NOT_FOR to cases where it doesn't make sense to build the package
 > (e.g. netbsd kernel modules on linux) and use BROKEN_ON where the
 > package does make sense but doesn't work, like most of the non-64-
 > bit-clean packages.

After reviewing the uses of NOT_FOR_COMPILER, I'm inclined to say we
should either leave it alone or rename the variable to
BROKEN_ON_COMPILER, depending on how we feel about minor consistency;
I don't think there's a measurable number of cases where it doesn't
make sense to build a package with a particular compiler, only cases
where it doesn't work. But also, the logic is used to pick a different
compiler rather than to fail, so it won't make any difference to
failure reporting and tracking.

I have the following preliminary patch for BROKEN_ON_PLATFORM,
though. It has been lightly tested and appears to DTRT. It doesn't
include changes to the Guide or to pkglint, and doesn't include review
and cleanup of PKG_FAIL_REASON vs. PKG_SKIP_REASON either. (But it
does trigger PKG_SKIP_REASON vs. PKG_FAIL_REASON for NOT_FOR vs.
BROKEN_ON.)

I'm not especially happy with the name BROKEN_EXCEPT_ON_PLATFORM; it
could be WORKS_ON_PLATFORM, but (a) I'd kind of rather have the word
BROKEN in it, and (b) that's too affirmative (not broken is not the
same thing as known to be working...) and so far I can't think of
anything else.

also I'm sure pkglint would be happier with BROKEN_ON_PLATFORMS and so
on, but as NOT_FOR_PLATFORM/ONLY_FOR_PLATFORM aren't lexicographically
plural I don't really want to change.


Index: mk/bsd.pkg.mk
===================================================================
RCS file: /cvsroot/pkgsrc/mk/bsd.pkg.mk,v
retrieving revision 1.2006
diff -u -r1.2006 bsd.pkg.mk
--- mk/bsd.pkg.mk	25 Nov 2014 18:27:17 -0000	1.2006
+++ mk/bsd.pkg.mk	30 Dec 2014 06:13:41 -0000
@@ -516,7 +516,24 @@
 
 .include "license.mk"
 
-# Define __PLATFORM_OK only if the OS matches the pkg's allowed list.
+#
+# Check for packages broken or inappropriate on this platform.
+# Set __PLATFORM_OK and __PLATFORM_WORKS only if the platform passes
+# both the NOT_FOR/ONLY_FOR and BROKEN_ON lists (respectively).
+#
+
+# 1. BROKEN_EXCEPT_ON_PLATFORM
+.  if defined(BROKEN_EXCEPT_ON_PLATFORM) && !empty(BROKEN_EXCEPT_ON_PLATFORM)
+.    for __tmp__ in ${BROKEN_EXCEPT_ON_PLATFORM}
+.      if ${MACHINE_PLATFORM:M${__tmp__}} != ""
+__PLATFORM_WORKS?=	yes
+.      endif	# MACHINE_PLATFORM
+.    endfor	# __tmp__
+.  else	# !BROKEN_EXCEPT_ON_PLATFORM
+__PLATFORM_WORKS?=	yes
+.  endif	# BROKEN_EXCEPT_ON_PLATFORM
+
+# 2. ONLY_FOR_PLATFORM
 .  if defined(ONLY_FOR_PLATFORM) && !empty(ONLY_FOR_PLATFORM)
 .    for __tmp__ in ${ONLY_FOR_PLATFORM}
 .      if ${MACHINE_PLATFORM:M${__tmp__}} != ""
@@ -526,15 +543,30 @@
 .  else	# !ONLY_FOR_PLATFORM
 __PLATFORM_OK?=	yes
 .  endif	# ONLY_FOR_PLATFORM
+
+# 3. BROKEN_ON_PLATFORM
+.  for __tmp__ in ${BROKEN_ON_PLATFORM}
+.    if ${MACHINE_PLATFORM:M${__tmp__}} != ""
+.      undef __PLATFORM_WORKS
+.    endif	# MACHINE_PLATFORM
+.  endfor	# __tmp__
+
+# 4. NOT_FOR_PLATFORM
 .  for __tmp__ in ${NOT_FOR_PLATFORM}
 .    if ${MACHINE_PLATFORM:M${__tmp__}} != ""
 .      undef __PLATFORM_OK
 .    endif	# MACHINE_PLATFORM
 .  endfor	# __tmp__
+
+# Check OK (NOT_FOR/ONLY_FOR) before WORKS (BROKEN_ON)
 .  if !defined(__PLATFORM_OK)
-PKG_FAIL_REASON+= "${PKGNAME} is not available for ${MACHINE_PLATFORM}"
+PKG_SKIP_REASON+= "${PKGNAME} is not available for ${MACHINE_PLATFORM}"
 .  endif	# !__PLATFORM_OK
-.endif
+.  if !defined(__PLATFORM_WORKS)
+PKG_FAIL_REASON+= "${PKGNAME} is marked broken on ${MACHINE_PLATFORM}"
+.  endif	# !__PLATFORM_WORKS
+
+.endif # NO_SKIP
 
 # Add these defs to the ones dumped into +BUILD_DEFS
 _BUILD_DEFS+=	PKGPATH
Index: mk/bsd.prefs.mk
===================================================================
RCS file: /cvsroot/pkgsrc/mk/bsd.prefs.mk,v
retrieving revision 1.352
diff -u -r1.352 bsd.prefs.mk
--- mk/bsd.prefs.mk	7 Dec 2014 06:22:52 -0000	1.352
+++ mk/bsd.prefs.mk	30 Dec 2014 06:13:42 -0000
@@ -820,10 +820,10 @@
 _SYS_VARS.dirs=		WRKDIR DESTDIR PKG_SYSCONFBASEDIR
 
 # List of 64bit operating systems with sizeof(int) != sizeof(void *).
-# This can be used for software that is not 64bit clean.
-# amd64 is for OpenBSD.
+# This can be used with BROKEN_ON_PLATFORM for software that is not
+# 64bit clean. The "amd64" case is for OpenBSD.
 #
-# Keywords: ONLY_FOR_PLATFORM NOT_FOR_PLATFORM 64bit
+# Keywords: BROKEN_ON_PLATFORM 64bit
 #
 LP64PLATFORMS=		*-*-alpha *-*-sparc64 *-*-x86_64 *-*-amd64
 
Index: mk/help/notonly.help
===================================================================
RCS file: /cvsroot/pkgsrc/mk/help/notonly.help,v
retrieving revision 1.2
diff -u -r1.2 notonly.help
--- mk/help/notonly.help	12 Sep 2007 09:59:44 -0000	1.2
+++ mk/help/notonly.help	30 Dec 2014 06:13:42 -0000
@@ -2,21 +2,34 @@
 
 # === Package-settable variables ===
 #
+# BROKEN_ON_PLATFORM
+#	A list of platforms on which this package doesn't build or
+#	builds but doesn't run.
+#
 # NOT_FOR_PLATFORM
-#	A list of platforms (for which this package either doesn't build
-#	or isn't useful to have.
+#	A list of platforms for which this package isn't useful to
+#	have and doesn't make sense to attempt to build.
+#
 #
+# BROKEN_EXCEPT_ON_PLATFORM
+#	If a package only builds and runs on some platforms, they
+#	should be listed here. This variable should only be used if
+#	you are sure that the package won't work on other platforms.
+#	It is almost always better to use BROKEN_ON_PLATFORM.
 #
 # ONLY_FOR_PLATFORM
-#	If a package only builds on some platforms, they should be
-#	listed here. This variable should only be used if you are sure
-#	that the package won't work on other platforms. It is almost
-#	always better to use NOT_FOR_PLATFORM.
+#	If a package only makes sense on a fixed set of platforms
+#	(often exactly one), they should be listed here. This variable
+#	should only be used if you are sure that the package doesn't
+#	make sense on other platforms. For ordinary cases it is almost
+#	always better to use NOT_FOR_PLATFORM, and if the package merely
+#	doesn't work, use BROKEN_ON_PLATFORM.
 #
 # Platforms are triples of OPSYS, OS_VERSION and MACHINE_ARCH, separated
 # by dashes. Each of the components may be the wildcard "*".
 #
-# Whenever you use these variables in a package Makefile, add a comment
-# nearby _why_ you are restricting the list of platforms. Otherwise
-# these restrictions may be quickly removed by other developers.
+# Whenever you use these variables in a package Makefile, add a
+# comment nearby explaining _why_ you are restricting the list of
+# platforms. Otherwise these restrictions may be quickly removed by
+# other developers.
 #
Index: mk/misc/can-be-built-here.mk
===================================================================
RCS file: /cvsroot/pkgsrc/mk/misc/can-be-built-here.mk,v
retrieving revision 1.6
diff -u -r1.6 can-be-built-here.mk
--- mk/misc/can-be-built-here.mk	5 Nov 2008 08:24:23 -0000	1.6
+++ mk/misc/can-be-built-here.mk	30 Dec 2014 06:13:42 -0000
@@ -5,6 +5,7 @@
 #
 # * NOT_FOR_COMPILER, ONLY_FOR_COMPILER
 # * NOT_FOR_PLATFORM, ONLY_FOR_PLATFORM
+# * BROKEN_ON_PLATFORM, BROKEN_EXCEPT_ON_PLATFORM
 # * NOT_FOR_BULK_PLATFORM
 # * NOT_FOR_UNPRIVILEGED, ONLY_FOR_UNPRIVILEGED
 # * PKG_FAIL_REASON, PKG_SKIP_REASON
@@ -89,6 +90,31 @@
 .  endfor
 .endif
 
+# Check BROKEN_ON_PLATFORM
+_CBBH_CHECKS+=		bplat
+_CBBH_MSGS.bplat=	"This package is broken on these platforms: "${BROKEN_ON_PLATFORM:Q}"."
+
+_CBBH.bplat=		yes
+.for p in ${BROKEN_ON_PLATFORM}
+.  if !empty(MACHINE_PLATFORM:M${p})
+_CBBH.bplat=		no
+.  endif
+.endfor
+
+# Check BROKEN_EXCEPT_ON_PLATFORM
+_CBBH_CHECKS+=		beplat
+_CBBH_MSGS.beplat=	"This package is broken except on these platforms: "${BROKEN_EXCEPT_ON_PLATFORM:Q}"."
+
+_CBBH.beplat=		yes
+.if defined(BROKEN_EXCEPT_ON_PLATFORM) && !empty(BROKEN_EXCEPT_ON_PLATFORM)
+_CBBH.beplat=		no
+.  for p in ${BROKEN_EXCEPT_ON_PLATFORM}
+.    if !empty(MACHINE_PLATFORM:M${p})
+_CBBH.beplat=		yes
+.    endif
+.  endfor
+.endif
+
 # Check NOT_FOR_UNPRIVILEGED
 _CBBH_CHECKS+=		nunpriv
 _CBBH_MSGS.nunpriv=	"This package is not available in unprivileged mode."

-- 
David A. Holland
dholland%netbsd.org@localhost
Follow-Ups:
- Re: PKG_SKIP/FAIL_REASON
  - From: Greg Troxel
- NOT_FOR_COMPILER/BROKEN_ON_COMPILER (was: PKG_SKIP/FAIL_REASON)
  - From: Edgar Fuß
References:
- PKG_SKIP/FAIL_REASON
  - From: David Holland
Prev by Date: Re: builtin.mk for lua [take 2]
Next by Date: Re: to revbump or not to revbump?
Previous by Thread: PKG_SKIP/FAIL_REASON
Next by Thread: NOT_FOR_COMPILER/BROKEN_ON_COMPILER (was: PKG_SKIP/FAIL_REASON)
Indexes:
Home | Main Index | Thread Index | Old Index