Opened 2 months ago

Closed 2 months ago

Last modified 2 months ago

#33983 closed defect (fixed)

sage_setup: Add missing dependency

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

Status badges

Description

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'

Change History (11)

comment:1 Changed 2 months ago by mkoeppe

  • Branch set to u/mkoeppe/sage_setup__add_missing_dependency

comment:2 Changed 2 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Cc fbissey added
  • Commit set to d490b8f8b2e22edb74e225e2d8878aa0f47dcac7
  • Status changed from new to needs_review

New commits:

d490b8fbuild/pkgs/sage_setup/dependencies: Add jinja2

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

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.

comment:4 Changed 2 months ago by git

  • Commit changed from d490b8f8b2e22edb74e225e2d8878aa0f47dcac7 to 1176758659e48a56fe3cf8e5a871a212f5a5f8a9

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

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

comment:5 in reply to: ↑ 3 Changed 2 months ago by mkoeppe

Replying to fbissey:

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

comment:6 Changed 2 months ago by fbissey

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

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

comment:7 Changed 2 months ago by mkoeppe

Thanks!

comment:8 Changed 2 months ago by vbraun

  • Branch changed from u/mkoeppe/sage_setup__add_missing_dependency to 1176758659e48a56fe3cf8e5a871a212f5a5f8a9
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:9 Changed 2 months ago by fbissey

  • Commit 1176758659e48a56fe3cf8e5a871a212f5a5f8a9 deleted

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 Changed 2 months ago by mkoeppe

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 Changed 2 months ago by fbissey

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

Note: See TracTickets for help on using tickets.