Remove build/: two dead developer setups, and the autotools helpers moved to m4/ and build-aux/ - #78
Merged
Merged
Conversation
A 2013 developer script that cloned memtailor, mathic and mathicgb and built them against each other in several configurations. Its last substantive commit was 93d8985, on 2013-09-24, and it is broken three ways over: - It clones from https://github.com/broune/{memtailor,mathic,mathicgb}.git, the pre-Macaulay2 URLs. - It downloads gtest 1.6.0 from http://googletest.googlecode.com/files/. Google Code shut down in 2016, so the gtest target -- which every other target depends on -- cannot run at all. - It defines MEMTAILOR_DEBUG, which is not a macro. memtailor's is MEMT_DEBUG, so the assert-enabled targets never enabled memtailor's asserts even back when the script worked. Neither build system refers to it. doc/description.txt did, in three places, all updated here: the recipe for setting up several configurations at once is gone with the script, and the two project ideas that described the makefile it wrote no longer point at it.
Visual Studio 2012 project files. The last real change to them is from 2013-10-03; the two commits since are a file-permission fix and the CRLF normalization in f78d6be. Staleness is not the decisive part -- they cannot work. stdinc.h has required C++17 since PR #60 and VS2012 does not do complete C++11, so any build stops at #error "mathicgb requires C++17 or later" Independently, the file list has been wrong since 2021: MESClassicGBAlg.cpp, added in e6996a3, is missing from mathicgb-lib.vcxproj. notes.txt is a 300-line write-up of clicking through the VS2012 GUI, down to building gtest 1.6 with _VARIADIC_MAX=10. Three things go with them. .gitattributes set "*.sln text eol=crlf", a rule that existed solely for build/vs12/mathicgb.sln; no .sln, .vcxproj or .filters file remains anywhere in the tree, so it now matches nothing. .gitignore had a block of seven MSVC artifact rules, one of which (output/) named vs12's build output directory specifically; MSVC is not a supported compiler, so the block goes with the project files it was written for. And doc/description.txt had an entire "Installation for Visual Studio" section, which documented these project files and asserted that "as of the time of this writing (October 3, 2013) the code compiles in MSVC" -- removed here along with its table of contents entry and the MSVC-only project idea that closed the section. Not removed: the #ifdef _MSC_VER branch in stdinc.h, which is the ordinary shape of a compiler-abstraction header and is paired with a __GNUC__ branch; Atomic.hpp's MSVC path; and cmake/FindTBB.cmake, which is vendored third-party code that carries its own MSVC logic. With this and the previous commit, build/ holds only autotools/.
configure.ac put the autoconf macro directory at build/autotools/m4 and the auxiliary build tools at build/autotools -- two levels of directory for what amounts to one tracked file. Everything else under there (install-sh, missing, depcomp, compile, ar-lib, config.guess, config.sub, ltmain.sh, test-driver and the five libtool .m4 files) is written by "autoreconf --install" and ignored; the only thing we keep in the tree is m4/ax_cxx_compile_stdcxx.m4, which configure.ac calls for the C++17 requirement. Use the layout that autoconf's own manual and gnulib default to: m4/ for the macros and build-aux/ for the auxiliary tools. With the previous two commits having emptied build/ of everything else, the directory is gone. .gitignore moves with them. The three /build/autotools/* entries give way to build-aux/.gitignore, which now also lists compile and test-driver -- those two were in the top-level file while their seven siblings were in the subdirectory's, for no reason anyone recorded. One of the three was wrong regardless: configure has generated mathicgb.pc at the top level since PR #72, not under build/autotools, so the built file has been showing up as untracked ever since. That entry is now /mathicgb.pc. Verified from a tree with build/ deleted: autogen.sh, configure and make all succeed and "make check" passes 246/246, with the working tree clean afterwards -- autoreconf regenerates every moved file into its new home and .gitignore covers all of them. "make dist" ships m4/ and build-aux/ just as it shipped build/autotools/ before.
ACLOCAL_AMFLAGS predates automake 1.13, which taught aclocal to trace
AC_CONFIG_MACRO_DIRS out of configure.ac and add those directories to its
own search path. With the plural macro in place the -I in Makefile.am is
the same directory written a second time, in a second file, where it can
fall out of step with the first.
Verified from a tree with aclocal.m4, configure and Makefile.in deleted:
aclocal still writes m4_include([m4/ax_cxx_compile_stdcxx.m4]) into
aclocal.m4, configure still expands the C++17 check ("checking whether g++
supports C++17 features by default... yes"), and make check passes
246/246.
One cosmetic regression: libtoolize now ends autogen.sh with "Consider
adding '-I m4' to ACLOCAL_AMFLAGS in Makefile.am." It looks for the flag
without checking whether AC_CONFIG_MACRO_DIRS already covers it -- the
line above it in the same output reads "putting macros in
AC_CONFIG_MACRO_DIRS, 'm4'", so it has the answer and does not use it.
The suggestion is wrong on libtool 2.4.7 and nothing acts on it.
d-torrance
added a commit
that referenced
this pull request
Sep 2, 2026
Atomic.hpp carried a hand-written implementation of atomic load and store
-- compiler barriers via __asm__ __volatile__ ("" ::: "memory"), CPU
fences via __sync_synchronize(), and a CAS loop on
__sync_bool_compare_and_swap. stdinc.h defined
MATHICGB_USE_CUSTOM_ATOMIC_X86_X64 in both the _MSC_VER and __GNUC__
branches, so on x86 and x64 that, and not std::atomic, is what
MonomialMap and FixedSizeMonomialMap have been using.
Its own comment gave the premise: "it is surprising that any compiler
ships with a std::atomic that is worse than this - but that is very much
the case." That was written in 2013 against GCC 4.x and it is no longer
true. Compiling both implementations at -O2 with GCC 13.3 on x86-64
and comparing what comes out:
order custom std::atomic
------------------------------------------------------------------
load relaxed movq movq
load acquire movq movq
load seq_cst movq movq
store relaxed movq movq
store release movq movq
store seq_cst movq; lock cmpxchgq; jne xchgq
Identical for every operation on the hot path, and on the one operation
where they differ the hand-written version is the worse of the two: a
compare-and-swap with a retry branch where std::atomic emits a single
locked exchange. So this is not a wash. It removes a slower
implementation.
That store is used once, at MonomialMap.hpp:188, on the map-growth path.
Everything in the traversal and insertion paths -- relaxed, consume and
release -- compiles to the same instructions either way, which is why
timing cannot see the change: interleaved runs of hyclic8-101-trimmed and
cyclic7 at one and four threads, nine reps each, land between -0.5% and
+4.4% with run-to-run deviation of 3-7%. Where the emitted code is the
same there is nothing to time.
Also gone with it: the AtomicInternal barrier helpers and seqCstStore,
which existed only to serve the custom class, and the class comment's
claim about MSVC 2012, which cannot apply to a project that has required
C++17 since PR #60 and whose VS2012 files were removed in PR #78.
MATHICGB_USE_FAKE_ATOMIC is untouched and still compiles. It selects a
no-synchronization implementation for measuring the overhead of the
atomic operations, which is a separate facility from the one removed
here.
Verified: cmake Release and Debug both pass 246/246, autotools make check
and make distcheck pass, a build with -DMATHICGB_USE_FAKE_ATOMIC still
compiles, and single-threaded output is byte-identical to the old code on
cyclic5, cyclic7, hyclic8-101-trimmed and hilbertkunz1. Multithreaded
output cannot be compared this way: the same binary produces a different
.gb on five consecutive cyclic7 runs, and identical output at
-threadCount 1.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
d-torrance
added a commit
that referenced
this pull request
Sep 19, 2026
MESClassicGBAlg.cpp and .hpp arrived in e6996a3, a bulk backport of 14 files from Macaulay2/M2#2160 "Refactor mathic". They have had no substantive change since, only the repo-wide whitespace pass. They were never built. Both are absent from Makefile.am and src/CMakeLists.txt, including EXTRA_DIST, so no release tarball has ever contained them, and build/vs12's project file omitted them too until PR #78 deleted it. Nothing in the tree refers to them. They cannot be built. MESClassicGBAlg.cpp includes ClassicGBAlg.hpp, not its own header, and defines mgb::ClassicGBAlg -- the same class ClassicGBAlg.cpp defines. Adding it to a build is a duplicate symbol. Its own header declares something different again, mgbF4, which the .cpp never includes. There is nothing in them to keep. The algorithm is identical: computeGrobnerBasis matches outright, and step and insertReducedPoly differ only in spelling MATHICGB_ASSERT as plain assert. Every other difference is a removal -- the statistics block that ClassicGBAlg builds with 51 ColumnPrinter calls is gone, printMemoryUse is an empty body reading "TODO: bring over from mathicgb or rewrite", and LogDomain is dropped. 226 fewer lines, all of them things taken out. What it was is a port in progress: the file strips this project's own macros so that it compiles inside Macaulay2's engine instead, which its compile-command names as $M2BUILDDIR/Macaulay2/e. Macaulay2 does not have it today either -- the name appears nowhere in that repository. Deleting it closes the "in two files" half of review item 22, and removes the second copy of the timing label item 20 fixed and a third site of the ignored queueType parameter in item 27. It also stops every grep for a bug in ClassicGBAlg.cpp returning two answers, one of which compiles nowhere, which is how item 22 came to be written as it was. Recoverable from history if anyone wants it back. Verified: 253/253 in cmake Release and Debug, no build warnings, which is unsurprising since nothing compiled these files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
build/had three subdirectories. Two of them cannot work and are removed here;the third holds one tracked file, and moves to where autotools projects
conventionally keep it. The directory itself is gone afterwards.
build/setup/make-Makefile.sh— 4967c4eA 2013 developer script that cloned memtailor, mathic and mathicgb and built them
against each other in several configurations. Last substantive commit 93d8985,
2013-09-24. Broken three ways over:
https://github.com/broune/{memtailor,mathic,mathicgb}.git, thepre-Macaulay2 URLs;
http://googletest.googlecode.com/files/, andGoogle Code shut down in 2016 — every other target in the makefile it writes
depends on the
gtesttarget, so none of them can run;MEMTAILOR_DEBUG, which is not a macro. memtailor's isMEMT_DEBUG, as PR Don't define memtailor's and mathic's debug macros #68 established, so its assert-enabled configurations neverenabled memtailor's asserts even back when the script worked.
doc/description.txtreferred to it in three places, all updated: the recipe forsetting up several configurations at once goes with the script, and the two
project ideas that described the makefile it wrote no longer point at it.
build/vs12— b254bfaVisual Studio 2012 project files. Last real change 2013-10-03; the two commits
since are a file-permission fix and the CRLF normalization in f78d6be.
Staleness is not the decisive part — they cannot work.
stdinc.hhas requiredC++17 since PR #60 and VS2012 does not do complete C++11, so any build stops at
#error "mathicgb requires C++17 or later". Independently the file list has beenwrong since 2021:
MESClassicGBAlg.cpp, added in e6996a3, is missing frommathicgb-lib.vcxproj.Three things go with them, all of which existed only for these files: the
*.sln text eol=crlfrule in.gitattributes(no.sln,.vcxprojor.filtersfile remains anywhere in the tree); the seven MSVC artifact rules in.gitignore, one of which named vs12'soutput/directory specifically; anddoc/description.txt's entire "Installation for Visual Studio" section, whichasserted that "as of the time of this writing (October 3, 2013) the code compiles
in MSVC".
Deliberately kept: the
#ifdef _MSC_VERbranch instdinc.h, which is theordinary shape of a compiler-abstraction header and is paired with a
__GNUC__branch;
Atomic.hpp's MSVC path; andcmake/FindTBB.cmake, which is vendoredthird-party code carrying its own MSVC logic.
build/autotools→m4/andbuild-aux/— cd80f1d, 25735e1Two levels of directory for one tracked file. Everything else under there —
install-sh,missing,depcomp,compile,ar-lib,config.guess,config.sub,ltmain.sh,test-driverand the five libtool.m4files — iswritten by
autoreconf --installand ignored; the only thing kept in the tree isax_cxx_compile_stdcxx.m4, whichconfigure.accalls for the C++17 requirement.This uses the layout autoconf's own manual and gnulib default to:
m4/for themacros,
build-aux/for the auxiliary tools.The ignore rules are consolidated with the move.
compileandtest-driverwerelisted in the top-level
.gitignorewhile their seven siblings were in thesubdirectory's; they are together in
build-aux/.gitignorenow. One of the threetop-level entries was wrong regardless:
configurehas generatedmathicgb.pcatthe top level since PR #72, not under
build/autotools, so the built file hasbeen showing up as untracked ever since. That entry is now
/mathicgb.pc.The second commit drops
ACLOCAL_AMFLAGSfromMakefile.amin favour ofAC_CONFIG_MACRO_DIRS, which aclocal has traced out ofconfigure.acsinceautomake 1.13 — otherwise the same directory is written twice, in two files, where
the copies can fall out of step.
One cosmetic regression from that: libtoolize now ends every
autogen.shrun withConsider adding '-I m4' to ACLOCAL_AMFLAGS in Makefile.am.It checks for theflag without checking whether
AC_CONFIG_MACRO_DIRSalready covers it — the lineabove it in the same output reads
putting macros in AC_CONFIG_MACRO_DIRS, 'm4'—so the suggestion is wrong on libtool 2.4.7, but it is new noise. Happy to keep
ACLOCAL_AMFLAGSand drop that commit if the nag is worse than the duplication.Verification
Both build systems, from a tree with
build/deleted, on GCC 13.3 / TBB 2021.11.0:autogen.sh,configure,make distcheck—PASS: unittest,archive built. distcheck unpacks the tarball and builds out of tree against a
read-only srcdir, so it exercises the moved
m4/andbuild-aux/from scratch;they ship in the tarball exactly as
build/autotools/did.Multithreading with TBB: ON, build clean,src/mathicgb-unit-testspasses 246/246 in 29 suites.git statusis clean after both, soautoreconfregenerates every moved fileinto its new home and the ignore rules cover all of them.
configurestillexpands the C++17 check through the relocated macro:
checking whether g++ supports C++17 features by default... yes.🤖 Generated with Claude Code