After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 692122 - [PATCH] Corrupt Makefile causing a build error in tests/telepathy with automake 1.13
[PATCH] Corrupt Makefile causing a build error in tests/telepathy with automa...
Status: RESOLVED FIXED
Product: folks
Classification: Platform
Component: general
git master
Other Linux
: Normal normal
: Unset
Assigned To: folks-maint
folks-maint
Depends on:
Blocks:
 
 
Reported: 2013-01-20 02:09 UTC by Nuno Araujo (IRC: russo79)
Modified: 2013-01-24 00:07 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Enable serial tests suite (1.02 KB, patch)
2013-01-20 02:14 UTC, Nuno Araujo (IRC: russo79)
rejected Details | Review
Don't use make functions for defining TESTS (999 bytes, patch)
2013-01-21 22:55 UTC, Nuno Araujo (IRC: russo79)
accepted-commit_now Details | Review
Don't use make functions for defining TESTS (1.83 KB, patch)
2013-01-23 21:50 UTC, Nuno Araujo (IRC: russo79)
none Details | Review
Don't use make functions for defining TESTS (1.94 KB, patch)
2013-01-23 22:06 UTC, Nuno Araujo (IRC: russo79)
committed Details | Review

Description Nuno Araujo (IRC: russo79) 2013-01-20 02:09:41 UTC
Since automake 1.13 is out, the parallel testsuite harness (previously only enabled by the 'parallel-tests' option) is now the default. [1]

Build stops with an error.

The following patch enables the serial testsuite harness.

[1]http://lists.gnu.org/archive/html/automake/2012-12/msg00038.html
Comment 1 Nuno Araujo (IRC: russo79) 2013-01-20 02:14:36 UTC
Created attachment 233926 [details] [review]
Enable serial tests suite

For the moment folks tests can not be run with the parallel testsuite harness[1].

[1]https://bugzilla.gnome.org/show_bug.cgi?id=679826
Comment 2 Philip Withnall 2013-01-20 21:58:42 UTC
Review of attachment 233926 [details] [review]:

The correct fix would be to ensure that the tests _do_ work correctly with parallel-tests enabled. The first step towards doing that would be to provide the error message you get when running the tests.
Comment 3 Nuno Araujo (IRC: russo79) 2013-01-21 22:55:57 UTC
Created attachment 234055 [details] [review]
Don't use make functions for defining TESTS

> The correct fix would be to ensure that the tests _do_ work correctly with
> parallel-tests enabled. The first step towards doing that would be to provide
> the error message you get when running the tests.

Yes, you are right about that. It's just that I had spent all the day jhbuilding a full gnome stack and fixing automake 1.13 errors. I didn't took the time to make a deep diagnosis of the problem.

But it's done now :-)

The cause was the usage of make functions for defining TESTS in tests/telepathy/Makefile.am

Usage of 'make' functions as $(filter-out) in the definition of TESTS special variable can lead to the creation of a corrupt Makefile.in, thus a corrupt Makefile.

The value of TESTS is parsed and transformed by automake before the execution of the 'make' functions.

As an example, automake tries to append the $(EXEEXT) variable to each of the test programs.

For tests/telepathy/Makefile.am, the Makefile.in is as follows:

TESTS = $(filter-out fake-tp-backend$(EXEEXT) \
        individual-zeitgeist,$(noinst_PROGRAMS))

This works accidentaly because $(EXEEXT) is empty in this case.

With the release of automake 1.13, the parallel testsuite harness is now the default.
This testsuite harness tries to create .log targets for each of the test programs.

This leads to the following Makefile.in:

$(filter-out.log: $(filter-out
	@p='$(filter-out'; \
	b='$(filter-out'; \
	$(am__check_pre) $(LOG_DRIVER) --test-name "$$f" \
	--log-file $$b.log --trs-file $$b.trs \
	$(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \
	"$$tst" $(AM_TESTS_FD_REDIRECT)

Building folks with this version of automake, gives the following error:

make[3]: Entering directory `/home/nuno/work/gnome/code/folks/tests/telepathy'
Makefile:1087: *** unterminated variable reference.  Stop.


Attached is a new patch that fixes the problem by manually expanding the list of tests to be executed.

Note, this goal of this patch is not to fix any failing test, just to allow folks to be built correctly when using the parallel testsuite harness.
Comment 4 Philip Withnall 2013-01-21 23:55:19 UTC
Review of attachment 234055 [details] [review]:

Thanks for investigating this so thoroughly (and thanks for building the entire jhbuild stack; it’s a thankless but really useful task). If you make the change noted below and add a suitable NEWS entry, you can commit this to master. Thanks again.

::: tests/telepathy/Makefile.am
@@ +69,3 @@
+	individual-properties \
+	init \
+	$(NULL)

It would be slightly more maintainable if you also changed noinst_PROGRAMS to reference $(TESTS) rather than listing the tests twice in the Makefile.
Comment 5 Nuno Araujo (IRC: russo79) 2013-01-22 00:12:35 UTC
No problem with the investigation. When you really want to learn you got to dig deep! :-) If we only always had the time...

OK for the NEWS entry and for the noinst_PROGRAMS.

But I don't see what you want me to change in tests/telepathy/Makefile.am.
Those lines are already in it. Or should I remove the other entries?
Comment 6 Philip Withnall 2013-01-22 19:39:56 UTC
(In reply to comment #5)
> But I don't see what you want me to change in tests/telepathy/Makefile.am.
> Those lines are already in it. Or should I remove the other entries?

Change noinst_PROGRAMS so that it doesn’t explicitly list the same programs as TESTS; and instead just lists $(TESTS).

i.e.

noinst_PROGRAMS = \
	$(TESTS) \
	fake-tp-backend \
	individual-zeitgeist \
	$(NULL)
Comment 7 Nuno Araujo (IRC: russo79) 2013-01-23 21:50:01 UTC
Created attachment 234255 [details] [review]
Don't use make functions for defining TESTS

Patch adapted to take previous comments into account.

Just one note, I do not have commit access to git.gnome.org, so it would be nice if you could commit the patch.
Comment 8 Nuno Araujo (IRC: russo79) 2013-01-23 22:06:10 UTC
Created attachment 234256 [details] [review]
Don't use make functions for defining TESTS

Oups, forgot to put TESTS before noinst_PROGRAMS in previous version of patch.

This is the good one :-)

Sorry for the noise.
Comment 9 Philip Withnall 2013-01-24 00:07:22 UTC
Review of attachment 234256 [details] [review]:

Committed to master, thanks!

commit 5ffdeac6899925871da0d32aa5643fed5c2cb954
Author: Nuno Araujo <nuno.araujo@russo79.com>
Date:   Mon Jan 21 13:00:41 2013 +0100

    build: Don't use make functions for defining TESTS
    
    Usage of 'make' functions in the definition of TESTS special variable
    can lead to the creation of a corrupt Makefile.in, thus a corrupt Makefile.
    
    Closes: https://bugzilla.gnome.org/show_bug.cgi?id=692122

 NEWS                        |  1 +
 tests/telepathy/Makefile.am | 22 ++++++++++++----------
 2 files changed, 13 insertions(+), 10 deletions(-)