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

Pip / Conda Packaging Draft PR #84

Draft
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

georgiastuart
Copy link

@georgiastuart georgiastuart commented Sep 4, 2024

This is still very much a rough draft, but I wanted to get this PR open for discussion and iteration!

The aim here is to enable installation via pip, per discussion with @The9Cat . There's also some work on conda packaging, but that's not in working form. The pip installation method does work, but I think I did too much manipulation of the existing CMakeLists.txt for this to be a viable PR.

What this PR achieves:

  • creates the pip install structure for EXP as three packages: EXP-libraries, EXP-nbody, and pyEXP. The first two can be installed independently of pyEXP.

Side effects:

  • Mostly impacts the current CMake build files since I needed greater separation between the component parts.

Things that need to be done here:

  • Add a pypi package for the util executables
  • revisit how CMakeLists.txt was altered to remove the impact on the standard build system
  • add a lot more documentation on how the pip packages are built
  • CICD for building the pip packages for hosting on github or elsewhere
  • Either complete or remove the conda additions for another PR

Signed-off-by: Georgia Stuart <gstuart@umass.edu>
Signed-off-by: Georgia Stuart <gstuart@umass.edu>
Signed-off-by: Georgia Stuart <gstuart@umass.edu>
Signed-off-by: Georgia Stuart <gstuart@umass.edu>
Signed-off-by: Georgia Stuart <gstuart@umass.edu>
Signed-off-by: Georgia Stuart <gstuart@umass.edu>
@georgiastuart georgiastuart marked this pull request as draft September 4, 2024 23:49
Signed-off-by: Georgia Stuart <gstuart@umass.edu>
Copy link
Member

@michael-petersen michael-petersen left a comment

Choose a reason for hiding this comment

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

I'm happy to be a 'standard build' test subject for this PR! I ran my native build script on a fresh pull of this fork, and am working through some kinks.

CMakeLists.txt Outdated
include_directories(${PROJECT_SOURCE_DIR}/extern/pybind11/include)
endif()

find_package(yaml-cpp REQUIRED)
Copy link
Member

Choose a reason for hiding this comment

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

For my standard build, this line addition is a problem. Perhaps it needs to be inside some if statement?

CMake Error at CMakeLists.txt:248 (find_package):
  By not providing "Findyaml-cpp.cmake" in CMAKE_MODULE_PATH this project has
  asked CMake to find a package configuration file provided by "yaml-cpp",
  but CMake did not find one.

  Could not find a package configuration file provided by "yaml-cpp" with any
  of the following names:

    yaml-cppConfig.cmake
    yaml-cpp-config.cmake

  Add the installation prefix of "yaml-cpp" to CMAKE_PREFIX_PATH or set
  "yaml-cpp_DIR" to a directory containing one of the above files.  If
  "yaml-cpp" provides a separate development package or SDK, be sure it has
  been installed.

Copy link
Author

@georgiastuart georgiastuart Sep 5, 2024

Choose a reason for hiding this comment

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

Ah! I think it should only use find_package for yaml-cpp if USE_SUBMODULES=OFF, so I'll fix that. Merging in the latest updates and fixing up any oddities.

Signed-off-by: Georgia Stuart <gstuart@umass.edu>
@michael-petersen
Copy link
Member

Thanks for working on this! The good news is that the continuous integration is now triggering for this PR, and is replicating what I'm seeing on my machine, so there is now a whole traceback. Perhaps pybind11 also needs a USE_SUBMODULES=OFF trigger? I'm also happy to tinker if helpful.

@georgiastuart
Copy link
Author

I've got some changes in progress to fix the nbody pip packaging after pulling in the updates. I'll push those up tonight or tomorrow, then I think we can fix up the standard build! I just don't want to clobber your fixes while I'm still making changes.

Signed-off-by: Georgia Stuart <gstuart@umass.edu>
@georgiastuart
Copy link
Author

Main exe works now! Fixing up pyexp and then it should pass the tests.

georgiastuart and others added 15 commits September 17, 2024 12:11
Signed-off-by: Georgia Stuart <gstuart@umass.edu>
Signed-off-by: Georgia Stuart <gstuart@umass.edu>
Signed-off-by: Georgia Stuart <gstuart@umass.edu>
Signed-off-by: Georgia Stuart <gstuart@umass.edu>
Signed-off-by: Georgia Stuart <gstuart@umass.edu>
Signed-off-by: Georgia Stuart <gstuart@umass.edu>
Signed-off-by: Georgia Stuart <gstuart@umass.edu>
Signed-off-by: Georgia Stuart <gstuart@umass.edu>
Signed-off-by: Georgia Stuart <gstuart@umass.edu>
Signed-off-by: Georgia Stuart <gstuart@umass.edu>
Signed-off-by: Georgia Stuart <gstuart@umass.edu>
Signed-off-by: Georgia Stuart <gstuart@umass.edu>
Signed-off-by: Georgia Stuart <gstuart@umass.edu>
Signed-off-by: Georgia Stuart <gstuart@umass.edu>
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.

2 participants