-
-
Notifications
You must be signed in to change notification settings - Fork 644
Parse .npmrc files
#4608
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
base: develop
Are you sure you want to change the base?
Parse .npmrc files
#4608
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -500,6 +500,48 @@ def update_workspace_members(cls, workspace_members, codebase): | |||||
| for member in workspace_members: | ||||||
| member.save(codebase) | ||||||
|
|
||||||
| class NpmrcHandler(BaseNpmHandler): | ||||||
| datasource_id = 'npmrc' | ||||||
| path_patterns = ('*/.npmrc',) | ||||||
| default_package_type = 'npm' | ||||||
| default_primary_language = None | ||||||
| description = 'npm .npmrc configuration file' | ||||||
| documentation_url = 'https://docs.npmjs.com/cli/v11/configuring-npm/npmrc' | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
this would pin the link to this version and will soon be outdated. I see some other npm doc URLs also have versions, can you also remove those in the PR? |
||||||
|
|
||||||
| @classmethod | ||||||
| def parse(cls, location, package_only=False): | ||||||
| """ | ||||||
| parse [.npmrc] file and store result in key : value pair. | ||||||
| convert key : value pair to object and return it. | ||||||
| """ | ||||||
| extra_data = {} | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are .npmrc examples with license/author info which we need to parse and store properly, this is not just extra data. Please also research examples found in the wild/docs to see what other fields we can use like this to map useful data to package data fields.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i'll update the handler to map meaningful fields (license, author, homepage, etc.) to ScanCode’s structured package fields. and other config-only fields (like proxy, cafile, always-auth) in extra_data, since they don’t map to package metadata. |
||||||
| with io.open(location, encoding='utf-8') as lines: | ||||||
| for line in lines: | ||||||
| line = line.strip() | ||||||
| if not line or line.startswith(';') or line.startswith('#'): | ||||||
| continue | ||||||
| if '=' not in line: | ||||||
| continue | ||||||
| key, value = line.split('=', 1) | ||||||
| key = key.strip() | ||||||
| value = value.strip() | ||||||
| # ignore empty key but allow empty values | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are there cases with empty values that is still useful to keep? can you provide examples?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks for pointing it out.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yet i have not seen any empty values case, should i include it to handler or just skip it (example: registry= ) ? |
||||||
| if not key: | ||||||
| continue | ||||||
| # if value is in single quote or in double quote, strip them | ||||||
| if (value.startswith('"') and value.endswith('"')) or (value.startswith("'") and value.endswith("'")): | ||||||
| if len(value) >= 2: | ||||||
| value = value[1:-1] | ||||||
| extra_data[key] = value | ||||||
|
|
||||||
| package_data = dict( | ||||||
| datasource_id=cls.datasource_id, | ||||||
| type=cls.default_package_type, | ||||||
| primary_language=cls.default_primary_language, | ||||||
| description=cls.description, | ||||||
| extra_data=extra_data, | ||||||
| ) | ||||||
| yield models.PackageData.from_data(package_data, package_only) | ||||||
|
|
||||||
| def get_urls(namespace, name, version, **kwargs): | ||||||
| return dict( | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| ; sample .npmrc for tests | ||
| # a comment line | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use real .npmrc files seen in the wild like with: https://github.com/search?q=path%3A*.npmrc&type=code This also helps you looks for what kind of data is present and whether we can use these differently rather than just storing in the |
||
| registry=https://registry.npmjs.org/ | ||
| cache=~/.npm | ||
| strict-ssl=true | ||
| //registry.npmjs.org/:_authToken="abc123" | ||
| init.author.name=John Doe | ||
| emptykey= | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| [ | ||
| { | ||
| "type": "npm", | ||
| "namespace": null, | ||
| "name": null, | ||
| "version": null, | ||
| "qualifiers": {}, | ||
| "subpath": null, | ||
| "primary_language": null, | ||
| "description": "npm .npmrc configuration file", | ||
| "release_date": null, | ||
| "parties": [], | ||
| "keywords": [], | ||
| "homepage_url": null, | ||
| "download_url": null, | ||
| "size": null, | ||
| "sha1": null, | ||
| "md5": null, | ||
| "sha256": null, | ||
| "sha512": null, | ||
| "bug_tracking_url": null, | ||
| "code_view_url": null, | ||
| "vcs_url": null, | ||
| "copyright": null, | ||
| "holder": null, | ||
| "declared_license_expression": null, | ||
| "declared_license_expression_spdx": null, | ||
| "license_detections": [], | ||
| "other_license_expression": null, | ||
| "other_license_expression_spdx": null, | ||
| "other_license_detections": [], | ||
| "extracted_license_statement": null, | ||
| "notice_text": null, | ||
| "source_packages": [], | ||
| "file_references": [], | ||
| "is_private": false, | ||
| "is_virtual": false, | ||
| "extra_data": { | ||
| "registry": "https://registry.npmjs.org/", | ||
| "cache": "~/.npm", | ||
| "strict-ssl": "true", | ||
| "//registry.npmjs.org/:_authToken": "abc123", | ||
| "init.author.name": "John Doe", | ||
| "emptykey": "" | ||
| }, | ||
| "dependencies": [], | ||
| "repository_homepage_url": null, | ||
| "repository_download_url": null, | ||
| "api_data_url": null, | ||
| "datasource_id": "npmrc", | ||
| "purl": null | ||
| } | ||
| ] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| import os | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need to create a new file, add the tests in the test_npm.py file |
||
|
|
||
| from packagedcode import npm | ||
| from packages_test_utils import PackageTester | ||
| from scancode_config import REGEN_TEST_FIXTURES | ||
|
|
||
|
|
||
| class TestNpmrc(PackageTester): | ||
| test_data_dir = os.path.join(os.path.dirname(__file__), 'data') | ||
|
|
||
| def test_parse_basic_npmrc(self): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also add a test to check |
||
| test_file = self.get_test_loc('npm/basic/.npmrc') | ||
| expected_loc = self.get_test_loc('npm/basic/.npmrc.expected') | ||
| packages_data = npm.NpmrcHandler.parse(test_file) | ||
| self.check_packages_data(packages_data, expected_loc, regen=REGEN_TEST_FIXTURES) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be part of the
npmassembly only if the data present in .npmrc files can be used to updated package data information in other npm manifests. The registry information present could be used to update npm urls in npm packages found alongside these manifests but these URLs are not always usable. And you'd have to modify theBaseNpmHandler.assemblefunction to handle/use these too.So let's keep this as a subclass of
models.NonAssemblableDatafileHandlerto avoid any assembly from these files and keep things simple for now.