pkgs/sagemath*: Exclude all__*.py files of other distributions

[mkoeppe]

Fixes #35661 (comment)

📝 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

[kiwifb]

I may be saying stupid here, how will the PR in its current state prevent sagemath-standard to ship, say, sage/graphs/bliss.pyx? Or do I have the design of the thing completely wrong?

[mkoeppe]

Sorry, I missed the part about bliss.pyx in your report. I'll take a look.

[kiwifb]

Similarly, bits belonging to meataxe and sirocco were shipped as part of sagemath-standard. I cannot say for the other two but it may be the case as well.

[kiwifb]

I have just inspected the "Manifest.in" for sagemath-standard and it does include an exclude line for bliss

# Exclude extension modules shipped by optional packages
exclude sage/graphs/bliss.pyx

but none for the other optional packages. But even so it is there, I still have bliss.pyx shipped currently. I am not sure why it is ignored.

[kiwifb]

OK, MANIFEST.in is used for building sdits. Which means that when I use the sdist for sagemath-standard-10.0, bliss.pyx is not included and everything is fine. But MANIFEST.in is not used when building wheels. My current install process produces a wheel and install it (for both master and develop) but because develop is a git clone, it includes bliss.pyx and other files that would otherwise be excluded in a sdist. It seems the only way to exclude a file from a wheel is to explicitly exclude it with a

find_packages(exclude=[...])

pattern. Which I find problematic from a maintenance point of view but probably doable.

[kiwifb]

We may want exclude_package_data https://setuptools.pypa.io/en/latest/userguide/datafiles.html#exclude-package-data

[mkoeppe]

I think we need to build the wheels via the sdists so that filtering works properly.

I'm already doing this in #35095 for most distributions.
I'll look into this next.

[mkoeppe]

There is unfortunately a limitation of setuptools: If I want to declare package data, I need to first declare a package. But this then leads to include all Python files in that package as source files.

This will all go away when we move from setuptools to something better (#34630). In the meantime, building wheel via sdist is the solution. This is also the default way how the "build" tool builds wheels.

[kiwifb]

Well it is a bit of an issue when you start from a git clone. The process right now would be

• git clone
• prepare sdist
• use sdist to build package
  The prepare sdist step is unusual and not well catered for in an ebuild environment. The normal solution is to pre-package the sdist but that nullify any benefits from the cloning ability to stay on top of what is happening.

But it remains that the current MANIFEST.in for sage needs to exclude files for meataxe, sirocco and the other optional packages so they do not get in the sdist. Let's deal with this particular problem here.

[mkoeppe]

• The prepare sdist step is unusual and not well catered for in an ebuild environment.

You mean ebuild insists on using setup.py bdist_wheel instead of using modern tools such as https://pypa-build.readthedocs.io/en/latest/#python--m-build ?

[kiwifb]

• The prepare sdist step is unusual and not well catered for in an ebuild environment.

You mean ebuild insists on using setup.py bdist_wheel instead of using modern tools such as https://pypa-build.readthedocs.io/en/latest/#python--m-build ?

Out of my hands at this stage. I use the standard boiler plate I am provided with - which mostly expect a sdist or sdist like thing. Part of the problem is that this process takes precedence over whatever the package maintainer would want to do.

Philosophically, I consider the whole thing as python wanting to have nothing to do with downstream packaging and not supporting it. This is the way we package things in python, deal with it. A current that seems to have gain prevalence in a number of ecosystems in the last decade for good or bad. I do consider it a mixed bag myself, but coming from a rolling release distribution like Gentoo, I appreciate wanting to escape the normal binary distribution model. Using binary python package from a traditional distribution does put serious limits on the user.

[kiwifb]

OK, I can make things work in Gentoo so long as MANIFEST.in is correct. I can leverage that. I am adding stuff for sirocco, meataxe, mcqd, coxeter3 and tdlib. What about all those "all__*` empty files? Should they also be excluded from sagemath-standard? It looks like they are all installed by default in develop.

[mkoeppe]

What about all those "all__*` empty files? Should they also be excluded from sagemath-standard?

Yes, done in a8d6ef0

[kiwifb]

What about all those "all__*` empty files? Should they also be excluded from sagemath-standard?

Yes, done in a8d6ef0

Thanks and now I have to refresh and restart my edits on sagemath-standard's MANIFEST.in.

[mkoeppe]

Just pushed another fix of files that shouldn't be shipped by sagemath-standard any more.

[kiwifb]

That take care of what I was working on. For tdlib I think there should be a pyx file as well as the .cpp.

…

On Fri, 4 Aug 2023 at 12:11, Matthias Köppe ***@***.***> wrote: Just pushed another fix of files that shouldn't be shipped by sagemath-standard any more. — Reply to this email directly, view it on GitHub <#36015 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABE7M5BVDR5XO33EQ645XDXTQ43DANCNFSM6AAAAAA3AMVMRA> . You are receiving this because your review was requested.Message ID: ***@***.***>

[kiwifb]

Apart from the last question about that .cpp file, I tested that this branch worked properly at least for bliss, meataxe and sirocco. I think we are able to ship it ASAP.

[mkoeppe]

I thought I took care of the tdlib things already in cf02a42

[mkoeppe]

I'll postpone the switch to using python-build to a follow-up PR

[kiwifb]

I thought I took care of the tdlib things already in cf02a42

Did you read my review comment? It works as is, but the line excluding sage_tdlib.cpp that comes from the previous commit is redundant, removing the corresponding include line was sufficient to make sure it is not shipped.

[mkoeppe]

Did you read my review comment?

I don't see a comment, maybe you didn't "submit" the review?

It works as is, but the line excluding sage_tdlib.cpp that comes from the previous commit is redundant, removing the corresponding include line was sufficient to make sure it is not shipped.

That's true, I'll remove it again.

[kiwifb]

Yes I must have "forgotten" to submit. It should go now.

[github-actions]

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

[mkoeppe]

May I set to positive review then?

[kiwifb]

Sorry forgot to change the label.

[mkoeppe]

Thank you!

[kiwifb]

Now that rc0 is out and I have run proper doctests on sage-on-gentoo, I have noticed unintended consequences that I missed before.

sage -t --long --random-seed=118675773581622755375337172702839208422 /usr/lib/python3.11/site-packages/sage_setup/find.py
**********************************************************************
File "/usr/lib/python3.11/site-packages/sage_setup/find.py", line 117, in sage_setup.find.?
Failed example:
    find_python_sources(SAGE_SRC, distributions=['sagemath-tdlib'])
Expected:
    ([], [], [<setuptools.extension.Extension('sage.graphs.graph_decompositions.tdlib')...>])
Got:
    ([], [], [])
**********************************************************************
File "/usr/lib/python3.11/site-packages/sage_setup/find.py", line 196, in sage_setup.find.filter_cython_sources
Failed example:
    any(f.endswith('sage/graphs/graph_decompositions/tdlib.pyx') for f in cython_modules)
Expected:
    True
Got:
    False

and

sage -t --long --random-seed=118675773581622755375337172702839208422 /usr/lib/python3.11/site-packages/sage/misc/package_dir.py
**********************************************************************
File "/usr/lib/python3.11/site-packages/sage/misc/package_dir.py", line 110, in sage.misc.package_dir.read_distribution
Failed example:
    read_distribution(os.path.join(SAGE_SRC, 'sage', 'graphs', 'graph_decompositions', 'tdlib.pyx'))
Exception raised:
    Traceback (most recent call last):
      File "/usr/lib/python3.11/site-packages/sage/doctest/forker.py", line 709, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/lib/python3.11/site-packages/sage/doctest/forker.py", line 1144, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.misc.package_dir.read_distribution[2]>", line 1, in <module>
        read_distribution(os.path.join(SAGE_SRC, 'sage', 'graphs', 'graph_decompositions', 'tdlib.pyx'))
      File "/usr/lib/python3.11/site-packages/sage/misc/package_dir.py", line 116, in read_distribution
        with open_source_file(src_file, error_handling='ignore') as fh:
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/usr/lib/python3.11/site-packages/Cython/Utils.py", line 255, in open_source_file
        raise FileNotFoundError(source_filename)
    FileNotFoundError: /usr/lib/python3.11/site-packages/sage/graphs/graph_decompositions/tdlib.pyx

prior to this PR, the tests were fine because the files concerned would be just shipped as part of sagemath-standard. So far we have dealt with doctests requiring real access to the sources with #optional - sage_spkg. Should we do mark those three in the same way? Or is it time we think about something more formal when running sage -t --installed?