Handle C extensions not inside a package directory (at the root)#39
Handle C extensions not inside a package directory (at the root)#39natefoo wants to merge 1 commit intomatthew-brett:masterfrom
Conversation
0dcf21e to
0667367
Compare
matthew-brett
left a comment
There was a problem hiding this comment.
Thanks - and sorry to be so slow to review, the start of term got the better of me.
|
|
||
| def find_package_dirs(root_path): | ||
| """ Find python package directories in directory `root_path` | ||
| def find_packages(root_path): |
There was a problem hiding this comment.
Sorry - I'm a little sensitive about changing the API without warning.
Would you mind making and deprecating something like:
def find_package_dirs(root_path):
return [p for p, is_dir in find_packages(root) if is_dir]
and importing into delocating?
| package_sdirs : set | ||
| Set of strings where each is a subdirectory of `root_path`, containing | ||
| an ``__init__.py`` file. Paths prefixed by `root_path` | ||
| Set of strings where each is a subdirectory of `root_path` containing an |
There was a problem hiding this comment.
Actually a set of (string, is_dir) tuples no?
| if isdir(fname) and exists(pjoin(fname, '__init__.py')): | ||
| package_sdirs.add(fname) | ||
| package_sdirs.add((fname, True)) | ||
| elif isfile(fname) and ((fname.endswith('.so') or fname.endswith('.dylib'))): |
There was a problem hiding this comment.
Use lib_filt_func here? See delocate_wheel docstring.
There was a problem hiding this comment.
I'm not sure how to use lib_filt_func here - here we're not checking the dependencies of found objects, but only whether a member of root_path is a module. What would lib_filt_func is None mean in this context? It can't mean that every directory/file found is a package, nor should lib_filt_func == "dylibs-only" cause the function to ignore packages in subdirs.
Perhaps I should move _dylibs_only() to tools and use it here (and import it in delocating)?
There was a problem hiding this comment.
I'm sure sure I understand, I might have been too hasty but lib_filt_func is to decide what files to consider when looking for libraries. I guess it's fine print. Yes, moving to tools sounds like a reasonable thing to do.
eac59ce to
aa5c4ab
Compare
|
looks like this has a conflict now -- I'm very interested in using this branch |
|
@natefoo - OK to rebase? I promise to review quickly. |
|
the conflict is pretty trivial -- it's just mad about imports -- (can compare against https://github.com/asottile/delocate/tree/top-level-fix-squash_asottile) |
aa5c4ab to
58106c6
Compare
|
@matthew-brett ping, I rebased this (also feel free to modify directly). |
|
FWIW, I'm using this patch to build |
|
@matthew-brett Anything I help with to get this over the line? |
|
reaching for this again -- unfortunately the conflict is pretty messy now so I'm having trouble rebasing this :( |
|
@bsolomon1124, thank you for continue working on this. BTW recently per pypa/auditwheel#90, auditwheel has just adopted the new naming scheme for dynamic libraries ( |
|
Superseded by #123. |
@matthew-brett how's this look for an implementation? It's working for me but needs a few tests.
Root packages end up with a
.dylib<pkg>directory for delocated libs, this mirrors what auditwheel does.