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

Add Python Source Distribution (sdist) support #457

Open
wants to merge 3 commits into
base: development
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,7 @@ wheelhouse/

# pip/enscons build output
soar_sml/
/pyproject.toml

### Soar ###
out/
Expand Down
2 changes: 1 addition & 1 deletion Core/ClientSMLSWIG/Python/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ requires = [
# and other python build/install frontends (such as pip itself).
#
# We use a forked version of enscons to add python 3.12 support.
"enscons @ git+https://github.com/ShadowJonathan/enscons-soar@544f39f",
"enscons @ git+https://github.com/ShadowJonathan/enscons-soar@7d6f4ca",

# Required sub-dependencies of enscons.
"toml>=0.1",
Expand Down
2 changes: 1 addition & 1 deletion Core/SConscript
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ if not CheckSWIG(env):
print('SWIG not found. Will not define sml_* build targets. Install swig to build wrappers for other languages.')
else:
for x in 'Python Java Tcl CSharp'.split():
SConscript(os.path.join('ClientSMLSWIG', x, 'SConscript'))
SConscript(os.path.join('ClientSMLSWIG', x, 'SConscript'), must_exist=False)

if 'MSVSProject' in kernel_env['BUILDERS']:
vcproj = kernel_env.MSVSProject(
Expand Down
39 changes: 38 additions & 1 deletion SConstruct
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,6 @@ py_sources += [
env.Alias(SML_PYTHON_ALIAS + "_dev", py_sources)

if enscons_active:
env['PACKAGE_METADATA'] = enscons.get_pyproject(env)['project']
# Instead of giving an explicit tag, we tell enscons that we're not building a "pure" (python-only) library,
# and so we let it determine the wheel tag by itself.
env['ROOT_IS_PURELIB'] = False
Expand All @@ -431,6 +430,44 @@ if enscons_active:

env.WhlFile(source=whl)

sdist_sources = [
# Add SCons related files
"SConstruct",
"Core/SConscript",
*env.Glob("Core/*/SConscript"),
"Core/ClientSMLSWIG/Python/SConscript",

# Add SWIG files
*env.Glob("Core/ClientSMLSWIG/*.i"),
*env.Glob("Core/ClientSMLSWIG/Python/*.i"),

# Add build support files
*env.Glob("build_support/*.py"),

# Entire SVS source tree
"Core/SVS",

# Misc files
"Core/ClientSMLSWIG/Python/README.md",
]

# Look for files under Core with the following file extensions, for up to 5 levels deep.
#
# We cannot just add the directories, as they will then start to include the .tar.gz file,
# which scons treats as a "target", creating a circular dependency.
SOURCE_EXTS = {"h", "c", "cpp", "hpp", "cxx"}
for i in range(0, 5):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know that we'll ever go more than 5 deep, but if we did this would break and we might not notice. I would feel more comfortable with some logic that checks if any more files are being adde.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could just bump it from 5 to 10, would that suffice?

And add a CI job that does a basic sdist packaging + sdist-based build-and-install, possibly?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, 10 sounds fine. Testing in CI would also be a good idea.

for ext in SOURCE_EXTS:
sdist_sources.extend(
env.Glob("Core/" + ("*/" * i) + "*." + ext)
)

env.SDist(
source=sdist_sources,
# We tell enscons to include a generated / corrected pyproject.toml into the source distribution
pyproject=True,
)

# We make sure that an editable (`pip install -e`) installation always properly installs
# the files in the correct places.
env.Depends("editable", py_sources)
Expand Down
Loading