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

fix: Ignore pre-installed self-package from prefix_path #862

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

LecrisUT
Copy link
Collaborator

@LecrisUT LecrisUT commented Aug 13, 2024

Context:


I am opening to brainstorm some ideas on how to implement.

TODO:

  • Expose the ignore_prefix_path as a configure variable
  • Do not use canonical_name as self project name, instead find out the path from installed python files, entry-point etc.
  • How to deal when the project shared the CMake install path with another project?
  • Figure out why CMAKE_IGNORE_PREFIX_PATH does not works as intended
  • Write tests

About CMAKE_IGNORE_PREFIX_PATH not working as intended. From what I've tested, if I put the path as site-packages/spglib, then this does not work, it still continues to find the pre-installed package files, but if I remove the spglib path and make it point to effectively the same folder as CMAKE_PREFIX_PATH, it does work. This seems like a bug in CMake that it doesn't resolve itself when CMAKE_IGNORE_PREFIX_PATH is a child of CMAKE_PREFIX_PATH, which as much as I think about it, it should be this way right?

Here is what I managed to track down from upstream:

  1. Parse CMAKE_IGNORE_PREFIX_PATH
  2. Pass CMAKE_IGNORE_PREFIX_PATH to construct search path list
  3. Logic for filtering search paths

Copy link

codecov bot commented Aug 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.95%. Comparing base (4f06e8c) to head (a09cf06).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #862      +/-   ##
==========================================
+ Coverage   83.91%   83.95%   +0.04%     
==========================================
  Files          74       74              
  Lines        4363     4375      +12     
==========================================
+ Hits         3661     3673      +12     
  Misses        702      702              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

1 participant