Skip to content

sage --package list: Sort output, add switches --{in,ex}clude-dependencies#36393

Merged
vbraun merged 2 commits intosagemath:developfrom
mkoeppe:sage_package_list_dependencies
Oct 14, 2023
Merged

sage --package list: Sort output, add switches --{in,ex}clude-dependencies#36393
vbraun merged 2 commits intosagemath:developfrom
mkoeppe:sage_package_list_dependencies

Conversation

@mkoeppe
Copy link
Copy Markdown
Contributor

@mkoeppe mkoeppe commented Oct 3, 2023

We enhance the command sage --package list:

  • New switches --{in,ex}clude-dependencies walk the tree of dependencies declared in dependencies files
  • Output is always (uniquified and) sorted, eliminating the use of sort in various scripts
  • Handling of --exclude is moved to the end, which improves its applicability.

The new parsing of the dependencies* files in sage_bootstrap is generic preparation for handling more of the bootstrapping phase in Python, as sketched in #33860 (part of #29146).

This PR is immediate preparation for #35593, where it is desired to create "minimized" lists of system packages that do not include all standard packages that are really only needed as dependencies of "top-level" packages.

📝 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
Copy Markdown

github-actions bot commented Oct 3, 2023

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

@orlitzky
Copy link
Copy Markdown
Contributor

orlitzky commented Oct 4, 2023

Do we have a notion of "top level" package yet, or is that TBD? I ask because things like gengetopt and patch are listed in the output of sage --package list --exclude-dependencies :standard: and I'm fairly sure that those are only needed as dependencies of other things that sage needs.

Unrelated, this outputs a list of packages,

$ ./sage --package list --exclude-dependencies :all:

but this,

$ ./sage --package list --exclude-dependencies

outputs nothing. IIUC they should do the same thing?

@mkoeppe
Copy link
Copy Markdown
Contributor Author

mkoeppe commented Oct 4, 2023

Do we have a notion of "top level" package yet, or is that TBD?

No, this cannot be declared yet (any thoughts?); for use in #35593 I just with --exclude-dependencies to eliminate packages that will be pulled in by others.

I ask because things like gengetopt and patch are listed in the output of sage --package list --exclude-dependencies :standard: and I'm fairly sure that those are only needed as dependencies of other things that sage needs.

gengetopt and patch are order-only dependencies, but the new switches only deal with proper dependencies.

Unrelated, this outputs a list of packages,

$ ./sage --package list --exclude-dependencies :all:

but this,

$ ./sage --package list --exclude-dependencies

outputs nothing. IIUC they should do the same thing?

Without the new switches, the default is indeed :all:, which is nice for interactive use. But with the new switches given, I have changed the default to empty, which works better for scripting uses.

@orlitzky
Copy link
Copy Markdown
Contributor

orlitzky commented Oct 5, 2023

Do we have a notion of "top level" package yet, or is that TBD?

No, this cannot be declared yet (any thoughts?); for use in #35593 I just with --exclude-dependencies to eliminate packages that will be pulled in by others.

It's hard to do properly without writing a package manager. The entries in build/pkgs/sagelib/dependencies are a good start, but there are lots of runtime dependencies missing, and the $(FOO) deps to worry about, and the things controlled by ./configure --disable-foo...

We may wind up with a hard-coded list for a while., but at least the distribution maintainers have already come up with that list (they should all be roughly the same).

Without the new switches, the default is indeed :all:, which is nice for interactive use. But with the new switches given, I have changed the default to empty, which works better for scripting uses.

Can the --help output mention this then? That's the only documentation we have.

@mkoeppe
Copy link
Copy Markdown
Contributor Author

mkoeppe commented Oct 6, 2023

Without the new switches, the default is indeed :all:, which is nice for interactive use. But with the new switches given, I have changed the default to empty, which works better for scripting uses.

Can the --help output mention this then?

Good idea, done

orlitzky
orlitzky approved these changes Oct 7, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@orlitzky orlitzky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that this in-between approach is how I'd do it: if we came up with that list of direct dependencies now, we might also be able to find a way to hide the other packages from ./configure and make. That would be nice for conda and the distros because the package manager install instructions would contain only a minimal list, and ./configure would be able to skip a few hundred pointless checks for non-direct dependencies.

Still, this doesn't hurt anything, so it's OK too.

@mkoeppe
Copy link
Copy Markdown
Contributor Author

mkoeppe commented Oct 7, 2023

Thanks!

@vbraun
Copy link
Copy Markdown
Member

vbraun commented Oct 8, 2023

Merge conflict

@vbraun vbraun merged commit 699dbf1 into sagemath:develop Oct 14, 2023
vbraun pushed a commit to vbraun/sage that referenced this pull request Oct 19, 2023
sagemathgh-36476: Fix bootstrapping on centos-7, fedora-30, `ubun…
c47788b 
…tu-xenial`

<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

<!-- Why is this change required? What problem does it solve? -->
On `centos-7`, still supported until June 2024, we have to use sage-
bootstrap-python = python2.
Support for Python 2 was broken accidentally in sagemath#36393, see https://gith
ub.com/sagemath/sage/actions/runs/6520548150/job/17767608828#step:10:192
7

Fixed here, as tested with `tox -e docker-centos-7-devtoolset-
gcc_11-standard -- config.status`.

Same issue also affects `fedora-30`, which is still tested by the CI as
a proxy for old RHEL, and `ubuntu-xenial`. See sagemath#32074

<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

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

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->

URL: sagemath#36476
Reported by: Matthias Köppe
Reviewer(s): Tobias Diez
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dimpase dimpase Awaiting requested review from dimpase

@jhpalmieri jhpalmieri Awaiting requested review from jhpalmieri

1 more reviewer

@orlitzky orlitzky orlitzky approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@mkoeppe mkoeppe

Labels

c: scripts

Projects

None yet

Milestone

sage-10.2

Development

Successfully merging this pull request may close these issues.

3 participants

@mkoeppe @orlitzky @vbraun