aboutcode-org / aboutcode-org/scancode-toolkit
Streamline packagedcode.licensing
- Ngôn ngữ chính
- Python
- Star
- 2.6k
- Fork
- 791
- Merge trung bình
- 1 ngày 12 giờ
- Pull request đã merge (30 ngày)
- 5
Mô tả
There could be ways to streamline and simplify the code in packagedcode.licensing which may be trying to handle too many special cases for all package types at once, with a special case for Maven. I feel that the code has grown complex and could benefit from some love.
There is some classes relationship that need clarification like `packagedcode.models.DatafileHandler` and `packagedcode.models.PackageData` with:
- the instance method `packagedcode.models.PackageData.populate_license_fields`
- and the class method `packagedcode.models.DatafileHandler.populate_license_fields`
These are called sometimes or some other by different package types and is a source of confusion:
- about, conan, conda, npm, pypi, readme and swift call the PackageData instance method
- alpine, freebsd call the DatafileHandler class method
- Maven is special
Here is how the code works today, starting in `packagedcode.models`:
- `packagedcode.models.PackageData.normalize_extracted_license_statement`: stores the extracted_license_statement as a YAML string in the extracted_license_statement
- `packagedcode.models.PackageData.populate_license_fields` detects the license from the statement called from a few places and calls:
- `packagedcode.models.PackageData.get_license_detections_and_expression` which calls:
- `packagedcode.licensing.get_license_detections_and_expression`
- This calls also before `packagedcode.models.get_default_relation_license` which does a lookup in to find the PackageData class by `datasource_id` .... the default_relation_between licenses could instead just use a self attribute on PackageData called directly rather than having an indirection.
Then in `packagedcode.licensing` starting with the call to `packagedcode.licensing.get_license_detections_and_expression`:
- Get a `package_data_class` from a `datasource_id` which has only one entry for Maven `packagedcode.maven.MavenPackageData` where IMHO we should instead always get the method from a `packagedcode.models.PackageData` class directly via self, and this not only for Maven but all classes.
- For Maven only, this calls `packagedcode.maven.MavenPackageData.get_license_detections_for_extracted_license_statement` which does a very basic data structure check: is this a list of mappings as found in licenses/license/name|url|comments data in a POM or not? (??? (how could this ever happen?) )
- if not a regular Maven data structure (how could this ever happen?), this calls `packagedcode.licensing.get_normalized_license_detections`
- otherwise it cleans and converts the list of mappings everything back to a YAML string and calls `packagedcode.licensing.get_license_detections_for_extracted_license_statement` passing a string
- For packages other than Maven, this calls first: `packagedcode.licensing.get_normalized_license_detections`
This goes through variable processing depending on whether the the extracted license is:
- a str, then call get_license_detections_for_extracted_license_statement on that value
- a dict, then call get_license_detections_for_extracted_license_statement on each values of the dict (weird, but keys are fully ignored)
- if nothing found, then call get_unknown_license_detection on a REPR (weird we should use a YAML dump)
- anything but a list: call get_license_detections_for_extracted_license_statement on a YAML dump
- a list: iterate on each item then
- if a str, call get_license_detections_for_extracted_license_statement on it
- if a dict, call get_license_detections_for_extracted_license_statement on each item value (weird, but keys are fully ignored)
- anything else, call get_license_detections_for_extracted_license_statement on a YAML dump
- For all package types, if no license detections after detection above, call get_unknown_license_detection on a YAML dump
- Finally, call get_mapping_and_expression_from_detections
Overall that process tries too much to be generic for any and all package types and data files, but each data file has a specific data shape for license (or a few limited well know data shapes) and it does not make sense to have such a complicated, one size fits all process.
Instead we should have a set of common primitives to work from lists, str, mappings, list of mappings, with our without keys, and unknown detection that should be called uniquely for each package type and adapted mildly as needed and better suited to handle special cases like Maven (with and without XML comments at the top), Legacy npm, old and new PyPI (with classifiers, license and now license-expressions) and much much more small variants that are making one size fits all hard to understand and debug.
So an improved design could look like this:
- for each DatafileHandler we would have an overriden `get_license_detections()` method, possibly with a base class minimal implementation assuming that the data is a string
- we would have multiple specialized primitive functions doing one thing well to detect:
- from a string as a license expression
- from a string that references a file
- from a string
- from a mapping using values, or key/values
- from a list of strings
- from a list of mappings
- for unknown license detection
- each overriden `get_license_detections()` method would be responsible for the call sequence and cascading using these functions in the special way that the data shape and data content of the package type demands
Hướng dẫn đóng góp
Đánh giá
Issue này chưa được đánh giá.