search

SoarGroup / Soar

Soar, a general cognitive architecture for systems that exhibit intelligent behavior.
http://soar.eecs.umich.edu
Other
322 stars 70 forks source link

Add Python Source Distribution (sdist) support #457

Open ShadowJonathan opened 1 month ago

ShadowJonathan commented 1 month ago

This PR depends on #448, and will remain a draft until it merges, and can be rebased.

This PR adds experimental "Source Distribution" (or; sdist) support for python packaging. Basically, sdist distributions are all source files necessary to build a package, so that when pip can't find a pre-built binary, it will pull the sdist .tar.gz file, and attempt to build it itself.

As you can guess, this is prone to failure, and is only meant as a last-resort usecase, the CI conditions in #448 aren't present on all machines that Soar would want to be installed on, nor have all edgecases been enumerated in the SConstruct file, to support building on a variety of targets.

Yet, I wanted to try, and this PR - at least experimentally - allows for the creation of an sdist .tar.gz file when ran with python -m build Core/ClientSMLSWIG/Python --sdist.

This file can then be uploaded to pypi alongside the binary .whl distributions, or installed locally with pip install soar_sml-[...].tar.gz

There is an argument to be had about "Should we even have this? We don't support every single system that's out there" and that's a fair point, which is why I separated this from #448, to talk/discuss that, and also to extend the invitation of tons of testing on all sorts of systems.

If sdist files are uploaded for any version of soar-sml, pip will attempt to build it, when it has no binary release to grab.

This may result in errors (as the system does not have the right build tools, or other conditions preventing its success), and may result in extra support requests (or other similar issues).

ShadowJonathan commented 1 month ago

Rebased on main.

This is more a discussion of "should we have this" instead of "we should have this" PR, I'm just bringing the prototype code/changes that can achieve it ^^

ShadowJonathan commented 1 month ago

Roughly, you can use the following lines to test this;

pip install build

pushd Core/ClientSMLSWIG/Python

# -f to ignore if it doesnt exist
rm -f dist/*
pyproject-build . -vvvv -s
pip install dist/* -vvvv

popd
garfieldnate commented 1 month ago

Well, first what are the additional platforms that could be supported with this that might be interesting?

ShadowJonathan commented 1 month ago

FTR, we could support most of those with wheel distributions.

Other platforms I could add to that list would be AIX / z/OS / freeBSD, etc.

I'd argue RISC-V would become more of an interest in the future ^^

WASM would be outside scope for python source packaging, imo that one could be better served with some rust-esc interface that people can then build upon and compile for WASM.

We could possibly attach this file to github releases, to provide people a way to install directly? Though, that would be almost indistinguishable from a full clone + local build.

An embedded system of some kind? But then you normally build on another system and deploy to the embedded one, and I don't know if this change covers that case.

Cross-compilation would be better done within CI and specialised environments, I don't know if this method would allow anyone to more effectively and easier re-package Soar, but since its a build method limited to python distribution, I doubt it.

I'll have to be honest, like this, it feels like this usecase is a solution looking for a problem. It is nice to define the sdist method, so people can be expected that it works, but since we might have a problem with releasing it as-is (because we want to prevent build errors on users' systems, and instead CI it for as many systems as possible), it might not be too useful.

Yeah, unless we want to make soar-sml buildable on systems, there are better ways to do this (such as the install-via-github command), and the only other use-case for this would be someone packaging soar-sml for an offline machine.

With that, I think adding the machinery for an sdist would maybe be useful for developers and researchers if they need it, but it doesn't solve a concrete problem at the moment.

garfieldnate commented 1 month ago

It doesn't look like a huge piece of implementation, honestly, so I wouldn't mind just adding it. Soar is the type of thing that might get used on a weird platform.

ShadowJonathan commented 1 month ago

Let me then also add the appropriate documentation to explain such a use-case :)