From owner-freebsd-testing@freebsd.org Thu Nov 19 18:14:41 2015 Return-Path: Delivered-To: freebsd-testing@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 455BAA33369 for ; Thu, 19 Nov 2015 18:14:41 +0000 (UTC) (envelope-from yaneurabeya@gmail.com) Received: from mail-pa0-x22c.google.com (mail-pa0-x22c.google.com [IPv6:2607:f8b0:400e:c03::22c]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 145B01A02; Thu, 19 Nov 2015 18:14:41 +0000 (UTC) (envelope-from yaneurabeya@gmail.com) Received: by pabfh17 with SMTP id fh17so91181868pab.0; Thu, 19 Nov 2015 10:14:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=content-type:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=oBi3XVKca7vKN+atPv05A0ief6x501RqLOTDfw60m9o=; b=JmGL2rzSYk7T3OsBBtFjpvmETx9IZHNAMKBmN8/abjKSCqNw31fs1Jy10PeBNOpkrW 4yo/4GXMSil275KHC1iWDtWLc2sLev/d4yIClj2qdyd55KPeGvG/yieL6fu6nij61rkH ZqpzJg0s+Roy5VZfDiLJMkLd99ltJg3HzpX3fCNKb5gpGo2HLO2m/9BHzXTBSjykfDA+ DIWpRQ4zGbm4krDTLQEMogRju/M9MwZPawlWUuYgwcErtVkL80NJT2yXBl3Kjb4niVuL OB25nFzivEH/IzL2b8fL5hifm8bLhhyg3CfmNqpbBSK2toIBYZkHAgwwHXZVMn5jl/n7 QgOQ== X-Received: by 10.66.188.49 with SMTP id fx17mr12581178pac.95.1447956880497; Thu, 19 Nov 2015 10:14:40 -0800 (PST) Received: from [33.172.230.229] (ma82036d0.tmodns.net. [208.54.32.168]) by smtp.gmail.com with ESMTPSA id dd4sm12021364pbb.52.2015.11.19.10.14.36 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 19 Nov 2015 10:14:38 -0800 (PST) Content-Type: text/plain; charset=cp932 Mime-Version: 1.0 (1.0) Subject: Re: [RFC] Rename `make test` in suite.test.mk with `make regress` From: Garrett Cooper X-Mailer: iPhone Mail (13B143) In-Reply-To: Date: Thu, 19 Nov 2015 10:14:35 -0800 Cc: "freebsd-testing@freebsd.org" Content-Transfer-Encoding: quoted-printable Message-Id: <27967032-F7AD-481F-A397-9495699B41AF@gmail.com> References: <5C39F495-5717-450F-8460-14B69DD48AD6@freebsd.org> To: Julio Merino X-BeenThere: freebsd-testing@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Testing on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 19 Nov 2015 18:14:41 -0000 > On Oct 17, 2015, at 21:27, NGie Cooper wrote: >=20 >>> On Oct 17, 2015, at 19:21, Julio Merino wrote: >>>=20 >>>> On Oct 17, 2015, at 21:56, NGie Cooper wrote: >>>>=20 >>>>=20 >>>> On Oct 17, 2015, at 18:50, Julio Merino wrote: >>>>=20 >>>>> On Oct 17, 2015, at 18:03, NGie Cooper wrote: >>>>>=20 >>>>> Hi all, >>>>> There=81fs a lesser known target in suite.test.mk that runs `kyua t= est` in a similar manner to how Jenkins and other groups have integrated kyu= a into their test infrastructures. >>>>> The legacy target on FreeBSD was `regress`, but the target created w= ith the bsd.test.mk creation back a few years ago was `test`. Why change fro= m `test` to `regress`? There are places in the tree (bin/test for example) t= hat have targets named test, so in order to avoid clashing with a common tar= get (name), it=81fs best to use the legacy target name. >>>>> Would anyone have any serious heartburn over the change? >>>>=20 >>>> Is this only because of bin/test? Seems like renaming a target to avoi= d that one collision is just moving the problem around. It is possible that= some other directory could later grow a target that conflicts with your new= name. >>>=20 >>> Yes, I realize that. The goal though is I want to be able to call `make <= something>` from the top and it would iterate down each and every subdirecto= ry and run tests. >>=20 >> Other for the bin/test naming collision, that should work already, should= n't it? >=20 > make test isn=81ft currently hooked in to any subdirectories. So this is w= hat happens today: >=20 > $ make -VRELDIR > bin > $ make test > =3D=3D=3D> tests (buildconfig) > =3D=3D=3D> tests (all) > $ cd test > $ make test > `test' is up to date. > $ >=20 > If I fake hooking it up, things get a bit weird because it=81fs now going t= o evaluate all targets named `test`, which currently causes a build break wi= th bin/sh and the builds test binary when I do it from bin/ : >=20 > $ env SUBDIR_TARGETS=3Dtest make test > =3D=3D=3D> tests (test) > *** Using this test does not preclude you from running the tests > *** installed in /usr/tests. This test run may raise false > *** positives and/or false negatives. >=20 > *** WARNING: make test is experimental > *** > *** Using this test does not preclude you from running the tests > *** installed in /usr/tests. This test run may raise false > *** positives and/or false negatives. >=20 > legacy_test:main -> passed [0.056s] >=20 > Results file id is usr_tests_bin_test.20151018-023326-624852 > Results saved to /home/ngie/.kyua/store/results.usr_tests_bin_test.2015101= 8-023326-624852.db >=20 > 1/1 passed (0 failed) >=20 > *** Once again, note that make test is unsupported. > cc -O2 -pipe -fno-strict-aliasing -O2 -pipe -std=3Dgnu99 -fstack-protect= or-strong -Wsystem-headers -Werror -Wall -Wno-format-y2k -W -Wno-unused-para= meter -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Wreturn-type= -Wcast-qual -Wwrite-strings -Wswitch -Wshadow -Wunused-parameter -Wcast-ali= gn -Wchar-subscripts -Winline -Wnested-externs -Wredundant-decls -Wold-style= -definition -Wno-pointer-sign -Wmissing-variable-declarations -Wthread-safet= y -Wno-empty-body -Wno-string-plus-int -Wno-unused-const-variable -Qunused-a= rguments -o test test.o >=20 > Neither case works, so this is why test is currently out of the picture. >=20 >> Not sure how ugly this would be, but you'd also define an internal "run-t= ests" target or similar that is used exclusively by the recursion code. The= n, in the top-level Makefile, you define a bare "test" target that starts th= e recursion so that "make test" works. And then, in bsd.test.mk, you define= a "test" target if - and only if - there is no "test" target yet. bin/test= would behave differently than the rest if you ever went in there and ran "m= ake test", but I think that's a minor problem. >=20 > I=81fm pretty sure make testworld would be ok for a top-level target. Hone= stly though, I don=81ft think people really care as long as they can execute= one command and it would do everything for them (run_tests, etc). >=20 >> This is offtopic, but do you have a plan anywhere on how to make the test= s invocation this way reliable? Without installed binaries, the tests might= not do the right thing, and I'm not sure if the object tree is enough for t= his to work properly. That's the reason why "make test" from the top-level d= irectory is currently disallowed. >=20 > I can=81ft make it 100% reliable and it=81fs not a replacement for really r= unning the tests or doing other CI testing as the tests might rely on tools o= n the base system. As a base set of requirements: > - Compile the tests > - Install the tests and supporting files to a temporary directory under MA= KEOBJDIRPREFIX. > - Run the tests >=20 > Once this has been resolved, we can start looking at other items, like: > - How=20 >=20 >> I think I see what you mean: that because "regress" already existed in th= e FreeBSD tree, no other targets were introduced to collide with it? If yes= , I wouldn't expect that to be the case. Tests were not interleaved with th= e source tree in the past, so the "regress" target only existed in a subset o= f the tree, didn't it? >=20 > Correct. It=81fs been established as a =81ghands-off=81h target for some t= ime. >=20 > You=81fre correct about regress only existing in part of the tree but in r= eality it was hooked in to other places in the build system, even if it was u= nused outside tools/regression: >=20 > $ grep -r regress share/mk Makefile* > share/mk/bsd.subdir.mk: realinstall regress tags \ > share/mk/bsd.sys.mk: realdepend realinstall regress subdir-all s= ubdir-depend \ > Makefile: obj objlink regress rerelease showconfig tags toolchain up= date \ > $ >=20 > I admit that it=81fs a poorly named target, but would probably raise less e= yebrows than telling people to invoke =81gkyua test=81h has in the past/woul= d... >=20 >>>> Have you considered "check"? That'd be in line with what automake does= , for example, which would homogenize the target name with a ton of other pr= ojects out there. >>>=20 >>> =81c `make check` would be ok too. I just have to comb the tree for bina= ries that don=81ft match `check`. >=20 > This doesn=81ft seem like a doable option right now, but I=81fll need to d= o more investigation as this checkout doesn=81ft have the rest of my changes= : >=20 > $ find . -name check.\* > ./sbin/fsck_msdosfs/check.c > ./tools/build/make_check/check.mk > ./contrib/xz/src/liblzma/api/lzma/check.h > ./contrib/xz/src/liblzma/check/check.c > ./contrib/xz/src/liblzma/check/check.h > ./contrib/atf/atf-c/check.c > ./contrib/atf/atf-c/check.h > ./contrib/atf/atf-c++/check.cpp > ./contrib/atf/atf-c++/check.hpp > ./contrib/serf/build/check.py > ./contrib/ntp/sntp/libopts/check.c > ./crypto/heimdal/kadmin/check.c > $ cd sbin/fsck_msdosfs; make check > cc -O2 -pipe -fno-strict-aliasing -O2 -pipe -I/usr/src/svn/sbin/fsck_msd= osfs/../fsck -std=3Dgnu99 -fstack-protector-strong -Wsystem-headers -Werror -= Wall -Wno-format-y2k -W -Wno-unused-parameter -Wstrict-prototypes -Wmissing-= prototypes -Wpointer-arith -Wreturn-type -Wcast-qual -Wwrite-strings -Wswitc= h -Wshadow -Wunused-parameter -Wcast-align -Wchar-subscripts -Winline -Wnest= ed-externs -Wredundant-decls -Wold-style-definition -Wno-pointer-sign -Wmiss= ing-variable-declarations -Wthread-safety -Wno-empty-body -Wno-string-plus-i= nt -Wno-unused-const-variable -Qunused-arguments /usr/src/svn/sbin/fsck_msd= osfs/check.c -o check > /usr/lib/crt1.o: In function `_start': > /usr/src/git/lib/csu/amd64/crt1.c:(.text+0x17b): undefined reference to `m= ain' > /tmp/check-014c0d.o: In function `checkfilesys': > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x2f): undefined reference t= o `alwaysno' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x35): undefined reference t= o `rdonly' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x3b): undefined reference t= o `preen' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x53): undefined reference t= o `rdonly' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x77): undefined reference t= o `rdonly' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x9d): undefined reference t= o `pwarn' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0xa3): undefined reference t= o `rdonly' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0xaf): undefined reference t= o `preen' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0xc4): undefined reference t= o `preen' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0xd7): undefined reference t= o `rdonly' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0xef): undefined reference t= o `readboot' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x10c): undefined reference t= o `perr' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x148): undefined reference t= o `preen' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x14e): undefined reference t= o `skipclean' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x164): undefined reference t= o `checkdirty' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x191): undefined reference t= o `preen' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x1cc): undefined reference t= o `readfat' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x235): undefined reference t= o `readfat' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x25c): undefined reference t= o `comparefat' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x28a): undefined reference t= o `preen' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x2aa): undefined reference t= o `checkfat' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x2cb): undefined reference t= o `preen' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x2eb): undefined reference t= o `resetDosDirSection' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x317): undefined reference t= o `handleDirTree' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x32a): undefined reference t= o `preen' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x34d): undefined reference t= o `checklost' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x371): undefined reference t= o `ask' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x390): undefined reference t= o `writefat' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x3f3): undefined reference t= o `pwarn' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x401): undefined reference t= o `pwarn' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x420): undefined reference t= o `ask' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x43f): undefined reference t= o `pwarn' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x454): undefined reference t= o `pwarn' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x46f): undefined reference t= o `writefat' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x48a): undefined reference t= o `finishDosDirSection' > /usr/src/svn/sbin/fsck_msdosfs/check.c:(.text+0x4b6): undefined reference t= o `pwarn' > cc: error: linker command failed with exit code 1 (use -v to see invocatio= n) > *** Error code 1 >=20 > Stop. > make: stopped in /usr/src/svn/sbin/fsck_msdosfs >=20 > Sometimes FreeBSD needs to bite the bullet and conform to something that=81= fs been widely accepted for the last 15 years, even if there will be teeth g= nashing. I've mulled this over for a while and the best path forward is "make check".= I'll go fix all non-conforming areas of the tree and send out a note to arc= h as well. Thanks! -NGie=