Opened 14 months ago
Closed 9 months ago
#31528 closed enhancement (fixed)
Cleanup spkg-configure.m4 files that mix tabs and spaces
Reported by: | slelievre | Owned by: | |
---|---|---|---|
Priority: | critical | Milestone: | sage-9.5 |
Component: | build: configure | Keywords: | |
Cc: | dimpase, embray, mjo, mkoeppe, slelievre | Merged in: | |
Authors: | Frédéric Chapoton, Michael Orlitzky | Reviewers: | Michael Orlitzky, Dima Pasechnik |
Report Upstream: | N/A | Work issues: | |
Branch: | f9e58da (Commits, GitHub, GitLab) | Commit: | f9e58dab2a38012cb0d684b63d5c7934b94963b5 |
Dependencies: | Stopgaps: |
Description (last modified by )
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.
Change History (15)
comment:1 Changed 14 months ago by
- Milestone changed from sage-9.3 to sage-9.4
comment:2 Changed 12 months ago by
- Description modified (diff)
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 patches
In 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
comment:3 Changed 10 months ago by
- Milestone changed from sage-9.4 to sage-9.5
comment:4 Changed 9 months ago by
- Branch set to u/chapoton/31528
- Commit set to 30988c777a5492aecf5198463aabe4a33b5bb602
- Status changed from new to needs_review
comment:5 Changed 9 months ago by
- Commit changed from 30988c777a5492aecf5198463aabe4a33b5bb602 to 2de538d920229e2c098cd1cbc7db1d54362d1348
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
2de538d | trac 32401 change one doctest for acosh
|
comment:6 Changed 9 months ago by
- Commit changed from 2de538d920229e2c098cd1cbc7db1d54362d1348 to 30988c777a5492aecf5198463aabe4a33b5bb602
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
30988c7 | no tab in spkg-configure
|
comment:7 Changed 9 months ago by
- Cc dimpase embray mjo mkoeppe added
Thanks Frédéric for this branch.
Would someone who knows about spkg-configure.m4 files give the green light on this?
comment:8 follow-up: ↓ 9 Changed 9 months ago by
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?
comment:9 in reply to: ↑ 8 Changed 9 months ago by
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.
comment:10 follow-up: ↓ 11 Changed 9 months ago by
- Branch changed from u/chapoton/31528 to u/mjo/ticket/31528
- Commit changed from 30988c777a5492aecf5198463aabe4a33b5bb602 to f9e58dab2a38012cb0d684b63d5c7934b94963b5
- Reviewers set to Michael Orlitzky
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.
comment:11 in reply to: ↑ 10 ; follow-up: ↓ 12 Changed 9 months ago by
Replying to mjo:
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?
comment:12 in reply to: ↑ 11 Changed 9 months ago by
Replying to slelievre:
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.
comment:13 Changed 9 months ago by
- Reviewers changed from Michael Orlitzky to Michael Orlitzky, Dima Pasechnik
- Status changed from needs_review to positive_review
lgtm
comment:14 Changed 9 months ago by
- Priority changed from major to critical
Setting to critical as this conditions work on spkg-configure files in other tickets.
comment:15 Changed 9 months ago by
- Branch changed from u/mjo/ticket/31528 to f9e58dab2a38012cb0d684b63d5c7934b94963b5
- Resolution set to fixed
- Status changed from positive_review to closed
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.