-
Notifications
You must be signed in to change notification settings - Fork 116
fix: Python installation #520
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #520 +/- ##
========================================
Coverage 73.30% 73.30%
========================================
Files 26 26
Lines 8068 8068
Branches 1691 1692 +1
========================================
Hits 5914 5914
Misses 1627 1627
Partials 527 527
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Great, it seems we need to resolve #466 for this 😮💨 |
b56e4a8
to
c8d58da
Compare
Current status:
|
The commit looks good to me. |
I was rather referring to the generated documentation, e.g.: |
Oh weird that must have sneaked in when we introduced
Wasn't that some weird quirk of |
Ok, managed to fix the windows and macos issues. But this PR is blocked by an upstream issue: scikit-build/scikit-build-core#860 basically if we run |
Final blocker: scikit-build/scikit-build-core#880 |
2014ede
to
a93de94
Compare
Ok, I've managed to figure out a workaround for the pip isolation failure. This PR is ready for review, but I don't know if we want to add it to Could you manually check with $ pip install "spglib@git+https://github.com/LecrisUT/spglib@fix/510" Particularly test that it imports correctly every time you re-run You should also have a full set of wheel artifacts here |
Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <git@lecris.dev>
- `add_subdirectory` must not be conditional to a cache variable before it is defined (`Spglib_SOURCE_DIR` is a cache variable) - The only appropriate conditional to use here is `TARGET` - Moved main target definitions before "External packages". Must be after `BUILD_SHARED_LIBS` is set. Signed-off-by: Cristian Le <git@lecris.dev>
- Python installation within CMake only is no longer supported. Avoid missing metadata files. - Install everything under the `spglib` package using default `GNUInstallDirs` paths Signed-off-by: Cristian Le <git@lecris.dev> Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <git@lecris.dev>
Awesome! I've checked both normal and editable installs, and they work. |
Reworked a lot of the CMake build
scikit-build-core
is no longer supported. Avoids dangling files that cannot be removed due to lacking python package metadata files (spglib.dist-info
)Spglib::symspg
(more robust than checking for any cache variables)find_package
to check for system installedSpglib
CMAKE_DISABLE_FIND_PACKAGE_Spglib=ON
to make sure the bundled version is usedCMAKE_REQUIRE_FIND_PACKAGE_Spglib=ON
to make sure a system version is usedfind_package
failed (andCMAKE_REQUIRE_FIND_PACKAGE_Spglib
was not set), use the bundled sourcesspglib
C library,libsymspg
must be resolved at build timeINSTALL_RPATH
for the python_spglib
library pointing to$ORIGIN/${CMAKE_INSTALL_LIBDIR}
. If you want to use a system installed library, there must not be alibsysmspg
library therecibuildweel
workaround from Update CI dependencies #507Depends-on: #458
Closes #510 Closes #462