Merge ~alexmurray/ubuntu-cve-tracker:source-map-performance-improvements into ubuntu-cve-tracker:master

Proposed by Alex Murray
Status: Rejected
Rejected by: Alex Murray
Proposed branch: ~alexmurray/ubuntu-cve-tracker:source-map-performance-improvements
Merge into: ubuntu-cve-tracker:master
Diff against target: 141 lines (+33/-14)
4 files modified
scripts/cve_lib.py (+3/-2)
scripts/source_map.py (+16/-9)
scripts/test_source_map.py (+11/-2)
test/esm-fake-supported.txt (+3/-1)
Reviewer Review Type Date Requested Status
Alex Murray Disapprove
Review via email: mp+447863@code.launchpad.net

Description of the change

This is currently a work-in-progress

To post a comment you must log in.
e76ede6... by Alex Murray

scripts/test_source_map.py: test get_packages_from_generic_name()

Signed-off-by: Alex Murray <email address hidden>

1d08db8... by Alex Murray

scripts/test_source_map.py: test get_aliases_of_ubuntu_package()

Signed-off-by: Alex Murray <email address hidden>

Revision history for this message
Alex Murray (alexmurray) wrote :

Closing this since the current approach of using a global variable to store the cache directly in source_map doesn't make sense given that source_map in general doesn't store global state like this. Instead will either need to encapsulate the cache in the returned map, or return it in a separate variable.

review: Disapprove

Unmerged commits

1d08db8... by Alex Murray

scripts/test_source_map.py: test get_aliases_of_ubuntu_package()

Signed-off-by: Alex Murray <email address hidden>

Succeeded
[SUCCEEDED] unit-tests:0 (build)
[SUCCEEDED] check-cves:0 (build)
12 of 2 results
e76ede6... by Alex Murray

scripts/test_source_map.py: test get_packages_from_generic_name()

Signed-off-by: Alex Murray <email address hidden>

Succeeded
[SUCCEEDED] unit-tests:0 (build)
[SUCCEEDED] check-cves:0 (build)
12 of 2 results
24c7dec... by Alex Murray

scripts/source_map.py: add comments about future proposed caches

Signed-off-by: Alex Murray <email address hidden>

Succeeded
[SUCCEEDED] unit-tests:0 (build)
[SUCCEEDED] check-cves:0 (build)
12 of 2 results
73c31b7... by Alex Murray

scripts/source_map.py: add a cache for generic name lookups

Signed-off-by: Alex Murray <email address hidden>

54f07e4... by Alex Murray

scripts: load aliases when loading a subproject

Signed-off-by: Alex Murray <email address hidden>

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/scripts/cve_lib.py b/scripts/cve_lib.py
2index 120dd8f..6439bd5 100755
3--- a/scripts/cve_lib.py
4+++ b/scripts/cve_lib.py
5@@ -950,8 +950,9 @@ def load_external_subprojects():
6
7 # check if aliases for packages exist
8 if os.path.isfile(supported_txt[:-len("supported.txt")] + 'aliases.yaml'):
9- subprojects[rel].setdefault("aliases",
10- supported_txt[:-len("supported.txt")] + 'aliases.yaml')
11+ # load aliases
12+ aliases = yaml.safe_load(open(supported_txt[:-len("supported.txt")] + 'aliases.yaml'))
13+ subprojects[rel].setdefault("aliases", aliases)
14
15 try:
16 # use config to populate other parts of the
17diff --git a/scripts/source_map.py b/scripts/source_map.py
18index 58ef5c3..65f0bdd 100755
19--- a/scripts/source_map.py
20+++ b/scripts/source_map.py
21@@ -17,7 +17,6 @@ import re
22 import subprocess
23 import sys
24 import cve_lib
25-import yaml
26
27 built_using_tags = ["Built-Using", "Static-Built-Using", "X-Cargo-Built-Using"]
28 apt_pkg.init_system()
29@@ -388,37 +387,43 @@ def get_built_using_header():
30 header += "\n" + "-" * 120
31 return header
32
33+# TODO - add a cache which we update when we load the sources data so that we
34+# don't have to loop through all the pkgs in the source map
35 def get_all_aliases(sources, rel):
36 aliases = []
37
38 if not rel in sources:
39 return aliases
40
41+ # this is very expensive
42 for pkg in sources[rel]:
43 if 'aliases' in sources[rel][pkg]:
44- for alias in sources[rel][pkg]['aliases']:
45- aliases.append(alias)
46+ aliases += sources[rel][pkg]['aliases']
47 return aliases
48
49+# TODO - add a cache which we update when we load the sources data so that we
50+# don't have to loop through all the pkgs in the source map
51 def get_aliases_of_ubuntu_package(sources, pkg_name, rel):
52 aliases = []
53 if not rel in sources:
54 return aliases
55
56+ # this is very expensive
57 for pkg in sources[rel]:
58 if 'aliases' in sources[rel][pkg]:
59 if pkg_name in sources[rel][pkg]['aliases']:
60 aliases.append(pkg)
61 return aliases
62
63+generic_name_cache = {}
64 def get_packages_from_generic_name(sources, generic_name, rel):
65 pkgs = []
66 if not rel in sources:
67 return pkgs
68
69- for pkg in sources[rel]:
70- if 'generic_name' in sources[rel][pkg] and sources[rel][pkg]['generic_name'] == generic_name:
71- pkgs.append(pkg)
72+ if generic_name in generic_name_cache and rel in generic_name_cache[generic_name]:
73+ return generic_name_cache[generic_name][rel]
74+
75 return pkgs
76
77 def load_subprojects_lists(releases=None):
78@@ -484,11 +489,13 @@ def load_subprojects_lists(releases=None):
79 if '|' in pkg:
80 main_package_name = pkg.split('|')[0]
81 map[rel][pkg]['generic_name'] = main_package_name
82+ generic_name_cache.setdefault(main_package_name, {})
83+ generic_name_cache[main_package_name].setdefault(rel, [])
84+ generic_name_cache[main_package_name][rel].append(pkg)
85
86- if 'aliases' in details:
87- with open(details['aliases'], 'r') as file:
88- aliases = yaml.safe_load(file)
89
90+ if 'aliases' in details:
91+ aliases = details['aliases']
92 for pkg in aliases:
93 for src_pkg in map[rel]:
94 if pkg == src_pkg or \
95diff --git a/scripts/test_source_map.py b/scripts/test_source_map.py
96index 8e99825..04316d3 100755
97--- a/scripts/test_source_map.py
98+++ b/scripts/test_source_map.py
99@@ -15,6 +15,9 @@ mock_subprojects = {
100 "codename": "Fabulous Fake",
101 "ppa": "ubuntu-esm/esm-bar-security/ubuntu",
102 "parent": "ubuntu/bar",
103+ "aliases": {
104+ "fooishbar": ["bar"],
105+ },
106 "description": "Available with Ubuntu Pro: https://ubuntu.com/pro",
107 "stamp": 1618963200,
108 }
109@@ -26,7 +29,13 @@ class TestSubprojects:
110 def test_load_subproject_supported(self, _find_sources_mock):
111 _find_sources_mock.return_value = []
112 map = source_map.load(releases=['esm-foo/bar'])
113- assert set(map['esm-foo/bar'].keys()) == set(['foo', 'bar'])
114+ assert set(map['esm-foo/bar'].keys()) == set(['foo', 'baz|1.0', 'bar', 'fooishbar'])
115+
116+ pkgs = source_map.get_packages_from_generic_name(map, "baz", "esm-foo/bar")
117+ assert pkgs == ['baz|1.0']
118+
119+ aliases = source_map.get_aliases_of_ubuntu_package(map, "bar", "esm-foo/bar")
120+ assert aliases == ['fooishbar']
121
122 @pytest.mark.parametrize("release", cve_lib.get_active_esm_releases())
123 @mock.patch("source_map._find_sources")
124@@ -52,4 +61,4 @@ class TestSubprojects:
125 packages_clean.add(package)
126
127 map = source_map.load(releases=[release])
128- assert set(map[release].keys()) == packages_clean
129\ No newline at end of file
130+ assert set(map[release].keys()) == packages_clean
131diff --git a/test/esm-fake-supported.txt b/test/esm-fake-supported.txt
132index a907ec3..9dcc581 100644
133--- a/test/esm-fake-supported.txt
134+++ b/test/esm-fake-supported.txt
135@@ -1,2 +1,4 @@
136 foo
137-bar
138\ No newline at end of file
139+bar
140+baz|1.0
141+fooishbar

Subscribers

People subscribed via source and target branches