Merge pull request #2997 from nexB/scio-npm-bug-448

Improve npm package processing

Author: pombredanne

src/packagedcode/build.py

@@ -177,6 +177,8 @@ def parse(cls, location):
     @classmethod
     def assign_package_to_resources(cls, package, resource, codebase, skip_name=None):
         package_uid = package.package_uid
+        if not package_uid:
+            return
         parent = resource.parent(codebase)
         for res in walk_build(resource=parent, codebase=codebase, skip_name=skip_name):
             res.for_packages.append(package_uid)

src/packagedcode/debian.py

@@ -397,20 +397,21 @@ def assemble(cls, package_data, resource, codebase):
         assemblable_paths = (
             f'usr/share/doc/{package_name}/copyright',
         )
-        for res in root_resource.walk(codebase):
-            if not res.path.endswith(assemblable_paths):
-                continue
-
-            for pkgdt in res.package_data:
-                package.update(
-                    package_data=pkgdt,
-                    datafile_path=res.path,
-                )
-
-            res.for_packages.append(package_uid)
-            res.save(codebase)
-
-            yield res
+        if package_uid:
+            for res in root_resource.walk(codebase):
+                if not res.path.endswith(assemblable_paths):
+                    continue
+    
+                for pkgdt in res.package_data:
+                    package.update(
+                        package_data=pkgdt,
+                        datafile_path=res.path,
+                    )
+    
+                res.for_packages.append(package_uid)
+                res.save(codebase)
+    
+                yield res
 
         yield package
 

src/packagedcode/models.py

@@ -970,7 +970,7 @@ def assign_package_to_resources(cls, package, resource, codebase):
         # NOTE: we do not attach files to the Package level. Instead we
         # update `for_packages` of a codebase resource.
         package_uid = package.package_uid
-        if resource:
+        if resource and package_uid:
             resource.for_packages.append(package_uid)
             resource.save(codebase)
             for res in resource.walk(codebase):
@@ -1030,8 +1030,9 @@ def assemble_from_many(cls, pkgdata_resources, codebase,):
                         datafile_path=resource.path,
                     )
                     package_uid = package.package_uid
-                    resource.for_packages.append(package_uid)
-                    resource.save(codebase)
+                    if package_uid:
+                        resource.for_packages.append(package_uid)
+                        resource.save(codebase)
             else:
                 # FIXME: What is the package_data is NOT for the same package as package?
                 # FIXME: What if the update did not do anything? (it does return True or False)
@@ -1040,8 +1041,9 @@ def assemble_from_many(cls, pkgdata_resources, codebase,):
                     package_data=package_data,
                     datafile_path=resource.path,
                 )
-                resource.for_packages.append(package_uid)
-                resource.save(codebase)
+                if package_uid:
+                    resource.for_packages.append(package_uid)
+                    resource.save(codebase)
 
             # in all cases yield possible dependencies
             dependent_packages = package_data.dependencies
@@ -1057,9 +1059,10 @@ def assemble_from_many(cls, pkgdata_resources, codebase,):
             yield resource
 
         # the whole parent subtree of the base_resource is for this package
-        for res in base_resource.walk(codebase):
-            res.for_packages.append(package_uid)
-            res.save(codebase)
+        if package_uid:
+            for res in base_resource.walk(codebase):
+                res.for_packages.append(package_uid)
+                res.save(codebase)
 
         if package:
             if not package.license_expression:
@@ -1336,9 +1339,10 @@ def get_packages_files(self, codebase):
         Yield all the Resource of this package found in codebase.
         """
         package_uid = self.package_uid
-        for resource in codebase.walk():
-            if package_uid in resource.for_packages:
-                yield resource
+        if package_uid:
+            for resource in codebase.walk():
+                if package_uid in resource.for_packages:
+                    yield resource
 
 
 @attr.attributes(slots=True)

src/packagedcode/npm.py

@@ -95,12 +95,12 @@ def assemble(cls, package_data, resource, codebase):
                 root = package_resource.parent(codebase)
                 if root:
                     for npm_res in cls.walk_npm(resource=root, codebase=codebase):
-                        if package_uid not in npm_res.for_packages:
+                        if package_uid and package_uid not in npm_res.for_packages:
                             npm_res.for_packages.append(package_uid)
                             npm_res.save(codebase)
                         yield npm_res
                 elif codebase.has_single_resource:
-                    if package_uid not in package_resource.for_packages:
+                    if package_uid and package_uid not in package_resource.for_packages:
                         package_resource.for_packages.append(package_uid)
                         package_resource.save(codebase)
                 yield package_resource
@@ -119,7 +119,7 @@ def assemble(cls, package_data, resource, codebase):
                 if lock_file.name in lockfile_names:
                     yield from yield_dependencies_from_package_resource(lock_file, package_uid)
 
-                    if package_uid not in lock_file.for_packages:
+                    if package_uid and package_uid not in lock_file.for_packages:
                         lock_file.for_packages.append(package_uid)
                         lock_file.save(codebase)
                     yield lock_file
@@ -226,8 +226,7 @@ def parse(cls, location):
 
         if not package.download_url:
             # Only add a synthetic download URL if there is none from the dist mapping.
-            dnl_url = npm_download_url(package.namespace, package.name, package.version)
-            package.download_url = dnl_url.strip()
+            package.download_url = npm_download_url(package.namespace, package.name, package.version)
 
         # licenses are a tad special with many different data structures
         lic = package_data.get('license')
@@ -781,12 +780,15 @@ def npm_homepage_url(namespace, name, registry='https://www.npmjs.com/package'):
 
     >>> expected = 'https://yarnpkg.com/en/package/@ang/angular'
     >>> assert npm_homepage_url('@ang', 'angular', 'https://yarnpkg.com/en/package') == expected
+
+    >>> assert not npm_homepage_url(None, None)
     """
-    if namespace:
-        ns_name = f'{namespace}/{name}'
-    else:
-        ns_name = name
-    return f'{registry}/{ns_name}'
+    if name:
+        if namespace:
+            ns_name = f'{namespace}/{name}'
+        else:
+            ns_name = name
+        return f'{registry}/{ns_name}'
 
 
 def npm_download_url(namespace, name, version, registry='https://registry.npmjs.org'):
@@ -803,13 +805,15 @@ def npm_download_url(namespace, name, version, registry='https://registry.npmjs.
 
     >>> expected = 'https://registry.npmjs.org/angular/-/angular-1.6.6.tgz'
     >>> assert npm_download_url(None, 'angular', '1.6.6') == expected
-    """
-    if namespace:
-        ns_name = f'{namespace}/{name}'
 
-    else:
-        ns_name = name
-    return f'{registry}/{ns_name}/-/{name}-{version}.tgz'
+    >>> assert not npm_download_url(None, None, None)
+    """
+    if name and version:
+        if namespace:
+            ns_name = f'{namespace}/{name}'
+        else:
+            ns_name = name
+        return f'{registry}/{ns_name}/-/{name}-{version}.tgz'
 
 
 def npm_api_url(namespace, name, version=None, registry='https://registry.npmjs.org'):
@@ -827,20 +831,25 @@ def npm_api_url(namespace, name, version=None, registry='https://registry.npmjs.
     >>> assert result == 'https://registry.yarnpkg.com/@invisionag%2feslint-config-ivx'
 
     >>> assert npm_api_url(None, 'angular', '1.6.6') == 'https://registry.npmjs.org/angular/1.6.6'
+
+    >>> assert not npm_api_url(None, None, None)
     """
-    version = version or ''
-    if namespace:
-        # this is a legacy wart: older registries used to always encode this /
-        # FIXME: do NOT encode and use plain / instead
-        ns_name = '%2f'.join([namespace, name])
-        # there is no version-specific URL for scoped packages
-        version = ''
-    else:
-        ns_name = name
+    if name:
+        if namespace:
+            # this is a legacy wart: older registries used to always encode this /
+            # FIXME: do NOT encode and use plain / instead
+            ns_name = '%2f'.join([namespace, name])
+            # there is no version-specific URL for scoped packages
+            version = ''
+        else:
+            ns_name = name
+
+        if version:
+            version = f'/{version}'
+        else:
+            version = ''
 
-    if version:
-        version = f'/{version}'
-    return f'{registry}/{ns_name}{version}'
+        return f'{registry}/{ns_name}{version}'
 
 
 def is_scoped_package(name):

src/packagedcode/pypi.py

@@ -212,12 +212,12 @@ def assemble(cls, package_data, resource, codebase):
                 for py_res in cls.walk_pypi(resource=root, codebase=codebase):
                     if py_res.is_dir:
                         continue
-                    if package_uid not in py_res.for_packages:
+                    if package_uid and package_uid not in py_res.for_packages:
                         py_res.for_packages.append(package_uid)
                         py_res.save(codebase)
                     yield py_res
             elif codebase.has_single_resource:
-                if package_uid not in package_resource.for_packages:
+                if package_uid and package_uid not in package_resource.for_packages:
                     package_resource.for_packages.append(package_uid)
                     package_resource.save(codebase)
 
@@ -320,9 +320,10 @@ def assign_package_to_resources(cls, package, resource, codebase):
 
         package_uid = package.package_uid
 
-        # save thyself!
-        resource.for_packages.append(package_uid)
-        resource.save(codebase)
+        if package_uid:
+            # save thyself!
+            resource.for_packages.append(package_uid)
+            resource.save(codebase)
 
         # collect actual paths based on the file references
         for file_ref in package_data.file_references:
@@ -342,15 +343,16 @@ def assign_package_to_resources(cls, package, resource, codebase):
                     # TODO:w e should log these kind of things
                     continue
                 else:
-                    ref_resource.for_packages.append(package_uid)
-                    ref_resource.save(codebase)
+                    if package_uid:
+                        ref_resource.for_packages.append(package_uid)
+                        ref_resource.save(codebase)
             else:
                 ref_resource = get_resource_for_path(
                     path=path_ref,
                     root=site_packages,
                     codebase=codebase,
                 )
-                if ref_resource:
+                if ref_resource and package_uid:
                     ref_resource.for_packages.append(package_uid)
                     ref_resource.save(codebase)
 

src/packagedcode/rpm.py

@@ -210,10 +210,11 @@ def assemble(cls, package_data, resource, codebase):
             if not res:
                 missing_file_references.append(ref)
             else:
-                # path is found and processed: remove it, so we can check if we
-                # found all of them
-                res.for_packages.append(package_uid)
-                res.save(codebase)
+                if package_uid:
+                    # path is found and processed: remove it, so we can check if we
+                    # found all of them
+                    res.for_packages.append(package_uid)
+                    res.save(codebase)
 
         # if we have left over file references, add these to extra data
         if missing_file_references:

src/packagedcode/win_reg.py

@@ -380,8 +380,9 @@ def get_root_resource(cls, resource, codebase):
     @classmethod
     def assign_package_to_resources(cls, package, resource, codebase):
         package_uid = package.package_uid
-        resource.for_packages.append(package_uid)
-        resource.save(codebase)
+        if package_uid:
+            resource.for_packages.append(package_uid)
+            resource.save(codebase)
 
         refs = package.file_references
         if not refs:
@@ -406,11 +407,12 @@ def assign_package_to_resources(cls, package, resource, codebase):
             if not ref:
                 continue
 
-            # path is found and processed: remove it, so we can check if we
-            # found all of them
-            del refs_by_path[res.path]
-            res.for_packages.append(package_uid)
-            res.save(codebase)
+            if package_uid:
+                # path is found and processed: remove it, so we can check if we
+                # found all of them
+                del refs_by_path[res.path]
+                res.for_packages.append(package_uid)
+                res.save(codebase)
 
         # if we have left over file references, add these to extra data
         if refs_by_path: