Skip to content

Commit

Permalink
feat: return depgraph from plugin
Browse files Browse the repository at this point in the history
this change will return a depgraph instead of a deptree from the plugin

because a package can only have one version, it doesn't make sense to create it more than once in "pip_resolve.py" script
this change will remove the duplicates from the scripts and replace them with a "true" value
later the "buildDepGraph" function will iterate over the tree and replace all "true" values with a reference to the original package

this will improve performance for those reasons:
1. no serializing dep tree with repeating dependencies
2. no deserializing repeating dependencies (memory and cpu affects)
3. return a depGraph
  • Loading branch information
admons committed Aug 24, 2021
1 parent ef5118d commit d58d135
Show file tree
Hide file tree
Showing 11 changed files with 4,139 additions and 51 deletions.
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
__pycache__/
*.pyc
npm-debug.log
.idea

# tox testing
/.tox/
Expand All @@ -22,4 +23,4 @@ reports/
venv/

# Diagnostic reports (https://nodejs.org/api/report.html)
report.[0-9]*.[0-9]*.[0-9]*.[0-9]*.json
report.[0-9]*.[0-9]*.[0-9]*.[0-9]*.json
50 changes: 50 additions & 0 deletions lib/dependencies/build-dep-graph.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { DepGraph } from '@snyk/dep-graph';
import { depTreeToGraph, DepTree } from '@snyk/dep-graph/dist/legacy';

type PackageName = string;

// This is a partially dep tree: every package exists once, all additional reference will have "true" as the value
export interface PartialDepTree {
name?: string;
version?: string;
dependencies?: Dependencies;
labels?: {
[key: string]: string;
};
}

type Dependencies = {
[depName: string]: PartialDepTree | 'true';
};

export function buildDepGraph(
partialDepTree: PartialDepTree
): Promise<DepGraph> {
const packageToDepTreeMap = new Map<PackageName, PartialDepTree>();

const queue: Dependencies[] = [partialDepTree.dependencies];
const referencesToUpdate: { key: string; dependencies: Dependencies }[] = [];
while (queue.length > 0) {
const dependencies = queue.pop();
if (!dependencies) continue;

for (const [key, dependencyDepTree] of Object.entries(dependencies)) {
if (dependencyDepTree === 'true') {
referencesToUpdate.push({ key, dependencies });
} else {
packageToDepTreeMap.set(key, dependencyDepTree);
queue.push(dependencyDepTree.dependencies);
}
}
}

referencesToUpdate.forEach(({ key, dependencies }) => {
if (!packageToDepTreeMap.get(key)) {
// this should never happen
throw new Error(`key ${key} not found in packageToDepTreeMap`);
}
dependencies[key] = packageToDepTreeMap.get(key);
});

return depTreeToGraph(partialDepTree as DepTree, 'pip');
}
4 changes: 2 additions & 2 deletions lib/dependencies/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export async function getDependencies(
baseargs = ['run', 'python'];
}

const [plugin, pkg] = await Promise.all([
const [plugin, dependencyGraph] = await Promise.all([
getMetaData(command, baseargs, root, targetFile),
inspectInstalledDeps(
command,
Expand All @@ -57,5 +57,5 @@ export async function getDependencies(
options.args
),
]);
return { plugin, package: pkg };
return { plugin, dependencyGraph };
}
8 changes: 5 additions & 3 deletions lib/dependencies/inspect-implementation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import * as path from 'path';
import * as tmp from 'tmp';

import * as subProcess from './sub-process';
import { legacyCommon } from '@snyk/cli-interface';
import { DepGraph } from '@snyk/dep-graph';
import { buildDepGraph, PartialDepTree } from './build-dep-graph';
import { FILENAMES } from '../types';
import { EmptyManifestError, RequiredPackagesMissingError } from '../errors';

Expand Down Expand Up @@ -118,7 +119,7 @@ export async function inspectInstalledDeps(
allowMissing: boolean,
includeDevDeps: boolean,
args?: string[]
): Promise<legacyCommon.DepTree> {
): Promise<DepGraph> {
const tempDirObj = tmp.dirSync({
unsafeCleanup: true,
});
Expand All @@ -145,7 +146,8 @@ export async function inspectInstalledDeps(
}
);

return JSON.parse(output) as legacyCommon.DepTree;
const result = JSON.parse(output) as PartialDepTree;
return buildDepGraph(result);
} catch (error) {
if (typeof error === 'string') {
const emptyManifestMsg = 'No dependencies detected in manifest.';
Expand Down
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,15 @@
"author": "snyk.io",
"license": "Apache-2.0",
"dependencies": {
"@snyk/cli-interface": "^2.0.3",
"@snyk/cli-interface": "^2.11.2",
"@snyk/dep-graph": "^1.28.1",
"snyk-poetry-lockfile-parser": "^1.1.7",
"tmp": "0.2.1"
},
"devDependencies": {
"@snyk/types-tap": "^1.1.0",
"@types/jest": "^24.9.0",
"@types/node": "8.10.60",
"@types/node": "^14.14.31",
"@types/tmp": "^0.1.0",
"@typescript-eslint/eslint-plugin": "^3.8.0",
"@typescript-eslint/parser": "^3.8.0",
Expand Down
19 changes: 13 additions & 6 deletions pysrc/pip_resolve.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def create_tree_of_packages_dependencies(
p.key.lower() in lowercase_pkgs_names or
(p.project_name and p.project_name.lower()) in lowercase_pkgs_names]

def create_children_recursive(root_package, key_tree, ancestors):
def create_children_recursive(root_package, key_tree, ancestors, all_packages_map):
root_name = root_package[NAME].lower()
if root_name not in key_tree:
msg = 'Required packages missing: ' + root_name
Expand All @@ -80,15 +80,21 @@ def create_children_recursive(root_package, key_tree, ancestors):
if child_dist.project_name.lower() in ancestors:
continue

if DEPENDENCIES not in root_package:
root_package[DEPENDENCIES] = {}

if child_dist.project_name.lower() in all_packages_map:
root_package[DEPENDENCIES][child_dist.project_name] = 'true'
continue

child_package = {
NAME: child_dist.project_name.lower(),
VERSION: child_dist.installed_version,
}

create_children_recursive(child_package, key_tree, ancestors)
if DEPENDENCIES not in root_package:
root_package[DEPENDENCIES] = {}
create_children_recursive(child_package, key_tree, ancestors, all_packages_map)
root_package[DEPENDENCIES][child_dist.project_name] = child_package
all_packages_map[child_dist.project_name.lower()] = 'true'
return root_package

def create_dir_as_root():
Expand All @@ -113,13 +119,14 @@ def create_package_as_root(package, dir_as_root):
}
return package_as_root
dir_as_root = create_dir_as_root()
all_packages_map = {}
for package in packages_as_dist_obj:
package_as_root = create_package_as_root(package, dir_as_root)
if only_provenance:
package_as_root[LABELS] = {PROVENANCE: format_provenance_label(tlr_by_key[package_as_root[NAME]].provenance)}
dir_as_root[DEPENDENCIES][package_as_root[NAME]] = package_as_root
else:
package_tree = create_children_recursive(package_as_root, key_tree, set([]))
package_tree = create_children_recursive(package_as_root, key_tree, set([]), all_packages_map)
dir_as_root[DEPENDENCIES][package_as_root[NAME]] = package_tree
return dir_as_root

Expand Down Expand Up @@ -278,7 +285,7 @@ def create_dependencies_tree_by_req_file_path(requirements_file_path,

# build a tree of dependencies
package_tree = create_tree_of_packages_dependencies(
dist_tree, top_level_requirements, requirements_file_path, allow_missing, only_provenance)
dist_tree, top_level_requirements, requirements_file_path, allow_missing, only_provenance)
print(json.dumps(package_tree))


Expand Down
Loading

0 comments on commit d58d135

Please sign in to comment.