Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 14 Aug 2017 19:03:05 +0000 (UTC)
From:      Ngie Cooper <ngie@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r322511 - in head: share/mk tools/build/options
Message-ID:  <201708141903.v7EJ35LE013073@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: ngie
Date: Mon Aug 14 19:03:05 2017
New Revision: 322511
URL: https://svnweb.freebsd.org/changeset/base/322511

Log:
  Add limited sandbox capability to "make check"
  
  == Rationale ==
  
  r295380 introduced "make check" and consolidated means for running
  test code in an attempt to simplify running tests. One could either
  install files/libraries/programs and run "make check", or run "make check"
  with an explicit CHECKDIR, e.g., `make check CHECKDIR=$(make -V.OBJDIR)``.
  
  One criticism that was received is that "make check" should be run with
  the intent of making dev->test->commit easier, which means that the target
  audience's workflow should be developers. One developer pattern available
  in other opensource projects is to run test code from a developer sandbox,
  instead of installing to a system.
  
  == Method ==
  
  This approach is slightly different from the standard approach, in the sense
  that it builds and installs into a deterministic directory under .OBJDIR (as I call it,
  the "sandbox"), then runs "make check" against that. In the event the test
  run is successful, the deterministic directory is removed to save space.
  
  == Approach ==
  
  bsd.lib.mk, bsd.prog.mk:
  
  To support this functionality, a new variable `HAS_TESTS` is being added.
  
  HAS_TESTS enables appropriate behavior with bsd.lib.mk and bsd.prog.mk, as
  follows:
  - Add "make check" as an available target from the directory.
  - Pass down appropriate variables via ${TESTS_ENV}, i.e.,
    ${TESTS_LD_LIBRARY_PATH} and ${TESTS_PATH}.
  
  One should add "HAS_TESTS" to directories containing tests in them, e.g. from
  bin/sh/Makefile,
  
    HAS_TESTS=
    SUBDIR.${MK_TESTS}+= tests
  
  HAS_TESTS doesn't automatically add the tests subdirectory for flexibility
  reasons.
  
  bsd.opts.mk, src.opts.mk:
  - The knob ${MK_MAKE_CHECK_USE_SANDBOX} has been added, both to explicitly
    direct (internally) when to set a deterministic ${DESTDIR} and to also allow
    users to disable this behavior globally, i.e., via src.conf.
  - MK_TESTS has been promoted from src.opts.mk to bsd.opts.mk to leverage
    syntactic sugar for having MK_TESTS be a dependency for
    MK_MAKE_CHECK_USE_SANDBOX, but to also ensure that src.opts.mk isn't required
    to use suite.test.mk (which is a dependency of bsd.test.mk).
  
  suite.test.mk:
  - beforecheck behavior (when MK_MAKE_CHECK_USE_SANDBOX is enabled) is modified
    from a no-op to:
  -- Build.
  -- Run "make hierarchy" on the sandbox dir.
  -- Install the tests/files to the sandbox dir.
  - aftercheck behavior (when MK_MAKE_CHECK_USE_SANDBOX is enabled) is modified
    from a no-op to:
  -- Remove the sandbox dir.
  
  Again, because the dependency order set in bsd.test.mk is
  beforecheck -> check -> aftercheck, "make check" will not be run unless
  "beforecheck" completes successfully, and "aftercheck" will not be run unless
  "beforecheck" and "check" complete successfully.
  
  == Caveats ==
  
  - This target must either be run with MK_INSTALL_AS_USER or as root. Otherwise
    it will fail when running "make install" as the default user/group for many
    makefiles when calling INSTALL is root/wheel.
  - This target must be run from a suitable top-level directory. For example,
    running tests from `tests/sys/fs/tmpfs` won't work, but `tests/sys/fs` will,
    because `tests/sys/fs/tmpfs` relies on files installed by `tests/sys/fs`.
  - Running MK_INSTALL_AS_USER may introduce determinism issues. However, using
    it could identify deficiences in tests in terms of needing to be run as
    root, which are not properly articulated in the test requirements.
  - The doesn't negate the need for running "make installworld" and
    "make checkworld", etc. Again, this just is intended to simplify the
    dev->test->commit workflow.
  
  == Cleanup done ==
  - CHECKDIR is removed; one can use "MK_MAKE_CHECK_USE_SANDBOX=no" to enable
    "legacy" (r295380) behavior.
  
  MFC after:	2 months
  Relnotes:	yes (CHECKDIR removed; "make check" behavior changed)
  Requested by:	jhb
  Reviewed by:	arch (silence), testing (silence)
  Differential Revision:	D11905

Added:
  head/tools/build/options/WITHOUT_MAKE_CHECK_USE_SANDBOX
     - copied, changed from r322452, head/tools/build/options/WITHOUT_TESTS
Modified:
  head/share/mk/bsd.README
  head/share/mk/bsd.lib.mk
  head/share/mk/bsd.opts.mk
  head/share/mk/bsd.prog.mk
  head/share/mk/src.opts.mk
  head/share/mk/suite.test.mk

Modified: head/share/mk/bsd.README
==============================================================================
--- head/share/mk/bsd.README	Mon Aug 14 18:49:46 2017	(r322510)
+++ head/share/mk/bsd.README	Mon Aug 14 19:03:05 2017	(r322511)
@@ -566,7 +566,7 @@ It has seven targets:
 	all:
 		build the test programs.
 	check:
-		runs the test programs from CHECKDIR with kyua test.
+		runs the test programs with kyua test.
 
 		The beforecheck and aftercheck targets will be invoked, if
 		defined, to execute commands before and after the realcheck
@@ -574,8 +574,6 @@ It has seven targets:
 
 		The devel/kyua package must be installed before invoking this
 		target.
-
-		See CHECKDIR for more details.
 	clean:
 		remove the test programs and any object files.
 	cleandir:
@@ -602,10 +600,6 @@ ATF_TESTS_C	The names of the ATF C test programs to bu
 ATF_TESTS_CXX	The names of the ATF C++ test programs to build.
 
 ATF_TESTS_SH	The names of the ATF sh test programs to build.
-
-CHECKDIR	The directory that 'make check' executes tests from.
-
-		The value of CHECKDIR defaults to .OBJDIR.
 
 KYUAFILE	If 'auto' (the default), generate a Kyuafile out of the
 		test programs defined in the Makefile.  If 'yes', then a

Modified: head/share/mk/bsd.lib.mk
==============================================================================
--- head/share/mk/bsd.lib.mk	Mon Aug 14 18:49:46 2017	(r322510)
+++ head/share/mk/bsd.lib.mk	Mon Aug 14 19:03:05 2017	(r322511)
@@ -442,6 +442,12 @@ OBJS_DEPEND_GUESS.${_S:R}.pico+=	${_S}
 .endfor
 .endif
 
+.if defined(HAS_TESTS)
+MAKE+=			MK_MAKE_CHECK_USE_SANDBOX=yes
+SUBDIR_TARGETS+=	check
+TESTS_LD_LIBRARY_PATH+=	${.OBJDIR}
+.endif
+
 .include <bsd.dep.mk>
 .include <bsd.clang-analyze.mk>
 .include <bsd.obj.mk>

Modified: head/share/mk/bsd.opts.mk
==============================================================================
--- head/share/mk/bsd.opts.mk	Mon Aug 14 18:49:46 2017	(r322510)
+++ head/share/mk/bsd.opts.mk	Mon Aug 14 19:03:05 2017	(r322511)
@@ -55,6 +55,7 @@ __DEFAULT_YES_OPTIONS = \
     INCLUDES \
     INSTALLLIB \
     KERBEROS \
+    MAKE_CHECK_USE_SANDBOX \
     MAN \
     MANCOMPRESS \
     NIS \
@@ -63,6 +64,7 @@ __DEFAULT_YES_OPTIONS = \
     PROFILE \
     SSP \
     SYMVER \
+    TESTS \
     TOOLCHAIN \
     WARNS
 
@@ -73,6 +75,7 @@ __DEFAULT_NO_OPTIONS = \
     STALE_STAGED
 
 __DEFAULT_DEPENDENT_OPTIONS = \
+    MAKE_CHECK_USE_SANDBOX/TESTS \
     STAGING_MAN/STAGING \
     STAGING_PROG/STAGING \
     STALE_STAGED/STAGING \

Modified: head/share/mk/bsd.prog.mk
==============================================================================
--- head/share/mk/bsd.prog.mk	Mon Aug 14 18:49:46 2017	(r322510)
+++ head/share/mk/bsd.prog.mk	Mon Aug 14 19:03:05 2017	(r322511)
@@ -301,6 +301,13 @@ lint: ${SRCS:M*.c}
 .include <bsd.man.mk>
 .endif
 
+.if defined(HAS_TESTS)
+MAKE+=			MK_MAKE_CHECK_USE_SANDBOX=yes
+SUBDIR_TARGETS+=	check
+TESTS_LD_LIBRARY_PATH+=	${.OBJDIR}
+TESTS_PATH+=		${.OBJDIR}
+.endif
+
 .if defined(PROG)
 OBJS_DEPEND_GUESS+= ${SRCS:M*.h}
 .endif

Modified: head/share/mk/src.opts.mk
==============================================================================
--- head/share/mk/src.opts.mk	Mon Aug 14 18:49:46 2017	(r322510)
+++ head/share/mk/src.opts.mk	Mon Aug 14 19:03:05 2017	(r322511)
@@ -160,7 +160,6 @@ __DEFAULT_YES_OPTIONS = \
     TCP_WRAPPERS \
     TCSH \
     TELNET \
-    TESTS \
     TEXTPROC \
     TFTP \
     TIMED \

Modified: head/share/mk/suite.test.mk
==============================================================================
--- head/share/mk/suite.test.mk	Mon Aug 14 18:49:46 2017	(r322510)
+++ head/share/mk/suite.test.mk	Mon Aug 14 19:03:05 2017	(r322511)
@@ -8,6 +8,8 @@
 .error suite.test.mk cannot be included directly.
 .endif
 
+.include <bsd.opts.mk>
+
 # Name of the test suite these tests belong to.  Should rarely be changed for
 # Makefiles built into the FreeBSD src tree.
 TESTSUITE?= FreeBSD
@@ -75,8 +77,6 @@ Kyuafile: Makefile
 	@mv ${.TARGET}.tmp ${.TARGET}
 .endif
 
-CHECKDIR?=	${DESTDIR}${TESTSDIR}
-
 KYUA= ${LOCALBASE}/bin/kyua
 
 # Definition of the "make check" target and supporting variables.
@@ -99,4 +99,26 @@ realcheck: .PHONY
 		echo "LOCALBASE=\"${LOCALBASE}\""; \
 		false; \
 	fi
-	@${KYUA} test -k ${CHECKDIR}/Kyuafile
+	@env ${TESTS_ENV:Q} ${KYUA} test -k ${DESTDIR}${TESTSDIR}/Kyuafile
+
+MAKE_CHECK_SANDBOX_DIR=	${.OBJDIR}/checkdir
+CLEANDIRS+=	${MAKE_CHECK_SANDBOX_DIR}
+
+.if ${MK_MAKE_CHECK_USE_SANDBOX} != "no" && make(check)
+DESTDIR:=	${MAKE_CHECK_SANDBOX_DIR}
+
+beforecheck:
+.for t in clean depend all
+	@cd ${.CURDIR} && ${MAKE} $t
+.endfor
+	@cd ${SRCTOP} && ${MAKE} hierarchy DESTDIR=${DESTDIR}
+	@cd ${.CURDIR} && ${MAKE} install \
+	    DESTDIR=${DESTDIR}
+
+# NOTE: this is intentional to ensure that "make check" can be run multiple
+#       times. "aftercheck" won't be run if "make check" fails, is interrupted,
+#       etc.
+aftercheck:
+	@cd ${.CURDIR} && ${MAKE} clean
+
+.endif

Copied and modified: head/tools/build/options/WITHOUT_MAKE_CHECK_USE_SANDBOX (from r322452, head/tools/build/options/WITHOUT_TESTS)
==============================================================================
--- head/tools/build/options/WITHOUT_TESTS	Sun Aug 13 01:23:13 2017	(r322452, copy source)
+++ head/tools/build/options/WITHOUT_MAKE_CHECK_USE_SANDBOX	Mon Aug 14 19:03:05 2017	(r322511)
@@ -1,9 +1,10 @@
 .\" $FreeBSD$
-Set to not build nor install the
-.Fx
-Test Suite in
-.Pa /usr/tests/ .
+Set to not execute
+.Dq Li "make check"
+in limited sandbox mode.
+This option should be paired with
+.Va WITH_INSTALL_AS_USER
+if executed as an unprivileged user.
 See
 .Xr tests 7
 for more details.
-This also disables the build of all test-related dependencies, including ATF.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201708141903.v7EJ35LE013073>