Opened 20 months ago

Closed 3 months ago

#30845 closed enhancement (fixed)

Fixes for the conda-for-Sage-developers installation method, add GH Actions workflow

Reported by: mkoeppe Owned by:
Priority: critical Milestone: sage-9.6
Component: build Keywords: sd111
Cc: isuruf, gh-tobiasdiez, dimpase, saraedum Merged in:
Authors: Tobias Diez, Matthias Koeppe Reviewers: Matthias Koeppe, Dima Pasechnik, Tobias Diez
Report Upstream: N/A Work issues:
Branch: 93fce5a (Commits, GitHub, GitLab) Commit: 93fce5a1589fcfca91e8be56f13d15f4de646294
Dependencies: #33358, #33330, #33361, #33141 Stopgaps:

Status badges

Description (last modified by mkoeppe)

Add github action workflow that checks the build of sage in a conda environment, completely bypassing the installation of any sage package. This is based on the steps outlined at https://wiki.sagemath.org/Conda.

This also tests that the generated src/environment*.yml files work correctly. (See documentation added in #28745)

Run: https://github.com/sagemath/sagetrac-mirror/actions/workflows/ci-conda.yml

Fixes:

  • #33330: primecountpy is not installed in the conda env because its not listed in environment.yml
  • environment-optional: lcalc 1 is installed and then not recognized / leads to error
    building 'sage.libs.lcalc.lcalc_Lfunction' extension
        cc1plus: warning: command line option '-Wstrict-prototypes' is valid for C/ObjC but not for C++
        In file included from sage/libs/lcalc/lcalc_Lfunction.cpp:740:
        sage/libs/lcalc/lcalc_sage.h:1:10: fatal error: lcalc/L.h: No such file or directory
            1 | #include "lcalc/L.h"
              |          ^~~~~~~~~~~
        compilation terminated.
    
  • environment-optional: probably related, pari 2.11.4 is installed by conda instead of the newest 2.13.2. This version is then rejected by configure as being too old.
  • Disable conda distro information for gdb because it is broken on macOS.
  • Add conda distro information for lrcalc_python
  • Filter out conda-specific ld warnings in doctests
  • In the feature test for sage_spkg, check whether any Sage packages are actually installed
  • Include an upper version bound for ptyprocess in sagemath-standard's install-requires

Follow-ups: See Meta-ticket #33331

Attachments (1)

config.log (469.5 KB) - added by gh-tobiasdiez 5 months ago.

Download all attachments as: .zip

Change History (330)

comment:1 Changed 20 months ago by mkoeppe

  • Summary changed from GH Actions: Add test for conda without SPKG to tox.ini, GH Actions: Add test for conda without SPKG

comment:2 Changed 20 months ago by mkoeppe

  • Description modified (diff)

comment:3 Changed 19 months ago by mkoeppe

  • Keywords sd111 added

Hoping we can make progress on this ticket this week - https://wiki.sagemath.org/days111

comment:4 Changed 19 months ago by gh-tobiasdiez

  • Cc isuruf. gh-tobiasdiez added; isuruf removed

comment:5 Changed 19 months ago by gh-tobiasdiez

  • Cc isuruf added; isuruf. removed

comment:6 Changed 19 months ago by mkoeppe

  • Description modified (diff)

comment:7 Changed 19 months ago by gh-tobiasdiez

Something like https://github.com/tobiasdiez/sage/blob/public/build/conda_gh/.github/workflows/ci-conda.yml?

This currently fails with

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: creating build/pkgs/sage_conf/src/sage_conf.py
config.status: creating build/pkgs/sage_conf/src/setup.cfg
config.status: executing depfiles commands
config.status: executing mkdirs commands
config.status: creating directory /home/runner/work/sage/sage/logs/pkgs
config.status: creating directory 
mkdir: cannot create directory '': No such file or directory
config.status: error: could not create 
Error: Process completed with exit code 1.

https://github.com/tobiasdiez/sage/runs/1594237745?check_suite_focus=true Any idea what could be the reason and how to fix it?

Last edited 19 months ago by gh-tobiasdiez (previous) (diff)

comment:8 Changed 19 months ago by gh-tobiasdiez

  • Authors set to Tobias Diez
  • Branch set to public/build/conda_gh
  • Commit set to 9af4614153be945d5e56e2ffc7cdddaece705f76

New commits:

c68d262Add first version of workflow
9af4614Try with sudo (why is this needed now?!)

comment:9 Changed 19 months ago by git

  • Commit changed from 9af4614153be945d5e56e2ffc7cdddaece705f76 to a178b56349e645e6120832b0945a9a0e80327ce4

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

5ac74eeTry with conda-incubator
a178b56Run in the conda shell

comment:10 Changed 19 months ago by git

  • Commit changed from a178b56349e645e6120832b0945a9a0e80327ce4 to 86411b841fb32b1ced8d4c8989d2f30703448c2e

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

199886cInstall more conda dependencies
10de3f5Fix conda env name
86411b8Remove unnececesary install of conda dep

comment:11 Changed 19 months ago by gh-tobiasdiez

Managed to fix the above error, just to be stuck two lines further down:

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: creating build/pkgs/sage_conf/src/sage_conf.py
config.status: creating build/pkgs/sage_conf/src/setup.cfg
config.status: executing depfiles commands
config.status: executing broken-gcc commands
config.status: executing mkdirs commands
config.status: creating directory /home/runner/work/sage/sage/logs/pkgs
config.status: creating directory /usr/share/miniconda/envs/sage-build
config.status: creating directory /usr/share/miniconda/envs/sage-build/bin
config.status: creating directory /usr/share/miniconda/envs/sage-build/etc
config.status: creating directory /usr/share/miniconda/envs/sage-build/include
config.status: creating directory /usr/share/miniconda/envs/sage-build/lib
config.status: creating directory /usr/share/miniconda/envs/sage-build/lib/pkgconfig
config.status: creating directory /usr/share/miniconda/envs/sage-build/share
config.status: creating directory /usr/share/miniconda/envs/sage-build/var/lib/sage/installed
config.status: error: Cannot perform incremental update, run "make distclean && make"
config.status: /usr/share/miniconda/envs/sage-build/lib64 is not a symlink, see Trac #19782
Error: Process completed with exit code 1.

https://github.com/tobiasdiez/sage/runs/1594601374?check_suite_focus=true

comment:12 Changed 19 months ago by gh-tobiasdiez

Found a hack that fixed it by removing the lib64 folder. It only contained libcc1.so (not sure where/how this is needed).

comment:13 follow-up: Changed 19 months ago by gh-tobiasdiez

Made a bit more progress, but it currently errors while compiling gcc. Moreover, there is a problem in the detection of the conda/system gcc because "sage-env-config" doesn't exist. https://github.com/tobiasdiez/sage/runs/1594867631?check_suite_focus=true

comment:14 Changed 19 months ago by git

  • Commit changed from 86411b841fb32b1ced8d4c8989d2f30703448c2e to 741c55abee6e1cc125b16f62d56fc865982c01af

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

53503dcTry remove lib64 folder by hand
e530ac0Add c++ compiler to conda
ab47f5eInstall directly on top of conda
741c55aMake autogen work

comment:15 Changed 19 months ago by mkoeppe

The direct variant should use src/environment.yml - which installs the Python packages of Sage in addition to what is listed in environment.yml

comment:16 Changed 19 months ago by mkoeppe

The lib64 looks like a conda packaging bug to me - @isuruf, can you help?

comment:17 Changed 19 months ago by mkoeppe

environment.yml already includes compilers -- I don't think adding cxx-compiler should be necessary

comment:18 Changed 19 months ago by git

  • Commit changed from 741c55abee6e1cc125b16f62d56fc865982c01af to 3aba04885446291cb840e46712084185a5812eb1

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

1231a24Add env logging and fix SAGE_SRC
be024f8Fix build
8a46d04Improve output of print_env
9802c56Improve print statement
3aba048Use correct conda env file

comment:19 in reply to: ↑ 13 Changed 19 months ago by mkoeppe

Replying to gh-tobiasdiez:

Made a bit more progress, but it currently errors while compiling gcc. Moreover, there is a problem in the detection of the conda/system gcc because "sage-env-config" doesn't exist. https://github.com/tobiasdiez/sage/runs/1594867631?check_suite_focus=true

This is now #31097.

comment:20 Changed 19 months ago by mkoeppe

  • Dependencies changed from #28745 to #31097

comment:21 Changed 19 months ago by mkoeppe

  • Description modified (diff)

comment:22 Changed 19 months ago by isuruf

I don't see a lib64 folder. Which package is that coming from?

comment:23 Changed 19 months ago by gh-tobiasdiez

It contains libcc1.so so I guess gcc, g++ or some other compiler package.

comment:24 Changed 19 months ago by git

  • Commit changed from 3aba04885446291cb840e46712084185a5812eb1 to e9cb77b0c0755e08fc3c054f051d1ec1a9cd80d9

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

d853f14Ok, then install setup.py relative to src
cb5e284Add tests
1cf3de4build/pkgs/gcc/spkg-configure.m4: Fix SAGE_BROKEN_GCC test
1bc9c29Merge branch 'u/mkoeppe/build_pkgs_gcc_spkg_configure_m4__fix_sage_broken_gcc_test' of git://trac.sagemath.org/sage into public/build/conda_gh
c57d615Correct bootstrap command
e85f1baFix venv config
e29a9abCorrect test command
483de62Install autotools
8237b2cInstall gettext
e9cb77bForce configure to not install gcc

comment:25 Changed 19 months ago by gh-tobiasdiez

Current status:

  • Building sage using make on top of conda fails, since gcc is not found and the custom build of gcc fails.
  • Installing sagelib on top of a conda works, but many doctests fail.

https://github.com/sagemath/sagetrac-mirror/actions?query=workflow%3A%22Build+%26+Test+using+conda%22

I let somebody else with more experience in conda/the intricacies of sage's build system take over from here.

comment:26 Changed 19 months ago by mkoeppe

Thanks for getting this started!

comment:27 Changed 19 months ago by mkoeppe

Unfortunately the runs do not give the config.log

comment:28 follow-up: Changed 19 months ago by mkoeppe

Now I finally realize that for on-top-of-spkg you were trying to do ./configure --prefix=$CONDA_PREFIX -- that can't work.

comment:29 Changed 19 months ago by git

  • Commit changed from e9cb77b0c0755e08fc3c054f051d1ec1a9cd80d9 to 9857e36640c22e9bf7f1d072f21dfa9cd1455449

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

9857e36Without the prefix

comment:30 Changed 19 months ago by git

  • Commit changed from 9857e36640c22e9bf7f1d072f21dfa9cd1455449 to b50ae6a7e2e4f80dafbb27785ce9345163875163

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

b50ae6aRemove conda gcc distro again

comment:31 Changed 19 months ago by git

  • Commit changed from b50ae6a7e2e4f80dafbb27785ce9345163875163 to 68080fa95aed35a35813bddb7f35fbc73a373468

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

68080faCleanup workflow

comment:32 in reply to: ↑ 28 Changed 19 months ago by gh-tobiasdiez

Replying to mkoeppe:

Now I finally realize that for on-top-of-spkg you were trying to do ./configure --prefix=$CONDA_PREFIX -- that can't work.

Thanks! Removing the prefix part indeed fixed the gcc issue. Now we are further in the build, and find this beautiful error:

2020-12-22T21:44:17.7966259Z [cvxopt-1.2.5]   x86_64-conda_cos6-linux-gnu-gcc -pthread -shared -Wl,-O2 -Wl,--sort-common -Wl,--as-needed -Wl,-z,relro -Wl,-z,now -Wl,-rpath,/usr/share/miniconda/envs/sage-build/lib -L/usr/share/miniconda/envs/sage-build/lib -Wl,-O2 -Wl,--sort-common -Wl,--as-needed -Wl,-z,relro -Wl,-z,now -Wl,-rpath,/usr/share/miniconda/envs/sage-build/lib -L/usr/share/miniconda/envs/sage-build/lib -Wl,-rpath-link,/home/runner/work/sagetrac-mirror/sagetrac-mirror/local/lib -L/home/runner/work/sagetrac-mirror/sagetrac-mirror/local/lib -Wl,-rpath,/home/runner/work/sagetrac-mirror/sagetrac-mirror/local/lib -Wl,-O2 -Wl,--sort-common -Wl,--as-needed -Wl,-z,relro -Wl,-z,now -Wl,--disable-new-dtags -Wl,--gc-sections -Wl,-rpath,/usr/share/miniconda/envs/sage-build/lib -Wl,-rpath-link,/usr/share/miniconda/envs/sage-build/lib -L/usr/share/miniconda/envs/sage-build/lib -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe -isystem /usr/share/miniconda/envs/sage-build/include -DNDEBUG -D_FORTIFY_SOURCE=2 -O2 -isystem /usr/share/miniconda/envs/sage-build/include build/temp.linux-x86_64-3.8/src/C/lapack.o -L/usr/share/miniconda/envs/sage-build/lib -lopenblas -lopenblas -o build/lib.linux-x86_64-3.8/cvxopt/lapack.cpython-38-x86_64-linux-gnu.so
2020-12-22T21:44:17.7974135Z [cvxopt-1.2.5]   building 'umfpack' extension
2020-12-22T21:44:17.7979362Z [cvxopt-1.2.5]   /usr/share/miniconda/envs/sage-build/bin/x86_64-conda-linux-gnu-cc -Wno-unused-result -Wsign-compare -DNDEBUG -fwrapv -O2 -Wall -Wstrict-prototypes -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -pipe -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -pipe -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe -isystem /usr/share/miniconda/envs/sage-build/include -DNDEBUG -D_FORTIFY_SOURCE=2 -O2 -isystem /usr/share/miniconda/envs/sage-build/include -fPIC -I/usr/include -I/home/runner/work/sagetrac-mirror/sagetrac-mirror/local/include -I/usr/share/miniconda/envs/sage-build/include/python3.8 -c src/C/umfpack.c -o build/temp.linux-x86_64-3.8/src/C/umfpack.o
2020-12-22T21:44:17.7990748Z [cvxopt-1.2.5]   In file included from /usr/share/miniconda/envs/sage-build/include/python3.8/Python.h:11:0,
2020-12-22T21:44:17.7991831Z [cvxopt-1.2.5]                    from src/C/cvxopt.h:22,
2020-12-22T21:44:17.7992613Z [cvxopt-1.2.5]                    from src/C/umfpack.c:22:
2020-12-22T21:44:17.7993750Z [cvxopt-1.2.5]   /usr/include/limits.h:26:10: fatal error: bits/libc-header-start.h: No such file or directory
2020-12-22T21:44:17.7994924Z [cvxopt-1.2.5]    #include <bits/libc-header-start.h>
2020-12-22T21:44:17.7995741Z [cvxopt-1.2.5]             ^~~~~~~~~~~~~~~~~~~~~~~~~~
2020-12-22T21:44:17.7996470Z [cvxopt-1.2.5]   compilation terminated.
2020-12-22T21:44:17.7997728Z [cvxopt-1.2.5]   error: command '/usr/share/miniconda/envs/sage-build/bin/x86_64-conda-linux-gnu-cc' failed with exit status 1
2020-12-22T21:44:17.7999070Z [cvxopt-1.2.5]   Building wheel for cvxopt (setup.py): finished with status 'error'

Could this be related to the fact that libcc1 is installed in lib64?

comment:34 Changed 19 months ago by gh-tobiasdiez

Ah thx for the pointer!

comment:35 Changed 19 months ago by gh-tobiasdiez

  • Reviewers set to https://github.com/sagemath/sagetrac-mirror/actions?query=workflow%3A%22Build+%26+Test+using+conda%22
  • Status changed from new to needs_review

I guess the main objective of this ticket was to add a workflow verifying that sagelib installed on top of conda works. Since this is accomplished, I'll put it as ready for review.

Fixing the doctests and the build using make can be done later (e.g by resolving #30710).

comment:36 follow-up: Changed 19 months ago by mkoeppe

As on-top-of-spkg turns out to be a variant of local-conda-forge-ubuntu-standard, I think I would want to implement this variant using tox.

comment:37 Changed 19 months ago by mkoeppe

I have opened #31099 for this.

comment:38 in reply to: ↑ 36 Changed 19 months ago by gh-tobiasdiez

Replying to mkoeppe:

As on-top-of-spkg turns out to be a variant of local-conda-forge-ubuntu-standard, I think I would want to implement this variant using tox.

I thought about using tox as well, but there are

a) problems setting up conda on github actions (this is why the workflow uses a special action for the conda setup)

b) difficulties with conda and tox environment interactions (which I guess are handled by the tox-conda plugin, but I didn't wanted to dive into this)

c) I still think tox is not the right tool for managing the user's environment beyond the Python environment.

comment:39 Changed 19 months ago by mkoeppe

local-conda-forge-ubuntu-standard has been running for months on GH Actions, see for example https://github.com/sagemath/sage/runs/1553907433 for 9.3.beta4.

#31099 is just a matter of creating a variant that uses conda env create -f environment.yml instead of conda install ....

comment:40 Changed 17 months ago by chapoton

  • Status changed from needs_review to needs_work

red branch => needs work

comment:41 Changed 15 months ago by mkoeppe

  • Milestone changed from sage-9.3 to sage-9.4

Moving this ticket to 9.4, as it seems unlikely that it will be merged in 9.3, which is in the release candidate stage

comment:42 Changed 11 months ago by mkoeppe

  • Milestone changed from sage-9.4 to sage-9.5

comment:43 Changed 8 months ago by git

  • Commit changed from 68080fa95aed35a35813bddb7f35fbc73a373468 to 103f78b3f0163e18b5aa4287d29570fbd2872524

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

103f78b.github/workflows/ci-conda.yml: New

comment:44 Changed 8 months ago by mkoeppe

I have reduced the branch to one of the jobs, direct, to remove the duplication with the existing workflows.

comment:45 Changed 8 months ago by mkoeppe

  • Cc dimpase added
  • Dependencies #31097 deleted
  • Reviewers https://github.com/sagemath/sagetrac-mirror/actions?query=workflow%3A%22Build+%26+Test+using+conda%22 deleted

Still needs updating to follow the steps of "Conda for Sage Developers" - https://wiki.sagemath.org/Conda - help welcome

comment:46 Changed 8 months ago by mkoeppe

  • Description modified (diff)

comment:47 Changed 6 months ago by mkoeppe

  • Milestone changed from sage-9.5 to sage-9.6

comment:48 Changed 5 months ago by gh-tobiasdiez

  • Branch changed from public/build/conda_gh to public/build/conda-runci
  • Commit changed from 103f78b3f0163e18b5aa4287d29570fbd2872524 to 68e8330a3619f0b17934f1ff3d23cb8b9c392729
  • Summary changed from tox.ini, GH Actions: Add test for conda without SPKG to GH Actions: Add test for conda without SPKG

Last 10 new commits:

65e1b49Fix workflow syntax
85c31c3Make workflow at least try to compile sage
fd484d8Github needs sudo
909cd9eAdd a few missing python packages
a91da81Run in parallel
d4945e5Install gap manually
3171f82Split in more jobs
b87f2bcReuse existing bootstrap
eec7abcDelete env-python from gitignore again
68e8330Try with prefix instead of building ecl and gap

comment:49 Changed 5 months ago by gh-tobiasdiez

  • Description modified (diff)

comment:50 Changed 5 months ago by gh-tobiasdiez

  • Description modified (diff)
  • Status changed from needs_work to needs_review

Okay, this is now ready for review.

Any pointers to the two open issues are welcome. I looked for open tickets in these directions, but couldn't find one.

comment:51 Changed 5 months ago by git

  • Commit changed from 68e8330a3619f0b17934f1ff3d23cb8b9c392729 to b408f9e4eaf24bcd898e33d35bc596a0ec998462

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

b408f9eCleanup workflow

comment:52 Changed 5 months ago by mkoeppe

In https://github.com/sagemath/sagetrac-mirror/runs/5146995129?check_suite_focus=true I see

I see

building 'sage.libs.lcalc.lcalc_Lfunction' extension
    cc1plus: warning: command line option '-Wstrict-prototypes' is valid for C/ObjC but not for C++
    In file included from sage/libs/lcalc/lcalc_Lfunction.cpp:740:
    sage/libs/lcalc/lcalc_sage.h:1:10: fatal error: lcalc/L.h: No such file or directory
        1 | #include "lcalc/L.h"
          |          ^~~~~~~~~~~
    compilation terminated.
    building 'sage.libs.libecm' extension

comment:53 Changed 5 months ago by gh-tobiasdiez

  • Description modified (diff)

comment:54 Changed 5 months ago by gh-tobiasdiez

Good observation. I think, however, this is not the reason why the build terminates as this error occurs somewhere in the middle (and I had similar error messages with gap before). I've added it to the list of "issues" in the ticket description.

comment:55 follow-up: Changed 5 months ago by mkoeppe

  • Status changed from needs_review to needs_work

This is the reason for the failure. There's no mystery there.

comment:56 follow-up: Changed 5 months ago by isuruf

Do you get lcalc>=2 with conda?

comment:57 in reply to: ↑ 55 ; follow-up: Changed 5 months ago by gh-tobiasdiez

  • Status changed from needs_work to needs_review

Replying to mkoeppe:

This is the reason for the failure. There's no mystery there.

Could be. But the point of this ticket is to add the github action, so that one can fix these issues in follow-up tickets and have a direct verification. It might take quite some time until this workflow is green, especially since there are probably quite a few tests that fail.

So except if you think that there is something fundamentally flawed with the current steps which result in the lcalc error, this should be ready for review.

comment:58 in reply to: ↑ 56 Changed 5 months ago by gh-tobiasdiez

Replying to isuruf:

Do you get lcalc>=2 with conda?

Nope, its installing lcalc 1.23 h0d16fac_1004 conda-forge. That's probably the reason why its not working.

comment:59 follow-ups: Changed 5 months ago by mkoeppe

So build/pkgs/lcalc/distros/conda.txt should be changed

comment:60 in reply to: ↑ 57 ; follow-up: Changed 5 months ago by mkoeppe

Replying to gh-tobiasdiez:

the point of this ticket is to add the github action

Well, it fails early, so we cannot see whether the later steps work.

comment:61 in reply to: ↑ 59 Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

So build/pkgs/lcalc/distros/conda.txt should be changed

I don't think it will be that easy to solve:

mamba install lcalc=2.0.5 -n sage-dev
 > package libeantic-1.0.2-hf1f868f_1 requires antic >=0.2.4,<0.3.0a0, but none of the providers can be installed

mamba install libeantic=1.1.0 -n sage-dev
 > package libeantic-1.1.0-h52e3b27_0 requires arb >=2.21.1,<2.22.0a0, but none of the providers can be installed

mamba install arb=2.21.1 -n sage-dev
 > package e-antic-1.0.2-hf1f868f_0 requires arb >=2.19.0,<2.20.0a0, but none of the providers can be installed

I appears that there is a funny cyclic dependency thing going on.

comment:62 in reply to: ↑ 60 Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

Replying to gh-tobiasdiez:

the point of this ticket is to add the github action

Well, it fails early, so we cannot see whether the later steps work.

Should I un-comment the last test step for now?

comment:63 Changed 5 months ago by mkoeppe

Try switching from src/environment-optional.yml to src/environment.yml. e_antic is an optional package for Sage. Let's try without optional packages first.

comment:64 Changed 5 months ago by git

  • Commit changed from b408f9e4eaf24bcd898e33d35bc596a0ec998462 to 051b7b5730203843fda7a5a47bab8bbaffdf5c3c

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

051b7b5Don't use -optional env

comment:65 Changed 5 months ago by gh-tobiasdiez

That should indeed work to get loalc 2.0.5. Good suggestion.

For the record: first installing environment.yml and then updating the env to environment-optional.yml yields the following downgrade of packgages:

  Downgrade:
───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

  - arb                                               2.22.1  hbbd85db_0              installed
  + arb                                               2.19.0  h7e34412_0              conda-forge/linux-64        7MB
  - boost-cpp                                         1.77.0  h359cf19_1              installed
  + boost-cpp                                         1.74.0  h312852a_4              conda-forge/linux-64     Cached
  - cvxopt                                             1.2.7  py310h9fd565e_1         installed
  + cvxopt                                             1.2.6  py38h55e5319_0          conda-forge/linux-64     Cached
  - cysignals                                         1.11.2  py310h15010d2_0         installed
  + cysignals                                         1.10.3  py38hd3cf888_0          conda-forge/linux-64     Cached
  - eclib                                           20210625  h52f5ea4_4              installed
  + eclib                                           20190909  h5b7fb09_3              conda-forge/linux-64       36MB
  - gap-core                                          4.11.1  h1725ef4_2              installed
  + gap-core                                          4.11.0  h1725ef4_7              conda-forge/linux-64       67MB
  - gap-defaults                                      4.11.1  ha770c72_2              installed
  + gap-defaults                                      4.11.0  ha770c72_7              conda-forge/linux-64       88MB
  - gsl                                                  2.7  he838d99_0              installed
  + gsl                                                  2.6  he838d99_2              conda-forge/linux-64     Cached
  - harfbuzz                                           3.3.1  hb4a5f5f_0              installed
  + harfbuzz                                           3.1.1  h83ec7ef_0              conda-forge/linux-64     Cached
  - icu                                                 69.1  h9c3ff4c_0              installed
  + icu                                                 68.2  h9c3ff4c_0              conda-forge/linux-64     Cached
  - lcalc                                              2.0.5  h8cd7e2e_0              installed
  + lcalc                                               1.23  h0d16fac_1004           conda-forge/linux-64     Cached
  - libclang                                          13.0.1  default_hc23dcda_0      installed
  + libclang                                          11.1.0  default_ha53f305_1      conda-forge/linux-64     Cached
  - libflint                                           2.8.4  hd3cd37b_ntl_100        installed
  + libflint                                           2.6.3  h6b702e8_0              conda-forge/linux-64       11MB
  - libpq                                               14.2  hd57d9b9_0              installed
  + libpq                                               13.5  hd57d9b9_1              conda-forge/linux-64     Cached
  - libuv                                             1.43.0  h7f98852_0              installed
  + libuv                                             1.42.0  h7f98852_0              conda-forge/linux-64     Cached
  - pari                                              2.13.3  h82e6b41_1_pthread      installed
  + pari                                              2.11.4  hddaffa3_2              conda-forge/linux-64     Cached
  - python                                            3.10.2  h85951f9_3_cpython      installed
  + python                                            3.8.12  ha38a3c6_3_cpython      conda-forge/linux-64     Cached
  - python_abi                                          3.10  2_cp310                 installed
  + python_abi                                           3.8  2_cp38                  conda-forge/linux-64     Cached
  - r-base                                             4.1.2  hde4fec0_0              installed
  + r-base                                             4.1.1  hb67fd72_0              conda-forge/linux-64     Cached
  - singular                                        4.2.1.p3  haca423c_0              installed
  + singular                                        4.1.1.p2  ha188dad_4              conda-forge/linux-64       22MB

comment:66 Changed 5 months ago by mkoeppe

There must be some outdated pins / version constraints in the conda-forge packaging. Likely related to the fact that the Sage distribution has fallen behind upstream regarding e_antic/normaliz (see #31588).

comment:67 Changed 5 months ago by mkoeppe

@isuruf, I think Sage works fine with current upstream versions of normaliz/pynormaliz that use the e_antic 1.x series, so it should be safe to remove such constraints. (We just can't yet update within the Sage distribution because of portability bugs in e_antic, see #31588.)

comment:68 in reply to: ↑ 59 Changed 5 months ago by mkoeppe

Replying to mkoeppe:

So build/pkgs/lcalc/distros/conda.txt should be changed

... to say "lcalc >=2"

comment:69 Changed 5 months ago by gh-tobiasdiez

  • Description modified (diff)

comment:70 follow-up: Changed 5 months ago by mkoeppe

Regarding the pari issue mentioned in the ticket description, we should probably use configure --with-system-pari=force so the error is signalled earlier

Let's also keep the instructions in https://wiki.sagemath.org/Conda updated.

comment:71 follow-up: Changed 5 months ago by mkoeppe

Oh, and configure --with-system-lcalc=force too...

comment:72 Changed 5 months ago by mkoeppe

  • Cc saraedum added

comment:73 Changed 5 months ago by gh-tobiasdiez

  • Description modified (diff)

comment:74 in reply to: ↑ 70 Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

Regarding the pari issue mentioned in the ticket description, we should probably use configure --with-system-pari=force so the error is signalled earlier

Let's also keep the instructions in https://wiki.sagemath.org/Conda updated.

With the basic env, its now also using pari 2.13.3.

I guess some of the optional packages is lagging behind a bit and still requiring some older version of some package. This then leads to these various downgrades of arb, ecl, pari, lcalc etc

comment:75 Changed 5 months ago by mkoeppe

Yes, and we are missing many of the version constraints that are in https://github.com/conda-forge/sage-feedstock/blob/master/recipe/meta.yaml

comment:76 Changed 5 months ago by mkoeppe

Many of these should be added to the conda.txt files, so that they will be included in our src/environment*.yml.

comment:77 Changed 5 months ago by git

  • Commit changed from 051b7b5730203843fda7a5a47bab8bbaffdf5c3c to bd652746414a344a666261af7298631769dd94c3

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

bd65274Fix test command

comment:78 Changed 5 months ago by gh-tobiasdiez

Okay, after using environment.yml the build now succeeds. The tests fail almost immediately at the import of sage.all, because primecountpy is not installed in the conda env (it is also in neither environment file).

I propose to get this ticket in and then work on the improvements to fix the various issues.

comment:79 Changed 5 months ago by gh-tobiasdiez

  • Description modified (diff)

comment:80 Changed 5 months ago by gh-tobiasdiez

  • Description modified (diff)

comment:81 Changed 5 months ago by gh-tobiasdiez

  • Description modified (diff)

comment:82 follow-up: Changed 5 months ago by mkoeppe

Just add build/pkgs/primecountpy/distros/conda.txt

comment:83 follow-up: Changed 5 months ago by mkoeppe

At the current pace, it will take months before this ticket is merged. Little point in waiting for that to happen. There's a momentum now to fix it.

comment:84 Changed 5 months ago by mkoeppe

comment:85 Changed 5 months ago by mkoeppe

Also, the ticket description claims that it follows https://wiki.sagemath.org/Conda; but that uses pip install -r src/requirements.txt, which would install the missing primecountpy too.

comment:86 Changed 5 months ago by isuruf

It's better to not use pip.

comment:87 in reply to: ↑ 83 Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

At the current pace, it will take months before this ticket is merged. Little point in waiting for that to happen. There's a momentum now to fix it.

That's sadly true, but follow-up tickets can easily depend on this ticket here to run the CI.

comment:88 Changed 5 months ago by gh-tobiasdiez

  • Description modified (diff)

comment:89 Changed 5 months ago by mkoeppe

  • Status changed from needs_review to needs_work

comment:90 in reply to: ↑ 71 Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

Oh, and configure --with-system-lcalc=force too...

Do you really think we should add a --with-system-xyz=force for every system package?

comment:91 follow-up: Changed 5 months ago by mkoeppe

I don't have a better solution at the moment, but at least there's a reasonably short way to write it as configure --with-system-{gcc,lcalc}=force...

comment:92 follow-ups: Changed 5 months ago by mkoeppe

Or generate it as $(for pkg in $(./sage -package list :standard: --has-file spkg-configure.m4); do echo --with-system-$pkg=force; done)

comment:93 Changed 5 months ago by isuruf

Why do you need --with-system-lcalc=force?

comment:94 Changed 5 months ago by mkoeppe

So that it is an immediate error if the system package is not usable. Here we are testing the workflow that does not run make at all.

comment:95 in reply to: ↑ 92 ; follow-ups: Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

Or generate it as $(for pkg in $(./sage -package list :standard: --has-file spkg-configure.m4); do echo --with-system-$pkg=force; done)

But doesn't this trigger errors as soon as a system package is rejected? That currently happens with a lot of packages as you can see in the github workflow.

Moreover, we have complete control over which versions we install in the conda systems, so one could actually skip the whole system check that ̀ configure` currently performs.

comment:96 in reply to: ↑ 95 Changed 5 months ago by mkoeppe

Replying to gh-tobiasdiez:

Replying to mkoeppe:

Or generate it as $(for pkg in $(./sage -package list :standard: --has-file spkg-configure.m4); do echo --with-system-$pkg=force; done)

But doesn't this trigger errors as soon as a system package is rejected? That currently happens with a lot of packages as you can see in the github workflow.

Yes, that's the idea. For packages that have an spkg-configure.m4, our configure script knows which versions work. It should fail early if there is something that cannot be accepted.

Last edited 5 months ago by mkoeppe (previous) (diff)

comment:97 in reply to: ↑ 95 ; follow-up: Changed 5 months ago by mkoeppe

Replying to gh-tobiasdiez:

Moreover, we have complete control over which versions we install in the conda systems, so one could actually skip the whole system check that ̀ configure` currently performs.

That is something that is best left to the downstream packagers.

comment:98 in reply to: ↑ 95 Changed 5 months ago by mkoeppe

Replying to gh-tobiasdiez:

That currently happens with a lot of packages as you can see in the github workflow.

python3 is rejected because your script doesn't follow the instructions at https://wiki.sagemath.org/Conda - --with-python=$CONDA_PREFIX/bin/python is missing.

comment:99 follow-up: Changed 5 months ago by mkoeppe

And many others fail because you use --without-system-gcc, which is a bad idea.

comment:100 Changed 5 months ago by git

  • Commit changed from bd652746414a344a666261af7298631769dd94c3 to 8a297af947f5864afb5c8d5563293cf2b9391fe4

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

8a297afRemove without gcc, add with python

comment:101 in reply to: ↑ 99 ; follow-up: Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

And many others fail because you use --without-system-gcc, which is a bad idea.

Otherwise configure fails with

 configure: error: 

    Given --with-system-gcc=force, but no system package could be used.
    That's an error.  Please install the indicated package to continue.
    (To override this error, use ./configure --without-system-gcc)

comment:102 Changed 5 months ago by git

  • Commit changed from 8a297af947f5864afb5c8d5563293cf2b9391fe4 to bd652746414a344a666261af7298631769dd94c3

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

comment:103 in reply to: ↑ 97 ; follow-up: Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

Replying to gh-tobiasdiez:

Moreover, we have complete control over which versions we install in the conda systems, so one could actually skip the whole system check that ̀ configure` currently performs.

That is something that is best left to the downstream packagers.

The section has the title "Conda for Sage Developers" so downstream packagers are not involved here.

comment:104 Changed 5 months ago by gh-tobiasdiez

  • Status changed from needs_work to needs_review

comment:105 Changed 5 months ago by gh-tobiasdiez

  • Description modified (diff)

comment:106 in reply to: ↑ 82 Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

Just add build/pkgs/primecountpy/distros/conda.txt

This is now #33330.

comment:107 Changed 5 months ago by gh-tobiasdiez

  • Description modified (diff)

comment:108 Changed 5 months ago by dimpase

  • Reviewers set to Matthias Koeppe, Dima Pasechnik
  • Status changed from needs_review to positive_review

I undestand that the GH run on #33330 uses this branch, so it looks good to me.

comment:109 Changed 5 months ago by gh-tobiasdiez

Also here, thanks for the review.

comment:110 Changed 5 months ago by gh-tobiasdiez

I've now created #33331 for the follow-up issues.

comment:111 in reply to: ↑ 103 Changed 5 months ago by mkoeppe

Replying to gh-tobiasdiez:

Replying to mkoeppe:

Replying to gh-tobiasdiez:

Moreover, we have complete control over which versions we install in the conda systems, so one could actually skip the whole system check that ̀ configure` currently performs.

That is something that is best left to the downstream packagers.

The section has the title "Conda for Sage Developers" so downstream packagers are not involved here.

Exactly, so in other words we should not attempt to skip configure's checks.

comment:112 in reply to: ↑ 101 Changed 5 months ago by mkoeppe

Replying to gh-tobiasdiez:

Replying to mkoeppe:

And many others fail because you use --without-system-gcc, which is a bad idea.

Otherwise configure fails with

 configure: error: 

    Given --with-system-gcc=force, but no system package could be used.
    That's an error.  Please install the indicated package to continue.
    (To override this error, use ./configure --without-system-gcc)

Why does it reject gcc?

comment:113 follow-up: Changed 5 months ago by mkoeppe

comment:98 still unaddressed

comment:114 in reply to: ↑ 113 Changed 5 months ago by dimpase

Replying to mkoeppe:

comment:98 still unaddressed

It is addressed in comment:100 (commit ​8a297af), no?

comment:115 Changed 5 months ago by mkoeppe

That commit is not on the current branch

comment:116 follow-up: Changed 5 months ago by gh-tobiasdiez

Yes, I had to revert that commit since the workflow didn't passed. https://github.com/sagemath/sagetrac-mirror/actions/runs/1836383017

That conda's python is rejected is #31539. And since sage is not invoking python during the workflow run, it also shouldn't matter if its rejected or not.

comment:117 follow-up: Changed 5 months ago by mkoeppe

That failure was from gcc, not python. So why back out the fix for python?

comment:118 in reply to: ↑ 116 Changed 5 months ago by mkoeppe

Replying to gh-tobiasdiez:

since sage is not invoking python during the workflow run

Hm?

comment:119 in reply to: ↑ 117 ; follow-up: Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

That failure was from gcc, not python. So why back out the fix for python?

Since it is working without this "fix".

comment:120 follow-up: Changed 5 months ago by mkoeppe

Also, could you please add some steps that package up the logs, like the three last steps of local-ubuntu in tox.yml?

Right now we cannot see what went wrong in the gcc detection, as we cannot see config.log

comment:121 in reply to: ↑ 119 ; follow-up: Changed 5 months ago by mkoeppe

Replying to gh-tobiasdiez:

Replying to mkoeppe:

That failure was from gcc, not python. So why back out the fix for python?

Since it is working without this "fix".

But what does it test? Currently certainly not installation steps that we could recommend to developers. The configure run should really reflect the system configuration.

comment:122 Changed 5 months ago by dimpase

  • Status changed from positive_review to needs_review

comment:123 Changed 5 months ago by git

  • Commit changed from bd652746414a344a666261af7298631769dd94c3 to 7b76d0b4e85640ddb7ff5427ca93270595cb3e71

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

3e49a2eMerge branch 'develop' of https://github.com/sagemath/sage into public/build/conda-runci
c1423cdForce python
7b76d0bPrint logs

comment:124 in reply to: ↑ 120 Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

Also, could you please add some steps that package up the logs, like the three last steps of local-ubuntu in tox.yml?

Right now we cannot see what went wrong in the gcc detection, as we cannot see config.log

Done.

comment:125 in reply to: ↑ 121 ; follow-up: Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

Replying to gh-tobiasdiez:

Replying to mkoeppe:

That failure was from gcc, not python. So why back out the fix for python?

Since it is working without this "fix".

But what does it test? Currently certainly not installation steps that we could recommend to developers. The configure run should really reflect the system configuration.

I still don't understand why it's so important that sage's configure script accepts conda's python, but I've added the explicit --with-python argument.

comment:126 Changed 5 months ago by dimpase

I still don't understand why it's so important that sage's configure script accepts conda's python

How else would you expect to be using Conda's Python packages in Sage, if you used another Python?

comment:127 Changed 5 months ago by gh-tobiasdiez

The workflow is completely bypassing the make-scripts of sage. So using Conda's python packages follows the 'usual' strategy:

  • Create (virtual) python environment (done by conda)
  • Install other python packages (done by conda)
  • Install sagelib into the same environment (done via pip install)

So sage is not managing the python installation nor the python environment. It is simply a normal python package that gets installed in an existing environment. In this context, it is pretty non-standard to pose more requirements at the python installation then a simple version requirement (which is checked by pip). Ideally, you wouldn't even want to call configure, a simple pip install -e src should be enough (and that's for example the workflow numpy and scipy are using), but that's of course currently not possible.

comment:128 follow-up: Changed 5 months ago by dimpase

OK, but note that ./bootstrap and ./configure still need to be run to install various things and fill in *.in templates. Passing wrong parameters to ./configure is not a good idea, IMHO.

comment:129 in reply to: ↑ 125 ; follow-up: Changed 5 months ago by mkoeppe

Replying to gh-tobiasdiez:

Since it is working without this "fix".

But what does it test? Currently certainly not installation steps that we could recommend to developers. The configure run should really reflect the system configuration.

I still don't understand why it's so important that sage's configure script accepts conda's python

As I said before in comment:70, comment:92, etc., the configure script should really be invoked with --with-system-xyz=force for all packages for which we have spkg-configure.m4.

comment:130 in reply to: ↑ description Changed 5 months ago by mkoeppe

Replying to mkoeppe:

  • Documentation doesn't mention the use of --prefix=$CONDA_PREFIX (without this things like ecl-config are not found)

For example, this one about ecl-config is the result of the misconfiguration. The configure script does not accept system ecl because of the misconfiguration --without-system-gcc. So the configuration reflects use of an ECL built using SPKG (which is not done because make is not involved).

comment:131 in reply to: ↑ 128 Changed 5 months ago by gh-tobiasdiez

Replying to dimpase:

OK, but note that ./bootstrap and ./configure still need to be run to install various things and fill in *.in templates. Passing wrong parameters to ./configure is not a good idea, IMHO.

Sure, I agree with this!

comment:132 in reply to: ↑ 129 ; follow-ups: Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

Replying to gh-tobiasdiez:

Since it is working without this "fix".

But what does it test? Currently certainly not installation steps that we could recommend to developers. The configure run should really reflect the system configuration.

I still don't understand why it's so important that sage's configure script accepts conda's python

As I said before in comment:70, comment:92, etc., the configure script should really be invoked with --with-system-xyz=force for all packages for which we have spkg-configure.m4.

Not sure if that's really a good idea or if it would be better to just exclude versions of conda packages that are known to not work. But regardless, adding these force statements doesn't currently work.

Last edited 5 months ago by gh-tobiasdiez (previous) (diff)

comment:133 in reply to: ↑ 132 ; follow-up: Changed 5 months ago by mkoeppe

Replying to gh-tobiasdiez:

adding these force statements doesn't currently work.

You reported that --with-system-gcc=force does not work; what else does not work?

comment:134 in reply to: ↑ 133 Changed 5 months ago by mkoeppe

Replying to mkoeppe:

Replying to gh-tobiasdiez:

adding these force statements doesn't currently work.

You reported that --with-system-gcc=force does not work

... and we can fix it if you show us the config.log

comment:135 Changed 5 months ago by gh-tobiasdiez

I didn't check the force argument, but ecl, arb, ntl, ppl are not recognized.

comment:136 Changed 5 months ago by mkoeppe

yes, because of the misconfigured gcc (I just explained it in comment:130)

Last edited 5 months ago by mkoeppe (previous) (diff)

comment:137 in reply to: ↑ 132 ; follow-up: Changed 5 months ago by mkoeppe

Replying to gh-tobiasdiez:

the configure script should really be invoked with --with-system-xyz=force for all packages for which we have spkg-configure.m4.

Not sure if that's really a good idea or if it would be better to just exclude versions of conda packages that are known to not work.

Both.

comment:138 Changed 5 months ago by git

  • Commit changed from 7b76d0b4e85640ddb7ff5427ca93270595cb3e71 to 9af27b5b2c797a5e90cc0986870b695147029473

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

9af27b5Fix log output

comment:139 follow-up: Changed 5 months ago by git

  • Commit changed from 9af27b5b2c797a5e90cc0986870b695147029473 to 5940d76d46fa1f5762f69f446d756a5a1ca38559

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

5940d76Try again with system gcc

comment:140 in reply to: ↑ 137 ; follow-up: Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

Replying to gh-tobiasdiez:

the configure script should really be invoked with --with-system-xyz=force for all packages for which we have spkg-configure.m4.

Not sure if that's really a good idea or if it would be better to just exclude versions of conda packages that are known to not work.

Both.

What's the point of the force except that the workflow fails early? Wouldn't it make more sense to let it run through and see what error it produces?

comment:141 in reply to: ↑ 140 ; follow-up: Changed 5 months ago by mkoeppe

Replying to gh-tobiasdiez:

What's the point of the force except that the workflow fails early?

Failing early is good.

Wouldn't it make more sense to let it run through and see what error it produces?

No, because we do not want to send developers to manually diagnose build or run problems that have already been diagnosed and codified in our configure tests.

comment:142 in reply to: ↑ 139 ; follow-ups: Changed 5 months ago by gh-tobiasdiez

Replying to git:

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

5940d76Try again with system gcc

Locally this fails, output can be found at https://github.com/sagemath/sagetrac-mirror/runs/5189468934?check_suite_focus=true in a bit.

It accepts the conda gcc if I remove the --prefix=$CONDA_PREFIX part. But then a few other packages are still not properly recognized: ecl bdw-gc libhomfly (which was the reason why I added the prefix in the first place). Nothing of this is specific to the github action and can easily be investigated after creating a conda env locally.

But can we do this error analysis and corresponding improvements please in a follow-up ticket? The version using prefix works with almost no test failures (apart from giac, which is broken and has already a ticket). Moreover, in this ticket, I tried to codify the instructions of the wiki in a github workflow. The instructions didn't work for me verbatim in a fresh clone, nor via the github action. The current version works relatively well in my eyes. I'm happy to try a different combination of commands, but then please check yourself that these work locally for you.

comment:143 in reply to: ↑ 141 ; follow-ups: Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

Replying to gh-tobiasdiez:

What's the point of the force except that the workflow fails early?

Failing early is good.

Wouldn't it make more sense to let it run through and see what error it produces?

No, because we do not want to send developers to manually diagnose build or run problems that have already been diagnosed and codified in our configure tests.

What about adding a warning annotation in the github workflow when a system package is not recognized? Then you still get the information that the config didn't work as expected but keep the benefit of seeing error messages. This should become helpful down the road, as the conda packages are quite up to date and thus may fail to be recognized for the simple reason that they are too new.

comment:144 Changed 5 months ago by mkoeppe

comment:121 - the workflow should be testing something that we can recommend to developers. As of the current branch it's "let's misconfigure creatively so that configure does not end with an error; please ignore everything that the configure script says because it does not matter. There may be errors in build and run later, YMMV."

Last edited 5 months ago by mkoeppe (previous) (diff)

comment:145 in reply to: ↑ 142 ; follow-up: Changed 5 months ago by mkoeppe

Replying to gh-tobiasdiez:

I'm happy to try a different combination of commands, but then please check yourself that these work locally for you.

I've already offered to fix the problem with --with-system-gcc=force

comment:146 in reply to: ↑ 142 ; follow-ups: Changed 5 months ago by mkoeppe

Replying to gh-tobiasdiez:

It accepts the conda gcc if I remove the --prefix=$CONDA_PREFIX part. But then a few other packages are still not properly recognized: ecl bdw-gc libhomfly (which was the reason why I added the prefix in the first place).

These reports would be more useful if you attach the config.log

comment:147 in reply to: ↑ 145 Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

Replying to gh-tobiasdiez:

I'm happy to try a different combination of commands, but then please check yourself that these work locally for you.

I've already offered to fix the problem with --with-system-gcc=force

Can you please give the full commands that create a working conda env and are able to recognize gcc and ecl. As you can see here with-system-gcc doesn't work: https://github.com/sagemath/sagetrac-mirror/runs/5189468934?check_suite_focus=true

comment:148 in reply to: ↑ 146 Changed 5 months ago by mkoeppe

Replying to mkoeppe:

Replying to gh-tobiasdiez:

It accepts the conda gcc if I remove the --prefix=$CONDA_PREFIX part. But then a few other packages are still not properly recognized: ecl bdw-gc libhomfly (which was the reason why I added the prefix in the first place).

These reports would be more useful if you attach the config.log

((Yes, it's available now at https://github.com/sagemath/sagetrac-mirror/runs/5189468934?check_suite_focus=true)

Changed 5 months ago by gh-tobiasdiez

comment:149 in reply to: ↑ 146 ; follow-up: Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

Replying to gh-tobiasdiez:

It accepts the conda gcc if I remove the --prefix=$CONDA_PREFIX part. But then a few other packages are still not properly recognized: ecl bdw-gc libhomfly (which was the reason why I added the prefix in the first place).

These reports would be more useful if you attach the config.log

Now attached to the ticket.

comment:150 in reply to: ↑ 146 Changed 5 months ago by mkoeppe

Replying to mkoeppe:

Replying to gh-tobiasdiez:

It accepts the conda gcc if I remove the --prefix=$CONDA_PREFIX part.

OK, here's why it does not accept it when the (questionable, but perhaps currently necessary) option --prefix=$CONDA_PREFIX is given: build/pkgs/gcc/spkg-configure.m4 has these lines:

    if test -f "$SAGE_LOCAL/bin/gcc"; then
        # Special value for SAGE_INSTALL_GCC if GCC is already installed
        SAGE_INSTALL_GCC=exists
        # Set yes since this implies we have already installed GCC and want to keep
        # it selected
        sage_spkg_install_gcc=yes

        # Check whether it actually works...
        # See https://trac.sagemath.org/ticket/24599
        SAGE_CHECK_BROKEN_GCC()
[...]

comment:151 Changed 5 months ago by mkoeppe

I'll work on a fix for this later today.

comment:152 in reply to: ↑ 149 ; follow-up: Changed 5 months ago by mkoeppe

Replying to gh-tobiasdiez:

Replying to mkoeppe:

These reports would be more useful if you attach the config.log

Now attached to the ticket.

Thanks. From config.log:

## --------------------------------------------------- ##
## Checking whether SageMath should install SPKG gc... ##
## --------------------------------------------------- ##
configure:19428: checking whether any of libatomic_ops is installed as or will be installed as SPKG
configure:19437: result: no
configure:19440: checking whether we run on WSL
configure:19444: result: yes; disabling using of system gc
configure:19693: no suitable system package found for SPKG gc

This is why ecl and homfly are not used.

Last edited 5 months ago by mkoeppe (previous) (diff)

comment:153 follow-up: Changed 5 months ago by dimpase

ecl is rejected because gc is rejected on WSL, see #30629: Reject system libgc that breaks ECL build on WSL

This rejection should not happen if ECL is already provided, I think. (ditto for the other consumers of gc - libhomfly)

Last edited 5 months ago by dimpase (previous) (diff)

comment:154 in reply to: ↑ 153 Changed 5 months ago by mkoeppe

Replying to dimpase:

This rejection should not happen if ECL is already provided, I think. (ditto for the other consumers of gc - libhomfly)

This one is pretty subtle. gc is used by both ECL and libhomfly. So only if BOTH can be used from the system, then we do not need to build gc itself.

comment:156 Changed 5 months ago by mkoeppe

So we need an improved test

comment:157 follow-up: Changed 5 months ago by dimpase

is it gc from Conda (it should be OK, right?)

comment:158 in reply to: ↑ 157 ; follow-up: Changed 5 months ago by gh-tobiasdiez

Replying to dimpase:

is it gc from Conda (it should be OK, right?)

Yes it's bdw-gc 8.0.4, which is probably to old. There is a PR updating to 8.0.6 but that's unmerged for quite some time now https://github.com/conda-forge/bdw-gc-feedstock/pull/17

comment:159 Changed 5 months ago by git

  • Commit changed from 5940d76d46fa1f5762f69f446d756a5a1ca38559 to 970b01e290cf96f0d5607064c10fa360bf8e720c

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

970b01eRemove conda prefx

comment:160 Changed 5 months ago by dimpase

Conda's gc would only be too old if it were built with a particular option.

They probably use it to build ecl, and it won't fly if it was broken!

comment:161 Changed 5 months ago by git

  • Commit changed from 970b01e290cf96f0d5607064c10fa360bf8e720c to 9799fd7584ff844149cc1005e6629a25cdcb8b9a

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

9799fd7build/pkgs/gcc/spkg-configure.m4: Handle the case of SAGE_LOCAL = a system directory with gcc better

comment:162 Changed 5 months ago by mkoeppe

Here's an attempt, not really tested yet

comment:163 Changed 5 months ago by git

  • Commit changed from 9799fd7584ff844149cc1005e6629a25cdcb8b9a to 757bd73e08627853311ff17680b04417b2847de5

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

dcf3ddeRevert "Remove conda prefx"
757bd73build/pkgs/gcc/spkg-configure.m4: Handle the case of SAGE_LOCAL = a system directory with gcc better (fixup)

comment:164 Changed 5 months ago by mkoeppe

Tested locally with EXTRA_CONFIGURE_ARGS=--prefix=/opt/conda tox -e docker-conda-forge-standard -- config.status

comment:165 in reply to: ↑ 91 ; follow-up: Changed 5 months ago by mkoeppe

  • Dependencies set to #33330, #33345

Replying to mkoeppe:

I don't have a better solution at the moment, but at least there's a reasonably short way to write it as configure --with-system-{gcc,lcalc}=force...

Now I have a better solution: With #33345, we can use configure --with-system-standard-packages=force.

comment:166 Changed 5 months ago by git

  • Commit changed from 757bd73e08627853311ff17680b04417b2847de5 to b4f76aa13dfc153ff811da654d0ee543d99afca7

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

613198aAdd primecountpy
566664dMerge #33330
dda3c48configure --with-system-standard-packages: New option
bc8aaa3Merge #33345
b4f76aa.github/workflows/ci-conda.yml: Use configure --with-system-standard-packages=force

comment:167 Changed 5 months ago by git

  • Commit changed from b4f76aa13dfc153ff811da654d0ee543d99afca7 to a9812f3bd9e1780f3e3f94a8a84fc89afb4953dc

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

3bbaf44bootstrap: Only use SPKG_USE_SYSTEM_STANDARD_PACKAGES for packages with spkg-configure.m4
2998104Merge #33345
a9812f3.github/workflows/ci-conda.yml: Use configure --with-system-standard-packages=force

comment:168 in reply to: ↑ 152 Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

Replying to gh-tobiasdiez:

Replying to mkoeppe:

These reports would be more useful if you attach the config.log

Now attached to the ticket.

Thanks. From config.log:

## --------------------------------------------------- ##
## Checking whether SageMath should install SPKG gc... ##
## --------------------------------------------------- ##
configure:19428: checking whether any of libatomic_ops is installed as or will be installed as SPKG
configure:19437: result: no
configure:19440: checking whether we run on WSL
configure:19444: result: yes; disabling using of system gc
configure:19693: no suitable system package found for SPKG gc

This is why ecl and homfly are not used.

That makes sense. Sorry, should have investigated this before deviating from the instructions outlined in the wiki. Thanks for your patience and investigation.

comment:169 in reply to: ↑ 165 Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

Replying to mkoeppe:

I don't have a better solution at the moment, but at least there's a reasonably short way to write it as configure --with-system-{gcc,lcalc}=force...

Now I have a better solution: With #33345, we can use configure --with-system-standard-packages=force.

https://trac.sagemath.org/ticket/30845#comment:143 ?

comment:170 in reply to: ↑ 158 Changed 5 months ago by dimpase

Replying to gh-tobiasdiez:

Replying to dimpase:

is it gc from Conda (it should be OK, right?)

Yes it's bdw-gc 8.0.4, which is probably to old. There is a PR updating to 8.0.6 but that's unmerged for quite some time now https://github.com/conda-forge/bdw-gc-feedstock/pull/17

Upstream says one has to check that

strings /usr/lib/x86_64-linux-gnu/libgc.so | grep "Wrong __data_start/_end pair"

does not find anything. But this rules out all 8.0.4 (and possibly earlier) installations of libgc, not only broken ones.

comment:171 in reply to: ↑ 143 ; follow-up: Changed 5 months ago by mkoeppe

Replying to gh-tobiasdiez:

What about adding a warning annotation in the github workflow when a system package is not recognized?

We don't have a mechanism for that, see discussion in #29499.

comment:172 in reply to: ↑ 143 ; follow-up: Changed 5 months ago by mkoeppe

Replying to gh-tobiasdiez:

the conda packages are quite up to date and thus may fail to be recognized for the simple reason that they are too new.

That's not really specific to conda-forge but also true for various cutting-edge distributions including archlinux and gentoo

comment:173 follow-ups: Changed 5 months ago by mkoeppe

How about adding another job (via matrix) that uses src/environment-optional.yml?

comment:174 Changed 5 months ago by mkoeppe

Also, adding jobs that test other python versions could probably be useful

comment:175 in reply to: ↑ 171 ; follow-up: Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

Replying to gh-tobiasdiez:

What about adding a warning annotation in the github workflow when a system package is not recognized?

We don't have a mechanism for that, see discussion in #29499.

One could add a problem matcher for "no suitable system package; standard, will be installed as an SPKG" for now.

comment:176 in reply to: ↑ 173 ; follow-up: Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

How about adding another job (via matrix) that uses src/environment-optional.yml?

Probably wait with this until this workflow here passes without errors. There is not much of an advantage in having 5 different matrix flows fail.

comment:177 in reply to: ↑ 172 ; follow-up: Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

Replying to gh-tobiasdiez:

the conda packages are quite up to date and thus may fail to be recognized for the simple reason that they are too new.

That's not really specific to conda-forge but also true for various cutting-edge distributions including archlinux and gentoo

But in this case the workflow does not simply fail because the package is not accepted, right?

comment:178 in reply to: ↑ 175 ; follow-up: Changed 5 months ago by mkoeppe

Replying to gh-tobiasdiez:

Replying to mkoeppe:

Replying to gh-tobiasdiez:

What about adding a warning annotation in the github workflow when a system package is not recognized?

We don't have a mechanism for that, see discussion in #29499.

One could add a problem matcher for "no suitable system package; standard, will be installed as an SPKG" for now.

No, the problem is that the configuration variables need to be set for the accepted system packages. https://trac.sagemath.org/ticket/29499#comment:3

comment:179 in reply to: ↑ 176 Changed 5 months ago by mkoeppe

Replying to gh-tobiasdiez:

Replying to mkoeppe:

How about adding another job (via matrix) that uses src/environment-optional.yml?

Probably wait with this until this workflow here passes without errors.

Just reduce the errors in the ptest to warnings. This would be in line with the entire portability test suite.

comment:180 in reply to: ↑ 177 Changed 5 months ago by mkoeppe

Replying to gh-tobiasdiez:

Replying to mkoeppe:

Replying to gh-tobiasdiez:

the conda packages are quite up to date and thus may fail to be recognized for the simple reason that they are too new.

That's not really specific to conda-forge but also true for various cutting-edge distributions including archlinux and gentoo

But in this case the workflow does not simply fail because the package is not accepted, right?

That's right, Sage will just build the SPKG.

comment:181 Changed 5 months ago by mkoeppe

When a Sage user/developer follows our (to-be-updated) instructions with --with-system-standard-packages=force and it gives the configure error regarding the unsuitable system package, there is a clear recourse: Find and install a version that works.

When Sage developers see this in the CI run, there is also a clear course of action: Adjust the conda-specific version constraints in conda.txt files if possible; or file a conda bug report.

Seeing error messages from using a system package that configure knows to reject does not help. It only leads to developers engaging in unproductive guesswork.

For updating Sage and our spkg-configure.m4 scripts to work with new versions, local development and testing on many platforms is necessary anyway; it does not help if there's 1 platform that already speculatively displays some errors.

comment:182 in reply to: ↑ 178 ; follow-up: Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

Replying to gh-tobiasdiez:

Replying to mkoeppe:

Replying to gh-tobiasdiez:

What about adding a warning annotation in the github workflow when a system package is not recognized?

We don't have a mechanism for that, see discussion in #29499.

One could add a problem matcher for "no suitable system package; standard, will be installed as an SPKG" for now.

No, the problem is that the configuration variables need to be set for the accepted system packages. https://trac.sagemath.org/ticket/29499#comment:3

I meant as a github annotation. Just run configure without -with-system = force. Then configure will print "no suitable system package; standard, will be installed as an SPKG", which is detected and reported in the action overview.

comment:183 in reply to: ↑ 182 ; follow-ups: Changed 5 months ago by mkoeppe

Replying to gh-tobiasdiez:

I meant as a github annotation. Just run configure without -with-system = force. Then configure will print "no suitable system package; standard, will be installed as an SPKG", which is detected and reported in the action overview.

Sure, +1 on that, that's a good idea.

comment:184 Changed 5 months ago by mkoeppe

I have some of this in tox.yml, matching configure warnings and error

comment:185 follow-up: Changed 5 months ago by git

  • Commit changed from a9812f3bd9e1780f3e3f94a8a84fc89afb4953dc to 6ff193fe4608756ee7b99b0b9f465beaab5bc7bf

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

c1c9e61Add matrix
6ff193fAdd problem matcher

comment:186 Changed 5 months ago by git

  • Commit changed from 6ff193fe4608756ee7b99b0b9f465beaab5bc7bf to 3a25ddbe43563899df90142213fc99622e8a050b

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

3a25ddbFix path to problem matcher

comment:187 Changed 5 months ago by git

  • Commit changed from 3a25ddbe43563899df90142213fc99622e8a050b to a3fd48c0337090a9a042977600eb9f2832b77c1b

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

a3fd48cImprove problem matcher

comment:188 in reply to: ↑ 183 Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

Replying to gh-tobiasdiez:

I meant as a github annotation. Just run configure without -with-system = force. Then configure will print "no suitable system package; standard, will be installed as an SPKG", which is detected and reported in the action overview.

Sure, +1 on that, that's a good idea.

Okay, this is now implemented. See https://github.com/sagemath/sagetrac-mirror/actions/runs/1852417654

comment:189 Changed 5 months ago by gh-tobiasdiez

The tests now fail with the following error:

************************************************************************
It seems that you are attempting to run Sage from an unpacked source
tarball, but you have not compiled it yet (or maybe the build has not
finished). You should run `make` in the Sage root directory first.
If you did not intend to build Sage from source, you should download
a binary tarball instead. Read README.txt for more information.
************************************************************************

Is this a consequence of me merging the latest develop branch, or did I accidentally misconfigured something? How can one overwrite this check?

comment:190 follow-up: Changed 5 months ago by gh-tobiasdiez

  • Dependencies #33330, #33345 deleted

comment:191 in reply to: ↑ 173 ; follow-up: Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

How about adding another job (via matrix) that uses src/environment-optional.yml?

Done, but it is currently failing (as reported before). Also added a minimal matrix for python versions 3.8 + 3.9

comment:192 in reply to: ↑ 185 Changed 5 months ago by mkoeppe

Replying to git:

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

c1c9e61Add matrix
6ff193fAdd problem matcher

This nuked my changes, intended?

comment:193 in reply to: ↑ 190 ; follow-up: Changed 5 months ago by mkoeppe

Replying to gh-tobiasdiez:

Dependencies #33330, #33345 deleted

?

comment:194 in reply to: ↑ 193 Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

Replying to gh-tobiasdiez:

Dependencies #33330, #33345 deleted

?

No longer needed.

comment:195 in reply to: ↑ 191 ; follow-ups: Changed 5 months ago by isuruf

Replying to gh-tobiasdiez:

Replying to mkoeppe:

How about adding another job (via matrix) that uses src/environment-optional.yml?

Done, but it is currently failing (as reported before). Also added a minimal matrix for python versions 3.8 + 3.9

Should be fixed with #33358

comment:196 in reply to: ↑ 183 Changed 5 months ago by mkoeppe

Replying to mkoeppe:

Replying to gh-tobiasdiez:

I meant as a github annotation. Just run configure without -with-system = force. Then configure will print "no suitable system package; standard, will be installed as an SPKG", which is detected and reported in the action overview.

Sure, +1 on that, that's a good idea.

Wait, I missed that you said "run configure without -with-system = force".

I was +1 on adding the problem matchers.

comment:197 follow-up: Changed 5 months ago by mkoeppe

In comment:181 I explained why we DON'T want to have it run anyway and produce mysterious errors that send developers into doing unproductive guesswork.

comment:198 Changed 5 months ago by mkoeppe

  • Status changed from needs_review to needs_work

comment:199 in reply to: ↑ 197 ; follow-ups: Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

In comment:181 I explained why we DON'T want to have it run anyway and produce mysterious errors that send developers into doing unproductive guesswork.

When Sage developers see this in the CI run, there is also a clear course of action: Adjust the conda-specific version constraints in conda.txt files if possible; or file a conda bug report.

This is still accomplished with the current problem matchers, and even easier. Developer looks at the workflow run, sees that system packages are not accepted and can take action. In addition, she can directly see what errors result from not accepting the system package.

I don't see any advantage in hiding useful information (what does break because of the missing package).

comment:200 in reply to: ↑ 195 Changed 5 months ago by gh-tobiasdiez

Replying to isuruf:

Replying to gh-tobiasdiez:

Replying to mkoeppe:

How about adding another job (via matrix) that uses src/environment-optional.yml?

Done, but it is currently failing (as reported before). Also added a minimal matrix for python versions 3.8 + 3.9

Should be fixed with #33358

Thanks!

comment:201 follow-up: Changed 5 months ago by gh-tobiasdiez

comment:202 in reply to: ↑ 199 ; follow-up: Changed 5 months ago by mkoeppe

Replying to gh-tobiasdiez:

I don't see any advantage in hiding useful information

I've already explained that it's not useful for those who know how to update our configure tests.

comment:203 in reply to: ↑ 201 ; follow-up: Changed 5 months ago by mkoeppe

Replying to gh-tobiasdiez:

Any idea on how to fix https://trac.sagemath.org/ticket/30845#comment:189?

Yes, you didn't install the Sage library where you promised that it would be installed. SAGE_LOCAL (set by --prefix) defaults to SAGE_ROOT/local and SAGE_VENV defaults to a subdirectory of it. But you installed the Sage library instead into the system site-packages.

Last edited 5 months ago by mkoeppe (previous) (diff)

comment:204 in reply to: ↑ 199 Changed 5 months ago by mkoeppe

Replying to gh-tobiasdiez:

Replying to mkoeppe:

In comment:181 I explained why we DON'T want to have it run anyway and produce mysterious errors that send developers into doing unproductive guesswork.

When Sage developers see this in the CI run, there is also a clear course of action: Adjust the conda-specific version constraints in conda.txt files if possible; or file a conda bug report.

This is still accomplished with the current problem matchers, and even easier.

But it misses the criterion of comment:121 - does it test what we would recommend to developers.

comment:205 Changed 5 months ago by mkoeppe

Also comment:178.

comment:206 in reply to: ↑ 203 Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

Replying to gh-tobiasdiez:

Any idea on how to fix https://trac.sagemath.org/ticket/30845#comment:189?

Yes, you didn't install the Sage library where you promised that it would be installed. SAGE_LOCAL (set by --prefix) defaults to SAGE_ROOT/local and SAGE_VENV defaults to a subdirectory of it. But you installed the Sage library instead into the system site-packages.

And how does one fix it without using prefix (which according to you isn't desired)?

comment:207 follow-up: Changed 5 months ago by mkoeppe

For now, using --prefix is the best option. That's why I provided commits 9799fd7/757bd73.

comment:208 Changed 5 months ago by git

  • Commit changed from a3fd48c0337090a9a042977600eb9f2832b77c1b to 8426d2d57f710d2b64c46e0bcde09ee22ed75851

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

7666019Back to using prefix for now
8a0f319Fix for rename of conda e-antic package to libeantic
c8a7b0fMerge remote-tracking branch 'trac/u/isuruf/eantic' into public/build/conda-runci
613198aAdd primecountpy
8426d2dMerge remote-tracking branch 'trac/public/build/conda-addpackages-runci' into public/build/conda-runci

comment:209 Changed 5 months ago by gh-tobiasdiez

  • Dependencies set to #33358, #33330

comment:210 in reply to: ↑ 195 Changed 5 months ago by gh-tobiasdiez

Replying to isuruf:

Replying to gh-tobiasdiez:

Replying to mkoeppe:

How about adding another job (via matrix) that uses src/environment-optional.yml?

Done, but it is currently failing (as reported before). Also added a minimal matrix for python versions 3.8 + 3.9

Should be fixed with #33358

It is indeed correctly using lcalc 2 now. Thanks!

comment:211 in reply to: ↑ 207 Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

For now, using --prefix is the best option. That's why I provided commits 9799fd7/757bd73.

Thanks. I've reverted to using --prefix (with --without-system-gcc). Let's put your improvements to the gcc detection script in a follow-up ticket.

comment:212 in reply to: ↑ 202 ; follow-ups: Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

Replying to gh-tobiasdiez:

I don't see any advantage in hiding useful information

I've already explained that it's not useful for those who know how to update our configure tests.

There are also people that don't know much about these configure scripts but still want to work on the conda environment.

The point of this ticket (at least for me), is having a CI infrastructure to confirm that updates to the conda evniroment such as #33330 and #33358 indeed have a positive impact. For this it is not helpful if the workflow exists early because some conda package got updated in the meantime and now the corresponding "system" package is rejected by sage.

To be honest, I feel tired by this discussion and think we already invested more time discussing pros and cons then developer will ever spend while chasing mysterious bugs potentially coming from this. If its so important to you, I would like to ask you to implement it in a follow-up ticket.

comment:213 follow-up: Changed 5 months ago by gh-tobiasdiez

  • Status changed from needs_work to needs_review

It's now working: build succeeds and most tests pass in all tested configs. https://github.com/sagemath/sagetrac-mirror/actions/runs/1854640215

Let's move potential improvements (like removing --prefix, --without-system-gcc, fixes to giac, updates to the docs etc) to follow-up tickets.

comment:214 in reply to: ↑ 212 Changed 5 months ago by mkoeppe

Replying to gh-tobiasdiez:

Replying to mkoeppe:

Replying to gh-tobiasdiez:

I don't see any advantage in hiding useful information

I've already explained that it's not useful for those who know how to update our configure tests.

There are also people that don't know much about these configure scripts but still want to work on the conda environment.

Yes, and they can do that as I explained in comment:181. They don't need to see the errors if they don't want to work on the configure scripts.

comment:215 in reply to: ↑ 213 ; follow-up: Changed 5 months ago by mkoeppe

  • Status changed from needs_review to needs_work

Replying to gh-tobiasdiez:

It's now working: build succeeds and most tests pass in all tested configs.

It still doesn't pass the criterion of comment:121 - does it test what we would recommend to developers.

comment:216 in reply to: ↑ 212 Changed 5 months ago by mkoeppe

Replying to gh-tobiasdiez:

If it's so important to you, I would like to ask you to implement it in a follow-up ticket.

I already did, but you removed the commits that implemented it.

comment:217 in reply to: ↑ 215 Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

Replying to gh-tobiasdiez:

It's now working: build succeeds and most tests pass in all tested configs.

It still doesn't pass the criterion of comment:121 - does it test what we would recommend to developers.

C'mon, according to your own information using --prefix is the best way currently. Without any changes to gcc detection or whatever (which is not the point of this ticket), this only works with --without-system-gcc.

comment:218 follow-up: Changed 5 months ago by isuruf

@gh-tobiasdiez, can you restore @mkoeppe's commits that fixed the gcc detection? Then we can use --prefix.

comment:219 follow-up: Changed 5 months ago by mkoeppe

I'll put these commits on #33361 now

comment:220 Changed 5 months ago by git

  • Commit changed from 8426d2d57f710d2b64c46e0bcde09ee22ed75851 to 1c2c97ed3a08904ea91dd856cb09b1032d549c30

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

d547541build/pkgs/gcc/spkg-configure.m4: Handle the case of SAGE_LOCAL = a system directory with gcc better
ce6f8fdbuild/pkgs/gcc/spkg-configure.m4: Handle the case of SAGE_LOCAL = a system directory with gcc better (fixup)
5ba1005Merge remote-tracking branch 'trac/u/mkoeppe/configure__handle_the_case_of_sage_local___a_system_directory_with_gcc_better' into public/build/conda-runci
1c2c97eRemove without-system-gcc

comment:221 in reply to: ↑ 219 Changed 5 months ago by gh-tobiasdiez

Replying to mkoeppe:

I'll put these commits on #33361 now

Thanks

comment:222 in reply to: ↑ 218 Changed 5 months ago by gh-tobiasdiez

  • Status changed from needs_work to needs_review

Replying to isuruf:

@gh-tobiasdiez, can you restore @mkoeppe's commits that fixed the gcc detection? Then we can use --prefix.

Done by merging #33361. (For the record, I initially removed these commits because it seemed like the whole prefix thing wasn't necessary).

Still a bit confused why #33361 couldn't just be a follow-up on top of this ticket, but, well, I guess I'm used to a different developing/merging strategy.

comment:223 Changed 5 months ago by gh-tobiasdiez

  • Dependencies changed from #33358, #33330 to #33358, #33330, #33361

comment:224 Changed 5 months ago by gh-tobiasdiez

Seems to work well, no gc errors anymore and all standard conda packages are detected successfully (even with the `environment-optional.yml).

So should be good to go now.

comment:225 follow-up: Changed 5 months ago by dimpase

How is there no gc error any more? What has changed in this respect?

comment:226 in reply to: ↑ 225 ; follow-up: Changed 5 months ago by gh-tobiasdiez

Replying to dimpase:

How is there no gc error any more? What has changed in this respect?

#33361 fixed it.

comment:227 in reply to: ↑ 226 Changed 5 months ago by dimpase

Replying to gh-tobiasdiez:

Replying to dimpase:

How is there no gc error any more? What has changed in this respect?

#33361 fixed it.

this must be close a miracle - as only gcc was touched there.

comment:228 follow-ups: Changed 5 months ago by mkoeppe

gc was rejected via SPKG_DEPCHECK on gcc.

comment:229 in reply to: ↑ 228 Changed 5 months ago by dimpase

Replying to mkoeppe:

gc was rejected via SPKG_DEPCHECK on gcc.

Does this mean that somehow #33361 is blessing gcc's deps? Hmm, not sure I follow.

comment:230 Changed 5 months ago by dimpase

no, that's really weird. gc is not used by gcc... OK, this is a Church of gcc spkg, and I'm a non-believer...

comment:231 in reply to: ↑ 228 Changed 5 months ago by mkoeppe

Replying to mkoeppe:

gc was rejected via SPKG_DEPCHECK on gcc.

Sorry, I was wrong - I misremembered

comment:232 Changed 5 months ago by mkoeppe

There was no problem with gc except within Tobias's WSL install.

comment:233 Changed 5 months ago by gh-tobiasdiez

Sorry for the missing c and the confusion it caused :-)

comment:234 Changed 5 months ago by dimpase

https://i.stack.imgur.com/jiFfM.jpg

comment:235 in reply to: ↑ 92 ; follow-up: Changed 4 months ago by mkoeppe

Replying to mkoeppe:

Or generate it as $(for pkg in $(./sage -package list :standard: --has-file spkg-configure.m4); do echo --with-system-$pkg=force; done)

With dependency #33361 positively reviewed, but #33345 (configure --with-system-standard-packages=force) stalled, let's use the above as a solution.

comment:236 Changed 4 months ago by mkoeppe

@isuruf - --with-system-lrcalc=force fails because conda-forge does not provide lrcalc 2.x yet

comment:238 in reply to: ↑ 235 ; follow-up: Changed 4 months ago by gh-tobiasdiez

Replying to mkoeppe:

Replying to mkoeppe:

Or generate it as $(for pkg in $(./sage -package list :standard: --has-file spkg-configure.m4); do echo --with-system-$pkg=force; done)

With dependency #33361 positively reviewed, but #33345 (configure --with-system-standard-packages=force) stalled, let's use the above as a solution.

If a system package is not used, then an error message is displayed in the github flow, so these force arguments are not necessary (or can later be added as part of #33345) .

Can we please get this ticket in?

comment:239 Changed 4 months ago by git

  • Commit changed from 1c2c97ed3a08904ea91dd856cb09b1032d549c30 to 574c6dfa3f79cdefe1996087e5567a01e32a9352

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

9ea6061Merge tag '9.6.beta5' into t/30845/public/build/conda-runci
574c6df.github/workflows/ci-conda.yml: Use configure --with-system-...=force

comment:240 in reply to: ↑ 238 ; follow-up: Changed 4 months ago by mkoeppe

Replying to gh-tobiasdiez:

Can we please get this ticket in?

Working on it, as you can see

comment:241 in reply to: ↑ 237 Changed 4 months ago by mkoeppe

Replying to slelievre:

Fixed by

Great, now I see that lrcalc 2.1. Still need https://pypi.org/project/lrcalc/ in conda-forge, or we need to install it with pip.

comment:242 in reply to: ↑ 240 Changed 4 months ago by gh-tobiasdiez

Replying to mkoeppe:

Replying to gh-tobiasdiez:

Can we please get this ticket in?

Working on it, as you can see

Can you please revert 574c6df thanks!

comment:243 Changed 4 months ago by mkoeppe

I'm also getting

Checking whether SageMath should install SPKG singular...
checking whether any of gmp ntl flint readline mpfr cddlib is installed as or will be installed as SPKG... no
checking for Singular... /usr/share/miniconda/envs/sage-build/bin/Singular
checking for SINGULAR... no
...
configure: error: 

    Given --with-system-singular=force, but no system package could be used.
    That's an error.  Please install the indicated package to continue.
    (To override this error, use ./configure --without-system-singular)

(https://github.com/mkoeppe/sage/runs/5575546912?check_suite_focus=true)

Last edited 4 months ago by mkoeppe (previous) (diff)

comment:244 Changed 4 months ago by mkoeppe

  • Authors changed from Tobias Diez to Tobias Diez, Matthias Koeppe

comment:245 Changed 4 months ago by mkoeppe

  • Status changed from needs_review to needs_work

comment:246 Changed 4 months ago by git

  • Commit changed from 574c6dfa3f79cdefe1996087e5567a01e32a9352 to a01bdd9a6d5469431e1c1ef6cdef46c9446b26eb

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

a01bdd9build/pkgs/lrcalc/distros/conda.txt: Set version bound >= 2

comment:247 Changed 4 months ago by git

  • Commit changed from a01bdd9a6d5469431e1c1ef6cdef46c9446b26eb to 739e4cd599ed50b27b17a8c5b67d4036bb1a10c8

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

739e4cdbuild/pkgs/singular/distros/conda.txt: Set version lower bound

comment:248 Changed 4 months ago by isuruf

Working on both lrcalc and singular issue. Can you add python-lrcalc as the name for conda package for lrcalc_python?

comment:249 Changed 4 months ago by mkoeppe

Will do, thanks!

comment:250 Changed 4 months ago by git

  • Commit changed from 739e4cd599ed50b27b17a8c5b67d4036bb1a10c8 to 8af68a0815367328708edbb6e1967ce9a3e01aa6

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

587f57abuild/pkgs/lrcalc_python/distros/conda.txt: New
8af68a0build/pkgs/singular/distros/conda.txt: Change version specification

comment:251 follow-up: Changed 4 months ago by isuruf

Can you also do singular>=4.2.1.p3? Apparently conda thinks 4.2.1.p3 is a development version and is <4.2.1

comment:252 Changed 4 months ago by git

  • Commit changed from 8af68a0815367328708edbb6e1967ce9a3e01aa6 to 9cbee4cbbb26273eb6cc5a19205e8f0f085a6584

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

9cbee4cbuild/pkgs/singular/distros/conda.txt: Use singular>=4.2.1.p3

comment:253 follow-up: Changed 4 months ago by gh-tobiasdiez

Replying to mkoeppe:

I'm also getting

Checking whether SageMath should install SPKG singular...
checking whether any of gmp ntl flint readline mpfr cddlib is installed as or will be installed as SPKG... no
checking for Singular... /usr/share/miniconda/envs/sage-build/bin/Singular
checking for SINGULAR... no
...
configure: error: 

    Given --with-system-singular=force, but no system package could be used.
    That's an error.  Please install the indicated package to continue.
    (To override this error, use ./configure --without-system-singular)

(https://github.com/mkoeppe/sage/runs/5575546912?check_suite_focus=true)

Can we please do this in a follow-up ticket (i.e. remove the force for now). This has nothing to do with the github action action in this ticket.

Similarly, the changes to the conda package versions / package names should go to their own tickets (based on this ticket here to check if they indeed work as intended).


New commits:

9cbee4cbuild/pkgs/singular/distros/conda.txt: Use singular>=4.2.1.p3

comment:254 in reply to: ↑ 253 ; follow-up: Changed 4 months ago by mkoeppe

Replying to gh-tobiasdiez:

Can we please do this in a follow-up ticket (i.e. remove the force for now). This has nothing to do with the github action action in this ticket.

Similarly, the changes to the conda package versions / package names should go to their own tickets (based on this ticket here to check if they indeed work as intended).

Please, Tobias, an author repeatedly demanding that the ticket goes in "as is" is not helpful and is not part of the collaborative workflow of the Sage project.

comment:255 in reply to: ↑ 251 Changed 4 months ago by mkoeppe

Replying to isuruf:

Can you also do singular>=4.2.1.p3? Apparently conda thinks 4.2.1.p3 is a development version and is <4.2.1

Thanks, this works.

comment:256 in reply to: ↑ 254 Changed 4 months ago by gh-tobiasdiez

Replying to mkoeppe:

Replying to gh-tobiasdiez:

Can we please do this in a follow-up ticket (i.e. remove the force for now). This has nothing to do with the github action action in this ticket.

Similarly, the changes to the conda package versions / package names should go to their own tickets (based on this ticket here to check if they indeed work as intended).

Please, Tobias, an author repeatedly demanding that the ticket goes in "as is" is not helpful and is not part of the collaborative workflow of the Sage project.

That's not what I'm saying. I just don't see the need that these changes, that then need further changes etc need to be part of this ticket. I'm happy to adjust the github workflow if you have any comments/suggestions on this.

comment:257 Changed 4 months ago by mkoeppe

I see that python-lrcalc 2.1 is now available in the conda-forge channel. Thanks @isuruf!

Tests running at https://github.com/mkoeppe/sage/actions/runs/1996031338

comment:258 follow-ups: Changed 4 months ago by mkoeppe

There's something wrong with libgiac, in the doctests of src/sage/calculus/calculus.py it looks like infinite recursion

comment:259 Changed 4 months ago by git

  • Commit changed from 9cbee4cbbb26273eb6cc5a19205e8f0f085a6584 to 9f8cb1d94dfaaf8c3d556df47e07fafd582d67e2

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

9f8cb1d.github/workflows/ci-conda.yml: Also test on macos-latest

comment:260 follow-up: Changed 4 months ago by mkoeppe

I have also added tests on macos-latest. This reproduces an error that I saw on my machine: ppl is rejected by the configure script.

see https://github.com/mkoeppe/sage/runs/5581501610?check_suite_focus=true

comment:261 in reply to: ↑ 260 ; follow-up: Changed 4 months ago by dimpase

Replying to mkoeppe:

I have also added tests on macos-latest. This reproduces an error that I saw on my machine: ppl is rejected by the configure script.

see https://github.com/mkoeppe/sage/runs/5581501610?check_suite_focus=true

this looks to me like a bug in m4/ppl.m4. They extract -L, etc, flags into PPL_LDFLAGS but forget to append them to LDFLAGS for testing in AC_RUN_IF_ELSE. Something like this should make it work (assuming these flags are getting into PPL_LDFLAGS at all):

  • m4/ppl.m4

    a b else 
    9696  if test "x$enable_ppltest" = xyes
    9797  then
    9898    ac_save_CPPFLAGS="$CPPFLAGS"
     99    ac_save_LDFLAGS="$LDFLAGS"
    99100    ac_save_LIBS="$LIBS"
    100101    CPPFLAGS="$CPPFLAGS $PPL_CPPFLAGS"
    101102    LIBS="$LIBS $PPL_LIBS"
     103    LDFLAGS="$LDFLAGS $PPL_LDFLAGS"
    102104
    103105dnl Now check if the installed PPL is sufficiently new.
    104106dnl (Also sanity checks the results of ppl-config to some extent.)
    main() { 
    238240
    239241    CPPFLAGS="$ac_save_CPPFLAGS"
    240242    LIBS="$ac_save_LIBS"
     243    LDFLAGS="$ac_save_LDFLAGS"
    241244  fi
    242245fi
    243246
    else 
    261264      echo "*** Could not run PPL test program, checking why..."
    262265      CPPFLAGS="$CPPFLAGS $PPL_CPPFLAGS"
    263266      LIBS="$LIBS $PPL_LIBS"
     267      LDFLAGS="$LDFLAGS $PPL_LDFLAGS"
    264268      AC_LINK_IFELSE([AC_LANG_PROGRAM([[
    265269#include <ppl.hh>
    266270using namespace Parma_Polyhedra_Library;
    using namespace Parma_Polyhedra_Library; 
    287291])
    288292      CPPFLAGS="$ac_save_CPPFLAGS"
    289293      LIBS="$ac_save_LIBS"
     294      LDFLAGS="$ac_save_LDFLAGS"
    290295    fi
    291296  fi
    292297  PPL_CPPFLAGS=""

Untested.

comment:262 Changed 4 months ago by git

  • Commit changed from 9f8cb1d94dfaaf8c3d556df47e07fafd582d67e2 to 7bb23f9208d93660c60401605f9ed59c7569477a

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

7bb23f9Try again without lower limit for singular

comment:263 Changed 4 months ago by git

  • Commit changed from 7bb23f9208d93660c60401605f9ed59c7569477a to 886d8952e21c95388207df2509c6954ff3ed66c2

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

886d895...and without lrcalc lower bound

comment:264 in reply to: ↑ 261 Changed 4 months ago by isuruf

No, it's a bug in the conda package with binary relocation. Will fix shortly.

Replying to dimpase:

Replying to mkoeppe:

I have also added tests on macos-latest. This reproduces an error that I saw on my machine: ppl is rejected by the configure script.

see https://github.com/mkoeppe/sage/runs/5581501610?check_suite_focus=true

this looks to me like a bug in m4/ppl.m4. They extract -L, etc, flags into PPL_LDFLAGS but forget

comment:265 follow-up: Changed 4 months ago by gh-tobiasdiez

I've reverted the changes to the lower bounds to singular and lrcalc since for me they were not necessary. It also looks like on github lcalc 2.1 and singular 4.2.1.p3 are used automatically. This is also expected since conda always tries to use the most recent versions and only in very rare instances lower bounds are necessary (essentially only if there are two different branches in the dependency resolution tree, and conda would choose the 'wrong' one). Why did you added these lower bounds?

comment:266 Changed 4 months ago by gh-tobiasdiez

Replying to mkoeppe:

I have also added tests on macos-latest. This reproduces an error that I saw on my machine: ppl is rejected by the configure script.

see https://github.com/mkoeppe/sage/runs/5581501610?check_suite_focus=true

And which advantages does one now gain from the fact that the github workflow already quits after the configure step, and doesn't run build and tests?

Moreover, after the change with the force parameter the github matcher is no longer working correctly since now " no suitable system package; this is an error" is displayed in the logs.

comment:267 Changed 4 months ago by git

  • Commit changed from 886d8952e21c95388207df2509c6954ff3ed66c2 to a93b23651fa91e445eefd3122e1929f1a87c4a9b

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

a93b236.github/workflows/ci-conda.yml: Reorder matrix

comment:268 in reply to: ↑ 265 Changed 4 months ago by mkoeppe

Replying to gh-tobiasdiez:

I've reverted the changes to the lower bounds to singular and lrcalc since for me they were not necessary

Yes, they are not needed any more, thanks.

comment:269 follow-up: Changed 4 months ago by mkoeppe

@isuruf, on macos with environment-optional.yml (https://github.com/mkoeppe/sage/runs/5586815094?check_suite_focus=true), I am getting an error from gdb

Warning: ERROR conda.core.link:_execute(699): An error occurred while installing package 'conda-forge::gdb-11.1-py38h36b043a_3'.
  
  ERROR conda.core.link:_execute(699): An error occurred while installing package 'conda-forge::gdb-11.1-py38h36b043a_3'.
  Rolling back transaction: ...working... done
  Warning: 
  LinkError: post-link script failed for package conda-forge::gdb-11.1-py38h36b043a_3
  location of failed script: /usr/local/miniconda/envs/sage-build/bin/.gdb-post-link.sh
  ==> script messages <==
  <None>
  ==> script output <==
  stdout: Warning: GDB is not codesigned.
  Generating and installing gdb_codesign certificate
  
  stderr: cat: /usr/local/miniconda/envs/sage-build/etc/gdb/.messages.txt: No such file or directory
  error: Something went wrong when installing the certificate
  
  return code: 1
  
  ()
  
  
  
  LinkError: post-link script failed for package conda-forge::gdb-11.1-py38h36b043a_3
  location of failed script: /usr/local/miniconda/envs/sage-build/bin/.gdb-post-link.sh
  ==> script messages <==
  <None>
  ==> script output <==
  stdout: Warning: GDB is not codesigned.
  Generating and installing gdb_codesign certificate
  
  stderr: cat: /usr/local/miniconda/envs/sage-build/etc/gdb/.messages.txt: No such file or directory
  error: Something went wrong when installing the certificate
  
  return code: 1
  

comment:270 follow-up: Changed 4 months ago by mkoeppe

@isuruf, on macos I see lots of warnings ld: warning: -pie being ignored. It is only used when linking a main executable in the doctests. I see this is coming from LDFLAGS in the conda environments. Is this something that can be fixed, or should we silence this warning in our doctest machinery?

comment:271 in reply to: ↑ 258 Changed 4 months ago by mkoeppe

Replying to mkoeppe:

There's something wrong with libgiac, in the doctests of src/sage/calculus/calculus.py it looks like infinite recursion

This one is specific to Linux; it's fine on macos.

comment:272 follow-up: Changed 4 months ago by git

  • Commit changed from a93b23651fa91e445eefd3122e1929f1a87c4a9b to ed0967c881b63c928391ce4cbe8f7bf6370d187c

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

ed0967c.github/workflows/configure-systempackage-problem-matcher.json: Also match 'no suitable system package; this is an error'

comment:273 in reply to: ↑ 272 Changed 4 months ago by gh-tobiasdiez

Replying to git:

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

ed0967c.github/workflows/configure-systempackage-problem-matcher.json: Also match 'no suitable system package; this is an error'

Error: Unable to process command '::add-matcher::.github/workflows/configure-systempackage-problem-matcher.json' successfully. Error: After parsing a value an unexpected character was encountered: {. Path 'problemMatcher[0]', line 15, position 8.

comment:274 Changed 4 months ago by gh-tobiasdiez

Is there something else that is left to do?

comment:275 in reply to: ↑ 270 Changed 4 months ago by isuruf

Replying to mkoeppe:

@isuruf, on macos I see lots of warnings ld: warning: -pie being ignored. It is only used when linking a main executable in the doctests. I see this is coming from LDFLAGS in the conda environments. Is this something that can be fixed, or should we silence this warning in our doctest machinery?

Not easy to do. It's better to silence in the doctest machinery

comment:276 Changed 4 months ago by mkoeppe

OK, I'll do that so that the noise from these warnings does not hide real failures

comment:277 in reply to: ↑ 269 ; follow-up: Changed 4 months ago by isuruf

Can you open an issue in https://github.com/conda-forge/gdb-feedstock/?

Replying to mkoeppe:

@isuruf, on macos with environment-optional.yml (https://github.com/mkoeppe/sage/runs/5586815094?check_suite_focus=true), I am getting an error from gdb

Warning: ERROR conda.core.link:_execute(699): An error occurred while installing package 'conda-forge::gdb-11.1-py38h36b043a_3'.
  
  ERROR conda.core.link:_execute(699): An error occurred while installing package 'conda-forge::gdb-11.1-py38h36b043a_3'.
  Rolling back transaction: ...working... done
  Warning: 
  LinkError: post-link script failed for package conda-forge::gdb-11.1-py38h36b043a_3
  location of failed script: /usr/local/miniconda/envs/sage-build/bin/.gdb-post-link.sh
  ==> script messages <==
  <None>
  ==> script output <==
  stdout: Warning: GDB is not codesigned.
  Generating and installing gdb_codesign certificate
  
  stderr: cat: /usr/local/miniconda/envs/sage-build/etc/gdb/.messages.txt: No such file or directory
  error: Something went wrong when installing the certificate
  
  return code: 1
  
  ()
  
  
  
  LinkError: post-link script failed for package conda-forge::gdb-11.1-py38h36b043a_3
  location of failed script: /usr/local/miniconda/envs/sage-build/bin/.gdb-post-link.sh
  ==> script messages <==
  <None>
  ==> script output <==
  stdout: Warning: GDB is not codesigned.
  Generating and installing gdb_codesign certificate
  
  stderr: cat: /usr/local/miniconda/envs/sage-build/etc/gdb/.messages.txt: No such file or directory
  error: Something went wrong when installing the certificate
  
  return code: 1
  

comment:278 Changed 4 months ago by mkoeppe

A minor thing to fix: Installing sagelib downgrades scipy, networkx via install-requires; that's now #33520.

comment:279 Changed 4 months ago by git

  • Commit changed from ed0967c881b63c928391ce4cbe8f7bf6370d187c to d7dfa69ce69c843bd000a80266276f17b9421146

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

d7dfa69sage.features.pkg_systems.SagePackageSystem: Check if there are any installation records

comment:280 Changed 4 months ago by mkoeppe

  • Dependencies changed from #33358, #33330, #33361 to #33358, #33330, #33361, #33141

comment:281 Changed 4 months ago by git

  • Commit changed from d7dfa69ce69c843bd000a80266276f17b9421146 to 6b24c05f1b19135fcd50cedd535d749a48cf024c

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

07545e9Fix sage_setup doctests
aa971bdFix doctest in sage_docbuild/__init__.py. Most of the test rely on location that are writable or the presence of documentation sources.
54bd2f5Merge branch 'develop' into distro_doctests
6c1dc29replace optional build tag with sage_spkg
18308fcget rid of the last optional build tag
e2c9eeaMerge branch 'develop' into distro_doctests
5382264make doctest in find_extra_files more robust by sorting the result so that the order is stable.
6b24c05Merge #33141

comment:282 Changed 4 months ago by git

  • Commit changed from 6b24c05f1b19135fcd50cedd535d749a48cf024c to a408a001d19fc2466fb2c468814598d2379e0290

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

a408a00.github/workflows/configure-systempackage-problem-matcher.json: Fix syntax

comment:283 Changed 4 months ago by git

  • Commit changed from a408a001d19fc2466fb2c468814598d2379e0290 to 8631ae1d1770f7e6ea5e28463c523fc6cd29e42c

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

8631ae1src/sage/doctest/parsing.py: Filter out ld warnings on macOS conda

comment:284 Changed 4 months ago by git

  • Commit changed from 8631ae1d1770f7e6ea5e28463c523fc6cd29e42c to acf80dfe0c61f4b43f13c6ec168f7ba149b3bb65

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

acf80dfbuild/pkgs/gdb/distros/conda.txt: Disable

comment:285 in reply to: ↑ 277 Changed 4 months ago by mkoeppe

Replying to isuruf:

Can you open an issue in https://github.com/conda-forge/gdb-feedstock/?

Will do. For now I've disabled it

comment:287 follow-up: Changed 4 months ago by git

  • Commit changed from acf80dfe0c61f4b43f13c6ec168f7ba149b3bb65 to b431edb380db1a4bd7fc07c5619bc350a45ca428

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

b431edb.github/workflows/configure-systempackage-problem-matcher.json: Fix 'Duplicate owner name'

comment:288 follow-up: Changed 4 months ago by mkoeppe

  • Reviewers changed from Matthias Koeppe, Dima Pasechnik to Matthias Koeppe, Dima Pasechnik, https://github.com/mkoeppe/sage/actions/runs/2000712634

comment:289 in reply to: ↑ 287 Changed 4 months ago by gh-tobiasdiez

Replying to git:

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

b431edb.github/workflows/configure-systempackage-problem-matcher.json: Fix 'Duplicate owner name'

echo "::remove-matcher owner=configure-systempackage::" in the workflow has to be probably be changed as well now.

comment:290 in reply to: ↑ 288 ; follow-up: Changed 4 months ago by gh-tobiasdiez

Btw, the new workflow is running in the sagetrac-mirror repo (https://github.com/sagemath/sagetrac-mirror/actions/workflows/ci-conda.yml) so you don't have to run this manually in your own fork.

comment:291 Changed 4 months ago by mkoeppe

macos-latest-3.8-environment: https://github.com/mkoeppe/sage/runs/5591728773?check_suite_focus=true

sage -t --random-seed=295057506920321218473335936870781003380 src/sage/env.py  # 1 doctest failed
sage -t --random-seed=295057506920321218473335936870781003380 src/sage/interfaces/expect.py  # 40 doctests failed
sage -t --random-seed=295057506920321218473335936870781003380 src/sage/interfaces/gap.py  # 75 doctests failed
sage -t --random-seed=295057506920321218473335936870781003380 src/sage/interfaces/mwrank.py  # 6 doctests failed
sage -t --random-seed=295057506920321218473335936870781003380 src/sage/interfaces/quit.py  # 6 doctests failed
sage -t --random-seed=295057506920321218473335936870781003380 src/sage/repl/interpreter.py  # 2 doctests failed
sage -t --random-seed=295057506920321218473335936870781003380 src/sage/repl/interface_magic.py  # 1 doctest failed
sage -t --random-seed=295057506920321218473335936870781003380 src/sage/repl/ipython_kernel/install.py  # 1 doctest failed
sage -t --random-seed=295057506920321218473335936870781003380 src/sage/structure/coerce_actions.pyx  # 1 doctest failed
sage -t --random-seed=295057506920321218473335936870781003380 src/sage/tests/cmdline.py  # 2 doctests failed

comment:292 Changed 4 months ago by mkoeppe

I think the many failures from sage.interfaces are the failure of too recent versions of ptyprocess (#32147).

comment:293 Changed 4 months ago by mkoeppe

@isuruf unfortunately, the only working version of ptyprocess (0.5.1) on macOS is only available on outdated python versions in conda-forge.

ptyprocess                     0.5.1          py27_0  conda-forge         
ptyprocess                     0.5.1          py34_0  conda-forge         
ptyprocess                     0.5.1          py35_0  conda-forge         
ptyprocess                     0.5.1          py36_0  conda-forge         

comment:294 Changed 4 months ago by git

  • Commit changed from b431edb380db1a4bd7fc07c5619bc350a45ca428 to 6f9a2d052cae43ba87c35c601d443ac2ca7a6b84

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

6f9a2d0src/setup.cfg.m4: Add ptyprocess to install-requires to get version constraint

comment:295 in reply to: ↑ 290 Changed 4 months ago by mkoeppe

Replying to gh-tobiasdiez:

Btw, the new workflow is running in the sagetrac-mirror repo (https://github.com/sagemath/sagetrac-mirror/actions/workflows/ci-conda.yml) so you don't have to run this manually in your own fork.

Thanks, yes, but the pipeline is pretty clogged

comment:296 Changed 4 months ago by git

  • Commit changed from 6f9a2d052cae43ba87c35c601d443ac2ca7a6b84 to 4de700d8ed1265e7781a0d2296d68cc7846c7ac4

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

4de700d.github/workflows/ci-conda.yml: Update ::remove-matcher

comment:297 Changed 4 months ago by mkoeppe

  • Reviewers changed from Matthias Koeppe, Dima Pasechnik, https://github.com/mkoeppe/sage/actions/runs/2000712634 to Matthias Koeppe, Dima Pasechnik, https://github.com/mkoeppe/sage/actions/runs/2001401113

comment:298 Changed 4 months ago by mkoeppe

The other failures on macOS are very minor.

comment:299 in reply to: ↑ 258 Changed 4 months ago by mkoeppe

Replying to mkoeppe:

There's something wrong with libgiac, in the doctests of src/sage/calculus/calculus.py it looks like infinite recursion

This failure on Linux is the remaining big issue.

comment:300 Changed 4 months ago by mkoeppe

(The log of these runs is about 250 MB uncompressed, not fun to look at.)

comment:301 Changed 4 months ago by mkoeppe

There's also some cysignals-related breakage on Linux:

2022-03-17T21:25:16.4730359Z **********************************************************************
2022-03-17T21:25:16.4812929Z File "src/sage/functions/min_max.py", line 240, in sage.functions.min_max.MaxSymbolic._evalf_
2022-03-17T21:25:16.4813319Z Failed example:
2022-03-17T21:25:16.4813740Z     sig_on_count() # check sig_on/off pairings (virtual doctest)
2022-03-17T21:25:16.4814033Z Expected:
2022-03-17T21:25:16.4814224Z     0
2022-03-17T21:25:16.4814414Z Got:
2022-03-17T21:25:16.4814603Z     2947
2022-03-17T21:25:16.4935843Z **********************************************************************
Last edited 4 months ago by mkoeppe (previous) (diff)

comment:302 follow-up: Changed 4 months ago by isuruf

comment:303 Changed 4 months ago by git

  • Commit changed from 4de700d8ed1265e7781a0d2296d68cc7846c7ac4 to 93fce5a1589fcfca91e8be56f13d15f4de646294

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

b508d1dCleanup
93fce5aContinue on error in config step

comment:304 Changed 4 months ago by gh-tobiasdiez

  • Status changed from needs_work to needs_review

Most doctests are now passing, so this should be good to go now (leaving fixes to the other tests to follow-up tickets).

comment:305 follow-ups: Changed 4 months ago by mkoeppe

continue-on-error makes no sense because the unconfigured source tree cannot be tested.

comment:306 in reply to: ↑ 302 Changed 4 months ago by mkoeppe

Replying to isuruf:

giac error should be fixed with https://github.com/conda-forge/giac-feedstock/pull/29

Thanks @isuruf, looking great.

comment:307 in reply to: ↑ 305 ; follow-up: Changed 4 months ago by mkoeppe

Replying to mkoeppe:

continue-on-error makes no sense because the unconfigured source tree cannot be tested.

Other than that, it's in good shape now. Could someone review my changes please?

comment:308 in reply to: ↑ 305 ; follow-up: Changed 4 months ago by gh-tobiasdiez

Replying to mkoeppe:

continue-on-error makes no sense because the unconfigured source tree cannot be tested.

That largely depends on the nature of the error. If the error in the config step is critical, then the build step will fail and no tests are run. Moreover, there is no harm in executing also the other steps, the workflow will still be displayed as failed due to the error in the config job. You just get a bit more information.

comment:309 in reply to: ↑ 308 ; follow-up: Changed 4 months ago by mkoeppe

Replying to gh-tobiasdiez:

Replying to mkoeppe:

continue-on-error makes no sense because the unconfigured source tree cannot be tested.

That largely depends on the nature of the error.

No, it doesn't. On error, the generated files are not generated.

comment:310 in reply to: ↑ 307 ; follow-up: Changed 4 months ago by gh-tobiasdiez

  • Reviewers changed from Matthias Koeppe, Dima Pasechnik, https://github.com/mkoeppe/sage/actions/runs/2001401113 to Matthias Koeppe, Dima Pasechnik, Tobias Diez

Replying to mkoeppe:

Replying to mkoeppe:

continue-on-error makes no sense because the unconfigured source tree cannot be tested.

Other than that, it's in good shape now. Could someone review my changes please?

I'm fine with these changes (although they probably should have been extracted to their own tickets as they don't have to do anything with the github workflow; but lets not be overly pedantic).

comment:311 Changed 4 months ago by mkoeppe

  • Summary changed from GH Actions: Add test for conda without SPKG to Fixes for the conda-for-Sage-developers installation method, add GH Actions workflow

comment:312 Changed 4 months ago by mkoeppe

  • Description modified (diff)

comment:313 Changed 4 months ago by mkoeppe

  • Description modified (diff)

comment:314 in reply to: ↑ 309 ; follow-up: Changed 4 months ago by gh-tobiasdiez

Replying to mkoeppe:

Replying to gh-tobiasdiez:

Replying to mkoeppe:

continue-on-error makes no sense because the unconfigured source tree cannot be tested.

That largely depends on the nature of the error.

No, it doesn't. On error, the generated files are not generated.

I tried it locally and the necessary files seem to be still present when with-system-SPKG=force triggers an error. Having a quick look at the code these errors are also deferred to the very end of the configure script.

comment:315 Changed 4 months ago by mkoeppe

  • Description modified (diff)

comment:316 in reply to: ↑ 314 Changed 4 months ago by mkoeppe

Replying to gh-tobiasdiez:

I tried it locally and the necessary files seem to be still present when with-system-SPKG=force triggers an error. Having a quick look at the code these errors are also deferred to the very end of the configure script.

You are right.

comment:317 Changed 4 months ago by mkoeppe

Nevertheless, continue-on-error makes no sense as I have explained throughout the thread, comment:141 etc.

comment:318 follow-up: Changed 4 months ago by mkoeppe

  • Status changed from needs_review to positive_review

But it's fine for now, let's see that we can get it merged in 9.6.

comment:319 follow-up: Changed 4 months ago by mkoeppe

Onward to documenting it, #33426?

comment:320 Changed 4 months ago by mkoeppe

  • Priority changed from major to critical

comment:321 in reply to: ↑ 310 ; follow-up: Changed 4 months ago by mkoeppe

Replying to gh-tobiasdiez:

they probably should have been extracted to their own tickets as they don't have to do anything with the github workflow

Yes, the fix to src/sage/features/pkg_systems.py could perhaps have been made a separate ticket and a dependency here. But it would have been difficult to test it separately because the present ticket is its only use case.

Sometimes the scope of a ticket needs to be adjusted during development. You already know that sometimes the scope turns out to be too big and it is necessary break out smaller, more manageable and more clearly defined tickets.

But sometimes the scope of a ticket also turns out to be too small and the ticket is not viable on its own. This is what happened on the present ticket. The workflow documented in the wiki https://wiki.sagemath.org/Conda has never actually worked. Creating a GH Actions workflow based on a semi-quasi-working variant of it, by itself was not convincing as a ticket (comment:121 - "what does it test"). With the broader scope of actually fixing this way of installing Sage and testing something that we can recommend to developers, the ticket has a viable scope: The changes bring a clear improvement to Sage.

comment:322 Changed 4 months ago by mkoeppe

Some of the test suite failures that we saw are from ipython 8.x; #33170 updates the doctests

comment:323 in reply to: ↑ 318 Changed 4 months ago by gh-tobiasdiez

Replying to mkoeppe:

But it's fine for now, let's see that we can get it merged in 9.6.

Thanks for the review!

comment:324 in reply to: ↑ 319 Changed 4 months ago by gh-tobiasdiez

Replying to mkoeppe:

Onward to documenting it, #33426?

Yes!

comment:325 in reply to: ↑ 321 ; follow-up: Changed 4 months ago by gh-tobiasdiez

Replying to mkoeppe:

Replying to gh-tobiasdiez:

they probably should have been extracted to their own tickets as they don't have to do anything with the github workflow

Yes, the fix to src/sage/features/pkg_systems.py could perhaps have been made a separate ticket and a dependency here. But it would have been difficult to test it separately because the present ticket is its only use case.

Sometimes the scope of a ticket needs to be adjusted during development. You already know that sometimes the scope turns out to be too big and it is necessary break out smaller, more manageable and more clearly defined tickets.

But sometimes the scope of a ticket also turns out to be too small and the ticket is not viable on its own. This is what happened on the present ticket. The workflow documented in the wiki https://wiki.sagemath.org/Conda has never actually worked. Creating a GH Actions workflow based on a semi-quasi-working variant of it, by itself was not convincing as a ticket (comment:121 - "what does it test"). With the broader scope of actually fixing this way of installing Sage and testing something that we can recommend to developers, the ticket has a viable scope: The changes bring a clear improvement to Sage.

In general I agree that a ticket has to have a clear scope and improvement. For this ticket here, the github workflow essentially didn't change in the past 4 weeks, and at this point it was already clear that the workflow works in principle, with the caveat that a lot of docstests failed. In my opinion, it would have been better to merge the ticket already (with the few additional improvements to the workflow that we have discussed), then follow-up tickets could have easily improved different aspects of the conda environment, while the previous runs of the ci would have served as a baseline. Even thinks like the addition of the tests for macos could have been small follow-up tickets. I really think the tendency of the sage developers to create a lot of parallel, inter-dependent tickets makes reviewing harder and slows down the process. A more linear approach would increase the dev experience and velocity in my opinion.

Anyway, I'm happy that this ticket is finally ready to be merged.

comment:326 in reply to: ↑ 325 Changed 4 months ago by mkoeppe

Replying to gh-tobiasdiez:

it was already clear that the workflow works in principle

The project generally uses a standard for tickets higher than that. Yes, there's a tradeoff between velocity and stability.

comment:327 Changed 4 months ago by isuruf

Is there a ticket for the remaining failures after this ticket?

comment:328 Changed 4 months ago by gh-tobiasdiez

Yes, they are collected in #33331.

comment:329 Changed 3 months ago by vbraun

  • Branch changed from public/build/conda-runci to 93fce5a1589fcfca91e8be56f13d15f4de646294
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.