Opened 2 years ago

Closed 2 years ago

#30088 closed defect (fixed)

Fix "make dist", add test run to GitHub Actions

Reported by: Matthias Köppe Owned by:
Priority: critical Milestone: sage-9.2
Component: scripts Keywords:
Cc: Volker Braun, John Palmieri, Michael Orlitzky Merged in:
Authors: Matthias Koeppe Reviewers: Michael Orlitzky, John Palmieri
Report Upstream: N/A Work issues:
Branch: 160862f (Commits, GitHub, GitLab) Commit: 160862fb38046c22a4798f9aa00c6cad4415f540
Dependencies: Stopgaps:

Status badges

Description (last modified by Matthias Köppe)

make dist is currently broken.

https://github.com/mkoeppe/sage/runs/848070855

Finished cloning Sage sources
make[1]: Entering directory '/home/runner/work/sage/sage/build/make'
env SAGE_INSTALL_FETCH_ONLY=yes make -B SAGERUNTIME= \
	alabaster appnope arb babel bleach boost_cropped brial bzip2 cddlib c....
...
cd '/home/runner/work/sage/sage/build/pkgs/sage_conf' && . '/home/runner/work/sage/sage/src/bin/sage-env' && . '/home/runner/work/sage/sage/build/bin/sage-build-env-config' && sage-logger -p '/home/runner/work/sage/sage/build/pkgs/sage_conf/spkg-install' '/home/runner/work/sage/sage/logs/pkgs/sage_conf-none.log'
[sage_conf-none] Installing 
[sage_conf-none] Error: Tried to use Sage's Python which was not yet installed.
[sage_conf-none] If this was called from an spkg-install script for another 
[sage_conf-none] package you should add $(PYTHON) as a dependency in 
[sage_conf-none] build/pkgs/<pkg>/dependencies
[sage_conf-none] Error: could not determine package name
[sage_conf-none] ********************************************************************************
[sage_conf-none] Error installing
[sage_conf-none] *

On this ticket we fix it and add a test run to GH Actions so that it does not get broken again.

Related:

Change History (20)

comment:1 Changed 2 years ago by Matthias Köppe

Description: modified (diff)

comment:2 Changed 2 years ago by Matthias Köppe

Cc: John Palmieri Michael Orlitzky added

comment:3 Changed 2 years ago by Matthias Köppe

Description: modified (diff)
Priority: majorcritical
Summary: Fix "make sdist", add test run to GitHub ActionsFix "make dist", add test run to GitHub Actions

comment:4 Changed 2 years ago by Matthias Köppe

Branch: u/mkoeppe/fix__make_sdist___add_test_run_to_github_actions

comment:5 Changed 2 years ago by Michael Orlitzky

Commit: a17c655a2aac9c80a52893c5373805a233731186

Could you leave a comment in there explaining what's going on? It just took me about 20 minutes to figure out that this was failing on sage_conf because it's a standard package (which are usually incuded in SAGE_SDIST_PACKAGES) but not one whose sources need to be downloaded before being included with the source tarball (which is what the misnomer SAGE_SDIST_PACKAGES actually contains a list of). All script/pip packages should be similar, according to this change.

Also, per the description, I think we need a tox thingy.

comment:6 Changed 2 years ago by Matthias Köppe

Authors: Matthias Koeppe

I'm working on it currently

comment:7 Changed 2 years ago by Matthias Köppe

sage_conf currently is the only package that is type=standard, source=script. All other source=script packages are type=optional.

comment:8 Changed 2 years ago by git

Commit: a17c655a2aac9c80a52893c5373805a233731186de6464db6b94fcebd5f88337681994fc0eb07e4b

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

827923abootstrap: Accept 2020s in config.guess version check
c30ac54tox.ini: Add local-nobootstrap
de6464d.github/workflows/tox.yml: Add jobs "dist", "local-macos-nohomebrew

comment:9 Changed 2 years ago by Matthias Köppe

comment:10 Changed 2 years ago by Matthias Köppe

Status: newneeds_review

comment:11 Changed 2 years ago by Michael Orlitzky

Branch: u/mkoeppe/fix__make_sdist___add_test_run_to_github_actionsu/mjo/ticket/30088
Commit: de6464db6b94fcebd5f88337681994fc0eb07e4b775b7f7a8a406f9ae8aa9ef542384c060b4cbbd4

OK with an added comment? Otherwise, LGTM.


New commits:

775b7f7Trac #30088: add a few explanatory comments to m4/sage_spkg_collect.m4.

comment:12 Changed 2 years ago by Matthias Köppe

Thank you!

I'm still waiting for the test run to finish

comment:13 Changed 2 years ago by John Palmieri

Re "the only standard script package is sage_conf": sagelib is also a script package, isn't it? Or is that changing? (It is listed in build/bin/Makefile under SCRIPT_PACKAGES.)

comment:14 Changed 2 years ago by Matthias Köppe

You are right, I forgot that the ticket that made sagelib a script package was already merged.

comment:15 Changed 2 years ago by Matthias Köppe

Branch: u/mjo/ticket/30088u/mkoeppe/ticket/30088

comment:16 Changed 2 years ago by Matthias Köppe

Commit: 775b7f7a8a406f9ae8aa9ef542384c060b4cbbd4160862fb38046c22a4798f9aa00c6cad4415f540

The successful test run of the new dist workflow job can be seen at https://github.com/mkoeppe/sage/runs/864159160

It is possible that I may need to make some adjustments to the new local-macos-nohomebrew-nobootstrap tests, but I'll do that in #29929 as a follow up.

OK to set it to positive review?


New commits:

160862fm4/sage_spkg_collect.m4: Mention sagelib in the comment

comment:17 Changed 2 years ago by Michael Orlitzky

Reviewers: Michael Orlitzky
Status: needs_reviewpositive_review

Ok, you fixed the comment already. Yeah it's good now. John, please add yourself as a reviewer if you want some credit/blame here.

comment:18 Changed 2 years ago by John Palmieri

Reviewers: Michael OrlitzkyMichael Orlitzky, John Palmieri

I will confirm that, not only does mske dist work, but I turned off the internet on my computer and built Sage successfully from that tarball.

Last edited 2 years ago by John Palmieri (previous) (diff)

comment:19 Changed 2 years ago by Matthias Köppe

Thanks a lot!

comment:20 Changed 2 years ago by Volker Braun

Branch: u/mkoeppe/ticket/30088160862fb38046c22a4798f9aa00c6cad4415f540
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.