Skip to content

Remove sagemath-sirocco package#40363

Closed
tobiasdiez wants to merge 5 commits intosagemath:developfrom
tobiasdiez:remove-sagemath-sirocco
Closed

Remove sagemath-sirocco package#40363
tobiasdiez wants to merge 5 commits intosagemath:developfrom
tobiasdiez:remove-sagemath-sirocco

Conversation

@tobiasdiez
Copy link
Contributor

This modularized distribution was meant to optionally install some modules depending on sirocco. With the new meson build backend, we already optionally install these cython modules when sirocco is available so there is no need for a seperate distribution.

📝 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

@orlitzky
Copy link
Contributor

orlitzky commented Jul 3, 2025

Eventually it would be nice to have these all separated out like sagemath-giac, but in the meantime, is there some way to prevent the automatic detection during the sagelib build? What we want to avoid is,

  1. User installs (say) sirocco to use it
  2. User installs sagemath to use it
  3. User uninstalls sirocco which, as far as he knows, has nothing to do with sagemath
  4. Sage begins to crash

If something like meson configure --disable-sirocco is possible in step 2 we can avoid the problem by depending on sirocco iff the user requests sirocco support in sagelib.

@antonio-rojas
Copy link
Contributor

Eventually it would be nice to have these all separated out like sagemath-giac,

Please no. We are moving in the right direction with meson optionally enabling features at build time. There is no need to complicate packager's lives by splitting sage into multiple small optional packages.

  1. User installs (say) sirocco to use it
  2. User installs sagemath to use it
  3. User uninstalls sirocco which, as far as he knows, has nothing to do with sagemath
  4. Sage begins to crash

It will not crash - we have been shipping sage on Arch with all features enabled for years, with sirocco being optional. If you don't have the library, then you simply won't be able to load the corresponding module (with a quite explicit error message).

But even if it did crash - that is a general issue with users compiling stuff from source which you can't do much about. You can just replace "sirocco" with "pari" in your objections - nothing prevents a user from uninstalling pari after compiling and completely breaking sage (this time really crashing). That's just the way things work when you build stuff from source. If you want proper dependency management, install sage using a package manager.

@tobiasdiez
Copy link
Contributor Author

My proposal as a partial answer to this problem is #39412 - with this PR you get a quite explicit error message when you try to import a module whose dependencies are not met. But this will not solve the problem that when a user removes (or updates) a dependency, then the import of the corresponding modules may fail. I think there is nothing we can do except document that in this case the user should just rerun the installation/compilation.

Personally, I see value in extracting packages if they are purely python wrappers around some native library and there is a reasonable interest from other parties to use the same interface in other python projects. Not sure if that's the case for sirocco.

@orlitzky
Copy link
Contributor

orlitzky commented Jul 3, 2025

It will not crash - we have been shipping sage on Arch with all features enabled for years, with sirocco being optional. If you don't have the library, then you simply won't be able to load the corresponding module (with a quite explicit error message).

From the user's perspective, this is just a crash. Something that used to work in their code now raises an error message due to an apparently unrelated change, and the error message doesn't really explain how to fix it within the package manager. We get bug reports about it constantly. There's a whole page about it.

But even if it did crash - that is a general issue with users compiling stuff from source which you can't do much about. You can just replace "sirocco" with "pari" in your objections - nothing prevents a user from uninstalling pari after compiling and completely breaking sage (this time really crashing). That's just the way things work when you build stuff from source. If you want proper dependency management, install sage using a package manager.

Our package manager is source-based. There is something we can do about it though: we can have the package optionally depend on sirocco iff the user asks for it. But for that to have any effect, we need to be able to disable sirocco support in sage when the user has it disabled it in the package manager. Otherwise installing the sage package with the sirocco feature "disabled" will still build the sirocco component if sirocco happens to be installed. This also happens to help people who are building from source.

Having them as separate packages also solves the problem (just don't install sagemath-sirocco if you don't want the sirocco support), so it would be preferable if we could switch to the meson approach without this regression.

@enriqueartal
Copy link
Contributor

Just a comment. With ./configure --enable-sirocco, sirocco is built but without some libraries and it is unusable. This is not the case with ./configure --enable-satemath-sirocco; if removed some scripts should be kept I guess.

@orlitzky
Copy link
Contributor

I've added the explicit option in #40485. It should have no effect on anyone who wants to leave them all at "auto".

@enriqueartal
Copy link
Contributor

I've never used meson. This problem affected to the classical build and the problem is that when one enables sirocco, sage.libs.sirocco is not built.

@orlitzky
Copy link
Contributor

I've never used meson. This problem affected to the classical build and the problem is that when one enables sirocco, sage.libs.sirocco is not built.

This should incidentally be fixed when the sagelib build process is switched to meson within sage-the-distro, since meson will automatically detect that sirocco is present and build the corresponding extension module.

@enriqueartal
Copy link
Contributor

Thanks, I tried to use meson but honestly following the instructions I encountered errors that I do not know how to solve.

@enriqueartal
Copy link
Contributor

Deleting the line # sage_setup: distribution = sagemath-sirocco in src/sage/libs/sirocco.pyx allows to use sirocco without sagemath_sirocco

@tobiasdiez
Copy link
Contributor Author

Now that #39030 is merged, this one here should be ready as well.

@github-actions
Copy link

github-actions bot commented Sep 8, 2025

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

@antonio-rojas
Copy link
Contributor

What about all other optional packages? Shouldn't they be removed too?

@tobiasdiez
Copy link
Contributor Author

tobiasdiez commented Sep 8, 2025

What about all other optional packages? Shouldn't they be removed too?

From my point of view, yes everything under the pkgs directory can go now.
The only package that makes sense in my opinion would be a small sage-test so that sage packages could use sage-like doctests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants