Plugging ATF tests into the build and other cleanups

[Julio Merino]

On Tue, Oct 29, 2013 at 4:36 PM, Simon J. Gerraty <sjg at 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"?  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