Opened 6 months ago

Closed 4 weeks ago

#33295 closed enhancement (fixed)

Refactor sage_conf

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.7
Component: scripts Keywords:
Cc: fbissey, arojas, tornaria Merged in:
Authors: Matthias Koeppe Reviewers: François Bissey
Report Upstream: N/A Work issues:
Branch: eeddb03 (Commits, GitHub, GitLab) Commit: eeddb03c6ce0ccf80a6a823e16335f445ae26ed3
Dependencies: Stopgaps:

Status badges

Description

(split out from #31396)

We refactor sage_conf to make it easier to maintain variants of it, both the 2 in-tree variants (and another one to be added in #31396) and downstream variants:

  • The configure-generated configuration file is now pkgs/sage-conf/_sage_conf/_conf.py
  • The file pkgs/sage-conf/sage_conf.py is now static

Attachments (1)

config.log (387.9 KB) - added by gh-GMS103 6 weeks ago.

Download all attachments as: .zip

Change History (36)

comment:1 Changed 6 months ago by mkoeppe

  • Branch set to u/mkoeppe/refactor_sage_conf

comment:2 Changed 6 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Cc fbissey arojas tornaria added
  • Commit set to 70c2e95d852d65dd6e02d0d6cb7189cb12f4c155
  • Status changed from new to needs_review

New commits:

70c2e95pkgs/sage-conf*: Refactor

comment:3 follow-up: Changed 6 months ago by fbissey

Three version of sage_conf? So

  • "regular"
  • pypi
  • relocatable

when is each one used or intended to be used? Ideally, I want something from pypi or some other repo that will give me a source tarball of the package (and only that one package).

comment:4 in reply to: ↑ 3 Changed 6 months ago by mkoeppe

Replying to fbissey:

when is each one used or intended to be used?

Explained in pkgs/sage-conf/README.rst

comment:5 Changed 5 months ago by mkoeppe

  • Milestone changed from sage-9.6 to sage-9.7

comment:6 Changed 3 months ago by git

  • Commit changed from 70c2e95d852d65dd6e02d0d6cb7189cb12f4c155 to ffb5344a619ee2a417b1799ad10b547c24a5057f

Branch pushed to git repo; I updated commit sha1. New commits:

ffb5344Merge tag '9.6.rc3' into t/33295/refactor_sage_conf

comment:7 Changed 7 weeks ago by dimpase

rebase is needed

comment:8 Changed 7 weeks ago by git

  • Commit changed from ffb5344a619ee2a417b1799ad10b547c24a5057f to fbb318d1d130b8a20dcde0f0ac626dd7b69e50ec

Branch pushed to git repo; I updated commit sha1. New commits:

fbb318dMerge tag '9.7.beta2' into t/33295/refactor_sage_conf

comment:9 Changed 7 weeks ago by fbissey

  • Reviewers set to François Bissey
  • Status changed from needs_review to positive_review

I very much want this ticket in 9.7 and it looks in good shape now.

comment:10 Changed 7 weeks ago by fbissey

  • Status changed from positive_review to needs_work

I have spoken too soon. I see there is a file pkgs/sage_conf/sage_conf.py.in and its link in sage_conf_pypi that may still need removal. I believe it is functionally fully replaced by _sage_conf/conf.py.in.

Similarly, we have pkgs/sage_conf/setup.cfg and pkgs/sage_conf/setup.cfg.in, and the .in is not used anymore by configure either

diff --git a/configure.ac b/configure.ac
index eca011d..09f694d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -514,7 +514,7 @@ dnl AC_CONFIG_HEADERS([config.h])
 AC_CONFIG_FILES([build/make/Makefile-auto build/make/Makefile])
 AC_CONFIG_FILES([src/bin/sage-env-config src/bin/sage-src-env-config build/bin/sage-build-env-config])
 
-AC_CONFIG_FILES([pkgs/sage-conf/sage_conf.py pkgs/sage-conf/setup.cfg])
+AC_CONFIG_FILES([pkgs/sage-conf/_sage_conf/_conf.py])
 
 dnl Create basic directories needed for Sage
 AC_CONFIG_COMMANDS(mkdirs,

Once those are dealt with, I am OK with putting things straight back to positive review.

comment:11 Changed 6 weeks ago by mkoeppe

Thanks for catching this. Looks like I was not careful enough in one of the merges.

comment:12 Changed 6 weeks ago by git

  • Commit changed from fbb318d1d130b8a20dcde0f0ac626dd7b69e50ec to 9d100555f824ec0d7d6c509f750a3887692c3f98

Branch pushed to git repo; I updated commit sha1. New commits:

93ca1f7Merge tag '9.7.beta3' into t/33295/refactor_sage_conf
9d10055pkgs/sage-conf*: Remove some obsolete files

comment:13 Changed 6 weeks ago by mkoeppe

  • Status changed from needs_work to needs_review

comment:14 Changed 6 weeks ago by fbissey

  • Status changed from needs_review to positive_review

LGTM

comment:15 Changed 6 weeks ago by mkoeppe

Thanks!

comment:16 Changed 6 weeks ago by gh-GMS103

I am unable to merge this ticket on top of SageMath 9.7.beta3:

% rm -fr sage
% git clone -c core.symlinks=true --branch develop --tags https://github.com/sagemath/sage.git
% cd sage
% git-trac-merge 33295
% make configure
% source .homebrew-build-env
% ./configure

gives

config.status: error: cannot find input file: `pkgs/sage-conf/sage_conf.py.in'

(log attached).

Changed 6 weeks ago by gh-GMS103

comment:17 Changed 6 weeks ago by fbissey

OK, this is strange. It definitely shouldn't happen. Result of

grep sage_conf.py configure*

please. I mean it like it is, don't add an .in anywhere.

Last edited 6 weeks ago by fbissey (previous) (diff)

comment:18 Changed 6 weeks ago by gh-GMS103

% grep sage_conf.py configure*
configure:ac_config_files="$ac_config_files pkgs/sage-conf/sage_conf.py pkgs/sage-conf/setup.cfg"
configure:    "pkgs/sage-conf/sage_conf.py") CONFIG_FILES="$CONFIG_FILES pkgs/sage-conf/sage_conf.py" ;;
% 

But I must add that this happens on macOS 12.4. On macOS 11.6.7 it works.

comment:19 Changed 6 weeks ago by fbissey

Somehow the branch hasn't applied, those lines shouldn't be there.

comment:20 Changed 6 weeks ago by gh-GMS103

Last edited 6 weeks ago by gh-GMS103 (previous) (diff)

comment:21 follow-up: Changed 6 weeks ago by gh-GMS103

In fact the problem is the following.

./configure works, but ./configure --enable-download-from-upstream-url fails as indicated.

(I was merging several tickets, some needing the option.)

comment:22 in reply to: ↑ 21 Changed 6 weeks ago by fbissey

Replying to gh-GMS103:

In fact the problem is the following.

./configure works, but ./configure --enable-download-from-upstream-url fails as indicated.

(I was merging several tickets, some needing the option.)

And it should not whatever the configuration option. The lines I asked you to grep earlier should be absent if you are really on this branch.

comment:23 Changed 6 weeks ago by gh-GMS103

Thanks for bearing with my ignorance.

This is what happens for me every time on macOS 12.4 (but not on macOS 11.6.7).

% rm -fr sage
% git clone -c core.symlinks=true --branch develop --tags https://github.com/sagemath/sage.git
[...]
% cd sage
% git-trac-merge 33295
#33295
remote branch: u/mkoeppe/refactor_sage_conf

From git://trac.sagemath.org/sage
 * branch                  u/mkoeppe/refactor_sage_conf -> FETCH_HEAD
 * [new branch]            u/mkoeppe/refactor_sage_conf -> trac/u/mkoeppe/refactor_sage_conf

Removing pkgs/sage-conf_pypi/sage_conf.py.in
Merge made by the 'recursive' strategy.
 Makefile                                                   |  2 +-
 build/pkgs/sage_conf/dependencies                          |  2 +-
 configure.ac                                               |  2 +-
 pkgs/sage-conf/.gitignore                                  |  4 +++-
 pkgs/sage-conf/_sage_conf/__main__.py                      | 22 ++++++++++++++++++++++
 pkgs/sage-conf/{sage_conf.py.in => _sage_conf/_conf.py.in} | 25 -------------------------
 pkgs/sage-conf/sage_conf.py                                |  2 ++
 pkgs/sage-conf/{setup.cfg.in => setup.cfg}                 |  9 +++++----
 pkgs/sage-conf_pypi/.gitignore                             |  3 ++-
 pkgs/sage-conf_pypi/MANIFEST.in                            |  5 +++--
 pkgs/sage-conf_pypi/_sage_conf                             |  1 +
 pkgs/sage-conf_pypi/sage_conf.py                           |  1 +
 pkgs/sage-conf_pypi/sage_conf.py.in                        |  1 -
 pkgs/sage-conf_pypi/setup.cfg                              | 17 +----------------
 pkgs/sage-conf_pypi/setup.py                               | 10 ++--------
 src/bin/sage-env-config.in                                 |  2 +-
 src/bin/sage-src-env-config.in                             |  2 +-
 17 files changed, 47 insertions(+), 63 deletions(-)
 create mode 100644 pkgs/sage-conf/_sage_conf/__main__.py
 rename pkgs/sage-conf/{sage_conf.py.in => _sage_conf/_conf.py.in} (75%)
 create mode 100644 pkgs/sage-conf/sage_conf.py
 rename pkgs/sage-conf/{setup.cfg.in => setup.cfg} (81%)
 create mode 120000 pkgs/sage-conf_pypi/_sage_conf
 create mode 120000 pkgs/sage-conf_pypi/sage_conf.py
 delete mode 120000 pkgs/sage-conf_pypi/sage_conf.py.in
 mode change 100644 => 120000 pkgs/sage-conf_pypi/setup.cfg
% grep sage_conf.py configure*
% make configure
./bootstrap -d
rm -rf config configure build/make/Makefile-auto.in
rm -f src/doc/en/installation/*.txt
rm -rf src/doc/en/reference/spkg/*.rst
rm -f environment.yml
rm -f src/environment.yml
rm -f src/environment-dev.yml
rm -f environment-optional.yml
rm -f src/environment-optional.yml
rm -f src/Pipfile
rm -f src/pyproject.toml
rm -f src/requirements.txt
rm -f src/setup.cfg
bootstrap:83: installing 'm4/sage_spkg_configures.m4'
./bootstrap: installing /Users/sagemath/SageMath/Git/sage/build/pkgs/sagemath_objects/src/pyproject.toml
./bootstrap: installing /Users/sagemath/SageMath/Git/sage/build/pkgs/sagemath_objects/src/requirements.txt
./bootstrap: installing /Users/sagemath/SageMath/Git/sage/build/pkgs/sagemath_objects/src/setup.cfg
./bootstrap: installing /Users/sagemath/SageMath/Git/sage/build/pkgs/sagemath_environment/src/pyproject.toml
./bootstrap: installing /Users/sagemath/SageMath/Git/sage/build/pkgs/sagemath_environment/src/requirements.txt
./bootstrap: installing /Users/sagemath/SageMath/Git/sage/build/pkgs/sagemath_environment/src/setup.cfg
./bootstrap: installing /Users/sagemath/SageMath/Git/sage/build/pkgs/sagemath_repl/src/pyproject.toml
./bootstrap: installing /Users/sagemath/SageMath/Git/sage/build/pkgs/sagemath_repl/src/requirements.txt
./bootstrap: installing /Users/sagemath/SageMath/Git/sage/build/pkgs/sagemath_repl/src/setup.cfg
./bootstrap: installing /Users/sagemath/SageMath/Git/sage/build/pkgs/sagemath_categories/src/MANIFEST.in
./bootstrap: installing /Users/sagemath/SageMath/Git/sage/build/pkgs/sagemath_categories/src/pyproject.toml
./bootstrap: installing /Users/sagemath/SageMath/Git/sage/build/pkgs/sagemath_categories/src/requirements.txt
./bootstrap: installing /Users/sagemath/SageMath/Git/sage/build/pkgs/sagemath_categories/src/setup.cfg
./bootstrap: installing /Users/sagemath/SageMath/Git/sage/build/pkgs/sagelib/src/Pipfile-dist
./bootstrap: installing /Users/sagemath/SageMath/Git/sage/build/pkgs/sagelib/src/Pipfile
./bootstrap: installing /Users/sagemath/SageMath/Git/sage/build/pkgs/sagelib/src/pyproject.toml
./bootstrap: installing /Users/sagemath/SageMath/Git/sage/build/pkgs/sagelib/src/requirements.txt
./bootstrap: installing /Users/sagemath/SageMath/Git/sage/build/pkgs/sagelib/src/setup.cfg
./bootstrap-conda:48: generate conda enviroment files
src/doc/bootstrap:68: installing src/doc/en/installation/arch*.txt
src/doc/bootstrap:68: installing src/doc/en/installation/debian*.txt
src/doc/bootstrap:68: installing src/doc/en/installation/fedora*.txt
src/doc/bootstrap:68: installing src/doc/en/installation/cygwin*.txt
src/doc/bootstrap:68: installing src/doc/en/installation/homebrew*.txt
src/doc/bootstrap:78: installing src/doc/en/reference/spkg/*.rst
bootstrap:74: installing 'config/config.rpath'
./bootstrap: line 164: aclocal: command not found
Bootstrap failed, downloading required files instead.
Attempting to download package configure-b466c8ae02c04d287d28d2d669e1d111df3724ff.tar.gz from mirrors
Downloading the Sage mirror list
Searching fastest mirror
  177ms: http://files.sagemath.org/
  399ms: http://linorg.usp.br/sage/
   59ms: https://ftp.rediris.es/mirror/sagemath/
  523ms: https://ftp.riken.jp/sagemath/
  350ms: https://ftp.sun.ac.za/ftp/pub/mirrors/www.sagemath.org/
  264ms: https://ftp.yz.yamagata-u.ac.jp/pub/math/sage/
  174ms: https://mirror-hk.koddos.net/sagemath/
  305ms: https://mirror.aarnet.edu.au/pub/sage/
  245ms: https://mirror.csclub.uwaterloo.ca/sage/
   27ms: https://mirror.dogado.de/sage/
   12ms: https://mirror.koddos.net/sagemath/
   26ms: https://mirror.lyrahosting.com/sagemath/
   40ms: https://mirror.marwan.ma/sage/
  577ms: https://mirror.rcg.sfu.ca/mirror/sage/
  329ms: https://mirror.ufs.ac.za/sagemath/
  111ms: https://mirror.yandex.ru/mirrors/sage.math.washington.edu/
   81ms: https://mirrors.aliyun.com/sagemath/
   86ms: https://mirrors.mit.edu/sage/
  249ms: https://mirrors.nju.edu.cn/sagemath/
  685ms: https://mirrors.tuna.tsinghua.edu.cn/sagemath/
   55ms: https://mirrors.up.pt/pub/sage/
  245ms: https://mirrors.ustc.edu.cn/sagemath/
  126ms: https://mirrors.xmission.com/sage/
   40ms: https://sage.mirror.garr.it/mirrors/sage/
   15ms: https://www-ftp.lip6.fr/pub/math/sagemath/
   87ms: https://www.mirrorservice.org/sites/www.sagemath.org/
Fastest mirror: https://mirror.koddos.net/sagemath/
https://mirror.koddos.net/sagemath/spkg/upstream/configure/configure-b466c8ae02c04d287d28d2d669e1d111df3724ff.tar.gz
[......................................................................]
% grep sage_conf.py configure*
configure:ac_config_files="$ac_config_files pkgs/sage-conf/sage_conf.py pkgs/sage-conf/setup.cfg"
configure:    "pkgs/sage-conf/sage_conf.py") CONFIG_FILES="$CONFIG_FILES pkgs/sage-conf/sage_conf.py" ;;
% source .homebrew-build-env
% ./configure
[...]
zn_poly-0.9.2:                               no suitable system package; standard, will be installed as an SPKG
checking that generated files are newer than configure... done
configure: creating ./config.status
config.status: creating build/make/Makefile-auto
config.status: creating build/make/Makefile
config.status: creating src/bin/sage-env-config
config.status: creating src/bin/sage-src-env-config
config.status: creating build/bin/sage-build-env-config
config.status: error: cannot find input file: `pkgs/sage-conf/sage_conf.py.in'
% 

comment:24 Changed 6 weeks ago by mkoeppe

From the output:

bootstrap:74: installing 'config/config.rpath'
./bootstrap: line 164: aclocal: command not found
Bootstrap failed, downloading required files instead.

These downloaded files are unsuitable - they don't fit with the changes in the ticket.

Install the bootstrapping prerequisites (https://doc.sagemath.org/html/en/reference/spkg/_bootstrap.html#spkg-bootstrap)

comment:25 Changed 6 weeks ago by gh-GMS103

Thanks Matthias and François.

Now I understand what happened. I had uninstalled Homebrew because of some problems, and I thought I had reinstalled all that was needed. But I was wrong.

Once again, sorry for the noise.

Last edited 6 weeks ago by gh-GMS103 (previous) (diff)

comment:26 Changed 6 weeks ago by fbissey

Volker is trying to merge the ticket and I ended finding a problem. Not sure if it will have an impact on the mergability.

$ sage-config
Traceback (most recent call last):
  File "/usr/lib/python-exec/python3.10/sage-config", line 5, in <module>
    from sage_conf.__main__ import _main
ModuleNotFoundError: No module named 'sage_conf.__main__'; 'sage_conf' is not a package

It originates from setup.cfg

[options.entry_points]
console_scripts =
    sage-config = sage_conf.__main__:_main

I tried to change the last line to sage-config = _sage_conf.__main__:_main but it leads to a new error

$ sage-config
Traceback (most recent call last):
  File "/usr/lib/python-exec/python3.10/sage-config", line 5, in <module>
    from _sage_conf.__main__ import _main
  File "/usr/lib/python3.10/site-packages/_sage_conf/__main__.py", line 3, in <module>
    from sage_conf import *
  File "/usr/lib/python3.10/site-packages/sage_conf.py", line 2, in <module>
    from _sage_conf.__main__ import _main
ImportError: cannot import name '_main' from partially initialized module '_sage_conf.__main__' (most likely due to a circular import) (/usr/lib/python3.10/site-packages/_sage_conf/__main__.py)

comment:27 follow-up: Changed 6 weeks ago by mkoeppe

sage_conf is definitely a package after this ticket. Are you sure you installed it?

comment:28 Changed 6 weeks ago by fbissey

These are the file I have installed

/usr/lib/python3.10/site-packages/__pycache__
/usr/lib/python3.10/site-packages/__pycache__/sage_conf.cpython-310.opt-1.pyc
/usr/lib/python3.10/site-packages/__pycache__/sage_conf.cpython-310.opt-2.pyc
/usr/lib/python3.10/site-packages/__pycache__/sage_conf.cpython-310.pyc
/usr/lib/python3.10/site-packages/_sage_conf
/usr/lib/python3.10/site-packages/_sage_conf/__main__.py
/usr/lib/python3.10/site-packages/_sage_conf/__pycache__
/usr/lib/python3.10/site-packages/_sage_conf/__pycache__/__main__.cpython-310.opt-1.pyc
/usr/lib/python3.10/site-packages/_sage_conf/__pycache__/__main__.cpython-310.opt-2.pyc
/usr/lib/python3.10/site-packages/_sage_conf/__pycache__/__main__.cpython-310.pyc
/usr/lib/python3.10/site-packages/_sage_conf/__pycache__/_conf.cpython-310.opt-1.pyc
/usr/lib/python3.10/site-packages/_sage_conf/__pycache__/_conf.cpython-310.opt-2.pyc
/usr/lib/python3.10/site-packages/_sage_conf/__pycache__/_conf.cpython-310.pyc
/usr/lib/python3.10/site-packages/_sage_conf/_conf.py
/usr/lib/python3.10/site-packages/sage_conf-9.7b3.dist-info
/usr/lib/python3.10/site-packages/sage_conf-9.7b3.dist-info/METADATA
/usr/lib/python3.10/site-packages/sage_conf-9.7b3.dist-info/RECORD
/usr/lib/python3.10/site-packages/sage_conf-9.7b3.dist-info/WHEEL
/usr/lib/python3.10/site-packages/sage_conf-9.7b3.dist-info/top_level.txt
/usr/lib/python3.10/site-packages/sage_conf.py

Could it be some semantics issue? sage_conf.py does not have a main section. Or may be it shouldn't be present?

comment:29 in reply to: ↑ 27 Changed 6 weeks ago by mkoeppe

Replying to mkoeppe:

sage_conf is definitely a package after this ticket.

Sorry, I take this back

comment:30 Changed 6 weeks ago by mkoeppe

  • Status changed from positive_review to needs_work

comment:31 Changed 6 weeks ago by git

  • Commit changed from 9d100555f824ec0d7d6c509f750a3887692c3f98 to eeddb03c6ce0ccf80a6a823e16335f445ae26ed3

Branch pushed to git repo; I updated commit sha1. New commits:

eeddb03pkgs/sage-conf/setup.cfg: Fix up console_scripts

comment:32 Changed 6 weeks ago by mkoeppe

  • Status changed from needs_work to needs_review

comment:33 Changed 6 weeks ago by fbissey

  • Status changed from needs_review to positive_review

Much better. Although I am not sure I will keep sage-config in sage-on-gentoo. We may want a more graceful behavior for variables not found in some follow up ticket.

comment:34 Changed 5 weeks ago by mkoeppe

Thanks!

comment:35 Changed 4 weeks ago by vbraun

  • Branch changed from u/mkoeppe/refactor_sage_conf to eeddb03c6ce0ccf80a6a823e16335f445ae26ed3
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.