From owner-freebsd-testing@FreeBSD.ORG Wed Oct 30 02:48:45 2013 Return-Path: Delivered-To: freebsd-testing@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id 8230B470 for ; Wed, 30 Oct 2013 02:48:45 +0000 (UTC) (envelope-from julio@meroh.net) Received: from mail-la0-f42.google.com (mail-la0-f42.google.com [209.85.215.42]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 0DABE2C3B for ; Wed, 30 Oct 2013 02:48:44 +0000 (UTC) Received: by mail-la0-f42.google.com with SMTP id ea20so589939lab.15 for ; Tue, 29 Oct 2013 19:48:37 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-type; bh=ZnYevcLzcRrCWnq5hBnRM1rcSyfCjp6iDBXIXcXEmlo=; b=fvOBbdHG/wyHzGspDMI67odFjKu9ckDSbSWjJNB+2Rk26DuEeuNlZl5fAKEj+/giHS ka19dkIJBsUjTew9OJAWV7WKsmej+/TLPvYyQFFlEqgxy3UTqLqWq6Mf5jI6V45WCvSz qh6Of5F/BrA6BtXeKUCQiqWGmlBAPcGG361gGo+UQuBI2pDEUKP3AMhB9bgKhaUlpElJ orwSbBappKZMSnBxSe9kMu9oC+Tj1SUgiYo9sZFJi8ZilpMD2X91vCuyzN+GkT3/FLW8 eV0bsXPsYpd0BM+YBvogYc3oWomiW114TmdXFPi65FinQRG5iWnS2pmc7SAv2WnYloFM CM6g== X-Gm-Message-State: ALoCoQnjySRD99dfkYrXIjSsW+70sPd1Xsy4US9nreAUIoqLVVZ7VPuV0uJvxMjpkppjNYGuXFP/ X-Received: by 10.152.225.139 with SMTP id rk11mr1526818lac.27.1383101317140; Tue, 29 Oct 2013 19:48:37 -0700 (PDT) MIME-Version: 1.0 Received: by 10.112.132.135 with HTTP; Tue, 29 Oct 2013 19:48:17 -0700 (PDT) X-Originating-IP: [108.176.158.82] In-Reply-To: <20131029203658.75BF35807E@chaos.jnpr.net> References: <20131029203658.75BF35807E@chaos.jnpr.net> From: Julio Merino Date: Tue, 29 Oct 2013 22:48:17 -0400 Message-ID: Subject: Re: Plugging ATF tests into the build and other cleanups To: "Simon J. Gerraty" Content-Type: text/plain; charset=ISO-8859-1 Cc: freebsd-testing@freebsd.org, Rui Paulo X-BeenThere: freebsd-testing@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Testing on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 30 Oct 2013 02:48:45 -0000 Please disregard my previous reply! Turns out I could get to fixing the remaining rough edges earlier than I thought and thus some of the previous comments are already obsolete. I've replied below to your original mail and copy/pasted my previous replies where it made sense; in other places I've just rewritten them. The patches have been updated. On Tue, Oct 29, 2013 at 4:36 PM, Simon J. Gerraty wrote: > > On Sun, 27 Oct 2013 18:12:12 -0400, Julio Merino writes: >>The one concern I have here is having to keep track of all tests in >>tools/build/mk/OptionalObsoleteFiles.inc so that setting >>WITHOUT_TESTS=no cleans up /usr/tests. This will be a pain to maintain > > Yes. > >>and a sure source of inconsistencies. If we could special-case this to >>make it more automatic, do you have any suggestions? > > Iff ATF/Kyua were the only thing populating /usr/tests/ you could just > rm -rf that dir, but that's not how delete-old-* works else > > KYUAFILES!= cd $DESTDIR/ && find usr/tests -name Kyuafile > OLD_DIRS+= ${KYUAFILES:H} > > would be all you'd need. > You can't even do OLD_FILES+= ${KYUAFILES:H}/* I'm confused... Is that "can't" supposed to be "can"? I just gave this a try and it seems to work. Much simpler to manage. But this is obviously tied to the assumption that /usr/tests/ depends on the WITH_TESTS knob only.... which I think is a fair assumption to make: either you install the full test suite or you don't. Trying to support in-between cases that don't yet exist just makes things more complex; could be revised in the future if/when needed. And hence, this was mostly the rationale for the next patch. > It could be that a new delete-old- target might be useful, Would be nice if it weren't necessary though. I think delete-old-files should deal with this case just as it does for any other WITH_* knob so that there are no extra steps to the rebuild procedure. >>As usual: http://portal.meroh.net/~jmmv/freebsd-testing/ > > remove-without-atf.diff > > looks ok - assuming folk are ok with s,ATF,TESTS, > > freebsd-testing/recurse-subdir.diff > > I'm not so crazy about. > What's wrong with requiring folk to set TESTS_SUBDIR[S] ? See my previous email if you wish, but I have dropped this from the patchset. The other changes I've done below have made this unnecessary... for now at least! > freebsd-testing/move-kyuafiles.diff > > I'm also not crazy about this one (the change to lib/Makefile) > What is the expected use? To populate /usr/tests/lib accordingly. You need: /usr/tests/Kyuafile - auto-recurse into directories /usr/tests/lib/Kyuafile - auto-recurse into directories /usr/tests/lib/libc/Kyuafile - list of tests to run and possibly other subdirectories and that /usr/tests/lib/Kyuafile has to come from somewhere. I think that doing this from src/tests/lib/, as I did it originally, is confusing and it seems to me that doing it from somewhere within src/lib/ is clearer. The rationale would be that everything required to create /usr/tests/lib/ would happen from a single place: src/lib/ . (More below.) > and why can't everything that is needed be > done within the context of the individual lib/*/tests/ dir? > That's how we are doing it. We could do this from src/lib/tests/ as easily; didn't think about it. Gave this a try and updated the patches accordingly. Did the same for src/libexec/tests/, src/usr.bin/tests/ and the 'atf' subdirectories in each of the three. I think this is reasonable enough and should play well with your ideas. > build-atf-tests.diff > > FWIW I prefer ${.CURDIR:H:H} etc rather than ${.CURDIR}/../.. > makes for much neater paths (also bit quicker on nfs) Done; didn't think about that. > It would be nice to be able to rely on SRCTOP being correctly set > (easy with bmake, have to think about fmake...) > > ATF_SRC = ${SRCTOP}/contrib/atf > > could then be set on one place > but otherwise ok. Indeed, having a SRCTOP would have made things easier... but I didn't find that available. OK, patches ready for a second look. Still not ready for submission though as I have not yet had the time to look at the test failures themselves. -- Julio Merino / @jmmv