Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

configure --enable-system-site-packages: First check all non-site packages #36382

Merged
merged 1 commit into from
Oct 8, 2023

Conversation

mkoeppe
Copy link
Member

@mkoeppe mkoeppe commented Oct 1, 2023

We first test all the packages without SAGE_PYTHON_PACKAGE_CHECK, then those with SAGE_PYTHON_PACKAGE_CHECK.

This is purely cosmetic, but possibly also useful preparation for checking for the site packages faster, using only one venv, and with less repeated output.

📝 Checklist

  • The title is concise, informative, and self-explanatory.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation accordingly.

⌛ Dependencies

@github-actions
Copy link

github-actions bot commented Oct 2, 2023

Documentation preview for this PR (built with commit 9871ad6; changes) is ready! 🎉

@orlitzky
Copy link
Contributor

orlitzky commented Oct 2, 2023

This part works fine.

I thought about trying to optimize the repeated venv creation, but in the end, wasting time by repeating checks is the autotools way. (It also keeps the logic simpler and would make failed checks easier to debug.) But I was on the fence about the tradeoffs, so if you want to do it the other way, I don't mind.

@mkoeppe
Copy link
Member Author

mkoeppe commented Oct 2, 2023

Thanks!

@vbraun vbraun merged commit 6d838ba into sagemath:develop Oct 8, 2023
15 of 16 checks passed
@mkoeppe mkoeppe added this to the sage-10.2 milestone Oct 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants