Implement failover with libraries found in the system #18
Conversation
pombredanne
left a comment
pombredanne
left a comment
There was a problem hiding this comment.
@tardyp Thanks! this makes a lot of sense. The idea for using a system-provided libarchive or 7zip so far has been to have instead a plugin that provides "system paths" ... but these feel contrived compared to your cleaner and simpler approach here.
For the current code, see https://github.com/nexB/scancode-plugins/tree/main/builtins/extractcode_7z_system_provided and https://github.com/nexB/scancode-plugins/tree/main/builtins/extractcode_libarchive_system_provided
Then we have that extra requires in https://github.com/nexB/extractcode/blob/main/setup.cfg#L49
So if one wants to use system-installed binaries, the procedure would be:
- install them by pip first
pip install typecode[full]- install scancode-toolkit separately
With all that said, I made a few minor comments inline for your consideration.
And I am not sure we could have a test for these?
Also which OS and architecture are you getting support and problems with?
src/extractcode/sevenzip.py
Outdated
| 'plugin with a valid executable available.' | ||
| ) | ||
| sevenzip = which("7z") | ||
| if sevenzip is None: |
There was a problem hiding this comment.
What about this instead?
| if sevenzip is None: | |
| if not sevenzip: |
src/extractcode/libarchive2.py
Outdated
| ) | ||
| filepath = ctypes.util.find_library(_LIBRARY_NAME) | ||
| libarchive = ctypes.cdll.LoadLibrary(filepath) | ||
| if libarchive is None: |
There was a problem hiding this comment.
What about this instead?
| if libarchive is None: | |
| if not libarchive: |
|
hi @pombredanne thanks for review. please have a look at related issue. We are having problems with mac m1 and windows wsl2. aboutcode-org/scancode-toolkit#2470 according to https://github.com/nexB/scancode-toolkit/blob/develop/setup.cfg#L98 'extractcode[full]' is required for scancode. If I don't comment those lines, even if I install extractcode first, pip will still try to install the [full] dependencies which are not available because extractcode-libarchive is only available for a select number of systems. |
pombredanne
left a comment
pombredanne
left a comment
There was a problem hiding this comment.
Thanks for the updates! Do you mind to switch to using single quotes for strings?
We will eventually switch to using Black as a style at once, but until then I would prefer to keep single quotes consistently in the code.
src/extractcode/libarchive2.py
Outdated
| EXTRACTCODE_LIBARCHIVE_DLL = 'extractcode.libarchive.dll' | ||
| _LIBRARY_NAME = "libarchive" | ||
|
|
||
| def load_lib_failover(): |
There was a problem hiding this comment.
Why are you keep a no-op function there?
Sorry for missing your aboutcode-org/scancode-toolkit#2470 post
|
This allows scancode to install and run in systems not supported by extractcode-libarchive and extractcode-7zip Signed-off-by: Pierre Tardy <pierre.tardy@renault.com>
|
@tardyp please see also aboutcode-org/scancode-plugins#9 (comment) |
2 participants
This allows scancode to install and run in systems not supported by
extractcode-libarchive and extractcode-7zip