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

Implement new library loading approach #20

Merged
merged 43 commits into from
May 31, 2021
Merged

Implement new library loading approach #20

merged 43 commits into from
May 31, 2021

Conversation

pombredanne
Copy link
Member

@pombredanne pombredanne commented May 27, 2021

We now load the libmagic native library and its database from paths found in:

  1. environment variables,
  2. OR a location provider plugin,
  3. OR the system PATH,
  4. OR we fail with an informative error message.

Based on original code contributed by @priv-kweihmann to scancode-plugins
in aboutcode-org/scancode-plugins#9 and
moved here and adapted for use in the core code rather than in a plugin.

This also includes the merge of @tardyp #17 to search for common locations as a failover

Signed-off-by: Philippe Ombredanne pombredanne@nexb.com

Signed-off-by: Philippe Ombredanne pombredanne@nexb.com

pombredanne and others added 30 commits January 25, 2021 12:54
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Steven Esser <sesser@nexb.com>
This allows scancode to work on platforms not supported by typecode-libmagic
e.g. M1 Macs or windows WSL

Most of this code is from https://github.com/ahupp/python-magic/blob/e0ccc6d/magic/loader.py
by Adam Hupp, MIT License, compatible with typecode's Apache

Signed-off-by: Pierre Tardy <pierre.tardy@renault.com>
Use virtualenv-embedded libraries
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
This will work even from a git archive or when git is not installed.

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
This was they do not end up in the template CHANGELOG.rst

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
We now load native libraries and executables from:
1. an envt. variable path
2. OR a locatin provider plugin
3. OR the PATH
or we fail

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Its was skipping plugin loading. Also add tracing.

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Only do content-based detection for programing language if there is
no file extension as this avoids a large number of detection issues.

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Aslo ensure we use assertion in the correct order.

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
We now load the libmagic native library and its database from paths found in:
1. environment variables,
2. OR a location provider plugin,
3. OR the system PATH,
4. OR we fail with an informative error message.

Based on original code contributed by @priv-kweihmann to
scancode-plugins in aboutcode-org/scancode-plugins#9 and
moved here and adapted for use in the core code rather than in a plugin.

Contributed-by: Konrad Weihmann <kweihmann@outlook.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
@pombredanne
Copy link
Member Author

@tardyp I merged 17 in this branch which also carries over in a single place the code originally contributed by @priv-kweihmann for the typecode_libmagic_system_provided plugins aboutcode-org/scancode-plugins#9

Getting both of your reviews would be great before I merge

@pombredanne pombredanne changed the title Implement new envt. variables approach Implement new library loding approach May 30, 2021
@pombredanne pombredanne changed the title Implement new library loding approach Implement new library loading approach May 30, 2021
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Copy link
Contributor

@tardyp tardyp left a comment

Choose a reason for hiding this comment

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

Hello,
Thanks for pinging me.

I am having hard time reviewing this 272 files PR.
Franckly I am not a huge fan of vendoring for opensource projects, although I do understand the need to limit dependency variablility for support reason.

I didn't file too much issue though, but I cannot claim a deep review. :-)

configure.bat Outdated
set PYTHON_EXECUTABLE=%PROVIDED_PYTHON%
goto run
if "%1" EQU "--python" (
echo "The --python is now DEPRECATED. Use the PYTHON_EXECUTABLE environment
Copy link
Contributor

Choose a reason for hiding this comment

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

there is an extra CR here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks. Fixed in https://github.com/nexB/skeleton and merged here

logger_debug('get_magicdb_location:', 'got path magicdb location:', magicdb_loc)

if not magicdb_loc:
warnings.warn(
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this not needed as libmagic will (probably) already complain if it does not find its db in standard location.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point... I like keeping the warning on our side too though as the message would be more explicit

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
From #20

Reported-by: Pierre Tardy <pierre.tardy@renault.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
@pombredanne
Copy link
Member Author

@tardyp you wrote:

I am having hard time reviewing this 272 files PR.
Franckly I am not a huge fan of vendoring for opensource projects, although I do understand the need to limit dependency variablility for support reason

I agree yet the Pygments vendoring here is a must here as we have dependencies on specific versions and vendoring is the sane approach to avoid conflicts (that popped up often in the past) when this fairly common Pygments library is installed otherwise.

@pombredanne
Copy link
Member Author

@tardyp note also that we should be able to support Apple ARM chips as soon as I can find some CI supporting it.

configure.bat Outdated Show resolved Hide resolved
And remove v prefix from fallback version

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
@pombredanne
Copy link
Member Author

This looks decent now. Merging and releasing!

@pombredanne pombredanne merged commit d5a26bb into main May 31, 2021
@pombredanne pombredanne deleted the use-env-var branch May 31, 2021 17:38
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.

3 participants