Date: Tue, 29 Oct 2013 22:48:17 -0400 From: Julio Merino <julio@meroh.net> To: "Simon J. Gerraty" <sjg@juniper.net> Cc: freebsd-testing@freebsd.org, Rui Paulo <rpaulo@fnop.net> Subject: Re: Plugging ATF tests into the build and other cleanups Message-ID: <CADyfeQWobDRogS3-dM9bw=a7TDZAU8AX6e7UqQUnteZ-QatxHA@mail.gmail.com> In-Reply-To: <20131029203658.75BF35807E@chaos.jnpr.net> References: <CADyfeQU7Y8APwTMDo9aTR2NUi2EBq0ytQ3QcF7Ct3xC7_BatBQ@mail.gmail.com> <20131029203658.75BF35807E@chaos.jnpr.net>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <sjg@juniper.net> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CADyfeQWobDRogS3-dM9bw=a7TDZAU8AX6e7UqQUnteZ-QatxHA>