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

sage -package dependencies #37350

Merged
merged 9 commits into from
Feb 25, 2024
Merged

Conversation

mkoeppe
Copy link
Contributor

@mkoeppe mkoeppe commented Feb 15, 2024

We add a new command sage --package dependencies.
Like the command sage --package properties introduced in #37018, it can be used to eliminate direct references to build/pkgs/ from various scripts and reduce code duplication in reading SPKG metadata.

Here we only make such changes in sage-spkg-info.
The only user-visible change is that the listed dependencies are now sorted alphabetically; this hides subtle nuances that may be expressed in the sort order of some dependencies files.

Split out from:

📝 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

@mkoeppe mkoeppe requested a review from kwankyu February 16, 2024 03:15
@mkoeppe mkoeppe self-assigned this Feb 16, 2024
@kwankyu
Copy link
Collaborator

kwankyu commented Feb 18, 2024

I get

$ sage --package properties furo singular
furo:
        path:                        /Users/kwankyu/GitHub/sage-dev/build/pkgs/furo
        version_with_patchlevel:     2022.9.29
        type:                        standard
        source:                      wheel
        trees:                       SAGE_VENV
singular:
        path:                        /Users/kwankyu/GitHub/sage-dev/build/pkgs/singular
        version_with_patchlevel:     4.3.2p8
        type:                        standard
        source:                      normal
        trees:                       SAGE_LOCAL

$ sage --package dependencies furo singular
$(MP_LIBRARY)
$(PYTHON)
$(PYTHON_TOOLCHAIN)
beautifulsoup4
cddlib
flint
mpfr
ntl
pygments
readline
sphinx
sphinx_basic_ng

$ sage --package dependencies --format=shell furo singular
order_only_deps_furo='$(PYTHON_TOOLCHAIN) $(PYTHON)'
optional_deps_furo=''
runtime_deps_furo='beautifulsoup4 sphinx pygments sphinx_basic_ng'
check_deps_furo=''
order_only_deps_singular=''
optional_deps_singular=''
runtime_deps_singular='$(MP_LIBRARY) ntl flint readline mpfr cddlib'
check_deps_singular=''

It seems that the output of sage --package dependencies furo singular should be improved.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 20, 2024

It seems that the output of sage --package dependencies furo singular should be improved.

I've made this change and also moved the whole formatting of dependencies for text and RST into sage_bootstrap.

@kwankyu
Copy link
Collaborator

kwankyu commented Feb 20, 2024

Before

$ build/bin/sage-spkg-info furo

Dependencies
------------


Version Information
-------------------


Equivalent System Packages
--------------------------

(none known)

After

$ build/bin/sage-spkg-info furo
sage-spkg-info: unknown package furo

bootstrap Outdated Show resolved Hide resolved
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 20, 2024

$ build/bin/sage-spkg-info furo
sage-spkg-info: unknown package furo

Well, the tools in build/bin are not consistent regarding whether they work if build/bin is not in PATH.

@kwankyu
Copy link
Collaborator

kwankyu commented Feb 20, 2024

OK.

@mkoeppe mkoeppe force-pushed the sage_package_dependencies branch from 5053453 to c52a2c4 Compare February 21, 2024 00:21
@kwankyu
Copy link
Collaborator

kwankyu commented Feb 21, 2024

Because of unnecessary indentation in "Dependencies" section as seen here

$ ./sage-spkg-info furo
furo: A clean customizable Sphinx documentation theme

...

Type
----

standard


Dependencies
------------

                - $(PYTHON)
                - $(PYTHON_TOOLCHAIN)
                - beautifulsoup4
                - pygments
                - sphinx
                - sphinx_basic_ng

Version Information
-------------------

...

we have
Screen Shot 2024-02-21 at 2 10 54 PM
instead of (better looking)
Screen Shot 2024-02-21 at 2 12 50 PM

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 21, 2024

Thanks! Let's see if I got it right now

Copy link
Collaborator

@kwankyu kwankyu left a comment

Choose a reason for hiding this comment

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

Thanks. LGTM.

Copy link

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

@kwankyu
Copy link
Collaborator

kwankyu commented Feb 21, 2024

It works well.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 21, 2024

Thanks for the careful review!

@vbraun vbraun merged commit 0df697a into sagemath:develop Feb 25, 2024
34 of 40 checks passed
@mkoeppe mkoeppe added this to the sage-10.3 milestone Mar 7, 2024
vbraun pushed a commit to vbraun/sage that referenced this pull request Apr 1, 2024
…nts.txt`

    
<!-- ^^^^^
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 -->

As discussed in sagemath#36982:
- the name "install-requires"  has become outdated with the transition
to the new names set by the `pyproject.toml` format (see https://setupto
ols.pypa.io/en/latest/userguide/dependency_management.html#declaring-
required-dependency, compare the tabs "pyproject.toml" vs. "setup.cfg")
- this will help with sagemath#35890, as
GitHub will be able to just read the `version_requirements.txt` files

sagemath#36982 (comment)

Notes for reviewers:
- **This is a simple, limited-scope improvement and not intended as a
long-term commitment to the format of the files in build/pkgs/....**
- PRs sagemath#37430, sagemath#37350, sagemath#36740 remove direct access to build/pkgs in favor
of using the sage_bootstrap API (sage --package).

<!-- Why is this change required? What problem does it solve? -->
<!-- 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: ...
-->
- Depends on sagemath#37401

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36999
Reported by: Matthias Köppe
Reviewer(s): Dima Pasechnik, Kwankyu Lee, Nathan Dunfield
vbraun pushed a commit to vbraun/sage that referenced this pull request Apr 8, 2024
…nts.txt`

    
<!-- ^^^^^
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 -->

As discussed in sagemath#36982:
- the name "install-requires"  has become outdated with the transition
to the new names set by the `pyproject.toml` format (see https://setupto
ols.pypa.io/en/latest/userguide/dependency_management.html#declaring-
required-dependency, compare the tabs "pyproject.toml" vs. "setup.cfg")
- this will help with sagemath#35890, as
GitHub will be able to just read the `version_requirements.txt` files

sagemath#36982 (comment)

Notes for reviewers:
- **This is a simple, limited-scope improvement and not intended as a
long-term commitment to the format of the files in build/pkgs/....**
- PRs sagemath#37430, sagemath#37350, sagemath#36740 remove direct access to build/pkgs in favor
of using the sage_bootstrap API (sage --package).

<!-- Why is this change required? What problem does it solve? -->
<!-- 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: ...
-->
- Depends on sagemath#37401

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36999
Reported by: Matthias Köppe
Reviewer(s): Dima Pasechnik, Kwankyu Lee, Nathan Dunfield
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.

sage-package: Add commands "dependencies", "trees"
3 participants