Merge ~emitorino/ubuntu-security-tools:skip_on_package_downloaded into ubuntu-security-tools:master

Proposed by Emilia Torino
Status: Needs review
Proposed branch: ~emitorino/ubuntu-security-tools:skip_on_package_downloaded
Merge into: ubuntu-security-tools:master
Diff against target: 64 lines (+26/-8)
1 file modified
audits/uaudit (+26/-8)
Reviewer Review Type Date Requested Status
Ubuntu Security Team Pending
Review via email: mp+377790@code.launchpad.net

Commit message

Skipping running umt download if the package was downloaded already

Description of the change

If for some reason (like previous errors) you need to run uaudit more than one time; it will fail on the initial step while performing umt download (as umt download fails if the directory already exists).

Following the same validations done for the following steps, I am adding a "WARN: Skipping package download build ({package} already exists)" in the case the package was downloaded already, allowing the script to continue.

To post a comment you must log in.
f12cec1... by Emilia Torino

adding release to directory validation

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

Would it be better to *always* download the package so that it is fresh? In that case perhaps just adding -f to umt download would be better? Or if not, should this prompt so the user can choose to overwrite it?

f7ccecd... by Emilia Torino

Reverting previous suggestions and just adding -f to um download as suggested by Alex

aabb484... by Emilia Torino

Asking for user consent to download package again as suggested by Alex

Revision history for this message
Emilia Torino (emitorino) wrote :

> Would it be better to *always* download the package so that it is fresh? In
> that case perhaps just adding -f to umt download would be better? Or if not,
> should this prompt so the user can choose to overwrite it?

I originally thought about adding the -f option to umt download but was not sure if that was the ideal behavior (mainly to avoid resources usage when not needed). I like the idea of the user prompt though! I did a bit of refactoring over my previous merge request so I can reuse the prompt part in both cases (asking for tool installation & asking for force download), Thanks!

Unmerged commits

aabb484... by Emilia Torino

Asking for user consent to download package again as suggested by Alex

f7ccecd... by Emilia Torino

Reverting previous suggestions and just adding -f to um download as suggested by Alex

f12cec1... by Emilia Torino

adding release to directory validation

d8c8d6e... by Emilia Torino

validating if package was already downloaded to be able to continue running uaudit if thats the case

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/audits/uaudit b/audits/uaudit
2index 7c794a3..9f5ced9 100755
3--- a/audits/uaudit
4+++ b/audits/uaudit
5@@ -257,10 +257,8 @@ def uaudit_setup_env():
6 add_to_path([audits_path, package_tools_path, build_tools_path])
7
8
9-def query_user_consent_to_install_tool(tool):
10+def quey_user_consent(question):
11 valid_answers = {"yes": True, "y": True, "no": False, "n": False}
12- question = f"{tool} is not present in the system. Do you want to install it now? Please note this command will " \
13- f"run as sudo. [Y/n]"
14
15 while True:
16 sys.stdout.write(question)
17@@ -272,6 +270,13 @@ def query_user_consent_to_install_tool(tool):
18 "(or 'y' or 'n').\n")
19
20
21+def query_user_consent_to_install_tool(tool):
22+ question = f"{tool} is not present in the system. Do you want to install it now? Please note this command will " \
23+ f"run as sudo. [Y/n]"
24+
25+ quey_user_consent(question)
26+
27+
28 def install_tool(cmd_args, tool):
29 if query_user_consent_to_install_tool(tool):
30 rc, out = cmd(cmd_args)
31@@ -368,15 +373,28 @@ def coverity_cmd(command):
32 return rc, out
33
34
35-def download(package, release, proposed=False):
36- '''Download package'''
37- prog = ['umt', 'download', '-r', release, package]
38+def download_cmd(args, proposed):
39 if proposed:
40- prog.insert(2, '-p')
41- rc, out = cmd(prog)
42+ args.insert(2, '-p')
43+ rc, out = cmd(args)
44 msg(out)
45 if rc != 0 or "Skipping" in out:
46 error("Problem downloading package")
47+
48+
49+def download(package, release, proposed=False):
50+ '''Download package'''
51+ package_release_dir = os.path.join(package, release)
52+ if os.path.isdir(package_release_dir) and os.listdir(package_release_dir):
53+ question = f"{package} is already downloaded in this directory. Do you want to download it again?"
54+ if quey_user_consent(question):
55+ args = ['umt', 'download', '-f', '-r', release, package]
56+ download_cmd(args, proposed)
57+ else:
58+ warn(f"Skipping package download ({package} already exists)")
59+ else:
60+ args = ['umt', 'download', '-r', release, package]
61+ download_cmd(args, proposed)
62 # This is probably not good enough
63 d = glob.glob(os.path.join(package, release, "%s-*" % package))[0]
64 msg("Unpacked source: %s" % os.path.join(os.getcwd(), d))

Subscribers

People subscribed via source and target branches