Cleanup spkg-configure.m4 files that mix tabs and spaces
Closed this issue · 25 comments
The spkg-configure files for the following packages
mix tabs and spaces:
arb brial cmake flint gcc gfortran git gmp iml
ninja_build pari patch pcre ppl python3 symmetrica.
This ticket is to make them use spaces only.
Note however discussion at #13899 warns against
changing whitespace in a lot of files at once.
CC: @dimpase @embray @orlitzky @mkoeppe @slel
Component: build: configure
Author: Frédéric Chapoton, Michael Orlitzky
Branch/Commit: f9e58da
Reviewer: Michael Orlitzky, Dima Pasechnik
Issue created by migration from https://trac.sagemath.org/ticket/31528
Sage development has entered the release candidate phase for 9.3. Setting a new milestone for this ticket based on a cursory review of ticket status, priority, and last modification date.
Previous related tickets:
- #15436: Cksum uses tabs instead of spaces, breaking sage-spkg's regex
- #13899: Don't use TAB characters for indentation
- #13146: Removing tabs in .rst, .tex and .pxi files
This shell command spots files with tabs
when run from the Sage root folder:
$ git grep $'\t' . | cut -f 1 -d ":" | uniq \
| grep -v Make | grep -v -e '^Binary' | grep -v patchesIn Sage 9.3 it reveals:
bootstrap
build/bin/write-dockerfile.sh
build/pkgs/_prereq/distros/freebsd.txt
build/pkgs/arb/spkg-configure.m4
build/pkgs/brial/spkg-configure.m4
build/pkgs/bzip2/spkg-configure.m4
build/pkgs/cmake/spkg-configure.m4
build/pkgs/cvxopt/spkg-install.in
build/pkgs/ecm/spkg-install.in
build/pkgs/flint/spkg-configure.m4
build/pkgs/gcc/spkg-configure.m4
build/pkgs/gfortran/spkg-configure.m4
build/pkgs/git/spkg-configure.m4
build/pkgs/gmp/spkg-configure.m4
build/pkgs/iml/spkg-configure.m4
build/pkgs/ninja_build/spkg-configure.m4
build/pkgs/pari/spkg-configure.m4
build/pkgs/patch/spkg-configure.m4
build/pkgs/pcre/spkg-configure.m4
build/pkgs/ppl/spkg-configure.m4
build/pkgs/python3/spkg-configure.m4
build/pkgs/symmetrica/spkg-configure.m4
m4/ax_absolute_header.m4
m4/ax_compiler_vendor.m4
m4/ax_gcc_option.m4
m4/ax_gxx_option.m4
m4/ax_prog_perl_modules.m4
src/.relint.yml
src/doc/en/developer/portability_testing.rst
src/doc/en/reference/parallel/media/map_reduce_arch.fig
src/sage/ext_data/graphs/graph_plot_js.html
src/sage/ext_data/magma/latex/latex.m
src/sage/ext_data/pari/simon/ell.gp
src/sage/graphs/cliquer/cl.c
src/sage/groups/perm_gps/partn_ref2/refinement_generic.h
src/sage/modular/arithgroup/farey.cpp
src/sage/modular/arithgroup/farey.hpp
src/sage/modular/arithgroup/sl2z.hpp
src/sage/rings/polynomial/weil/power_sums.c
src/sage/rings/polynomial/weil/power_sums.h
Description changed:
---
+++
@@ -1,5 +1,10 @@
The spkg-configure files for the following packages
mix tabs and spaces:
-arb brial cmake flint gcc gfortran git gmp iml ninja_build pari patch pcre ppl python3 symmetrica.
+arb brial cmake flint gcc gfortran git gmp iml
+ninja_build pari patch pcre ppl python3 symmetrica.
This ticket is to make them use spaces only.
+
+Note however discussion at #13899 warns against
+changing whitespace in a lot of files at once.
+Branch: u/chapoton/31528
Author: Frédéric Chapoton
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
2de538d | trac 32401 change one doctest for acosh |
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
30988c7 | no tab in spkg-configure |
Thanks Frédéric for this branch.
Would someone who knows about spkg-configure.m4
files give the green light on this?
The m4 and C-language changes all look fine to me at first glance. I'm probably responsible for a few of the tabs in spkg-configure.m4, but I do indeed prefer spaces when I'm paying attention.
Are magma and GP whitespace-insensitive too?
Are magma and GP whitespace-insensitive too?
I think that gp does not care. No idea about magma, but it certainly does not require TABs.
I took a closer look and added two commits to fix the indentation where it was weird. There are still others, but I don't have time to go through and fix them all. They're not new issues.
The GP change is OK too. I have no way to run the magma code, but since there are already spaces in that file, I guess it's unlikely that changing some existing tabs into more spaces will cause a problem.
If someone can double-check my two new commits (which should contain only whitespace changes), please set to positive review after.
Changed branch from u/chapoton/31528 to u/mjo/ticket/31528
Changed author from Frédéric Chapoton to Frédéric Chapoton, Michael Orlitzky
Reviewer: Michael Orlitzky
Replying to @orlitzky:
If someone can double-check my two new commits
(which should contain only whitespace changes),
please set to positive review after.
In build/pkgs/iml/spkg-configure.m4, besides
fixing indentation, you remove two lines.
Is that intended?
Replying to @slel:
In
build/pkgs/iml/spkg-configure.m4, besides
fixing indentation, you remove two lines.Is that intended?
The "dnl" lines? Yeah, those were comments, believe it or not.
Changed reviewer from Michael Orlitzky to Michael Orlitzky, Dima Pasechnik
lgtm
Setting to critical as this conditions work on
spkg-configure files in other tickets.
Changed branch from u/mjo/ticket/31528 to f9e58da