FLINT 2.4.5's test suite fails unconditionally (t-NTL-interface)

[kiwifb]

FLINT 2.4.5 (and 2.4.4 at least) unconditionally fail its test suite when building the NTL interface test due to a bug in the Makefile:

/make[4]: Entering directory
'/home/laurent/sage-6.6-src/sage-6.6/local/var/tmp/sage/build/flint-2.4.5/src'
   CXX   build/interfaces/test/t-NTL-interface
g++: error: build/interfaces/NTL-interface.o: No such file or directory
Makefile:248: recipe for target 'build/interfaces/test/t-NTL-interface'
failed
make[4]: *** [build/interfaces/test/t-NTL-interface] Error 1
make[4]: Leaving directory
'/home/laurent/sage-6.6-src/sage-6.6/local/var/tmp/sage/build/flint-2.4.5/src'
/bin/sh: 3: build/interfaces/test/t-NTL-interface: not found
Makefile:182: recipe for target 'check' failed
make[3]: *** [check] Error 127
make[3]: Leaving directory
'/home/laurent/sage-6.6-src/sage-6.6/local/var/tmp/sage/build/flint-2.4.5/src'
Error: FLINT failed to pass its test suite.

sage-on-gentoo and now Gentoo proper have carried a patch for the issue for some time now. See
https://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/sci-mathematics/flint/files/flint-2.4.4-test.patch?view=markup

Upstream: Fixed upstream, but not in a stable release.

Component: packages: standard

Keywords: NTL-interface.o check

Author: François Bissey

Branch: 5cdb5f9

Reviewer: Volker Braun

Issue created by migration from https://trac.sagemath.org/ticket/18424

[kiwifb]

comment:1

I will create a branch with the appropriate patch in a couple of hours.

[kiwifb]

New commits:

5cdb5f9

fix building of flint's ntl-interface test.

[kiwifb]

Branch: u/fbissey/flint-test

[kiwifb]

Author: François Bissey

[kiwifb]

Commit: 5cdb5f9

[nexttime]

comment:4

The proper fix is IMHO not to rename the file, but to add a dependency:

--- flint-2.4.5/Makefile.in	2015-02-17 15:56:55.000000000 +0100
+++ flint-2.4.5/Makefile.in	2015-05-15 15:41:47.203690000 +0200
@@ -211,7 +211,7 @@
 build/interfaces/NTL-interface.o: interfaces/NTL-interface.cpp NTL-interface.h
 	$(QUIET_CXX) $(CXX) $(CFLAGS) $(INCS) -c $< -o $@;
 
-build/interfaces/test/t-NTL-interface$(EXEEXT): interfaces/test/t-NTL-interface.cpp
+build/interfaces/test/t-NTL-interface$(EXEEXT): interfaces/test/t-NTL-interface.cpp build/interfaces/NTL-interface.o
 	$(QUIET_CXX) $(CXX) $(CFLAGS) $(INCS) $< build/interfaces/NTL-interface.o -o $@ $(LIBS);
 
 print-%:

Nice Makefile btw. ... B-)

[nexttime]

comment:5

Pretty embarrassing this already went into a stable Sage release, apparently without anybody even noticing.

[nexttime]

Changed keywords from none to NTL-interface.o check

[JohnCremona]

comment:7

Reported upstream yet?

[nexttime]

comment:8

Replying to @JohnCremona:

Reported upstream yet?

No idea; I can't imagine they haven't meanwhile noticed themselves... (although not everybody builds the NTL interface)

[nexttime]

comment:9

Looks like the bug was still there:

https://github.com/wbhart/flint2/blob/trunk/Makefile.in#L234

[nexttime]

comment:10

Replying to @nexttime:

Looks like the bug was still there:

https://github.com/wbhart/flint2/blob/trunk/Makefile.in#L234

Sorry, misread that. They just renamed the file, but didn't add a dependency, so it's half-fixed upstream.

[nexttime]

Upstream: Fixed upstream, but not in a stable release.

[nexttime]

comment:11

There's no corresponding issue on github I could comment on, just the commit by the author of the Gentoo patch.

[kiwifb]

comment:12

Actually the patch is originally mine. The name on the patch is the guy who committed it to the main tree. I found my original bug report for it with some explanations.
https://bugs.gentoo.org/show_bug.cgi?id=516028

I didn't report it upstream because I couldn't reproduce it from a plain upstream tarball. By default upstream builds static stuff and if you do that there is no problem. Originally it wasn't present in sage either, I'd have to check but at some point we must have stopped building static libraries or objects in flint. It only happens when you build shared libraries exclusively.

[nexttime]

comment:13

Replying to @kiwifb:

I didn't report it upstream because I couldn't reproduce it from a plain upstream tarball. By default upstream builds static stuff and if you do that there is no problem. Originally it wasn't present in sage either, I'd have to check but at some point we must have stopped building static libraries or objects in flint. It only happens when you build shared libraries exclusively.

commit 6f2750ed7bfb5caf1be3b47ead442e402b667db4
Author: Volker Braun <vbraun.name@gmail.com>
Date:   Thu Oct 2 15:18:09 2014 +0100

    Never build static library

Who else...

I'll polish my patch a bit since t-NTL-interface$(EXEEXT) should also depend on $(LIBS) (and the trailing semicolon doesn't make sense either), then submit it upstream I think.

[nexttime]

comment:14

Replying to @nexttime:

I'll polish my patch a bit since t-NTL-interface$(EXEEXT) should also depend on $(LIBS) (and the trailing semicolon doesn't make sense either), then submit it upstream I think.

Ah, no, $(LIBS) contains chains of -L/path/to/foo -lfoo, so it should -- if at all explicitly -- depend on library. (But t-NTL-interface is made in a rule which in turn already depends on library, namely check, so we don't really have to add it.)

[vbraun]

Reviewer: Volker Braun

[vbraun]

Changed branch from u/fbissey/flint-test to 5cdb5f9

[nexttime]

comment:17

FWIW, a proper fix is now in trunk, so we can remove the patch on the next upgrade.

[nexttime]

Changed commit from 5cdb5f9 to none