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.

Sign up for GitHub

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

Jump to bottom

build/pkgs/attrs: Change to wheel package, update dependencies #36429

Merged
merged 1 commit into from
Oct 14, 2023
Merged

build/pkgs/attrs: Change to wheel package, update dependencies #36429

merged 1 commit into from
Oct 14, 2023

Conversation

mkoeppe
Copy link
Contributor

@mkoeppe mkoeppe commented Oct 9, 2023

Fixes #36428

📝 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 self-assigned this Oct 9, 2023
@kwankyu
Copy link
Collaborator

kwankyu commented Oct 9, 2023 •
edited
Loading

Locally this is what I get

[attrs-23.1.0] Attempting to download package attrs-23.1.0-py3-none-any.whl from mirrors
[attrs-23.1.0] https://mirrors.aliyun.com/sagemath/spkg/upstream/attrs/attrs-23.1.0-py3-none-any.whl
[attrs-23.1.0] [xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx]
[attrs-23.1.0] ERROR [transfer|run:135]: [Errno socket error] [Errno 404] Not Found: '//mirrors.aliyun.com/sagemath/spkg/upstream/attrs/attrs-23.1.0-py3-none-any.whl'
[attrs-23.1.0] https://mirrors.mit.edu/sage/spkg/upstream/attrs/attrs-23.1.0-py3-none-any.whl
[attrs-23.1.0] [xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx]
[attrs-23.1.0] ERROR [transfer|run:135]: [Errno socket error] [Errno 404] Not Found: 
...
[attrs-23.1.0] https://mirror.yandex.ru/mirrors/sage.math.washington.edu/spkg/upstream/attrs/attrs-23.1.0-py3-none-any.whl
[attrs-23.1.0] [xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx]
[attrs-23.1.0] ERROR [transfer|run:135]: [Errno socket error] [Errno 404] Not Found: '//mirror.yandex.ru/mirrors/sage.math.washington.edu/spkg/upstream/attrs/attrs-23.1.0-py3-none-any.whl'
[attrs-23.1.0] https://mirrors.nju.edu.cn/sagemath/spkg/upstream/attrs/attrs-23.1.0-py3-none-any.whl
(seemingly hangs here forever...)

from make attrs after configure.

@kwankyu
Copy link
Collaborator

kwankyu commented Oct 9, 2023

Should it be attempted to install attrs by pip?

@kwankyu
Copy link
Collaborator

kwankyu commented Oct 9, 2023

attrs was installed anyway by make -j4. Just that there is no log about when and how it was installed.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 9, 2023 

seemingly hangs here forever...)

from make attrs after configure.

The upstream tarballs of various packages updated in this beta have not been uploaded to the Sage mirrors - #36381 (comment), so the Sage installer tries all mirrors including unreliable ones

@mkoeppe mkoeppe requested a review from dimpase October 9, 2023 16:22
@dimpase
Copy link
Member

dimpase commented Oct 9, 2023

are we now installing wheels as if it's normal?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 9, 2023

Platform-independent wheels, yes; any objections?

@dimpase
Copy link
Member

dimpase commented Oct 9, 2023

Can't platform-independent wheels, upon being installed, pull binary wheels of its dependencies?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 9, 2023

No. Both for normal and wheel packages, use of PyPI is disabled.

@kwankyu
Copy link
Collaborator

kwankyu commented Oct 9, 2023

The upstream tarballs of various packages updated in this beta have not been uploaded to the Sage mirrors - #36381 (comment), so the Sage installer tries all mirrors including unreliable ones

Yes. I know it. It is normal that sage fails in pulling the wheel from the sage mirrors. But after failing in that, sage (pip) successfully installed the wheel (from pypi).

Still I could not find any lines in the build log showing pip installing attrs, which puzzles me. Naturally I want to see those lines in the log to check that sage installs attrs of version 23.1.0, instead of existence of that after sage is built. Perhaps just my mistake...

@kwankyu
Copy link
Collaborator

kwankyu commented Oct 9, 2023

No. Both for normal and wheel packages, use of PyPI is disabled.

Then where does sage obtain the wheel after failing in getting it from the mirrors?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 9, 2023

from checksums.ini upstream_url

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 9, 2023

Still I could not find any lines in the build log showing pip installing attrs

It was probably installed as a side effect of installing a "pip" package (those packages have a file "requirements.txt" in their build/pkgs/SPKG/ directory.

@kwankyu
Copy link
Collaborator

kwankyu commented Oct 10, 2023 •
edited
Loading

I tried again, and saw the log :-)

...
[attrs-23.1.0] Finished installing attrs-23.1.0

All's well. (The wheel was downloaded from the upstream url, and installed by pip.)

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 10, 2023

Thank you!

vbraun pushed a commit to vbraun/sage that referenced this pull request Oct 12, 2023
sagemathgh-36429: build/pkgs/attrs: Change to wheel package, update d…
b85d719 
…ependencies

    
<!-- ^^^^^
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? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
Fixes sagemath#36428
<!-- 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.
- [ ] 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#36429
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee
vbraun pushed a commit to vbraun/sage that referenced this pull request Oct 12, 2023
sagemathgh-36429: build/pkgs/attrs: Change to wheel package, update d…
c697dde 
…ependencies

<!-- ^^^^^
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? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
Fixes sagemath#36428
<!-- 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.
- [ ] 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#36429
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee
@vbraun vbraun merged commit a84c11d into sagemath:develop Oct 14, 2023
@mkoeppe mkoeppe added this to the sage-10.2 milestone Oct 14, 2023
vbraun pushed a commit to vbraun/sage that referenced this pull request Dec 10, 2023
sagemathgh-36802: Python toolchain as wheel packages
3d75e2e 
    
<!-- ^^^^^
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 -->

We switch several pure Python packages that are part of
`PYTHON_TOOLCHAIN` from normal to wheel packages.

The only time that we carried a patch for any of these packages was in
2014–2016, a patch for pyparsing.

This reduces the complexity of our SPKGs, in line with previous PRs
sagemath#36267, sagemath#36429, sagemath#36129, sagemath#36794.



<!-- 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: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36802
Reported by: Matthias Köppe
Reviewer(s): Dima Pasechnik
vbraun pushed a commit to vbraun/sage that referenced this pull request Dec 13, 2023
sagemathgh-36802: Python toolchain as wheel packages
27a07da 
    
<!-- ^^^^^
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 -->

We switch several pure Python packages that are part of
`PYTHON_TOOLCHAIN` from normal to wheel packages.

The only time that we carried a patch for any of these packages was in
2014–2016, a patch for pyparsing.

This reduces the complexity of our SPKGs, in line with previous PRs
sagemath#36267, sagemath#36429, sagemath#36129, sagemath#36794.



<!-- 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: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36802
Reported by: Matthias Köppe
Reviewer(s): Dima Pasechnik
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kwankyu kwankyu kwankyu approved these changes

@dimpase dimpase Awaiting requested review from dimpase

Assignees

@mkoeppe mkoeppe

Labels
c: packages: standard p: blocker / 1
Projects
None yet
Milestone
sage-10.2
Development

Successfully merging this pull request may close these issues.

attrs depends on hatchling
4 participants
@mkoeppe @kwankyu @dimpase @vbraun