configure.ac: write build/make/Makefile within an AC_CONFIG_FILE, not during main configure

[mkoeppe]

build/make/Makefile should be written by config.status within an AC_CONFIG_FILE template, not at configure time.

Depends on #24729
Depends on #24703

Component: build: configure

Author: Erik Bray

Branch: 2a99185

Reviewer: Julian Rüth

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

[jdemeyer]

The title "Get rid of file descriptor 7 hack" is different from the description "build/make/Makefile should be written within an AC_CONFIG_COMMANDS procedure". Do you want to fix both things here? Can you please clarify?

And honestly, I think there is nothing wrong with the "file descriptor 7 hack". autoconf itself uses similar hacks internally.

[mkoeppe]

comment:2

I've updated the title

[mkoeppe]

comment:3

I agree, there's nothing wrong with using a file descriptor to write to a file ;)

[mkoeppe]

Dependencies: #21479

[embray]

comment:6

Does it necessarily need to use AC_CONFIG_COMMANDS? Most of the stuff that writes build/make/Makefile could just as easily be moved into its own macro in a separate file in the m4/ directory.

As I see it the main goal here is to simplify the configure.ac by moving more functionality into separate files. But moving the existing code into an m4 macro would take less work, and has the advantage that it would have full access to any other variables set in other parts of the configure script, without having to explicitly pass them on to an external command.

[embray]

comment:7

On second thought, I can see why this way would be better, as it's more in spirit with how configure is supposed to work w.r.t. generating files. I'll have to give that some more thought. Maybe there aren't so many variables that need to be passed in.

[embray]

Branch: u/embray/build/makefile-in

[embray]

Commit: db428a3

[embray]

Author: Erik Bray

[embray]

comment:8

Here's my proposed solution to this (I updated the ticket to mention AC_CONFIG_FILE since that's what this uses).

This adds a build/make/Makefile.in template that is used to generated build/make/Makefile in config.status. The configure script now just collects values for some variables that are inserted into the Makefile template. I tried to keep as much actual Makefile syntax out of the output variables as possible, sticking mostly to just lists of package names that are operated over. Most of the rules for building packages are generated by the Makefile itself in $(eval ...) loops. The end result I think is actually cleaner and easier to understand.

If any of this should be controversial, it may be the use of some more advanced GNU make constructs, the portability of which I'm not sure. This was tested with GNU make 3.81 (about 11 years old), and 4.2.1 (very recent). That said, it's using some relatively advanced GNU make features that may not be available in other makes. I think it's still better, but if need be more of the Makefile generation could be pushed back into configure, but still provided via output variables, instead of writing directly to a file.

Increased the priority since I have other build system work that would likely depend on this, and also because it makes some non-trivial performance improvements, especially on Cygwin.

---

New commits:

b034b3a	Fix minor bug in the conway_polynomials dependencies; it should just list the bare package name
f88d3cb	Use AC_CONFIG_FILES + AC_OUTPUT to generate build/make/Makefile
db428a3	Further rewrite of configure.ac to do away with the multiple

[embray]

comment:10

Weird that the patchbot is failing due to Numpy of all things. I wonder if it has something more to do with the fact that it's building in a temp dir (since this is an "unsafe" ticket) than anything to do with the patch itself, but I'll have to see if I can reproduce...

[numpy-1.12.1] Traceback (most recent call last):
[numpy-1.12.1]   File "<stdin>", line 3, in <module>
[numpy-1.12.1]   File "/tmp/tmp4YcJD2-sage-git-temp-21524/local/lib/python2.7/site-packages/numpy/__init__.py", line 142, in <module>
[numpy-1.12.1]     from . import add_newdocs
[numpy-1.12.1]   File "/tmp/tmp4YcJD2-sage-git-temp-21524/local/lib/python2.7/site-packages/numpy/add_newdocs.py", line 13, in <module>
[numpy-1.12.1]     from numpy.lib import add_newdoc
[numpy-1.12.1]   File "/tmp/tmp4YcJD2-sage-git-temp-21524/local/lib/python2.7/site-packages/numpy/lib/__init__.py", line 18, in <module>
[numpy-1.12.1]     from .polynomial import *
[numpy-1.12.1]   File "/tmp/tmp4YcJD2-sage-git-temp-21524/local/lib/python2.7/site-packages/numpy/lib/polynomial.py", line 20, in <module>
[numpy-1.12.1]     from numpy.linalg import eigvals, lstsq, inv
[numpy-1.12.1]   File "/tmp/tmp4YcJD2-sage-git-temp-21524/local/lib/python2.7/site-packages/numpy/linalg/__init__.py", line 51, in <module>
[numpy-1.12.1]     from .linalg import *
[numpy-1.12.1]   File "/tmp/tmp4YcJD2-sage-git-temp-21524/local/lib/python2.7/site-packages/numpy/linalg/linalg.py", line 29, in <module>
[numpy-1.12.1]     from numpy.linalg import lapack_lite, _umath_linalg
[numpy-1.12.1] ImportError: /tmp/tmp4YcJD2-sage-git-temp-21524/local/lib/python2.7/site-packages/numpy/linalg/lapack_lite.so: undefined symbol: zungqr_

[saraedum]

Work Issues: merge conflicts

[sagetrac-git]

Changed commit from bb9ff43 to 2a99185

[saraedum]

comment:58

Ok. Your recent changes look good to me.

[vbraun]

Changed branch from u/embray/build/makefile-in to 2a99185

[jhpalmieri]

Changed commit from 2a99185 to none

[jhpalmieri]

comment:60

This completely breaks upgrading Sage, it seems to me. On three different OS X machines with 8.2.beta7 installed, upgrading to 8.2.beta8, or in particular just using this branch, results in a bad build/make/Makefile: lines like

deps_4ti2 =  $(SAGE_LOCAL)/bin/gcczlib $(MP_LIBRARY) glpk
deps_alabaster =  $(SAGE_LOCAL)/bin/gcc$(PYTHON) | pip
deps_appnope =  $(SAGE_LOCAL)/bin/gcc$(PYTHON) | pip

Please fix this.

[embray]

comment:61

Indeed, it looks like a bug introduced upon the last rebase. I didn't catch it because it only affects if you are using Sage's gcc spkg.

[embray]

comment:62

Actually it appears the problem might come from #24703 in the first place, since it seems to misplace the space in the GCC_DEP variable...

[embray]

comment:63

Replying to @embray:

Actually it appears the problem might come from #24703 in the first place, since it seems to misplace the space in the GCC_DEP variable...

I take that back--the missing space is in my code.

[jdemeyer]

comment:65

You cannot just re-open a ticket. Only the release manager should do that.

[jdemeyer]

comment:66

Especially this ticket must not be re-opened since it has already been released. Just open a new ticket to fix it.

[embray]

comment:67

It's not in the latest beta yet.

[embray]

comment:68

Oops, apparently it is. If I had thought otherwise I wouldn't have reopened. Strange--I just checked too any I didn't see it.

[embray]

comment:69

Followup in #24961.

[jhpalmieri]

comment:70

Thanks for the quick diagnosis, by the way!

[embray]

comment:71

Replying to @jhpalmieri:

Thanks for the quick diagnosis, by the way!

No, thank you for pointing it out.

[jdemeyer]

comment:72

Why is this making changes to build/pkgs/gcc/spkg-install? That might have broken building Sage from scratch if GCC is built.

[jdemeyer]

comment:73

This might have broken $(SAGERUNTIME): #24995

[embray]

comment:74

Replying to @jdemeyer:

Why is this making changes to build/pkgs/gcc/spkg-install? That might have broken building Sage from scratch if GCC is built.

I just noticed this comment. That was not intentional. Probably a mis-resolved conflict (though I don't know why since I never made any edits to that file in the first place).

[jdemeyer]

comment:75

DUMMY_PACKAGES seems broken: #25188

[jdemeyer]

comment:76

What was the purpose of this change:

@@ -58,11 +58,11 @@ all-sage: \
 # option to make forces all targets to be built unconditionally)
 download-for-sdist: base
        env SAGE_INSTALL_FETCH_ONLY=yes $(MAKE) -B SAGERUNTIME= \
-               $(SDIST_PACKAGES)
+               $(SDIST_PACKAGE_INSTS)
 

It has broken sdists, see #25319.