From owner-freebsd-testing@FreeBSD.ORG Tue Oct 29 21:46:48 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 48C1B328 for ; Tue, 29 Oct 2013 21:46:48 +0000 (UTC) (envelope-from julio@meroh.net) Received: from mail-lb0-f177.google.com (mail-lb0-f177.google.com [209.85.217.177]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id B02012AE5 for ; Tue, 29 Oct 2013 21:46:47 +0000 (UTC) Received: by mail-lb0-f177.google.com with SMTP id u14so542546lbd.36 for ; Tue, 29 Oct 2013 14:46:39 -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=Njg1hDGMuokGg9R/2foH6xQNkfHNtuHUpgKioPGIcaM=; b=RbQB433lLU6dfMHdIkwiCmrkQ51F3C8jQAwpnfFLze70Kld08GqhC0GtjcV/nuH/ze 8Vd53ml/yrf7dVLZyACi1M2Kxyvy+Mk6Tfqu964ifaieLlVJg9u1WvYj9XK5/xKApEew SLSkYpMqllEhvzpjMtyyE6cb8p8/mySyNX1G1Rmp+AflEsn6gQ8oRtgDcv9uYcaBqMrN XuVBSVbiukfPKLGZmUETmeYdCBR04HcbgSHkSI50PRjzUAlcoEdKCm1CaM6AC+bMD4r9 /2jCc/5V/EiukmXzwJot1QrD2KF6eV57gswKy0qltz9fZ86C+lF8/P26gDdeg3w6ufDE rI1w== X-Gm-Message-State: ALoCoQlB4BWPxDZmRtDnnwWQhYSnO6Kv5SM6R9bWTmgj3SrLcVJHdS+V0p3NqqFva+veDPKDh6XC X-Received: by 10.152.29.38 with SMTP id g6mr913197lah.25.1383083199482; Tue, 29 Oct 2013 14:46:39 -0700 (PDT) MIME-Version: 1.0 Received: by 10.112.132.135 with HTTP; Tue, 29 Oct 2013 14:46:19 -0700 (PDT) X-Originating-IP: [172.26.105.74] In-Reply-To: <20131029203658.75BF35807E@chaos.jnpr.net> References: <20131029203658.75BF35807E@chaos.jnpr.net> From: Julio Merino Date: Tue, 29 Oct 2013 17:46:19 -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: Tue, 29 Oct 2013 21:46:48 -0000 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"? Or in other words, do you mean that this would or would not work if we assumed that ATF/Kyua are the only thing populating /usr/tests ? That's part of the rationale for the next patch: to make /usr/tests/ conditional on a single knob (WITH_TESTS) so that such assumptions can be made: either you install the full test suite or you don't. I don't think it makes sense to support in-between cases... or at least not until a real need for it appears). > 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] ? Because SUBDIR and TESTS_SUBDIRS serve different purposes. With SUBDIR you are telling make (and only make) that it has to recurse into those directories. With TESTS_SUBDIRS you are telling both make and kyua to recurse into the subdirectories. This is useful to build helper binaries/libraries/etc that live in subdirectories without tests (which is necessary when bsd.*.mk don't play well with the test stuff); however, this use-case is not needed yet so we can ignore it for now. The real reason I tossed in this patch here is the following. In the current patchset, this is useful in, for example, src/lib/atf/Makefile (see last patch) : this file needs to generate a Kyuafile that registers its 3 subdirectories (libatf-c, libatf-c++ and test-programs) so that the recursion can be performed at runtime. There is no need to list those as TESTS_SUBDIRS because they are already in SUBDIR. There are some alternatives to this behavior: 1. Move the generation of the Kyuafile to src/lib/atf/tests/Makefile. But if you do that, you cannot get the automatic generation to work because that tests/ directory does not have any children (and thus the automatic generation code has no idea about what to do). 2. Manually set TESTS_SUBDIR in src/lib/atf/Makefile to SUBDIR... which would be suboptimal because this is something that could be done automatically (this patch). 3. Make src/lib/atf/tests/Makefile install a generic Kyuafile that discovers its directories, just like /usr/tests/Kyuafile and /usr/tests/lib/Kyuafile do. This is a reasonable optional and would follow your comments below of keeping stuff in tests/ > 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; check them out. Still open is the question if we should do the same for src/lib/atf/Makefile et. al. I suspect the answer is yes, but then there is the issue mentioned above of the Kyuafile auto-generation. (Haven't done it yet as I don't have the time now and would appreciate your input.) > 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. Note: I have applied some of your suggestions to the existing patches but some things still remain open so don't bother doing an in-depth review again because they are broken. However, I gotta go now and I'd prefer to send this out for further comments while I have your attention ;) -- Julio Merino / @jmmv