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

build/pkgs/python3: Update to 3.12.4; declare support of Python 3.12.x stable #37834

Merged
merged 6 commits into from
Jun 22, 2024

Conversation

mkoeppe
Copy link
Contributor

@mkoeppe mkoeppe commented Apr 19, 2024

https://docs.python.org/3/whatsnew/changelog.html#python-3-12-4-final

📝 Checklist

  • The title is concise and informative.
  • 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 and checked the documentation preview.

⌛ Dependencies

Copy link

github-actions bot commented Apr 19, 2024

Documentation preview for this PR (built with commit 2d9dba3; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@tornaria
Copy link
Contributor

LGTM but I have not tested and there are failing CI here.

BTW, it seems to me that there is no value on having 7 separate commits here. Besides, the first commits seem they would be broken as updating python without removing the patches possibly leads to the patches not applying cleanly.

In any case, this looks like a simple way to update python in sage-the-distro, so thanks.

FTR, I've been shipping sagemath with python 3.12 without problems since October.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 21, 2024

Some of the commits are cherry-picked from elsewhere. Not squashing it makes it easier for me to track what has been merged.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 21, 2024

The "CI Linux Incremental" runs include some failures from unrelated problems. I'll look at it carefully in the next days. I have also run the full portability tests from a branch that has merged a number of other upgrade PRs, I'll inspect it as well. That's https://github.com/mkoeppe/sage/actions/runs/8761823524

@tornaria
Copy link
Contributor

tornaria commented Apr 21, 2024

Some of the commits are cherry-picked from elsewhere. Not squashing it makes it easier for me to track what has been merged.

I understand, but as a general rule, isn't it better to squash in working units (e.g. update version + adjust patches) so each commit is "correct"? OTOH, I understand the way sagemath works the "minimal unit of work" is a PR as merged by Volker.

For your problem, I think if you reorder and/or squash some commits, and later rebase your other branch on top of this one, it should do "the right thing". I do it all the time since we have rules about this on the void-packages repo (merges are not allowed and several commits to the same package have to be squashed together, so we edit the history a lot). If you want to make sure, you can rebase your other branch before squashing (or you squash on your other branch and then cherry-pick again).

EDIT: added a missing question mark in my first sentence above.


About the PR itself, I looked casually at the CI and you seem to be right. If you trust the CI, feel free to bump this. You have much better feeling for the CI, so I trust you on this.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 21, 2024

as a general rule, isn't it better to squash in working units (e.g. update version + adjust patches) so each commit is "correct"?

No, neither is there such a general rule, nor do we have such a policy as a project.

I'd say it's best to leave it to experienced users of version control systems to do what works best for them.

I understand the way sagemath works the "minimal unit of work" is a PR as merged by Volker.

Exactly.

@tornaria
Copy link
Contributor

as a general rule, isn't it better to squash in working units (e.g. update version + adjust patches) so each commit is "correct"?

No, neither is there such a general rule, nor do we have such a policy as a project.

We should strive to have a nice clean history that is useful in the future. Do you use git blame regularly? IME the history of sagemath is not very easy to work with, and some easy rules as above would go a long way, together with a judicious use of rebase and squash, and less reliance on merges (especially the ones done on the gh web interface).

I'd say it's best to leave it to experienced users of version control systems to do what works best for them.

No, because sagemath is not a single person project. This is similar to code style: we can have different ideas on what works best for an individual, but for collaboration is best to have some common ground.

I understand the way sagemath works the "minimal unit of work" is a PR as merged by Volker.

Exactly.

And --first-parent (which I learnt from you) is very useful, but unfortunately it doesn't work with git blame so having a better history in each PR is still useful. Also, there is a problem when PRs have "dependencies" that are merged in, because they will sometimes show as a single merge.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 22, 2024 via email

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 22, 2024

And --first-parent (which I learnt from you) is very useful, but unfortunately it doesn't work with git blame

Maybe you're looking for git annotate --first-parent?

@tornaria
Copy link
Contributor

And --first-parent (which I learnt from you) is very useful, but unfortunately it doesn't work with git blame

Maybe you're looking for git annotate --first-parent?

Yes, thanks! It does work with git blame as well.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 28, 2024

@mkoeppe mkoeppe changed the title build/pkgs/python3: Update to 3.12.3; declare support of Python 3.12.x stable build/pkgs/python3: Update to 3.12.4; declare support of Python 3.12.x stable Jun 9, 2024
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 9, 2024

vbraun pushed a commit to vbraun/sage that referenced this pull request Jun 10, 2024
…port of Python 3.12.x stable

    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

https://docs.python.org/3/whatsnew/changelog.html#python-3-12-4-final

- In part cherry-picked from sagemath#36181

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [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 and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
- Depends on sagemath#37914 (merged here)
- Depends on sagemath#37951 (merged here for sagemath#37026)
- Depends on sagemath#38144 (merged here for testing)
    
URL: sagemath#37834
Reported by: Matthias Köppe
Reviewer(s):
vbraun pushed a commit to vbraun/sage that referenced this pull request Jun 16, 2024
…port of Python 3.12.x stable

    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

https://docs.python.org/3/whatsnew/changelog.html#python-3-12-4-final

- In part cherry-picked from sagemath#36181

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [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 and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
- Depends on sagemath#37914 (merged here)
- Depends on sagemath#37951 (merged here for sagemath#37026)
- Depends on sagemath#38144 (merged here for testing)
    
URL: sagemath#37834
Reported by: Matthias Köppe
Reviewer(s):
vbraun pushed a commit to vbraun/sage that referenced this pull request Jun 16, 2024
…port of Python 3.12.x stable

    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

https://docs.python.org/3/whatsnew/changelog.html#python-3-12-4-final

- In part cherry-picked from sagemath#36181

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [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 and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
- Depends on sagemath#37914 (merged here)
- Depends on sagemath#37951 (merged here for sagemath#37026)
- Depends on sagemath#38144 (merged here for testing)
    
URL: sagemath#37834
Reported by: Matthias Köppe
Reviewer(s):
@vbraun vbraun merged commit de1d818 into sagemath:develop Jun 22, 2024
30 of 41 checks passed
@mkoeppe mkoeppe deleted the python-3.12.3 branch June 22, 2024 20:19
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.

4 participants