Hi Dan,
On Tue, Feb 7, 2012 at 1:44 AM, Dan McGee <dan@xxxxxxxxxxxxx> wrote:
> On Sat, Feb 4, 2012 at 10:01 PM, Lucas De Marchi
> <lucas.demarchi@xxxxxxxxxxxxxx> wrote:
>> Hi Dan,
>>
>> On Sat, Feb 4, 2012 at 12:29 AM, Dan McGee <dan@xxxxxxxxxxxxx> wrote:
>>> Ensure this dependency is explicit in the Makefile so rebuilding just
>>> one test works correctly. Also reduce some repetition in the test LDADD
>>
>> What exactly are you fixing here?
>
> Just because running `make check` works doesn't mean the build has
> correct dependencies stated:
>
> $ make testsuite/test-init
> (CDPATH="${ZSH_VERSION+.}:" && cd . && /bin/sh
> /home/dmcgee/projects/kmod/build-aux/missing --run autoheader)
> rm -f stamp-h1
> touch config.h.in
> cd . && /bin/sh ./config.status config.h
> config.status: creating config.h
> config.status: config.h is unchanged
> CC testsuite/testsuite_test_init-test-init.o
> CC testsuite/testsuite_libtestsuite_la-testsuite.lo
> GEN testsuite/rootfs
> CCLD testsuite/libtestsuite.la
> CC libkmod/libkmod.lo
> CC libkmod/libkmod-list.lo
> CC libkmod/libkmod-config.lo
> CC libkmod/libkmod-index.lo
> CC libkmod/libkmod-module.lo
> CC libkmod/libkmod-file.lo
> CC libkmod/libkmod-elf.lo
> CC libkmod/libkmod-hash.lo
> CC libkmod/libkmod-array.lo
> CC libkmod/libkmod-util.lo
> CCLD libkmod/libkmod-util.la
> CCLD libkmod/libkmod-private.la
> CCLD testsuite/test-init
>
> $ ./testsuite/test-init
> TESTSUITE: running test_initlib, in forked context
> TESTSUITE: 'test_initlib' [13561] exited with return code 0
> TESTSUITE: PASSED: test_initlib
> TESTSUITE: running test_insert, in forked context
> ERROR: ld.so: object
> '/home/dmcgee/projects/kmod/testsuite/.libs/path.so' from LD_PRELOAD
> cannot be preloaded: ignored.
> ERROR: ld.so: object
> '/home/dmcgee/projects/kmod/testsuite/.libs/init_module.so' from
> LD_PRELOAD cannot be preloaded: ignored.
> TESTSUITE: ERR: could not create module from path: No such file or directory
> TESTSUITE: ERR: 'test_insert' [13562] exited with return code 1
> TESTSUITE: ERR: FAILED: test_insert
>
Ok, you convinced me. Sad that check_LTLIBRARIES are not compiled
automatically when you compile tests without using 'make check'.
>
>>> bits by adding a new TESTSUITE_LDADD variable.
>>>
>>> Signed-off-by: Dan McGee <dan@xxxxxxxxxxxxx>
>>> ---
>>> Makefile.am | 16 +++++++++-------
>>> 1 files changed, 9 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/Makefile.am b/Makefile.am
>>> index 69ab986..4c03dbb 100644
>>> --- a/Makefile.am
>>> +++ b/Makefile.am
>>> @@ -163,11 +163,13 @@ EXTRA_DIST += testsuite/rootfs.tar.xz
>>> TESTSUITE_CPPFLAGS = $(AM_CPPFLAGS) \
>>> -DTESTSUITE_ROOTFS=\"$(abs_top_builddir)/testsuite/rootfs/\" \
>>> -DABS_TOP_BUILDDIR=\"$(abs_top_builddir)\"
>>> +TESTSUITE_LDADD = testsuite/libtestsuite.la libkmod/libkmod-private.la
>>
>> not every test depends on these 2 convenience library. Since they are
>> only tests and we are garbage collecting unused symbols, maybe to ease
>> the inclusion of more tests we can indeed do what you did.
> For consistency and sanity, I would think it is wise to use the same
> convenience helper libraries in all cases, unless there is a real good
> reason not to. I'm not too concerned about adding a few KB or 0.001
> seconds to the test binary builds; I'm more concerned about making
> this much more approachable for an outsider to follow the standard
> pattern to add new tests.
>
>>
>>>
>>> check_LTLIBRARIES += testsuite/libtestsuite.la
>>> testsuite_libtestsuite_la_SOURCES = testsuite/testsuite.c \
>>> testsuite/testsuite.h
>>> -testsuite_libtestsuite_la_DEPENDENCIES = testsuite/rootfs
>>> +testsuite_libtestsuite_la_DEPENDENCIES = testsuite/rootfs \
>>> + $(TESTSUITE_OVERRIDE_LIBS)
>>
>> humn.... /me thinking. You are only serializing the build and telling
>> autofoo to rebuild libtestsuite.la if, for example, testsuite/path.c
>> changes. However testsuite/path.c will be a separate dynamic lib.
>> There's no need to rebuild libtestsuite if we change it.
> But does it really hurt to? See above commands that I set out to fix
> here- if you can make that work with some other addition of
> dependencies, let me know, but I was trying to do minimally invasive
> surgery on the Makefile. I'm open to any replacement for this patch
> you have that makes that case work.
No, I'm fine with your approach.
Patch has been merged. Thanks.
Lucas De Marchi
--
To unsubscribe from this list: send the line "unsubscribe linux-modules" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
