-
-
Notifications
You must be signed in to change notification settings - Fork 649
remove bzip2 spkg #40011
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
remove bzip2 spkg #40011
Conversation
Documentation preview for this PR (built with commit eeabce5; changes) is ready! 🎉 |
Unless its absence causes no issues, we may want to disable it first (for a release cycle) to avoid creating extra work for others. It has no dependencies though so it should be easier to remove than (say) python. @culler is this something you can use the system copy of, or at least build yourself easily? |
@orlitzky Apple provides libbz2 and a bzip2 executable as part of the OS, and when you build python they find and use Apple's library with no issues. I don't know of any reason for not using Apple's library or executable. Your suggestion about the spkg is supported by basic principles of software engineering. The spkg should be disabled first, and not removed until it has been disabled for a long enough time that unexpected issues are likely to have appeared, if any exist. |
I am not sure what you mean. We are using Apple's bzip2 and libbz2 without any issues with Sage for years, it's all tested very, very well indeed. (And Linux has not been problematic here for many years too, needless to say) If it was not the case I wouldn't have proposed this removal like this. |
@culler not sure what principles you mean here. It has been tested a lot, as building this package from source is disabled by default already, for at least 3 years or more. On macOS as well as on Linuxes. It is 100% dead wood already. |
The principles are:
1. It is always something.
2. No good deed goes unpunished.
3. Nothing ever gets fixed without breaking something else, usually
something which is apparently unrelated.
4. Better safe than sorry.
- Marc
…On Mon, Apr 28, 2025 at 8:38 PM Dima Pasechnik ***@***.***> wrote:
*dimpase* left a comment (sagemath/sage#40011)
<#40011 (comment)>
@culler <https://github.com/culler> not sure what principles you mean
here. It has been tested a lot, as building this package from source is
disabled by default already, for at least 3 years or more. On macOS as well
as on Linuxes.
—
Reply to this email directly, view it on GitHub
<#40011 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAJ6CP3JBKVU633BU2KWKZT233JZRAVCNFSM6AAAAAB35TY2XGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQMZXGE4TIMRXG4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@culler your list can easily be adapted to make any progress and any development impossible. |
I am not demanding anything. And please do not ever ask me to comment on |
You wrote above: "The spkg should be disabled first." How is this anything but a demand? And I have not asked you to comment here, @orlitzky did. |
Marc hasn't objected in principle to removing these packages, he is only asking to go slowly so that we don't break anything and don't create too much extra work for him. It doesn't sound unreasonable to me, and I think progress on this will be a lot faster if we don't all hate each other, so it would be preferable to proceed in a way that leaves no one feeling slighted. |
Well, Marc is not even using this particular package. I am fine with providing a deprecation dance around the ones he is using, but for this one it just doesn't make sense. |
One question is, how do we know that no one has been building Sage's bzip2? What about changing the default argument for |
@jhpalmieri - we don't support ancient Linux installations which have a vintage bzip2, and all macOS systems have bzip2 installed with XCode. While it's in principle possible to come across a Linux system without bzip2 installed, bzip2 from the system will work (and the user will be best advised to use the system one). I don't see the point of a depreciation period for the empty set of users. |
We removed |
last but not the least, we ship a 10 years old bzip2 version, which is bad. This PR corrects this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion, it's a good idea to mark more and more packages as 'prerequisite'. Out of the 400 sage packages, 2/3 thirds are not up-to-date - which shows that the maintenance load of the current approach is too heavy (or nobody cares about some packages as they are never installed anyway).
Looking at https://repology.org/project/bzip2/versions, only three linux systems have the older version 1.0.5 (and those are oldtimers like Centos 6 with an eol of 2020). So it should be fine to drop it without a deprecation period.
It might be easier to first decide on a list of systems for which sage should work with only the prerequisite packages installed. Based on this list, it should be straight-forward to decide whether to mark a given sage package as prerequisite or not.
sagemathgh-40011: remove bzip2 spkg a good enough bzip2/libbz2 is readily available on all the systems we support, including "bare" macOS. Thus it's obsolete, and ripe for removal. See e.g. [sage-devel](https://groups.google.com/g/sage- devel/c/-ASHfAXqVYo/m/anPQmHxECwAJ) for the relevant context. ### 📝 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: ... --> URL: sagemath#40011 Reported by: Dima Pasechnik Reviewer(s): Tobias Diez
This breaks the buildbot (debian 12) with bzip installed (but no libbz2-dev)
our configure insists on building python, yet python's bz2 module is not built (presumably due to the lack of libbz2-dev):
In principle thats ok (require libbz2-dev), but our configure diagnostic doesn't mention that at all:
Shouldn't it at the very least say that you should apt-get install libbz2-dev? |
@vbraun I will add a test in configure.ac for availability of libbz2 and the corresponding header |
inst_patch no longer even defined, so we should remove it from the deps of the base target
sagemathgh-40011: remove bzip2 spkg a good enough bzip2/libbz2 is readily available on all the systems we support, including "bare" macOS. Thus it's obsolete, and ripe for removal. See e.g. [sage-devel](https://groups.google.com/g/sage- devel/c/-ASHfAXqVYo/m/anPQmHxECwAJ) for the relevant context. ### 📝 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: ... --> URL: sagemath#40011 Reported by: Dima Pasechnik Reviewer(s): Tobias Diez
@vbraun it should be fixed now |
sagemathgh-40011: remove bzip2 spkg a good enough bzip2/libbz2 is readily available on all the systems we support, including "bare" macOS. Thus it's obsolete, and ripe for removal. See e.g. [sage-devel](https://groups.google.com/g/sage- devel/c/-ASHfAXqVYo/m/anPQmHxECwAJ) for the relevant context. ### 📝 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: ... --> URL: sagemath#40011 Reported by: Dima Pasechnik Reviewer(s): Tobias Diez
Ok it does test now, but immediately bails out and does not make it to the diagnosis screen with suggestions for system packages:
|
This is not a regression, as we don't do it the packages in
Nothing system-specific. |
Done in eeabce5 - now you'll be pointed at https://doc.sagemath.org/html/en/reference/spkg/_prereq.html for more details |
sagemathgh-40011: remove bzip2 spkg a good enough bzip2/libbz2 is readily available on all the systems we support, including "bare" macOS. Thus it's obsolete, and ripe for removal. See e.g. [sage-devel](https://groups.google.com/g/sage- devel/c/-ASHfAXqVYo/m/anPQmHxECwAJ) for the relevant context. ### 📝 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: ... --> URL: sagemath#40011 Reported by: Dima Pasechnik Reviewer(s): Tobias Diez
sagemathgh-40011: remove bzip2 spkg a good enough bzip2/libbz2 is readily available on all the systems we support, including "bare" macOS. Thus it's obsolete, and ripe for removal. See e.g. [sage-devel](https://groups.google.com/g/sage- devel/c/-ASHfAXqVYo/m/anPQmHxECwAJ) for the relevant context. ### 📝 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: ... --> URL: sagemath#40011 Reported by: Dima Pasechnik Reviewer(s): Tobias Diez
sagemathgh-40011: remove bzip2 spkg a good enough bzip2/libbz2 is readily available on all the systems we support, including "bare" macOS. Thus it's obsolete, and ripe for removal. See e.g. [sage-devel](https://groups.google.com/g/sage- devel/c/-ASHfAXqVYo/m/anPQmHxECwAJ) for the relevant context. ### 📝 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: ... --> URL: sagemath#40011 Reported by: Dima Pasechnik Reviewer(s): Tobias Diez
As reported by @jeriedel24 [here](https://github.com/sagemath/sage/commi t/7f6dd7547ecf8c4d23099842f4ad4ace5f2f3072#commitcomment-160100961), old autoconf does not correctly deal with `test` clauses split across the lines. So we fix this regression, introduced in sagemath#40011, here. We still cannot drop autoconf 2.69, as various distros, e.g. Debian, are very slow in adapting 2.71+ <!-- ^ 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". --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [ ] 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 <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#40261 Reported by: Dima Pasechnik Reviewer(s): Tobias Diez
sagemathgh-40261: ensure autoconf 2.69 compatiblity in configure.ac As reported by @jeriedel24 [here](https://github.com/sagemath/sage/commi t/7f6dd7547ecf8c4d23099842f4ad4ace5f2f3072#commitcomment-160100961), old autoconf does not correctly deal with `test` clauses split across the lines. So we fix this regression, introduced in sagemath#40011, here. We still cannot drop autoconf 2.69, as various distros, e.g. Debian, are very slow in adapting 2.71+ <!-- ^ 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". --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [ ] 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 <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#40261 Reported by: Dima Pasechnik Reviewer(s): Tobias Diez
sagemathgh-40204: Remove pkgconf spkg `pkgconf`, a.k.a. `pkg-config`, is available on all systems we support - even on the "naked" (no homebrew/macports) macOS one can install a formally certified/notarised package https://github.com/donmccaughey/pkg-config_pkg - so there is no reason to keep it in the tree. There are big advantages to have pkg-config available at configure time, as recognition of several crucial external spkgs, such as (open)blas, zlib, etc. hinges upon pkg-config. With this PR in, we proceed to remove them. Last but not the least, it simplifies the Makefile by getting rid of `base` target, which becomes empty ## 📝 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: ... --> - sagemath#40011 - remove bzip2 spkg. - sagemath#40153 - planarity spkg fix (can't build otherwise) URL: sagemath#40204 Reported by: Dima Pasechnik Reviewer(s): Dima Pasechnik, Tobias Diez
sagemathgh-40261: ensure autoconf 2.69 compatiblity in configure.ac As reported by @jeriedel24 [here](https://github.com/sagemath/sage/commi t/7f6dd7547ecf8c4d23099842f4ad4ace5f2f3072#commitcomment-160100961), old autoconf does not correctly deal with `test` clauses split across the lines. So we fix this regression, introduced in sagemath#40011, here. We still cannot drop autoconf 2.69, as various distros, e.g. Debian, are very slow in adapting 2.71+ <!-- ^ 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". --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [ ] 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 <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#40261 Reported by: Dima Pasechnik Reviewer(s): Tobias Diez
sagemathgh-40204: Remove pkgconf spkg `pkgconf`, a.k.a. `pkg-config`, is available on all systems we support - even on the "naked" (no homebrew/macports) macOS one can install a formally certified/notarised package https://github.com/donmccaughey/pkg-config_pkg - so there is no reason to keep it in the tree. There are big advantages to have pkg-config available at configure time, as recognition of several crucial external spkgs, such as (open)blas, zlib, etc. hinges upon pkg-config. With this PR in, we proceed to remove them. Last but not the least, it simplifies the Makefile by getting rid of `base` target, which becomes empty ## 📝 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: ... --> - sagemath#40011 - remove bzip2 spkg. - sagemath#40153 - planarity spkg fix (can't build otherwise) URL: sagemath#40204 Reported by: Dima Pasechnik Reviewer(s): Dima Pasechnik, Tobias Diez
sagemathgh-40261: ensure autoconf 2.69 compatiblity in configure.ac As reported by @jeriedel24 [here](https://github.com/sagemath/sage/commi t/7f6dd7547ecf8c4d23099842f4ad4ace5f2f3072#commitcomment-160100961), old autoconf does not correctly deal with `test` clauses split across the lines. So we fix this regression, introduced in sagemath#40011, here. We still cannot drop autoconf 2.69, as various distros, e.g. Debian, are very slow in adapting 2.71+ <!-- ^ 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". --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [ ] 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 <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#40261 Reported by: Dima Pasechnik Reviewer(s): Tobias Diez
This breaks the sdist publishing: #40306 Should we install there also |
installing _prereq dependencies will fix it, sure. this might be a bit suboptimal, but it's still fast, so why not I don't understand how it worked before |
a good enough bzip2/libbz2 is readily available on all the systems we support, including "bare" macOS.
Thus it's obsolete, and ripe for removal.
See e.g. sage-devel for the relevant context.
📝 Checklist
⌛ Dependencies