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: |
Description (last modified by )
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)
Change History (330)
comment:1 Changed 20 months ago by
- 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
- Description modified (diff)
comment:3 Changed 19 months ago by
- Keywords sd111 added
comment:4 Changed 19 months ago by
- Cc isuruf. gh-tobiasdiez added; isuruf removed
comment:5 Changed 19 months ago by
- Cc isuruf added; isuruf. removed
comment:6 Changed 19 months ago by
- Description modified (diff)
comment:7 Changed 19 months ago by
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?
comment:8 Changed 19 months ago by
- Branch set to public/build/conda_gh
- Commit set to 9af4614153be945d5e56e2ffc7cdddaece705f76
comment:9 Changed 19 months ago by
- Commit changed from 9af4614153be945d5e56e2ffc7cdddaece705f76 to a178b56349e645e6120832b0945a9a0e80327ce4
comment:10 Changed 19 months ago by
- Commit changed from a178b56349e645e6120832b0945a9a0e80327ce4 to 86411b841fb32b1ced8d4c8989d2f30703448c2e
comment:11 Changed 19 months ago by
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
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: ↓ 19 Changed 19 months ago by
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
- Commit changed from 86411b841fb32b1ced8d4c8989d2f30703448c2e to 741c55abee6e1cc125b16f62d56fc865982c01af
comment:15 Changed 19 months ago by
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
The lib64
looks like a conda packaging bug to me - @isuruf, can you help?
comment:17 Changed 19 months ago by
environment.yml
already includes compilers
-- I don't think adding cxx-compiler
should be necessary
comment:18 Changed 19 months ago by
- Commit changed from 741c55abee6e1cc125b16f62d56fc865982c01af to 3aba04885446291cb840e46712084185a5812eb1
comment:19 in reply to: ↑ 13 Changed 19 months ago by
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
- Dependencies changed from #28745 to #31097
comment:21 Changed 19 months ago by
- Description modified (diff)
comment:22 Changed 19 months ago by
I don't see a lib64
folder. Which package is that coming from?
comment:23 Changed 19 months ago by
It contains libcc1.so
so I guess gcc, g++ or some other compiler package.
comment:24 Changed 19 months ago by
- Commit changed from 3aba04885446291cb840e46712084185a5812eb1 to e9cb77b0c0755e08fc3c054f051d1ec1a9cd80d9
Branch pushed to git repo; I updated commit sha1. New commits:
d853f14 | Ok, then install setup.py relative to src
|
cb5e284 | Add tests
|
1cf3de4 | build/pkgs/gcc/spkg-configure.m4: Fix SAGE_BROKEN_GCC test
|
1bc9c29 | Merge 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
|
c57d615 | Correct bootstrap command
|
e85f1ba | Fix venv config
|
e29a9ab | Correct test command
|
483de62 | Install autotools
|
8237b2c | Install gettext
|
e9cb77b | Force configure to not install gcc
|
comment:25 Changed 19 months ago by
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.
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
Thanks for getting this started!
comment:27 Changed 19 months ago by
Unfortunately the runs do not give the config.log
comment:28 follow-up: ↓ 32 Changed 19 months ago by
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
- Commit changed from e9cb77b0c0755e08fc3c054f051d1ec1a9cd80d9 to 9857e36640c22e9bf7f1d072f21dfa9cd1455449
Branch pushed to git repo; I updated commit sha1. New commits:
9857e36 | Without the prefix
|
comment:30 Changed 19 months ago by
- Commit changed from 9857e36640c22e9bf7f1d072f21dfa9cd1455449 to b50ae6a7e2e4f80dafbb27785ce9345163875163
Branch pushed to git repo; I updated commit sha1. New commits:
b50ae6a | Remove conda gcc distro again
|
comment:31 Changed 19 months ago by
- Commit changed from b50ae6a7e2e4f80dafbb27785ce9345163875163 to 68080fa95aed35a35813bddb7f35fbc73a373468
Branch pushed to git repo; I updated commit sha1. New commits:
68080fa | Cleanup workflow
|
comment:32 in reply to: ↑ 28 Changed 19 months ago by
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:33 Changed 19 months ago by
This is known. https://trac.sagemath.org/ticket/30710
comment:34 Changed 19 months ago by
Ah thx for the pointer!
comment:35 Changed 19 months ago by
- 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: ↓ 38 Changed 19 months ago by
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
I have opened #31099 for this.
comment:38 in reply to: ↑ 36 Changed 19 months ago by
Replying to mkoeppe:
As
on-top-of-spkg
turns out to be a variant oflocal-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
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
- Status changed from needs_review to needs_work
red branch => needs work
comment:41 Changed 15 months ago by
- 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
- Milestone changed from sage-9.4 to sage-9.5
comment:43 Changed 8 months ago by
- 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
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
- 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
- Description modified (diff)
comment:47 Changed 6 months ago by
- Milestone changed from sage-9.5 to sage-9.6
comment:48 Changed 5 months ago by
- 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:
65e1b49 | Fix workflow syntax
|
85c31c3 | Make workflow at least try to compile sage
|
fd484d8 | Github needs sudo
|
909cd9e | Add a few missing python packages
|
a91da81 | Run in parallel
|
d4945e5 | Install gap manually
|
3171f82 | Split in more jobs
|
b87f2bc | Reuse existing bootstrap
|
eec7abc | Delete env-python from gitignore again
|
68e8330 | Try with prefix instead of building ecl and gap
|
comment:49 Changed 5 months ago by
- Description modified (diff)
comment:50 Changed 5 months ago by
- 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
- Commit changed from 68e8330a3619f0b17934f1ff3d23cb8b9c392729 to b408f9e4eaf24bcd898e33d35bc596a0ec998462
Branch pushed to git repo; I updated commit sha1. New commits:
b408f9e | Cleanup workflow
|
comment:52 Changed 5 months ago by
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
- Description modified (diff)
comment:54 Changed 5 months ago by
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: ↓ 57 Changed 5 months ago by
- Status changed from needs_review to needs_work
This is the reason for the failure. There's no mystery there.
comment:56 follow-up: ↓ 58 Changed 5 months ago by
Do you get lcalc>=2
with conda?
comment:57 in reply to: ↑ 55 ; follow-up: ↓ 60 Changed 5 months ago by
- 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
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: ↓ 61 ↓ 68 Changed 5 months ago by
So build/pkgs/lcalc/distros/conda.txt
should be changed
comment:60 in reply to: ↑ 57 ; follow-up: ↓ 62 Changed 5 months ago by
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
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
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
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
- Commit changed from b408f9e4eaf24bcd898e33d35bc596a0ec998462 to 051b7b5730203843fda7a5a47bab8bbaffdf5c3c
Branch pushed to git repo; I updated commit sha1. New commits:
051b7b5 | Don't use -optional env
|
comment:65 Changed 5 months ago by
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
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
@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
comment:69 Changed 5 months ago by
- Description modified (diff)
comment:70 follow-up: ↓ 74 Changed 5 months ago by
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: ↓ 90 Changed 5 months ago by
Oh, and configure --with-system-lcalc=force
too...
comment:72 Changed 5 months ago by
- Cc saraedum added
comment:73 Changed 5 months ago by
- Description modified (diff)
comment:74 in reply to: ↑ 70 Changed 5 months ago by
Replying to mkoeppe:
Regarding the
pari
issue mentioned in the ticket description, we should probably useconfigure --with-system-pari=force
so the error is signalled earlierLet'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
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
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
- Commit changed from 051b7b5730203843fda7a5a47bab8bbaffdf5c3c to bd652746414a344a666261af7298631769dd94c3
Branch pushed to git repo; I updated commit sha1. New commits:
bd65274 | Fix test command
|
comment:78 Changed 5 months ago by
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
- Description modified (diff)
comment:80 Changed 5 months ago by
- Description modified (diff)
comment:81 Changed 5 months ago by
- Description modified (diff)
comment:82 follow-up: ↓ 106 Changed 5 months ago by
Just add build/pkgs/primecountpy/distros/conda.txt
comment:83 follow-up: ↓ 87 Changed 5 months ago by
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
comment:70, comment:71, comment:76, comment:82 are actionable
comment:85 Changed 5 months ago by
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
It's better to not use pip.
comment:87 in reply to: ↑ 83 Changed 5 months ago by
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
- Description modified (diff)
comment:89 Changed 5 months ago by
- Status changed from needs_review to needs_work
comment:90 in reply to: ↑ 71 Changed 5 months ago by
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: ↓ 165 Changed 5 months ago by
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: ↓ 95 ↓ 235 Changed 5 months ago by
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
Why do you need --with-system-lcalc=force
?
comment:94 Changed 5 months ago by
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: ↓ 96 ↓ 97 ↓ 98 Changed 5 months ago by
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
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.
comment:97 in reply to: ↑ 95 ; follow-up: ↓ 103 Changed 5 months ago by
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
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: ↓ 101 Changed 5 months ago by
And many others fail because you use --without-system-gcc
, which is a bad idea.
comment:100 Changed 5 months ago by
- Commit changed from bd652746414a344a666261af7298631769dd94c3 to 8a297af947f5864afb5c8d5563293cf2b9391fe4
Branch pushed to git repo; I updated commit sha1. New commits:
8a297af | Remove without gcc, add with python
|
comment:101 in reply to: ↑ 99 ; follow-up: ↓ 112 Changed 5 months ago by
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
- 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: ↓ 111 Changed 5 months ago by
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
- Status changed from needs_work to needs_review
comment:105 Changed 5 months ago by
- Description modified (diff)
comment:106 in reply to: ↑ 82 Changed 5 months ago by
comment:107 Changed 5 months ago by
- Description modified (diff)
comment:108 Changed 5 months ago by
- 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
Also here, thanks for the review.
comment:110 Changed 5 months ago by
I've now created #33331 for the follow-up issues.
comment:111 in reply to: ↑ 103 Changed 5 months ago by
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
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: ↓ 114 Changed 5 months ago by
comment:98 still unaddressed
comment:114 in reply to: ↑ 113 Changed 5 months ago by
Replying to mkoeppe:
comment:98 still unaddressed
It is addressed in comment:100 (commit 8a297af), no?
comment:115 Changed 5 months ago by
That commit is not on the current branch
comment:116 follow-up: ↓ 118 Changed 5 months ago by
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: ↓ 119 Changed 5 months ago by
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
comment:119 in reply to: ↑ 117 ; follow-up: ↓ 121 Changed 5 months ago by
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: ↓ 124 Changed 5 months ago by
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: ↓ 125 Changed 5 months ago by
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
- Status changed from positive_review to needs_review
comment:123 Changed 5 months ago by
- Commit changed from bd652746414a344a666261af7298631769dd94c3 to 7b76d0b4e85640ddb7ff5427ca93270595cb3e71
comment:124 in reply to: ↑ 120 Changed 5 months ago by
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: ↓ 129 Changed 5 months ago by
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
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
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: ↓ 131 Changed 5 months ago by
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: ↓ 132 Changed 5 months ago by
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
Replying to mkoeppe:
- Documentation doesn't mention the use of
--prefix=$CONDA_PREFIX
(without this things likeecl-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
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: ↓ 133 ↓ 137 Changed 5 months ago by
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 havespkg-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.
comment:133 in reply to: ↑ 132 ; follow-up: ↓ 134 Changed 5 months ago by
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
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
I didn't check the force argument, but ecl, arb, ntl, ppl are not recognized.
comment:136 Changed 5 months ago by
yes, because of the misconfigured gcc (I just explained it in comment:130)
comment:137 in reply to: ↑ 132 ; follow-up: ↓ 140 Changed 5 months ago by
Replying to gh-tobiasdiez:
the configure script should really be invoked with
--with-system-xyz=force
for all packages for which we havespkg-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
- Commit changed from 7b76d0b4e85640ddb7ff5427ca93270595cb3e71 to 9af27b5b2c797a5e90cc0986870b695147029473
Branch pushed to git repo; I updated commit sha1. New commits:
9af27b5 | Fix log output
|
comment:139 follow-up: ↓ 142 Changed 5 months ago by
- Commit changed from 9af27b5b2c797a5e90cc0986870b695147029473 to 5940d76d46fa1f5762f69f446d756a5a1ca38559
Branch pushed to git repo; I updated commit sha1. New commits:
5940d76 | Try again with system gcc
|
comment:140 in reply to: ↑ 137 ; follow-up: ↓ 141 Changed 5 months ago by
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 havespkg-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: ↓ 143 Changed 5 months ago by
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: ↓ 145 ↓ 146 Changed 5 months ago by
Replying to git:
Branch pushed to git repo; I updated commit sha1. New commits:
5940d76 Try 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: ↓ 171 ↓ 172 Changed 5 months ago by
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
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."
comment:145 in reply to: ↑ 142 ; follow-up: ↓ 147 Changed 5 months ago by
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: ↓ 148 ↓ 149 ↓ 150 Changed 5 months ago by
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
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
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
comment:149 in reply to: ↑ 146 ; follow-up: ↓ 152 Changed 5 months ago by
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
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
I'll work on a fix for this later today.
comment:152 in reply to: ↑ 149 ; follow-up: ↓ 168 Changed 5 months ago by
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.
comment:153 follow-up: ↓ 154 Changed 5 months ago by
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
)
comment:154 in reply to: ↑ 153 Changed 5 months ago by
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:155 Changed 5 months ago by
Actually, Jammy already has fixed gc
.
https://launchpad.net/ubuntu/+source/libgc
https://bugs.launchpad.net/ubuntu/+source/libgc/+bug/1897371/comments/1
comment:156 Changed 5 months ago by
So we need an improved test
comment:157 follow-up: ↓ 158 Changed 5 months ago by
is it gc
from Conda (it should be OK, right?)
comment:158 in reply to: ↑ 157 ; follow-up: ↓ 170 Changed 5 months ago by
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
- Commit changed from 5940d76d46fa1f5762f69f446d756a5a1ca38559 to 970b01e290cf96f0d5607064c10fa360bf8e720c
Branch pushed to git repo; I updated commit sha1. New commits:
970b01e | Remove conda prefx
|
comment:160 Changed 5 months ago by
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
- Commit changed from 970b01e290cf96f0d5607064c10fa360bf8e720c to 9799fd7584ff844149cc1005e6629a25cdcb8b9a
Branch pushed to git repo; I updated commit sha1. New commits:
9799fd7 | build/pkgs/gcc/spkg-configure.m4: Handle the case of SAGE_LOCAL = a system directory with gcc better
|
comment:162 Changed 5 months ago by
Here's an attempt, not really tested yet
comment:163 Changed 5 months ago by
- Commit changed from 9799fd7584ff844149cc1005e6629a25cdcb8b9a to 757bd73e08627853311ff17680b04417b2847de5
comment:164 Changed 5 months ago by
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: ↓ 169 Changed 5 months ago by
- Dependencies set to #33330, #33345
comment:166 Changed 5 months ago by
- Commit changed from 757bd73e08627853311ff17680b04417b2847de5 to b4f76aa13dfc153ff811da654d0ee543d99afca7
comment:167 Changed 5 months ago by
- Commit changed from b4f76aa13dfc153ff811da654d0ee543d99afca7 to a9812f3bd9e1780f3e3f94a8a84fc89afb4953dc
comment:168 in reply to: ↑ 152 Changed 5 months ago by
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 gcThis is why
ecl
andhomfly
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
comment:170 in reply to: ↑ 158 Changed 5 months ago by
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: ↓ 175 Changed 5 months ago by
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: ↓ 177 Changed 5 months ago by
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: ↓ 176 ↓ 191 Changed 5 months ago by
How about adding another job (via matrix) that uses src/environment-optional.yml
?
comment:174 Changed 5 months ago by
Also, adding jobs that test other python versions could probably be useful
comment:175 in reply to: ↑ 171 ; follow-up: ↓ 178 Changed 5 months ago by
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: ↓ 179 Changed 5 months ago by
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: ↓ 180 Changed 5 months ago by
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: ↓ 182 Changed 5 months ago by
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
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
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
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: ↓ 183 Changed 5 months ago by
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: ↓ 188 ↓ 196 Changed 5 months ago by
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
I have some of this in tox.yml, matching configure warnings and error
comment:185 follow-up: ↓ 192 Changed 5 months ago by
- Commit changed from a9812f3bd9e1780f3e3f94a8a84fc89afb4953dc to 6ff193fe4608756ee7b99b0b9f465beaab5bc7bf
comment:186 Changed 5 months ago by
- Commit changed from 6ff193fe4608756ee7b99b0b9f465beaab5bc7bf to 3a25ddbe43563899df90142213fc99622e8a050b
Branch pushed to git repo; I updated commit sha1. New commits:
3a25ddb | Fix path to problem matcher
|
comment:187 Changed 5 months ago by
- Commit changed from 3a25ddbe43563899df90142213fc99622e8a050b to a3fd48c0337090a9a042977600eb9f2832b77c1b
Branch pushed to git repo; I updated commit sha1. New commits:
a3fd48c | Improve problem matcher
|
comment:188 in reply to: ↑ 183 Changed 5 months ago by
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
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: ↓ 193 Changed 5 months ago by
- Dependencies #33330, #33345 deleted
comment:191 in reply to: ↑ 173 ; follow-up: ↓ 195 Changed 5 months ago by
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
comment:193 in reply to: ↑ 190 ; follow-up: ↓ 194 Changed 5 months ago by
comment:194 in reply to: ↑ 193 Changed 5 months ago by
comment:195 in reply to: ↑ 191 ; follow-ups: ↓ 200 ↓ 210 Changed 5 months ago by
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
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: ↓ 199 Changed 5 months ago by
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
- Status changed from needs_review to needs_work
comment:199 in reply to: ↑ 197 ; follow-ups: ↓ 202 ↓ 204 Changed 5 months ago by
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
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: ↓ 203 Changed 5 months ago by
Any idea on how to fix https://trac.sagemath.org/ticket/30845#comment:189?
comment:202 in reply to: ↑ 199 ; follow-up: ↓ 212 Changed 5 months ago by
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: ↓ 206 Changed 5 months ago by
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.
comment:204 in reply to: ↑ 199 Changed 5 months ago by
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
Also comment:178.
comment:206 in reply to: ↑ 203 Changed 5 months ago by
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 toSAGE_ROOT/local
andSAGE_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: ↓ 211 Changed 5 months ago by
For now, using --prefix
is the best option. That's why I provided commits 9799fd7/757bd73.
comment:208 Changed 5 months ago by
- Commit changed from a3fd48c0337090a9a042977600eb9f2832b77c1b to 8426d2d57f710d2b64c46e0bcde09ee22ed75851
Branch pushed to git repo; I updated commit sha1. New commits:
7666019 | Back to using prefix for now
|
8a0f319 | Fix for rename of conda e-antic package to libeantic
|
c8a7b0f | Merge remote-tracking branch 'trac/u/isuruf/eantic' into public/build/conda-runci
|
613198a | Add primecountpy
|
8426d2d | Merge remote-tracking branch 'trac/public/build/conda-addpackages-runci' into public/build/conda-runci
|
comment:209 Changed 5 months ago by
- Dependencies set to #33358, #33330
comment:210 in reply to: ↑ 195 Changed 5 months ago by
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
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: ↓ 214 ↓ 216 Changed 5 months ago by
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: ↓ 215 Changed 5 months ago by
- 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
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: ↓ 217 Changed 5 months ago by
- 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
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
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: ↓ 222 Changed 5 months ago by
@gh-tobiasdiez, can you restore @mkoeppe's commits that fixed the gcc detection? Then we can use --prefix.
comment:219 follow-up: ↓ 221 Changed 5 months ago by
I'll put these commits on #33361 now
comment:220 Changed 5 months ago by
- Commit changed from 8426d2d57f710d2b64c46e0bcde09ee22ed75851 to 1c2c97ed3a08904ea91dd856cb09b1032d549c30
Branch pushed to git repo; I updated commit sha1. New commits:
d547541 | build/pkgs/gcc/spkg-configure.m4: Handle the case of SAGE_LOCAL = a system directory with gcc better
|
ce6f8fd | build/pkgs/gcc/spkg-configure.m4: Handle the case of SAGE_LOCAL = a system directory with gcc better (fixup)
|
5ba1005 | Merge remote-tracking branch 'trac/u/mkoeppe/configure__handle_the_case_of_sage_local___a_system_directory_with_gcc_better' into public/build/conda-runci
|
1c2c97e | Remove without-system-gcc
|
comment:221 in reply to: ↑ 219 Changed 5 months ago by
comment:222 in reply to: ↑ 218 Changed 5 months ago by
- 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
- Dependencies changed from #33358, #33330 to #33358, #33330, #33361
comment:224 Changed 5 months ago by
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: ↓ 226 Changed 5 months ago by
How is there no gc
error any more? What has changed in this respect?
comment:226 in reply to: ↑ 225 ; follow-up: ↓ 227 Changed 5 months ago by
comment:227 in reply to: ↑ 226 Changed 5 months ago by
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: ↓ 229 ↓ 231 Changed 5 months ago by
gc
was rejected via SPKG_DEPCHECK on gcc
.
comment:229 in reply to: ↑ 228 Changed 5 months ago by
comment:230 Changed 5 months ago by
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
comment:232 Changed 5 months ago by
There was no problem with gc
except within Tobias's WSL install.
comment:233 Changed 5 months ago by
Sorry for the missing c and the confusion it caused :-)
comment:234 Changed 5 months ago by
comment:235 in reply to: ↑ 92 ; follow-up: ↓ 238 Changed 4 months ago by
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
@isuruf - --with-system-lrcalc=force
fails because conda-forge does not provide lrcalc 2.x yet
comment:237 follow-up: ↓ 241 Changed 4 months ago by
comment:238 in reply to: ↑ 235 ; follow-up: ↓ 240 Changed 4 months ago by
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
- Commit changed from 1c2c97ed3a08904ea91dd856cb09b1032d549c30 to 574c6dfa3f79cdefe1996087e5567a01e32a9352
comment:240 in reply to: ↑ 238 ; follow-up: ↓ 242 Changed 4 months ago by
comment:241 in reply to: ↑ 237 Changed 4 months ago by
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
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
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)
comment:244 Changed 4 months ago by
comment:245 Changed 4 months ago by
- Status changed from needs_review to needs_work
comment:246 Changed 4 months ago by
- Commit changed from 574c6dfa3f79cdefe1996087e5567a01e32a9352 to a01bdd9a6d5469431e1c1ef6cdef46c9446b26eb
Branch pushed to git repo; I updated commit sha1. New commits:
a01bdd9 | build/pkgs/lrcalc/distros/conda.txt: Set version bound >= 2
|
comment:247 Changed 4 months ago by
- Commit changed from a01bdd9a6d5469431e1c1ef6cdef46c9446b26eb to 739e4cd599ed50b27b17a8c5b67d4036bb1a10c8
Branch pushed to git repo; I updated commit sha1. New commits:
739e4cd | build/pkgs/singular/distros/conda.txt: Set version lower bound
|
comment:248 Changed 4 months ago by
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
Will do, thanks!
comment:250 Changed 4 months ago by
- Commit changed from 739e4cd599ed50b27b17a8c5b67d4036bb1a10c8 to 8af68a0815367328708edbb6e1967ce9a3e01aa6
comment:251 follow-up: ↓ 255 Changed 4 months ago by
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
- Commit changed from 8af68a0815367328708edbb6e1967ce9a3e01aa6 to 9cbee4cbbb26273eb6cc5a19205e8f0f085a6584
Branch pushed to git repo; I updated commit sha1. New commits:
9cbee4c | build/pkgs/singular/distros/conda.txt: Use singular>=4.2.1.p3
|
comment:253 follow-up: ↓ 254 Changed 4 months ago by
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:
9cbee4c | build/pkgs/singular/distros/conda.txt: Use singular>=4.2.1.p3
|
comment:254 in reply to: ↑ 253 ; follow-up: ↓ 256 Changed 4 months ago by
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
Replying to isuruf:
Can you also do
singular>=4.2.1.p3
? Apparently conda thinks4.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
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
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: ↓ 271 ↓ 299 Changed 4 months ago by
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
- 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: ↓ 261 Changed 4 months ago by
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: ↓ 264 Changed 4 months ago by
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 96 96 if test "x$enable_ppltest" = xyes 97 97 then 98 98 ac_save_CPPFLAGS="$CPPFLAGS" 99 ac_save_LDFLAGS="$LDFLAGS" 99 100 ac_save_LIBS="$LIBS" 100 101 CPPFLAGS="$CPPFLAGS $PPL_CPPFLAGS" 101 102 LIBS="$LIBS $PPL_LIBS" 103 LDFLAGS="$LDFLAGS $PPL_LDFLAGS" 102 104 103 105 dnl Now check if the installed PPL is sufficiently new. 104 106 dnl (Also sanity checks the results of ppl-config to some extent.) … … main() { 238 240 239 241 CPPFLAGS="$ac_save_CPPFLAGS" 240 242 LIBS="$ac_save_LIBS" 243 LDFLAGS="$ac_save_LDFLAGS" 241 244 fi 242 245 fi 243 246 … … else 261 264 echo "*** Could not run PPL test program, checking why..." 262 265 CPPFLAGS="$CPPFLAGS $PPL_CPPFLAGS" 263 266 LIBS="$LIBS $PPL_LIBS" 267 LDFLAGS="$LDFLAGS $PPL_LDFLAGS" 264 268 AC_LINK_IFELSE([AC_LANG_PROGRAM([[ 265 269 #include <ppl.hh> 266 270 using namespace Parma_Polyhedra_Library; … … using namespace Parma_Polyhedra_Library; 287 291 ]) 288 292 CPPFLAGS="$ac_save_CPPFLAGS" 289 293 LIBS="$ac_save_LIBS" 294 LDFLAGS="$ac_save_LDFLAGS" 290 295 fi 291 296 fi 292 297 PPL_CPPFLAGS=""
Untested.
comment:262 Changed 4 months ago by
- Commit changed from 9f8cb1d94dfaaf8c3d556df47e07fafd582d67e2 to 7bb23f9208d93660c60401605f9ed59c7569477a
Branch pushed to git repo; I updated commit sha1. New commits:
7bb23f9 | Try again without lower limit for singular
|
comment:263 Changed 4 months ago by
- 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
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 intoPPL_LDFLAGS
but forget
comment:265 follow-up: ↓ 268 Changed 4 months ago by
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
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
- 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
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: ↓ 277 Changed 4 months ago by
@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: ↓ 275 Changed 4 months ago by
@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
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: ↓ 273 Changed 4 months ago by
- 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
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
Is there something else that is left to do?
comment:275 in reply to: ↑ 270 Changed 4 months ago by
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 fromLDFLAGS
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
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: ↓ 285 Changed 4 months ago by
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 gdbWarning: 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
A minor thing to fix: Installing sagelib downgrades scipy
, networkx
via install-requires; that's now #33520.
comment:279 Changed 4 months ago by
- Commit changed from ed0967c881b63c928391ce4cbe8f7bf6370d187c to d7dfa69ce69c843bd000a80266276f17b9421146
Branch pushed to git repo; I updated commit sha1. New commits:
d7dfa69 | sage.features.pkg_systems.SagePackageSystem: Check if there are any installation records
|
comment:280 Changed 4 months ago by
- Dependencies changed from #33358, #33330, #33361 to #33358, #33330, #33361, #33141
comment:281 Changed 4 months ago by
- Commit changed from d7dfa69ce69c843bd000a80266276f17b9421146 to 6b24c05f1b19135fcd50cedd535d749a48cf024c
Branch pushed to git repo; I updated commit sha1. New commits:
07545e9 | Fix sage_setup doctests
|
aa971bd | Fix doctest in sage_docbuild/__init__.py. Most of the test rely on location that are writable or the presence of documentation sources.
|
54bd2f5 | Merge branch 'develop' into distro_doctests
|
6c1dc29 | replace optional build tag with sage_spkg
|
18308fc | get rid of the last optional build tag
|
e2c9eea | Merge branch 'develop' into distro_doctests
|
5382264 | make doctest in find_extra_files more robust by sorting the result so that the order is stable.
|
6b24c05 | Merge #33141
|
comment:282 Changed 4 months ago by
- 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
- Commit changed from a408a001d19fc2466fb2c468814598d2379e0290 to 8631ae1d1770f7e6ea5e28463c523fc6cd29e42c
Branch pushed to git repo; I updated commit sha1. New commits:
8631ae1 | src/sage/doctest/parsing.py: Filter out ld warnings on macOS conda
|
comment:284 Changed 4 months ago by
- Commit changed from 8631ae1d1770f7e6ea5e28463c523fc6cd29e42c to acf80dfe0c61f4b43f13c6ec168f7ba149b3bb65
Branch pushed to git repo; I updated commit sha1. New commits:
acf80df | build/pkgs/gdb/distros/conda.txt: Disable
|
comment:285 in reply to: ↑ 277 Changed 4 months ago by
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:286 Changed 4 months ago by
comment:287 follow-up: ↓ 289 Changed 4 months ago by
- 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: ↓ 290 Changed 4 months ago by
- 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
comment:290 in reply to: ↑ 288 ; follow-up: ↓ 295 Changed 4 months ago by
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
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
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
@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
- Commit changed from b431edb380db1a4bd7fc07c5619bc350a45ca428 to 6f9a2d052cae43ba87c35c601d443ac2ca7a6b84
Branch pushed to git repo; I updated commit sha1. New commits:
6f9a2d0 | src/setup.cfg.m4: Add ptyprocess to install-requires to get version constraint
|
comment:295 in reply to: ↑ 290 Changed 4 months ago by
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
- 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
- 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
The other failures on macOS are very minor.
comment:299 in reply to: ↑ 258 Changed 4 months ago by
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
(The log of these runs is about 250 MB uncompressed, not fun to look at.)
comment:301 Changed 4 months ago by
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 **********************************************************************
comment:302 follow-up: ↓ 306 Changed 4 months ago by
giac error should be fixed with https://github.com/conda-forge/giac-feedstock/pull/29
comment:303 Changed 4 months ago by
- Commit changed from 4de700d8ed1265e7781a0d2296d68cc7846c7ac4 to 93fce5a1589fcfca91e8be56f13d15f4de646294
comment:304 Changed 4 months ago by
- 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: ↓ 307 ↓ 308 Changed 4 months ago by
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
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: ↓ 310 Changed 4 months ago by
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: ↓ 309 Changed 4 months ago by
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: ↓ 314 Changed 4 months ago by
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: ↓ 321 Changed 4 months ago by
- 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
- 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
- Description modified (diff)
comment:313 Changed 4 months ago by
- Description modified (diff)
comment:314 in reply to: ↑ 309 ; follow-up: ↓ 316 Changed 4 months ago by
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
- Description modified (diff)
comment:316 in reply to: ↑ 314 Changed 4 months ago by
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
Nevertheless, continue-on-error
makes no sense as I have explained throughout the thread, comment:141 etc.
comment:318 follow-up: ↓ 323 Changed 4 months ago by
- 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: ↓ 324 Changed 4 months ago by
Onward to documenting it, #33426?
comment:320 Changed 4 months ago by
- Priority changed from major to critical
comment:321 in reply to: ↑ 310 ; follow-up: ↓ 325 Changed 4 months ago by
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
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
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
comment:325 in reply to: ↑ 321 ; follow-up: ↓ 326 Changed 4 months ago by
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
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
Is there a ticket for the remaining failures after this ticket?
comment:328 Changed 4 months ago by
Yes, they are collected in #33331.
comment:329 Changed 3 months ago by
- Branch changed from public/build/conda-runci to 93fce5a1589fcfca91e8be56f13d15f4de646294
- Resolution set to fixed
- Status changed from positive_review to closed
Hoping we can make progress on this ticket this week - https://wiki.sagemath.org/days111