Support for pip install as a standard Python package

[dpad]

• Tickets addressed:
  • Closes Rewrite packaging/setup to work better with pip #491
  • Closes Conan profile configuration doesn't work at first #525 (build should now work from the first time)
  • Issues that are kind of solved but only when using pip install, not when using python conanfile.py:
    • Conanfile incompatible with Docker #131 (caused due to calling pip as a subprocess from the Conanfile)
    • Basilisk always upgrades Pip, which sometimes fails if conflicts with user's cache  #459 (fixed when using pip install method)
    • Dockerfile for containerized Basilisk #350 (don't really need it, since you can just pip install Basilisk)
• Review: By file
• Merge strategy: Merge (no squash)

Description

CC: @juan-g-bonilla @schaubh

This is the first part of moving towards a modern Python build system with distributable pre-compiled packages that users can install directly, as based on #614 .

• Basilisk now builds and installs like any other Python package.
  • pip install . (-v flag to monitor compilation progress)
  • python -m build --sdist generates a "source distribution" (.tar.gz) file that can be pip-installed (don't need git).
  • pip wheel --no-deps . generates a "wheel" (.whl) that can be pip-installed (no compilation required).
    • NOTE: Will only work on similar systems with same architecture/runtime libs.
  • NOTE: Arguments to Conan must be passed via the CONAN_ARGS environment variable. e.g. CONAN_ARGS="--opNav True".
• Inverted the dependency of Conan and pip (pip now calls conan, instead of the opposite).
  • Conan build no longer assumes the user is running pip or tries to upgrade it or install dependencies.
  • Kept python conanfile.py installation behaviour, continues to work as before.
• Python dependencies are now managed through PEP-517 compliance.
  • Build and runtime dependencies are installed by the frontend tool (i.e. pip).
  • With pip, everything is built automatically within isolated temporary build environments.
    • NOTE: pip itself is no longer upgraded automatically. The installation should error if the version of pip is too low to correctly support the above features.
  • setup.py takes care of compilation by calling the Conan build system.
    • NOTE: Removed all previous python setup.py functionality. Do not run python setup.py, it is NOT supported in modern Python.

Motivation & Rationale

In recent years, Python "wheels" have become the standard way to distribute pre-compiled Python packages to end-users. Until now, Basilisk has been compiled and installed by users manually, using some invocation of python conanfile.py. However, in future, it would be desirable for an end-user to simply install a pre-compiled Basilisk package, for example by calling pip install Basilisk.

The Python community has established several standards for which all build tools are expected to follow in the future. These include:

• PEP-517, which defines "frontend" and "backend" packaging tools that take instructions from a pyproject.toml file
• PEP-660, which adds support for "editable" (development) installations

Most end-users interact directly with a frontend tool (e.g. pip) to install Python packages. However, the actual package is built by the "backend" tool, as specified in the [build-system] part of the pyproject.toml file. For Basilisk, we need a backend tool that can build custom "extension" modules (not just pure Python, but compiled from C/C++ source code).

This merge request adds the above pyproject.toml file and configures the setuptools backend build tool to correctly build the Basilisk extension modules. It is a little hacky in places, but setuptools was selected because:

• It is still the defacto standard backend used by most packages and familiar to most users.
• It has a (somewhat clunky but functional) API for adding custom build commands.
• It doesn't have stable support for --config-settings, but we assume it will in the future.
  • See Deprecate --global-option and --build-option pypa/pip#11859 (comment)
  • Currently, we get around this by using the CONAN_ARGS environment variable.

The following backends were also investigated, but rejected:

• hatchling doesn't support --config-settings (no indication of whether it ever will) and isn't as well-supported as setuptools.
• scikit-build and scikit-build-core assume the invocation is "cmake", which conflicts with our use of Conan.
• flit doesn't support build plugins at all.
• poetry likes to do its own thing, doesn't always follow standards.
• pdm works, but has some fiddly file discovery, and is not as well-supported as setuptools.
• meson-python is the build tool for numpy/scipy now, and seems like a good alternative if we ever want to switch away from Conan.

For further reference, refer to:

• https://packaging.python.org/en/latest/key_projects/
• https://packaging.python.org/en/latest/guides/tool-recommendations/

Verification

Verified to work on my local Linux (Ubuntu 22.04) machine:

• Build and install via pip install .. Passes all tests, including scenario tests. (pytest src/)
• Build a Python wheel via pip wheel . and install it. Passes all tests as above.
• Build a source distribution via python -m build --sdist and install it. Passes all tests as above.
• Build and install via python conanfile.py --opNav True. Passes all tests as above.
• All of the above with CONAN_ARGS="--opNav True", confirmed that opNav installs and all tests pass.

TODO (I'm leaving these for the maintainers, CC @juan-g-bonilla ):

• Needs to be tested on Mac.
• Needs to be tested on Windows.

Documentation

TODO (I'm leaving these for the maintainers, CC @juan-g-bonilla ):

• Update the documentation to talk about the pip install method.
  • Need to also decide on an appropriate workflow for developers. See "Editable Installations" below.

Editable Installations

Installation for developers with pip in editable mode is a little fiddly, so I've disabled the build step when calling pip install -e .. This maintains backwards compatibility with python conanfile.py as per the current developer workflow, but might be a little confusing if people try to pip install -e . expecting Basilisk to work immediately. For that reason, I added an error case that checks whether the Conanfile has already prepared the packages in the "dist3" directory.

Rationale

Basically, pip installs in an isolated build environment by default, including the appropriate build requirements (cmake, parse, setuptools, swig, etc.). This is very convenient, but when the CMake files get generated, they point to this temporary build environment (somewhere in /tmp). For an editable install, ideally we want developers to just call make in dist3 folder after they make changes to Basilisk C/C++ files, so they don't have to recompile everything from scratch.

For pip, there are two options:

1. pip install --no-clean -e .

• This forces pip to leave the temporary build environment intact (somewhere in /tmp), so that all the CMake files which point to it will still work. I personally use this option, because I develop in Docker containers that don't get restarted, so I don't lose any temporary files.
• ✅ Developer doesn't need to install/manage the build tools themselves, because pip is installing them automatically.
• ❌ The temporary build environment is in /tmp, so it may get deleted at any time (e.g. when the system restarts?).

2. pip install --no-build-isolation -e .

• This forces pip to not create a temporary build environment at all. Instead, it basically works as a "raw" installation, where it will use any system-installed tools (including setuptools, cmake, swig, parse, etc). I don't like this option as much because developers have to manually manage their build tools, but it's more convenient for people who know what they're doing and have an environment that might delete their temporary files.
• ✅ The CMake build system will always work so long as the build tools are available in the local environment.
• ❌ User needs to install and manage the versions of the build tools manually. pip will not check them and there are no standard tools to extract them from pyproject.toml.

Future work

• Build and test the wheels for each platform on CI, using cibuildwheel
  • I already did a lot of work towards this in develop...dpad:basilisk:basilisk/build-python-limited-api
  • I think some specific platforms and combinations of Python versions have some lingering run-time issues to track down.
• Publish Basilisk on Pypi
  • This will allow users to just pip install Basilisk
  • Basically, the maintainers need to create a Pypi account for Basilisk, and just push the ".tar.gz" and ".whl" files generated by cibuildwheel whenever they want to release a new version.
• Move generation scripts from Conanfile into CMake.
  • For developers who want to just make their modifications, if they add a new message file, it won't show up in the CMake build system until they run Conan again (i.e. by pip install again). So, these scripts should be run and managed by the CMake build system instead.
• Allow External Modules to be compiled separately from Basilisk.
  • Currently, developers must compile the entire Basilisk system to build their own custom modules.
  • I have some thoughts on how we can decouple the External Modules build system from the rest of the system, and allow users to just compile separately. I believe it is feasible but will take a fair bit of work.
  • Also should allow multiple External Modules folders to be specified. This should be a pretty easy change to the CMake files and can be done first.
• Upgrade to Conan 2.0 or Meson
  • This is mostly for convenience and to avoid future compatibility issues for the underlying requirements.
  • If we move to Meson, we can use the meson-python backend instead, the same as numpy/scipy do.

See also #614 for further context.

[dpad]

@juan-g-bonilla Please have a look at this. It's the minimal set of changes to get pip install to work.

I had some issues with the old CMake findPython a while back, here I've pre-emptively added support for building with the limited Python API which we will need for generic builds, so I've made some minor changes to the CMake files (and upgraded the minimum CMake version required). This changes how Python is found by CMake a bit so there's a small chance it might break some people's existing builds (if they have Python installed in a weird place, I guess), but I recommend keeping these changes in place regardless.

[dpad]

@juan-g-bonilla I've converted back to draft, because I'm still not happy with some of this structure. Although this follows the latest standards, there's too many hacks to get around issues with setuptools to my liking. I'm going to continue experimenting a bit to try and trim this down as far as possible, but I would appreciate if you could at least try out the current state on a Windows/Mac machine and let me know what issues (if any) you observe.

[schaubh]

Howdy @dpad , let's keep this at draft at the moment until we can fully iterate on this. I'll be on travel for the next two weeks and will be able to look at this in earnest. We definitely need to test this on all the supported platforms. I would like to minimize any changes to the user facing build system if possible to ease any disruption to other users build process. I see you added the legacy build option for now. Thanks. This will allow us to depreciate the old build system. Naturally we will have to update the build documentation :-)

I'll definitely want to do many test builds on different computers. I have general questions.

• You say we build BSK now using pip install . This will install dependencies and build BSK from the command line. Is the requirements.txt file not needed anymore? We do have optional python packages for some behaviors, can we retain this?
• Can we still do incremental builds with IDEs like Xcode, VS Studio, CLion, etc?
• How do we handle build options like vizInterface and opNav?
• Regarding build time, how does the new system compare to prior, about the same?
• on macOS with Python 3.11 and cmake 3.28.1, I was able to run pip install . from the root BSK folder. I had a clean new venv environment and deleted .conan folder. When trying to run scenarioBasicOrbit.py, I got an error that colorama package was not installed? After installing it, it couldn't find tqdm. Note I created a new .venv and ran pip install, I didn't manually install other packages first.  After installing these two packages I was able to run this basic orbit example script.
• I ran the basic orbit script to save off the Vizard binary, that worked, so vizInterface must be built.
• what are the instructions to do a clean build option with this build system?
• How do you build for Release or Debug to have break points?

Looking forward to playing with this branch more when I return from travel.

[dpad]

Hi @schaubh , thanks for checking and the questions!

There are some annoying limitations of pip and setuptools, despite these being the de-facto standard for packaging in Python. I'm going to continue iterating on this for a bit, but it is helpful to know if it works as is on your existing Windows/Mac workflows, and that the existing python conanfile.py install method is preserved.

You say we build BSK now using pip install . This will install dependencies and build BSK from the command line. Is the requirements.txt file not needed anymore? We do have optional python packages for some behaviors, can we retain this?

requirements.txt is not needed, you should specify the requirements (and optional requirements) in pyproject.toml instead. However, to maintain backwards compatibility, I've kept the requirements_*.txt files and read them in "dynamically" into the pyproject.toml (this is a standard feature of the setuptools build backend).

I included the requirements_optional.txt as the optional requirements group. If you prefer, you could group requirements separately, for example to split up test and documentation requirements. These are all installed in the standard way:

• pip install . -- Basilisk only, no optional requirements
• pip install .[optional] -- (implemented) Basilisk + optional requirements
• pip install .[test] -- (example only) Basilisk + test requirements
• pip install .[test,docs] -- (example only) Basilisk + test + documentation requirements

Can we still do incremental builds with IDEs like Xcode, VS Studio, CLion, etc?

I'm not sure if these IDEs do anything special, but incremental builds are possible for editable installations, but it's a little tricky. Basically, there is a trade-off:

Pip with build isolation (default):
By default, pip creates a new temporary directory to install all the build requirements. Therefore, it must do a clean CMake build to correctly point CMake to the right build tools. An incremental build would require --no-clean so that pip leaves its temporary build directory intact, and you can simply call make inside dist3 to do an incremental build afterwards. The risk is that the temporary directory would probably get deleted after a reboot, so you'd have to do this again (it works great if you're developing in a Docker environment that doesn't reboot, as you can in VSCode, for example). This is the option I currently use and recommended.

Pip without build isolation (--no-build-isolation):
If we disable build isolation, we also disable pip's management of the build tools. Therefore, the user needs to ensure they have all the correct build tools installed and up-to-date. Sadly, there is no standard mechanism for installing just the required build tools specified in pyproject.toml, so this is becomes a semi-manual process (of course, we can provide a helper that installs the requirements as per the current conanfile.py). This option essentially would work the same as the current conanfile.py build, so it might actually be the best for Basilisk, but I will have to think a bit more about it.

How do we handle build options like vizInterface and opNav?

Conan arguments can be passed in directly through an environment variable:

CONAN_ARGS="-o opNav=True -o vizInterface=True" pip install .

I used an environment variable because pip and setuptools don't have a stable or clean way to share command-line arguments between each other (even now in 2024...). Also, this is probably more convenient than creating a custom configuration file.

Regarding build time, how does the new system compare to prior, about the same?

I have not noticed much difference personally, but I suspect it is a tad slower. It installs the build tools every time, so there are at least a few extra seconds at the beginning.

on macOS with Python 3.11 and cmake 3.28.1, I was able to run pip install . from the root BSK folder. I had a clean new venv environment and deleted .conan folder. When trying to run scenarioBasicOrbit.py, I got an error that colorama package was not installed? After installing it, it couldn't find tqdm. Note I created a new .venv and ran pip install, I didn't manually install other packages first. After installing these two packages I was able to run this basic orbit example script.

Thank you for checking on Mac+3.11! Actually, pip installs and uses an appropriate version of CMake automatically, so your CMake version was not relevant to this build. This is the magic of the build requirements :)

The reason for the error you noticed is that both tqdm and colorama are dependencies of conan, which is a build requirement, but not a runtime requirement of Basilisk. Build requirements are used only during the build of Basilisk, and are not installed together with Basilisk.

If you need tqdm and colorama as a run-time requirement for Basilisk, then they need to be explicitly specified in the requirements.txt file. (I intentionally removed build requirements such as conan from this file, since they were not meant as run-time requirements.)

(In addition, I guess this scenario does not get exercised during unit tests, so this error is not caught automatically.)

what are the instructions to do a clean build option with this build system?

Due to the above annoyances of pip, all installs using pip are clean builds. See the incremental build question above.

I ran the basic orbit script to save off the Vizard binary, that worked, so vizInterface must be built.

Yes, I think the vizInterface option is True by default in the conanfile. You should be able to disable it (if you wanted) with:

CONAN_ARGS="-o vizInterface=False" pip install .

How do you build for Release or Debug to have break points?

By default, we specify the -s Release option to Conan. You can override this with the CONAN_ARGS environment variable as per above. (I assume this is all that you need for a Debug release?)

Please let me know if anything is unclear, and enjoy your travels!

[schaubh]

Thanks for the info @dpad . From what you are explaining, it sounds like the python conanfile.py build system is still the preferred way to build BSK if you are developing, but these changes would enable also building wheels? We would document how to build wheels if desired, and we could build and upload wheels for tagged BSK releases. Is this the expected use?

I have pinged some other BSK developers to take a look at this branch as well to get broad community feedback. Making wheels would be very exciting ;-)

[dpad]

@schaubh Yes, for now, the python conanfile.py is still the most convenient method for developers. This mainly just adds support for building the wheels using pip. Future merge requests will add cibuildwheel to build generic wheels for each architecture in CI, and later on we can add twine to upload these wheels directly to PyPi so users can just pip install Basilisk from anywhere.

For reference, I've now pushed a cleaner approach that simply wraps the python conanfile.py-based workflow, so it can be supported more easily going forward. This drops a lot of the ugliness from before, now we effectively just inject a call to python conanfile.py <some args> directly and then package up the results. I've tested this again locally for sdist/wheels and opNav on my Linux machine, and all tests pass as expected, but I'll leave it to the maintainers to test further.

I considered some alternative build backends, but had issues with each of them for maintaining backwards compatibility. As dirty as the current setuptools one is, it seems to be the best option for now.

Also note, I've added tqdm and colorama to the run-time requirements.txt, since they are needed for the SimulationProgressBar.

[juan-g-bonilla]

Hi @dpad ! Awesome work with this PR. I've been trying to build in Windows these past days with little success. The installation seems to be failing to link a missing file python3.lib. I'm attaching two build logs, one for a successful build on develop with the following command:

python conanfile.py --clean --vizInterface False --generator "Visual Studio 17 2022"  3>&1 2>&1 > build_log_develop.txt

and the other for the failed build with this PR:

set $env:CONAN_ARGS='-o vizInterface=False -o generator="Visual Studio 17 2022"'
pip install .  3>&1 2>&1 > build_log_pip_packaging_minimal.txt

These are my env variables:

PS ~> echo $env:PYTHON_INCLUDE
C:\Users\juang\AppData\Local\Programs\Python\Python312\include
PS ~> echo $env:PYTHON_LIB    
C:\Users\juang\AppData\Local\Programs\Python\Python312\libs\python312.lib

I tried changing the PYTHON_LIB to C:\Users\juang\AppData\Local\Programs\Python\Python312\libs\python3.lib it didnt seem to have any effect.

I can keep tinkering, but thought I would post to see if you had any ideas.

build_log_pip_packaging_minimal.txt
build_log_develop.txt

[dpad]

Hi @juan-g-bonilla , thanks a lot for trying it out! I had a look at my old branch that did more work on the Windows side. It seems I had to add a hack-around for MSVC. Have a look at this:

https://github.com/dpad/basilisk/blob/7a3bf92224342cf49a47866b8e6280f40fc94927/src/CMakeLists.txt#L72C1-L83C10

Alternatively, you could try reverting my find_package(Python3) changes and just keep using find_package(Python), but I think I had to update to find_package(Python3) for some other platform reason that I can no longer remember. Maybe we can just leave that upgrade to later when we see issues during the platform builds.

Or, just disable the "PY_LIMITED_API" definition inside conanfile.py. It's not strictly necessary, but will be useful later when we're building platform-specific wheels (otherwise we'll need one wheel per platform+Python version combination, which is madness).

(I'll either add the above hack or revert to find_package(Python) in this merge request when I get the chance later this week, maybe tomorrow.)

[juan-g-bonilla]

Thanks @dpad ! With following it builds correctly:

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 13b659ae9..ef19d3d8e 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -65,6 +65,20 @@ if(${SWIG_VERSION} VERSION_GREATER_EQUAL 4.2)
     message(STATUS "SWIG>=4.2.0 found, building with Py_LIMITED_API=${PY_LIMITED_API}")
     message(STATUS "(Refer to https://docs.python.org/3/c-api/stable.html#limited-c-api for details)")
     add_definitions("-DPy_LIMITED_API=${PY_LIMITED_API}")  # Support for current Python version
+
+    if (MSVC)
+      # XXX: Hacky! Py_LIMITED_API somehow forces the Windows compiler to link to
+      # `python3.lib`, but it can't find it because that doesn't match the name
+      # found by CMake's findPython3. Actually, CMake 3.26+ can search for the the
+      # "Development.SABIModule" in findPython3 instead, but in yet another cruel
+      # bug, setting the cmake_minimum_version() to 3.21 or higher ends up
+      # crashing CMake on Windows as well. Reason is still TBD.
+      # So let's just overwrite the Python3 library name here and be done with it.
+      # See https://gitlab.kitware.com/cmake/cmake/-/issues/24141
+      link_directories(AFTER "${Python3_LIBRARY_DIRS}")
+      set(Python3_LIBRARIES "${Python3_LIBRARY_DIRS}/python3.lib")
+    endif()
+
   endif()
 endif()

[dpad]

@juan-g-bonilla
Thanks for the confirmation!

I pushed some additional minor changes, including one that should fix the above Windows build issue properly (without that "hacky" work-around). However, I believe last time I tried it, Windows had issues with more recent versions of CMake. Would you mind trying out the latest changes again? If they work on both Windows and Mac, then I think this is ready for review.

[juan-g-bonilla]

@dpad The branch (at d8a981b) builds on my machine!

[dpad]

@juan-g-bonilla Thanks again for your help! I've finished cleaning up and have updated the main merge request comment above, including a new "Motivation & Rationale" section. I have maintained full backwards compatibility with the existing python conanfile.py workflow, but please let me know if any of the changes in conanfile.py are suspicious or unclear or if you see any issues. Note that the cleaned up version just wraps python conanfile.py directly, so CONAN_ARGS now takes the same arguments as that. In the future, if we deprecate or upgrade Conan, we can change the calls in setup.py easily.

I'm going to take this off Draft for now and leave it with you, if that's okay.

On a separate note, I raised a new bug here: #738
Could you please check if you observe the same behaviour? I'm getting that bug on the normal develop branch, so I don't think it's related to the changes here, but since the pip install . uses SWIG 4.2.0, it also fails the above test. I'm not sure why the test wasn't failing for me before... See my updates in that bug ticket, it's just some bizarre unrelated behaviour. The changes in this pull request should still pass.

[juan-g-bonilla]

Hi @dpad , thanks for all the work! Before we go through final review, would you find cleaning up the commits? In Basilisk, once we are satisfied with a PR, we usually rebase all commits and re-commit such that all related changes go together (while usually changes in commits appear chronologically as they are developed). Some commits might make better sense together or split, for example. A good example in this PR is the deprecatedBuild folder, which is added in the first commit but later removed. Similarly, there are line changes in some commit that are just re-changed in following commits. I would be best if we just had commits that reflect the final solution, and not the development process. I trust your judgement on how best to do this!

Moreover, it would be great if you could update the release notes with this change, as well as the documentation for how to setup Basilisk.

[dpad]

@juan-g-bonilla Thanks for getting back to me! I've cleaned up the commits and added some documentation + release notes. Please have a look.

[schaubh]

I'm back from my travel and catching up on lots of other work tasks. I plan to return to this PR early next week to review and test in more detail. I also want some of my researchers to pull and do test builds across more computers.

[juan-g-bonilla]

Checked again that the PR builds on my Windows machine without issue. Don't know enough about the build system and libraries here to do little more than bikeshedding, but I reviewed the code and didn't find any obvious code smell. Let's get more people to build this on their systems and then run the workflows.

[schaubh]

Ok, did a clean macOS build of your branch using python conanfile.py and at the end the terminal it showed:

note: Run script build phase 'CMake PreBuild Rules' will be run during every build because the option to run the script phase "Based on dependency analysis" is unchecked. (in target 'GeneratedProtobufMessages' from project 'basilisk')
note: Run script build phase 'Generate utilities/vizProtobuffer/CMakeFiles/GeneratedProtobufMessages' will be run during every build because the option to run the script phase "Based on dependency analysis" is unchecked. (in target 'GeneratedProtobufMessages' from project 'basilisk')
note: Run script build phase 'Generate CMakeFiles/ZERO_CHECK' will be run during every build because the option to run the script phase "Based on dependency analysis" is unchecked. (in target 'ZERO_CHECK' from project 'basilisk')
note: Run script build phase 'CMake PostBuild Rules' will be run during every build because the option to run the script phase "Based on dependency analysis" is unchecked. (in target 'ModuleIdGenerator' from project 'basilisk')
note: Run script build phase 'CMake PostBuild Rules' will be run during every build because the option to run the script phase "Based on dependency analysis" is unchecked. (in target 'vizardLib' from project 'basilisk')
note: Run script build phase 'CMake PostBuild Rules' will be run during every build because the option to run the script phase "Based on dependency analysis" is unchecked. (in target 'environmentLib' from project 'basilisk')
note: Run script build phase 'CMake PostBuild Rules' will be run during every build because the option to run the script phase "Based on dependency analysis" is unchecked. (in target 'dynamicsLib' from project 'basilisk')
note: Run script build phase 'CMake PostBuild Rules' will be run during every build because the option to run the script phase "Based on dependency analysis" is unchecked. (in target 'transDeterminationLib' from project 'basilisk')
note: Run script build phase 'CMake PostBuild Rules' will be run during every build because the option to run the script phase "Based on dependency analysis" is unchecked. (in target 'onboardDataHandlingLib' from project 'basilisk')
note: Run script build phase 'CMake PostBuild Rules' will be run during every build because the option to run the script phase "Based on dependency analysis" is unchecked. (in target 'effectorInterfacesLib' from project 'basilisk')
note: Run script build phase 'CMake PostBuild Rules' will be run during every build because the option to run the script phase "Based on dependency analysis" is unchecked. (in target 'architectureLib' from project 'basilisk')
note: Run script build phase 'CMake PostBuild Rules' will be run during every build because the option to run the script phase "Based on dependency analysis" is unchecked. (in target 'powerLib' from project 'basilisk')
note: Run script build phase 'CMake PostBuild Rules' will be run during every build because the option to run the script phase "Based on dependency analysis" is unchecked. (in target 'OverwriteSwig' from project 'basilisk')

Is this expected? Would it make sense to turn on "Based on dependency analysis"?

Good news is the build worked as expected and all associated unit tests passed. Trying next with opNav modules.

[schaubh]

Built with opNav flag and that worked as expected. I here I was able to make an Xcode project and build within Xcode. All unit tests passed again. Looking promising 🤠

[dpad]

@schaubh Thanks very much for your and your researchers' efforts in checking this! I'm sorry that I have not had a chance to make the requested review changes yet, I may have some time later this week or next, but if not please feel free of course to update this branch as you see fit.

[schaubh]

Howdy @dpad, you say "update the branch". I did add your fork to my BSK source setup so I can directly pull your branch. But, I can't push back any edits to it, I don't have write access. I applied my requested changes to feature/pip-packaging-minimal-HPS so you could readily cherry-pick them over to your branch. If there is a better way to for me to contribute directly to your fork's branch, please let me know. Glad to learn better ways to collaborate with external contributors.

[dpad]

I have the option below ticked, so I believe you should be able to push directly to this branch.

[schaubh]

Thanks, I'll give this a try. This would make collaboration with external contributors that much easier. I'll apply my edits and squash them into your commits to leave a clean, rebased branch that I was testing.

[schaubh]

Howdy @dpad , I pulled your latest branch, applied all my small tweaks and edits, fixed the footnote RST issue I found. However, when I try to force push the rebased branch to your fork I get this error:

Pushing to https://github.com/dpad/basilisk.git
error: Authentication error: Authentication required: You must have push access to verify locks
error: failed to push some refs to 'https://github.com/dpad/basilisk.git'
Completed with errors, see above

Any idea what is happening here?

[dpad]

Hi @schaubh , I also haven't used this feature before, but it sounds like potentially an issue with git-lfs. I'm not sure if it'll work, but could you try the setting below? (The --local flag should ensure this setting only applies to my fork and shouldn't affect your other repos.)

git config --local lfs.https://github.com/.locksverify false

[schaubh]

Howdy @dpad , I ran that command and tried to force push my branch to your branch, but got this error message now.
error.txt

I tried moving my local copy of your branch to origin/pip-packaging-minimal instead of origin/feature/pip-packaging-minimal, but that made no different. Shucks...

[schaubh]

Wheels are working well though! I am wondering how to reduce their size though, the Spice data is the biggest hit. We should think about how this is packaged in the future. I don't see a reason the data files should be part of the wheel? If someone runs BSK scripts and loads up data files, then they have to ensure the files are in the proper location relative to the script?

[dpad]

@schaubh It seems your push worked, no?

Regarding wheel sizes, I haven't looked into or thought about it too much yet, but I definitely noticed the SPICE data being the biggest. One option could be to have a kind of "lazy load" of SPICE data, where it only downloads the data the first time it is used.¹ Otherwise, you could just leave the data as is, since I guess most users will probably need the SPICE data anyway, it won't save much time or effort downloading them later.

Looking into this though, I've just learned that Pypi apparently has a size limit of 100MB (although it can be increased in some cases), so we would probably have to host the wheel files ourselves anyway (e.g. as Github release artifacts), which is probably not a huge deal either. But any effort to reducing the size might be worth it.

Footnotes

1. This is something I did in an old library I made, although it's in Julia so not of much help... ↩

[schaubh]

Glad to see this force push worked! It wasn't apparent from my side.

Regarding the wheel size, we can refine that in a separate branch. The data files are loaded dynamically in the BSK scripts. They just need to be available to the script, they don't need to be part of the wheel. We can do a separate branch to refine the wheel size now that this is working. I would like to get rid of the LFS requirement as well and find an alternate way to install the data files that are just needed to run BSK sample scripts, not for the software package itself.

[spiggottCO]

After reading further, I think my comment isn't necessary. This also seems like a solid change to the build system that can unlock distributing Basilisk as a binary package which is EXTREMELY COOL!

[schaubh]

Great contribution. Many users will appreciate the ability to generate and install via wheels.

[dpad]

@schaubh @spiggottCO Thanks a lot for all the efforts from you and the other maintainers! Hopefully this is helpful going forward.

The next step I recommend is to build these wheels as part of the CI process using a tool called cibuildwheel, which automates a lot of the work (including building against old compilers and system libraries so that the wheels have the highest possible compatibility across a wide range of machines).

The biggest will be that the CI builds take a fairly long time, and I'm not sure what limits there are for CI on Github...

For reference, I've been using cibuildwheel successfully for Basilisk to build Linux wheels automatically. When I next get the chance, I'll push changes to the CI here for you to have a look.

[schaubh]

Howdy @dpad , thanks for sharing the info on automated CI builds. I'm still very new to this process and appreciate your insight. I did create a PyPi account and need to read up on the process of contributing in my spare time.

[Mark2000]

@dpad I spent a bit of time looking into cibuildwheel today before I saw that you said you got it working. In particular, I was running into errors about the right compiler version not found when generating protobufs:

        [ 10%] Generating protobuf (C++): vizMessage
        /root/.conan/data/protobuf/3.17.1/_/_/package/53f76f058631e1c3a500008c75af3f37d7786131/bin/protoc: /lib64/libstdc++.so.6: version `GLIBCXX_3.4.20' not found (required by /root/.conan/data/protobuf/3.17.1/_/_/package/53f76f058631e1c3a500008c75af3f37d7786131/bin/protoc)
        /root/.conan/data/protobuf/3.17.1/_/_/package/53f76f058631e1c3a500008c75af3f37d7786131/bin/protoc: /lib64/libstdc++.so.6: version `CXXABI_1.3.8' not found (required by /root/.conan/data/protobuf/3.17.1/_/_/package/53f76f058631e1c3a500008c75af3f37d7786131/bin/protoc)
        /root/.conan/data/protobuf/3.17.1/_/_/package/53f76f058631e1c3a500008c75af3f37d7786131/bin/protoc: /lib64/libstdc++.so.6: version `GLIBCXX_3.4.21' not found (required by /root/.conan/data/protobuf/3.17.1/_/_/package/53f76f058631e1c3a500008c75af3f37d7786131/bin/protoc)
        gmake[2]: *** [/project/src/utilities/vizProtobuffer/vizMessage.pb.cc] Error 1
        gmake[1]: *** [utilities/vizProtobuffer/CMakeFiles/vizMessage.dir/all] Error 2
        gmake[1]: *** Waiting for unfinished jobs....

I'm now looking at the route of just exporting the wheels generated by the result of our pip-based install test that does work (I want the wheel for use in another CI system and thus don't need a full set of platforms and versions, so cibuildwheel is overkill for my use case), but I'm curious if this is an issue you faced and found a solution for.

[dpad]

Hi @Mark2000 , thanks for looking into it!

I'm not sure I can tell you much without seeing the full context of what you're trying, but it looks like you might be trying to use a pre-compiled version of protobuf? If this was running in cibuildwheel inside a manylinux Docker image, then those are specifically designed to have really old versions of GLIBC/GLIBCXX for maximum compatibility.

Did the call to cibuildwheel build this protoc binary from source, or did it download it as a prebuilt binary from Conan's servers? If the latter, then the pre-built binary was probably compiled with a GLIBCXX that is too new to work on the old Docker image you ran on. You would need to forcibly set Conan to build from source code directly.

For reference, the settings below for cibuildwheel seem to build Basilisk appropriately for Linux (these would go in the pyproject.toml file):

[tool.cibuildwheel]
skip = [
    # Don't build musllinux or on Pypy.
    "*-musllinux_*",
    "pp*",
    # TODO: Fix support for Python 3.12+.
    "cp312-*",
    "cp313-*",
]

# Run simple build test
test-command = [
    "ctest -C Release --test-dir {package}/dist3",
    "pytest {package}/src/tests -m \"not scenarioTest\"",
]
test-extras = ["test"]

# Build using the future-compatible manylinux_2_28 image.
# NOTE: This image contains a version of gcc that is not used by Conan's build servers,
# so it forces Conan to build all dependencies from source instead of downloading prebuilt binaries.
manylinux-x86_64-image = "manylinux_2_28"

# Only compile on 64-bit architectures for now.
[tool.cibuildwheel.linux]
environment-pass = ["CONAN_ARGS"]
archs = "auto64"

repair-wheel-command = [
    # Add --strip to the repair command to minimise the wheel size.
    # See https://github.com/AVSLab/basilisk/issues/814#issuecomment-2377263412
    "auditwheel repair --strip -w {dest_dir} {wheel}",
]

[Mark2000]

Ah, okay. Some chatgpt-aided debugging got me to the point of realizing that I needed to get Conan to build protocol from source, but couldn't get the configs quite right.

[ephraim271]

@dpad

Allow External Modules to be compiled separately from Basilisk.
Currently, developers must compile the entire Basilisk system to build their own custom modules.
I have some thoughts on how we can decouple the External Modules build system from the rest of the system, and allow users to just compile separately. I believe it is feasible but will take a fair bit of work.
Also should allow multiple External Modules folders to be specified. This should be a pretty easy change to the CMake files and can be done first.

I have a basilisk build with some External Modules, so is this feature currently not applicable for that case? In that case, whats the best alternative to distribute, docker?

[sassy-asjp]

@ephraim271 If you build with External Modules, the resulting wheel will have them, and you can distribute that just like you would any other python wheel. The separate External Modules compilation feature would be to be able to build separate wheels for the core Basilisk and the External Modules, e.g., a future where you could install Basilisk from the public pypi server, and your own External Modules from an internal server.