sagemath/sage

sage_setup: Add missing dependency

mkoeppe opened this issue · 16 comments

This is blocking the creation of sdists for PyPI

https://github.com/sagemath/sage/runs/6849918030?check_suite_focus=true

 File "/home/runner/work/sage/sage/local/var/lib/sage/venv-python3.8/lib/python3.8/site-packages/sage_setup/autogen/interpreters/utils.py", line 19, in <module>
    from jinja2 import Environment
ModuleNotFoundError: No module named 'jinja2'

CC: @kiwifb

Component: build

Author: Matthias Koeppe

Branch: 1176758

Reviewer: François Bissey

Issue created by migration from https://trac.sagemath.org/ticket/33983

Commit: d490b8f

New commits:

d490b8fbuild/pkgs/sage_setup/dependencies: Add jinja2

Author: Matthias Koeppe

comment:3

I'll say that's damning evidence. I did a quick inspection to see if something else obvious was missing but I couldn't find anything. Shouldn't it also be in requirements.txt and setup.cfg? Especially if it blocks stuff on Pypi, I would expect those to be looked at rather than dependencies which is a sage build system artifact not a pure python one.

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

1176758pkgs/sage-setup: Add jinja2 dep to requirements.txt, setup.cfg

Changed commit from d490b8f to 1176758

comment:5

Replying to @kiwifb:

Shouldn't it also be in requirements.txt and setup.cfg?

Indeed. Thanks!

Especially if it blocks stuff on Pypi, I would expect those to be looked at rather than dependencies which is a sage build system artifact not a pure python one.

We do build our sdists for PyPI by going through the Sage build system

Reviewer: François Bissey

comment:6

LGTM. Glad that I put a little bit more thought in that review.

comment:7

Thanks!

Changed commit from 1176758 to none

comment:9

In between this ticket and my previous one about the build target that was also related to autogen, it occured to me that the whole autogen should probably moved somewhere else in the near future.

I should expand, with modularization sage_setup is providing some build tools for other sage modules and possibly downstream package. autogen is only used by one of these modules at most and should therefore live with it. It is not at the present a common infrastructure used by many modules. May be you already have a ticket about this that I am unaware off?

comment:10

Yes, this is a good point. I haven't worked on it yet because I'm not 100% sure whether all of the "interpreters" that are built by autogen will end up in the same distribution package.

comment:11

That's a fair point. But I am happy that it is on your mind as well.