diff --git a/package_linter.py b/package_linter.py
index 8475762..add83ce 100755
--- a/package_linter.py
+++ b/package_linter.py
@@ -10,6 +10,7 @@ import urllib.request
import codecs
import subprocess
import time
+import statistics
reader = codecs.getreader("utf-8")
@@ -162,17 +163,6 @@ official_helpers = {
# Utilities
# ############################################################################
-# Taken from https://stackoverflow.com/a/49518779
-def check_for_duplicate_keys(ordered_pairs):
- dict_out = {}
- for key, val in ordered_pairs:
- if key in dict_out:
- print_warning("Duplicated key '%s' in %s" % (key, ordered_pairs))
- else:
- dict_out[key] = val
- return dict_out
-
-
class c:
HEADER = '\033[94m'
OKBLUE = '\033[94m'
@@ -184,6 +174,23 @@ class c:
BOLD = '\033[1m'
UNDERLINE = '\033[4m'
+class TestReport:
+
+ def __init__(self, message):
+ self.message = message
+
+ def display(self):
+ _print(self.style, self.message, c.END)
+
+class Warning(TestReport):
+ style = c.WARNING + "!"
+
+class Error(TestReport):
+ style = c.FAIL + "✘"
+
+class Info(TestReport):
+ style = c.OKBLUE
+
def header(app):
_print("""
@@ -213,24 +220,10 @@ def print_header(str):
_print("\n [" + c.BOLD + c.HEADER + str.title() + c.END + "]\n")
-def print_warning_not_reliable(str):
+def report_warning_not_reliable(str):
_print(c.MAYBE_FAIL + "?", str, c.END)
-warnings = []
-
-
-def print_warning(str):
- warnings.append(str)
- _print(c.WARNING + "!", str, c.END)
-
-
-errors = []
-
-
-def print_error(str):
- errors.append(str)
- _print(c.FAIL + "✘", str, c.END)
def print_happy(str):
@@ -257,12 +250,53 @@ def spdx_licenses():
if os.path.exists(cachefile) and time.time() - os.path.getmtime(cachefile) < 3600:
return open(cachefile).read()
- link = "https://spdx.org/licenses/"
- content = urlopen(link)['content']
+ url = "https://spdx.org/licenses/"
+ content = urlopen(url)['content']
open(cachefile, "w").write(content)
return content
+def app_list():
+
+ cachefile = "./.apps.json"
+ if os.path.exists(cachefile) and time.time() - os.path.getmtime(cachefile) < 3600:
+ try:
+ return json.loads(open(cachefile).read())
+ except:
+ _print("Uuuuh failed to load apps.json from cache...")
+
+ url = "https://raw.githubusercontent.com/YunoHost/apps/master/apps.json"
+ content = urlopen(url)['content']
+ open(cachefile, "w").write(content)
+ return json.loads(content)
+
+
+tests = {}
+tests_reports = []
+
+def test(**kwargs):
+ def decorator(f):
+ clsname = f.__qualname__.split(".")[0]
+ if clsname not in tests:
+ tests[clsname] = []
+ tests[clsname].append((f,kwargs))
+ return f
+ return decorator
+
+class TestSuite():
+
+ def run_tests(self):
+ for test, options in tests[self.__class__.__name__]:
+ if "only" in options and self.name not in options["only"]:
+ continue
+ if "ignore" in options and self.name in options["ignore"]:
+ continue
+ reports = list(test(self))
+ for report in reports:
+ if output == "plain":
+ report.display()
+ tests_reports.append((test.__qualname__, report))
+
# ############################################################################
# Actual high-level checks
# ############################################################################
@@ -270,134 +304,264 @@ def spdx_licenses():
scriptnames = ["_common.sh", "install", "remove", "upgrade", "backup", "restore"]
-class App():
+class App(TestSuite):
def __init__(self, path):
print_header("LOADING APP")
self.path = path
+ self.manifest_ = Manifest(self.path)
+ self.manifest = self.manifest_.manifest
self.scripts = {f: Script(self.path, f) for f in scriptnames}
+
def analyze(self):
- self.check_manifest()
- self.misc_file_checks()
- self.check_helpers_usage()
+ print_header("MANIFEST")
+ self.manifest_.run_tests()
for script in [self.scripts[s] for s in scriptnames if self.scripts[s].exists]:
- script.analyze()
+ print_header(script.name.upper() + " SCRIPT")
+ script.run_tests()
- def check_helpers_usage(self):
+ print_header("MISC HELPER USAGE / CONSISTENCY")
+ self.run_tests()
- print_header("HELPERS USAGE")
- # Check for custom helpers definition that are now official...
- cmd = "grep -IhEro 'ynh_\w+ *\( *\)' '%s/scripts' | tr -d '() '" % self.path
+ #######################################
+ # _ _ _ #
+ # | | | | | | #
+ # | |__| | ___| |_ __ ___ _ __ ___ #
+ # | __ |/ _ \ | '_ \ / _ \ '__/ __| #
+ # | | | | __/ | |_) | __/ | \__ \ #
+ # |_| |_|\___|_| .__/ \___|_| |___/ #
+ # | | #
+ # |_| #
+ #######################################
+
+ @test()
+ def helpers_now_official(app):
+
+ cmd = "grep -IhEro 'ynh_\w+ *\( *\)' '%s/scripts' | tr -d '() '" % app.path
custom_helpers = subprocess.check_output(cmd, shell=True).decode('utf-8').strip().split("\n")
custom_helpers = [c.split("__")[0] for c in custom_helpers]
for custom_helper in custom_helpers:
if custom_helper in official_helpers.keys():
- print_warning("%s is now an official helper since version '%s'" % (custom_helper, official_helpers[custom_helper] or '?'))
+ yield Warning("%s is now an official helper since version '%s'" % (custom_helper, official_helpers[custom_helper] or '?'))
- # Check for helpers usage that do not match version required in manifest...
- if self.yunohost_version_req:
- cmd = "grep -IhEro 'ynh_\w+' '%s/scripts'" % self.path
- helpers_used = subprocess.check_output(cmd, shell=True).decode('utf-8').strip().split("\n")
- helpers_used = sorted(set(helpers_used))
+ @test()
+ def helpers_version_requirement(app):
- manifest_req = [int(i) for i in self.yunohost_version_req.strip(">= ").split('.')] + [0,0,0]
- def validate_version_requirement(helper_req):
- if helper_req == '':
- return True
- helper_req = [int(i) for i in helper_req.split('.')]
- for i in range(0,len(helper_req)):
- if helper_req[i] == manifest_req[i]:
- continue
- return helper_req[i] <= manifest_req[i]
+ cmd = "grep -IhEro 'ynh_\w+ *\( *\)' '%s/scripts' | tr -d '() '" % app.path
+ custom_helpers = subprocess.check_output(cmd, shell=True).decode('utf-8').strip().split("\n")
+ custom_helpers = [c.split("__")[0] for c in custom_helpers]
+
+ yunohost_version_req = app.manifest.get("requirements", {}).get("yunohost", "").strip(">= ")
+
+ cmd = "grep -IhEro 'ynh_\w+' '%s/scripts'" % app.path
+ helpers_used = subprocess.check_output(cmd, shell=True).decode('utf-8').strip().split("\n")
+ helpers_used = sorted(set(helpers_used))
+
+ manifest_req = [int(i) for i in yunohost_version_req.split('.')] + [0,0,0]
+ def validate_version_requirement(helper_req):
+ if helper_req == '':
return True
-
- for helper in [h for h in helpers_used if h in official_helpers.keys()]:
- if helper in custom_helpers:
+ helper_req = [int(i) for i in helper_req.split('.')]
+ for i in range(0,len(helper_req)):
+ if helper_req[i] == manifest_req[i]:
continue
- helper_req = official_helpers[helper]
- if not validate_version_requirement(helper_req):
- major_diff = manifest_req[0] > int(helper_req[0])
- message = "Using official helper %s implies requiring at least version %s, but manifest only requires %s" % (helper, helper_req, self.yunohost_version_req)
- if major_diff:
- print_error(message)
- else:
- print_warning(message)
+ return helper_req[i] <= manifest_req[i]
+ return True
+
+ for helper in [h for h in helpers_used if h in official_helpers.keys()]:
+ if helper in custom_helpers:
+ continue
+ helper_req = official_helpers[helper]
+ if not validate_version_requirement(helper_req):
+ major_diff = manifest_req[0] > int(helper_req[0])
+ message = "Using official helper %s implies requiring at least version %s, but manifest only requires %s" % (helper, helper_req, yunohost_version_req)
+ yield Error(message) if major_diff else Warning(message)
- def misc_file_checks(self):
+ @test()
+ def helper_consistency_apt_deps(app):
+ """
+ check if ynh_install_app_dependencies is present in install/upgrade/restore
+ so dependencies are up to date after restoration or upgrade
+ """
- print_header("MISC FILE CHECKS")
+ install_script = app.scripts["install"]
+ if install_script.contains("ynh_install_app_dependencies"):
+ for name in ["upgrade", "restore"]:
+ if app.scripts[name].exists and not app.scripts[name].contains("ynh_install_app_dependencies"):
+ yield Warning("ynh_install_app_dependencies should also be in %s script" % name)
- #
- # Check for recommended and mandatory files
- #
+ @test()
+ def helper_consistency_service_add(app):
+ install_script = app.scripts["install"]
+ if install_script.contains("yunohost service add"):
+ if app.scripts["remove"].exists and not app.scripts["remove"].contains("yunohost service remove"):
+ yield Error(
+ "You used 'yunohost service add' in the install script, "
+ "but not 'yunohost service remove' in the remove script."
+ )
+ if app.scripts["upgrade"].exists and not app.scripts["upgrade"].contains("yunohost service add"):
+ yield Warning(
+ "You used 'yunohost service add' in the install script, "
+ "but not in the upgrade script"
+ )
+
+ if app.scripts["restore"].exists and not app.scripts["restore"].contains("yunohost service add"):
+ yield Warning(
+ "You used 'yunohost service add' in the install script, "
+ "but not in the restore script"
+ )
+
+ @test()
+ def helper_consistency_firewall(app):
+ install_script = app.scripts["install"]
+ if install_script.contains("yunohost firewall allow"):
+ if not install_script.contains("--needs_exposed_ports"):
+ yield Warning("The install script expose a port on the outside with 'yunohost firewall allow' but doesn't use 'yunohost service add' with --needs_exposed_ports ... If your are ABSOLUTELY SURE that the service needs to be exposed on THE OUTSIDE, then add --needs_exposed_ports to 'yunohost service add' with the relevant port number. Otherwise, opening the port leads to a significant security risk and you should keep the damn port closed !")
+
+
+ ###########################################################
+ # _____ __ __ _ #
+ # / ____| / _| / / (_) #
+ # | | ___ _ __ | |_ / / _ __ ___ _ ___ ___ #
+ # | | / _ \| '_ \| _| / / | '_ ` _ \| / __|/ __| #
+ # | |___| (_) | | | | | / / | | | | | | \__ \ (__ #
+ # \_____\___/|_| |_|_| /_/ |_| |_| |_|_|___/\___| #
+ # #
+ ###########################################################
+
+ @test()
+ def mandatory_scripts(app):
filenames = ("manifest.json", "LICENSE", "README.md",
"scripts/install", "scripts/remove",
"scripts/upgrade",
"scripts/backup", "scripts/restore")
- non_mandatory = ("script/backup", "script/restore")
for filename in filenames:
- if file_exists(self.path + "/" + filename):
- continue
- elif filename in non_mandatory:
- print_warning("Consider adding a file %s" % filename)
- else:
- print_error("Providing a %s is mandatory" % filename)
+ if not file_exists(app.path + "/" + filename):
+ yield Error("Providing %s is mandatory" % filename)
- #
- # Deprecated php-fpm.ini thing
- #
+ @test()
+ def change_url_script(app):
- if file_exists(self.path + "/conf/php-fpm.ini"):
- print_warning(
+ has_domain_arg = any(a["name"] == "is_public" for a in app.manifest["arguments"].get("install", []))
+ if has_domain_arg and not file_exists(app.path + "/scripts/change_url"):
+ yield Warning("Consider adding a change_url script to support changing where the app is installed")
+
+ @test()
+ def check_process_exists(app):
+
+ check_process_file = app.path + "/check_process"
+
+ if not file_exists(check_process_file):
+ yield Warning("You should add a 'check_process' file to properly interface with the continuous integration system")
+
+ @test()
+ def check_process_syntax(app):
+
+ check_process_file = app.path + "/check_process"
+ if not file_exists(check_process_file):
+ return
+
+ if os.system("grep -q 'Level 5=1' %s" % check_process_file) == 0:
+ yield Error("Do not force Level 5=1 in check_process...")
+
+ if os.system("grep -q ' *Level [^5]=' %s" % check_process_file) == 0:
+ yield Warning("Setting Level x=y in check_process is obsolete / not relevant anymore")
+
+ @test()
+ def check_process_consistency(app):
+
+ check_process_file = app.path + "/check_process"
+ if not file_exists(check_process_file):
+ return
+
+ has_is_public_arg = any(a["name"] == "is_public" for a in app.manifest["arguments"].get("install", []))
+ if has_is_public_arg:
+ if os.system(r"grep -q '^\s*setup_public=1' %s" % check_process_file) != 0:
+ yield Warning("It looks like you forgot to enable setup_public test in check_process ?")
+
+ if os.system(r"grep -q '^\s*setup_private=1' %s" % check_process_file) != 0:
+ yield Warning("It looks like you forgot to enable setup_private test in check_process ?")
+
+ has_path_arg = any(a["name"] == "path" for a in app.manifest["arguments"].get("install", []))
+ if has_path_arg:
+ if os.system(r"grep -q '^\s*setup_sub_dir=1' %s" % check_process_file) != 0:
+ yield Warning("It looks like you forgot to enable setup_sub_dir test in check_process ?")
+
+ if app.manifest.get("multi_instance") in [True, 1, "True", "true"]:
+ if os.system(r"grep -q '^\s*multi_instance=1' %s" % check_process_file) != 0:
+ yield Warning("It looks like you forgot to enable multi_instance test in check_process ?")
+
+ if app.scripts["backup"].exists:
+ if os.system(r"grep -q '^\s*backup_restore=1' %s" % check_process_file) != 0:
+ yield Warning("It looks like you forgot to enable backup_restore test in check_process ?")
+
+
+ @test()
+ def misc_legacy_phpini(app):
+
+ if file_exists(app.path + "/conf/php-fpm.ini"):
+ yield Error(
"Using a separate php-fpm.ini file is deprecated. "
"Please merge your php-fpm directives directly in the pool file. "
"(c.f. https://github.com/YunoHost-Apps/nextcloud_ynh/issues/138 )"
)
- #
- # Source management
- #
- source_dir = os.path.join(self.path, "sources")
+
+ @test()
+ def misc_source_management(app):
+
+ source_dir = os.path.join(app.path, "sources")
if os.path.exists(source_dir) \
and len([name for name in os.listdir(source_dir) if os.path.isfile(os.path.join(source_dir, name))]) > 5:
- print_warning(
- "[YEP-3.3] Upstream app sources shouldn't be stored in this 'sources' folder of this git repository as a copy/paste\n"
+ yield Error(
+ "Upstream app sources shouldn't be stored in this 'sources' folder of this git repository as a copy/paste\n"
"During installation, the package should download sources from upstream via 'ynh_setup_source'.\n"
"See the helper documentation. "
"Original discussion happened here : "
"https://github.com/YunoHost/issues/issues/201#issuecomment-391549262"
)
+
+
+ @test()
+ def misc_nginx_add_header(app):
+
#
# Analyze nginx conf
# - Deprecated usage of 'add_header' in nginx conf
# - Spot path traversal issue vulnerability
#
- for filename in os.listdir(self.path + "/conf") if os.path.exists(self.path + "/conf") else []:
+ for filename in os.listdir(app.path + "/conf") if os.path.exists(app.path + "/conf") else []:
# Ignore subdirs or filename not containing nginx in the name
- if not os.path.isfile(self.path + "/conf/" + filename) or "nginx" not in filename:
+ if not os.path.isfile(app.path + "/conf/" + filename) or "nginx" not in filename:
continue
- #
- # 'add_header' usage
- #
- content = open(self.path + "/conf/" + filename).read()
+ content = open(app.path + "/conf/" + filename).read()
if "location" in content and "add_header" in content:
- print_warning(
+ yield Error(
"Do not use 'add_header' in the nginx conf. Use 'more_set_headers' instead. "
"(See https://www.peterbe.com/plog/be-very-careful-with-your-add_header-in-nginx "
"and https://github.com/openresty/headers-more-nginx-module#more_set_headers )"
)
+
+ @test()
+ def misc_nginx_path_traversal(app):
+ for filename in os.listdir(app.path + "/conf") if os.path.exists(app.path + "/conf") else []:
+ # Ignore subdirs or filename not containing nginx in the name
+ if not os.path.isfile(app.path + "/conf/" + filename) or "nginx" not in filename:
+ continue
+ content = open(app.path + "/conf/" + filename).read()
+
#
# Path traversal issues
#
@@ -420,6 +584,9 @@ class App():
for block in nginxconf:
for location, alias in find_location_with_alias(block):
+ # Ignore locations which are regexes..?
+ if location.startswith("^") and location.endswith("$"):
+ continue
alias_path = alias[-1]
# For path traversal issues to occur, both of those are needed :
# - location /foo { (*without* a / after foo)
@@ -457,13 +624,13 @@ class App():
if do_path_traversal_check:
from lib.nginxparser import nginxparser
try:
- nginxconf = nginxparser.load(open(self.path + "/conf/" + filename))
+ nginxconf = nginxparser.load(open(app.path + "/conf/" + filename))
except Exception as e:
- print_warning_not_reliable("Could not parse nginx conf ... : " + str(e))
+ _print("Could not parse nginx conf ... : " + str(e))
nginxconf = []
for location in find_path_traversal_issue(nginxconf):
- print_warning(
+ yield Error(
"The nginx configuration (especially location %s) "
"appears vulnerable to path traversal issues as explained in\n"
" https://www.acunetix.com/vulnerabilities/web/path-traversal-via-misconfigured-nginx-alias/\n"
@@ -471,247 +638,279 @@ class App():
" https://github.com/YunoHost/example_ynh/blob/master/conf/nginx.conf" % location
)
- def check_helper_consistency(self):
- """
- check if ynh_install_app_dependencies is present in install/upgrade/restore
- so dependencies are up to date after restoration or upgrade
- """
+#############################################
+# __ __ _ __ _ #
+# | \/ | (_)/ _| | | #
+# | \ / | __ _ _ __ _| |_ ___ ___| |_ #
+# | |\/| |/ _` | '_ \| | _/ _ \/ __| __| #
+# | | | | (_| | | | | | || __/\__ \ |_ #
+# |_| |_|\__,_|_| |_|_|_| \___||___/\__| #
+# #
+#############################################
- install_script = self.scripts["install"]
- if install_script.exists:
- if install_script.contains("ynh_install_app_dependencies"):
- for name in ["upgrade", "restore"]:
- if self.scripts[name].exists and not self.scripts[name].contains("ynh_install_app_dependencies"):
- print_warning("ynh_install_app_dependencies should also be in %s script" % name)
+class Manifest(TestSuite):
- if install_script.contains("yunohost service add"):
- if self.scripts["remove"].exists and not self.scripts["remove"].contains("yunohost service remove"):
- print_error(
- "You used 'yunohost service add' in the install script, "
- "but not 'yunohost service remove' in the remove script."
- )
+ def __init__(self, path):
+ self.path = path
+ manifest_path = os.path.join(path, 'manifest.json')
+ assert os.path.exists(manifest_path), "manifest.json is missing"
- def check_manifest(self):
- manifest = os.path.join(self.path, 'manifest.json')
- if not os.path.exists(manifest):
- return
- print_header("MANIFEST")
- """
- Check if there is no comma syntax issue
- """
+ # Taken from https://stackoverflow.com/a/49518779
+ def check_for_duplicate_keys(ordered_pairs):
+ dict_out = {}
+ for key, val in ordered_pairs:
+ if key in dict_out:
+ raise Exception("Duplicated key '%s' in %s" % (key, ordered_pairs))
+ else:
+ dict_out[key] = val
+ return dict_out
+ manifest_path = os.path.join(path, 'manifest.json')
+ raw_manifest = open(manifest_path, encoding='utf-8').read()
try:
- with open(manifest, encoding='utf-8') as data_file:
- manifest = json.loads(data_file.read(), object_pairs_hook=check_for_duplicate_keys)
- except:
- print_error("[YEP-2.1] Syntax (comma) or encoding issue with manifest.json. Can't check file.")
+ self.manifest = json.loads(raw_manifest, object_pairs_hook=check_for_duplicate_keys)
+ except Exception as e:
+ raise Exception("Looks like there's a syntax issue in your json ? %s" % e)
- fields = ("name", "id", "packaging_format", "description", "url", "version",
- "license", "maintainer", "requirements", "multi_instance",
+ self.catalog_infos = app_list().get(self.manifest.get("id"), {})
+
+
+ @test()
+ def mandatory_fields(self):
+
+ fields = ("name", "id", "packaging_format", "description", "version",
+ "maintainer", "requirements", "multi_instance",
"services", "arguments")
+ missing_fields = [field for field in fields if field not in self.manifest.keys()]
- for field in fields:
- if field not in manifest:
- print_warning("[YEP-2.1] \"" + field + "\" field is missing")
+ if missing_fields:
+ yield Error("The following mandatory fields are missing: %s" % missing_fields)
- """
- Check values in keys
- """
+ fields = ("license", "url")
+ missing_fields = [field for field in fields if field not in self.manifest.keys()]
- if "packaging_format" not in manifest:
- print_error("[YEP-2.1] \"packaging_format\" key is missing")
- elif not isinstance(manifest["packaging_format"], int):
- print_error("[YEP-2.1] \"packaging_format\": value isn't an integer type")
- elif manifest["packaging_format"] != 1:
- print_error("[YEP-2.1] \"packaging_format\" field: current format value is '1'")
+ if missing_fields:
+ yield Warning("The following mandatory fields are missing: %s" % missing_fields)
- # YEP 1.1 Name is app
- if "id" in manifest:
- if not re.match('^[a-z0-9]((_|-)?[a-z0-9])+$', manifest["id"]):
- print_error("[YEP-1.1] 'id' field '%s' should respect this regex '^[a-z0-9]((_|-)?[a-z0-9])+$'")
+ @test()
+ def yunohost_version_requirement(self):
- if "name" in manifest:
- if len(manifest["name"]) > 22:
- print_warning(
- "[YEP-1.1] The 'name' field shouldn't be too long to be able to be with one line in the app list. "
- "The most current bigger name is actually compound of 22 characters."
- )
+ if not self.manifest.get("requirements", {}).get("yunohost", ""):
+ yield Error("You should add a yunohost version requirement in the manifest")
- # YEP 1.3 License
- def license_mentionned_in_readme(path):
- readme_path = os.path.join(path, 'README.md')
+ @test()
+ def yunohost_version_requirement_superold(app):
+
+ yunohost_version_req = app.manifest.get("requirements", {}).get("yunohost", "").strip(">= ")
+ if yunohost_version_req.startswith("2."):
+ yield Error("Your app only requires yunohost >= 2.x, which tends to indicate that your app may not be up to date with recommended packaging practices and helpers.")
+
+ @test()
+ def basic_fields_format(self):
+
+ if self.manifest.get("packaging_format") != 1:
+ yield Error("packaging_format should be 1")
+ if not re.match('^[a-z0-9]((_|-)?[a-z0-9])+$', self.manifest.get("id")):
+ yield Error("The app id is not a valid app id")
+ if len(self.manifest["name"]) > 22:
+ yield Warning("The app name is too long")
+
+ @test()
+ def license(self):
+
+ if not "license" in self.manifest:
+ return
+
+ def license_mentionned_in_readme():
+ readme_path = os.path.join(self.path, 'README.md')
if os.path.isfile(readme_path):
return "LICENSE" in open(readme_path).read()
return False
- if "license" in manifest:
- for license in manifest['license'].replace('&', ',').split(','):
- code_license = '' + license + ''
- if license == "nonfree":
- print_warning("[YEP-1.3] The correct value for non free license in license field is 'non-free' and not 'nonfree'")
- license = "non-free"
- if license in ["free", "non-free", "dep-non-free"]:
- if not license_mentionned_in_readme(self.path):
- print_warning(
- "[YEP-1.3] The use of '%s' in license field implies "
- " to write something about the license in your README.md" % (license)
- )
- if license in ["non-free", "dep-non-free"]:
- print_warning(
- "[YEP-1.3] 'non-free' apps can't be officialized. "
- " Their integration is still being discussed, especially for apps with non-free dependencies"
- )
- elif code_license not in spdx_licenses():
- print_warning(
- "[YEP-1.3] The license '%s' is not registered in https://spdx.org/licenses/ . "
- "It can be a typo error. If not, you should replace it by 'free' "
- "or 'non-free' and give some explanations in the README.md." % (license)
- )
-
- # YEP 1.4 Inform if we continue to maintain the app
- # YEP 1.5 Update regularly the app status
- # YEP 1.6 Check regularly the evolution of the upstream
-
- # YEP 1.2 Put the app in a weel known repo
- # YEP 1.7 - Add an app to the YunoHost-Apps organization
- if "id" in manifest:
-
- cachefile = "./.apps.json"
- app_list = None
- if os.path.exists(cachefile) and time.time() - os.path.getmtime(cachefile) < 3600:
- try:
- app_list = open(cachefile).read()
- app_list = json.loads(app_list)
- except:
- _print("Uuuuh failed to load apps.json from cache...")
- app_list = None
-
- if app_list is None:
- app_list_url = "https://raw.githubusercontent.com/YunoHost/apps/master/apps.json"
- app_list = urlopen(app_list_url)['content']
- open(cachefile, "w").write(app_list)
- app_list = json.loads(app_list)
-
- if manifest["id"] not in app_list:
- print_warning("[YEP-1.2] This app is not registered in our applications list")
-
- all_urls = [infos.get("url", "").lower() for infos in app_list.values()]
-
- repo_org = "https://github.com/YunoHost-Apps/%s_ynh" % (manifest["id"])
- repo_brique = "https://github.com/labriqueinternet/%s_ynh" % (manifest["id"])
-
- if repo_org.lower() not in all_urls and repo_brique.lower() not in all_urls:
- is_not_added_to_org = urlopen(repo_org)['code'] == 404
- is_not_added_to_brique = urlopen(repo_brique)['code'] == 404
-
- if is_not_added_to_org and is_not_added_to_brique:
- print_warning("[YEP-1.7] You should add your app in the YunoHost-Apps organisation.")
-
- # YEP 1.8 Publish test request
- # YEP 1.9 Document app
- if "description" in manifest:
- descr = manifest.get("description", "")
- if isinstance(descr, dict):
- descr = descr.get("en", "")
-
- if len(descr) < 5:
- print_warning(
- "[YEP-1.9] You should write a good description of the app, "
- "at least in english (1 line is enough)."
- )
-
- if len(descr) > 150:
- print_warning(
- "[YEP-1.9] Please use a shorter description (or the rendering on the webadmin / app list will be messy ...). Just describe in consise terms what the app is / does."
- )
-
- elif "for yunohost" in descr.lower() :
- print_warning(
- "[YEP-1.9] The 'description' should explain what the app actually does. "
- "No need to say that it is 'for YunoHost' - this is a YunoHost app "
- "so of course we know it is for YunoHost ;-)."
- )
- if descr.lower().startswith(manifest["id"].lower()) or descr.lower().startswith(manifest["name"].lower()):
- print_warning("[YEP-1.9] Try to avoid starting the description by '$app is' ... explain what the app is / does directly !")
-
- # TODO test a specific template in README.md
-
- # YEP 1.10 Garder un historique de version propre
-
- if not "version" in manifest or manifest["version"][-5:-1] != "~ynh":
- print_warning("Please specify a 'version' field in the manifest. It should match the format ~ynh. For example : 4.3-2~ynh3. It is composed of the upstream version number (in the example, 4.3-2) and an incremental number for each change in the package without upstream change (in the example, 3). This incremental number can be reset to 1 each time the upstream version changes.")
-
- # YEP 1.11 Cancelled
-
- # YEP 2.1
- if "multi_instance" in manifest and manifest["multi_instance"] != 1 and manifest["multi_instance"] != 0:
- print_error(
- "[YEP-2.1] \"multi_instance\" field must be boolean type values 'true' or 'false' and not string type")
-
- if "services" in manifest and self.scripts["install"].exists:
-
- known_services = ("nginx", "mysql", "uwsgi", "metronome",
- "php5-fpm", "php7.0-fpm", "php-fpm",
- "postfix", "dovecot", "rspamd")
-
- for service in manifest["services"]:
- if service not in known_services:
- if service == 'postgresql':
- if not self.scripts["install"].contains('ynh_psql_test_if_first_run')\
- or not self.scripts["restore"].contains('ynh_psql_test_if_first_run'):
- print_error("[YEP-2.1?] postgresql service present in the manifest, install and restore scripts must call ynh_psql_test_if_first_run")
- elif not self.scripts["install"].contains("yunohost service add %s" % service):
- print_error("[YEP-2.1?] " + service + " service not installed by the install file but present in the manifest")
-
- if "install" in manifest["arguments"]:
-
- recognized_types = ("domain", "path", "boolean", "app", "password", "user", "string", "display_text")
-
- for argument in manifest["arguments"]["install"]:
- if "optional" in argument.keys():
- if not isinstance(argument["optional"], bool):
- print_warning("The key 'optional' value for setting %s should be a boolean (true or false)" % argument["name"])
- if "type" not in argument.keys():
- print_warning(
- "[YEP-2.1] You should specify the type of the argument '%s'. "
- "You can use : %s." % (argument["name"], ', '.join(recognized_types))
- )
- elif argument["type"] not in recognized_types:
- print_warning(
- "[YEP-2.1] The type '%s' for argument '%s' is not recognized... "
- "it probably doesn't behave as you expect ? Choose among those instead : %s" % (argument["type"], argument["name"], ', '.join(recognized_types))
- )
-
- if "choices" in argument.keys():
- choices = [c.lower() for c in argument["choices"]]
- if len(choices) == 2:
- if ("true" in choices and "false" in choices) or ("yes" in choices and "no" in choices):
- print_warning(
- "Argument %s : you might want to simply use a boolean-type argument. "
- "No need to specify the choices list yourself." % argument["name"]
- )
-
- if argument["name"] == "is_public" and "help" not in argument.keys():
- print_warning_not_reliable(
- "Consider adding an 'help' key for argument 'is_public' "
- "to explain to the user what it means for *this* app "
- "to be public or private :\n"
- ' "help": {\n'
- ' "en": "Some explanation"\n'
- ' }')
+ license = self.manifest["license"]
- if "url" in manifest and manifest["url"].endswith("_ynh"):
- print_warning(
+ if "nonfree" in license.replace("-", ""):
+ yield Warning("'non-free' apps cannot be integrated in YunoHost's app catalog.")
+
+
+ code_license = '' + license + ''
+
+ if license == "free" and not license_mentionned_in_readme():
+ yield Warning(
+ "Setting the license as 'free' implies to write something about it in "
+ "the README.md. Alternatively, consider using one of the codes available "
+ "on https://spdx.org/licenses/"
+ )
+ elif code_license not in spdx_licenses():
+ yield Warning(
+ "The license id '%s' is not registered in https://spdx.org/licenses/. "
+ "It can be a typo error. If not, you should replace it by 'free' "
+ "or 'non-free' and give some explanations in the README.md." % (license)
+ )
+
+ @test()
+ def app_catalog(self):
+
+ if not self.catalog_infos:
+ yield Warning("This app is not in YunoHost's application catalog")
+
+ @test()
+ def app_catalog_revision(self):
+
+ if self.catalog_infos and self.catalog_infos.get("revision", "HEAD") != "HEAD":
+ yield Error("You should make sure that the revision used in YunoHost's apps catalog is HEAD...")
+
+ @test()
+ def app_catalog_state(self):
+
+ if self.catalog_infos and self.catalog_infos.get("state", "working") != "working":
+ yield Warning("The application is not flagged as working in YunoHost's apps catalog")
+
+ @test()
+ def app_catalog_maintained(self):
+
+ if self.catalog_infos and self.catalog_infos.get("maintained", True) is not True:
+ yield Warning("The application is flagged as not maintained in YunoHost's apps catalog")
+
+ @test()
+ def app_catalog_category(self):
+ if self.catalog_infos and not self.catalog_infos.get("category"):
+ yield Warning("The application has no associated category in YunoHost's apps catalog")
+
+ @test()
+ def app_in_github_org(self):
+
+ repo_org = "https://github.com/YunoHost-Apps/%s_ynh" % (self.manifest["id"])
+ repo_brique = "https://github.com/labriqueinternet/%s_ynh" % (self.manifest["id"])
+
+ if self.catalog_infos:
+ repo_url = self.catalog_infos["url"]
+
+ all_urls = [infos.get("url", "").lower() for infos in app_list().values()]
+
+ if repo_url.lower() not in [repo_org.lower(), repo_brique.lower()]:
+ if repo_url.lower().startswith("https://github.com/YunoHost-Apps/"):
+ yield Warning("The url for this app in the catalog should be ")
+ else:
+ yield Warning("Consider adding your app to the YunoHost-Apps organization to allow the community to contribute more easily")
+
+ else:
+ def is_in_github_org():
+ return urlopen(repo_org)['code'] != 404
+
+ def is_in_brique_org():
+ return urlopen(repo_brique)['code'] != 404
+
+ if not is_in_github_org() and not is_in_brique_org():
+ yield Warning("Consider adding your app to the YunoHost-Apps organization to allow the community to contribute more easily")
+
+ @test()
+ def description(self):
+
+ descr = self.manifest.get("description", "")
+ id = self.manifest["id"].lower()
+ name = self.manifest["name"].lower()
+
+ if isinstance(descr, dict):
+ descr = descr.get("en", "")
+
+ if len(descr) < 5 or len(descr) > 150:
+ yield Warning("The description of your app is either missing, too short or too long... Please describe in *consise* terms what the app is / does.")
+
+ if "for yunohost" in descr.lower():
+ yield Warning(
+ "The 'description' should explain what the app actually does. "
+ "No need to say that it is 'for YunoHost' - this is a YunoHost app "
+ "so of course we know it is for YunoHost ;-)."
+ )
+ if descr.lower().startswith(id) or descr.lower().startswith(name):
+ yield Warning(
+ "Try to avoid starting the description by '$app is' "
+ "... explain what the app is / does directly !"
+ )
+
+ @test()
+ def version_format(self):
+
+ if self.manifest.get("version", "")[-5:-1] != "~ynh":
+ yield Error(
+ "The 'version' field should match the format ~ynh. "
+ "For example : 4.3-2~ynh3. It is composed of the upstream version number (in the "
+ "example, 4.3-2) and an incremental number for each change in the package without "
+ "upstream change (in the example, 3). This incremental number can be reset to 1 "
+ "each time the upstream version changes."
+ )
+
+ @test()
+ def multiinstance_format(self):
+
+ if self.manifest["multi_instance"] not in [True, False, 0, 1]:
+ yield Error("\"multi_instance\" field should be boolean 'true' or 'false' and not string type")
+
+ @test()
+ def url(self):
+ if self.manifest.get("url", "").endswith("_ynh"):
+ yield Warning(
"'url' is not meant to be the url of the yunohost package, "
"but rather the website or repo of the upstream app itself..."
)
- self.yunohost_version_req = manifest.get("requirements", {}).get("yunohost", None)
+ @test()
+ def install_args(self):
+
+ recognized_types = ("domain", "path", "boolean", "password", "user", "string", "display_text")
+
+ for argument in self.manifest["arguments"].get("install", []):
+ if not isinstance(argument.get("optional", False), bool):
+ yield Warning(
+ "The key 'optional' value for setting %s should be a boolean (true or false)" % argument["name"]
+ )
+ if "type" not in argument.keys():
+ yield Warning(
+ "You should specify the type of the argument '%s'. "
+ "You can use : %s." % (argument["name"], ', '.join(recognized_types))
+ )
+ elif argument["type"] not in recognized_types:
+ yield Warning(
+ "The type '%s' for argument '%s' is not recognized... "
+ "it probably doesn't behave as you expect ? Choose among those instead : %s" % (argument["type"], argument["name"], ', '.join(recognized_types))
+ )
+
+ if "choices" in argument.keys():
+ choices = [c.lower() for c in argument["choices"]]
+ if len(choices) == 2:
+ if ("true" in choices and "false" in choices) or ("yes" in choices and "no" in choices):
+ yield Warning(
+ "Argument %s : you might want to simply use a boolean-type argument. "
+ "No need to specify the choices list yourself." % argument["name"]
+ )
+
+ @test()
+ def is_public_help(self):
+ for argument in self.manifest["arguments"].get("install", []):
+ if argument["name"] == "is_public" and "help" not in argument.keys():
+ yield Info(
+ "Consider adding an 'help' key for argument 'is_public' "
+ "to explain to the user what it means for *this* app "
+ "to be public or private :\n"
+ ' "help": {\n'
+ ' "en": "Some explanation"\n'
+ ' }')
-class Script():
+##################################
+# _____ _ _ #
+# / ____| (_) | | #
+# | (___ ___ _ __ _ _ __ | |_ #
+# \___ \ / __| '__| | '_ \| __| #
+# ____) | (__| | | | |_) | |_ #
+# |_____/ \___|_| |_| .__/ \__| #
+# | | #
+# |_| #
+##################################
+class Script(TestSuite):
def __init__(self, app_path, name):
self.name = name
@@ -744,7 +943,7 @@ class Script():
if not some_parsing_failed:
_print("Some lines could not be parsed in script %s. (That's probably not really critical)" % self.name)
some_parsing_failed = True
- print_warning_not_reliable("%s : %s" % (e, line))
+ report_warning_not_reliable("%s : %s" % (e, line))
def contains(self, command):
"""
@@ -764,161 +963,196 @@ class Script():
return any(re.search(regex, line)
for line in [' '.join(line) for line in self.lines])
- def analyze(self):
- print_header(self.name.upper() + " SCRIPT")
+ @test()
+ def error_handling(self):
- self.check_set_usage()
- self.check_helper_usage_dependencies()
- self.check_deprecated_practices()
-
- def check_set_usage(self):
-
- if self.name == "_common.sh":
- return
-
- present = False
-
- if self.name in ["backup", "remove"]:
+ if self.name in ["backup", "remove", "_common.sh"]:
present = self.contains("ynh_abort_if_errors") or self.contains("set -eu")
else:
present = self.contains("ynh_abort_if_errors")
- if self.name == "remove":
- # Remove script shouldn't use set -eu or ynh_abort_if_errors
+ if self.name in ["remove", "_common.sh"]:
if present:
- print_error(
- "[YEP-2.4] set -eu or ynh_abort_if_errors is present. "
- "If there is a crash, it could put yunohost system in "
- "a broken state. For details, look at "
- "https://github.com/YunoHost/issues/issues/419"
+ yield Error(
+ "Do not use set -eu or ynh_abort_if_errors in the remove or _common.sh: "
+ "If a single instruction fails, it will stop the script and is "
+ "likely to leave the system in a broken state."
)
elif not present:
- print_error(
- "[YEP-2.4] ynh_abort_if_errors is missing. For details, "
- "look at https://github.com/YunoHost/issues/issues/419"
+ yield Error(
+ "You should add 'ynh_abort_if_errors' in this script, "
+ "c.f. https://github.com/YunoHost/issues/issues/419"
)
- def check_helper_usage_dependencies(self):
- """
- Detect usage of ynh_package_* & apt-get *
- and suggest herlpers ynh_install_app_dependencies and ynh_remove_app_dependencies
- """
+ # Skip this in common.sh, sometimes custom not-yet-official helpers need this
+ @test(ignore=["_common.sh"])
+ def raw_apt_commands(self):
- # Skip this in common.sh, sometimes custom not-yet-official helpers need this
- if self.name == "_common.sh":
- return
-
- if self.contains("ynh_package_install") or self.contains("apt-get install"):
- print_warning(
+ if self.contains("ynh_package_install") or self.contains("apt install") or self.contains("apt-get install"):
+ yield Warning(
"You should not use `ynh_package_install` or `apt-get install`, "
"use `ynh_install_app_dependencies` instead"
)
- if self.contains("ynh_package_remove") or self.contains("apt-get remove"):
- print_warning(
+ if self.contains("ynh_package_remove") or self.contains("apt remove") or self.contains("apt-get remove"):
+ yield Warning(
"You should not use `ynh_package_remove` or `apt-get remove`, "
"use `ynh_remove_app_dependencies` instead"
)
- def check_deprecated_practices(self):
-
+ @test()
+ def obsolete_helpers(self):
if self.contains("yunohost app setting"):
- print_error("Do not use 'yunohost app setting' directly. Please use 'ynh_app_setting_(set,get,delete)' instead.")
+ yield Error("Do not use 'yunohost app setting' directly. Please use 'ynh_app_setting_(set,get,delete)' instead.")
if self.contains("yunohost app checkurl"):
- print_error("'yunohost app checkurl' is obsolete!!! Please use 'ynh_webpath_register' instead.")
+ yield Error("'yunohost app checkurl' is obsolete!!! Please use 'ynh_webpath_register' instead.")
if self.contains("yunohost app checkport"):
- print_error("'yunohost app checkport' is obsolete!!! Please use 'ynh_find_port' instead.")
+ yield Error("'yunohost app checkport' is obsolete!!! Please use 'ynh_find_port' instead.")
if self.contains("yunohost app initdb"):
- print_error("'yunohost app initdb' is obsolete!!! Please use 'ynh_mysql_setup_db' instead.")
- if self.contains("exit"):
- print_warning("'exit' command shouldn't be used. Please use 'ynh_die' instead.")
+ yield Error("'yunohost app initdb' is obsolete!!! Please use 'ynh_mysql_setup_db' instead.")
+ @test()
+ def safe_rm(self):
+ if self.contains("rm -rf"):
+ yield Error("You should avoid using 'rm -rf', please use 'ynh_secure_remove' instead")
+
+ @test()
+ def nginx_restart(self):
+ if self.contains("systemctl restart nginx") or self.contains("service nginx restart"):
+ yield Error(
+ "Restarting nginx is quite dangerous (especially for web installs) "
+ "and should be avoided at all cost. Use 'reload' instead."
+ )
+
+ @test(only=["install"])
+ def argument_fetching(self):
+
+ if self.containsregex(r"^\w+\=\$\{?[0-9]"):
+ yield Error(
+ "Do not fetch arguments from manifest using variable=$N (e.g."
+ " domain=$1 ...) Instead, use name=$YNH_APP_ARG_NAME"
+ )
+
+
+ @test(only=["install"])
+ def sources_list_tweaking(self):
+ if self.contains("/etc/apt/sources.list") \
+ or (os.path.exists(self.app_path + "/scripts/_common.sh") and "/etc/apt/sources.list" in open(self.app_path+"/scripts/_common.sh").read() and "ynh_add_repo" not in open(self.app_path+"/scripts/_common.sh").read()):
+ yield Error(
+ "Manually messing with apt's sources.lists is strongly discouraged "
+ "and should be avoided. Please use ynh_install_extra_app_dependencies is you "
+ "need to install dependencies from a custom apt repo."
+ )
+
+ @test()
+ def exit_ynhdie(self):
+
+ if self.contains(r"\bexit\b"):
+ yield Error("'exit' command shouldn't be used. Please use 'ynh_die' instead.")
+
+ @test()
+ def old_regenconf(self):
if self.contains("yunohost service regen-conf"):
- print_warning("'yunohost tools regen-conf' has been replaced by 'yunohost tools regen-conf'.")
+ yield Warning("'yunohost tools regen-conf' has been replaced by 'yunohost tools regen-conf'.")
+ @test()
+ def ssowatconf(self):
# Dirty hack to check only the 10 last lines for ssowatconf
# (the "bad" practice being using this at the very end of the script, but some apps legitimately need this in the middle of the script)
oldlines = list(self.lines)
self.lines = self.lines[-10:]
if self.contains("yunohost app ssowatconf"):
- print_warning("You probably don't need to run 'yunohost app ssowatconf' in the app script. It's supposed to be ran automatically after the script.")
+ yield Warning("You probably don't need to run 'yunohost app ssowatconf' in the app self. It's supposed to be ran automatically after the script.")
self.lines = oldlines
- if self.contains("rm -rf"):
- print_error("[YEP-2.12] You should avoid using 'rm -rf', please use 'ynh_secure_remove' instead")
-
+ @test()
+ def sed(self):
if self.containsregex(r"sed\s+(-i|--in-place)\s+(-r\s+)?s") or self.containsregex(r"sed\s+s\S*\s+(-i|--in-place)"):
- print_warning("[YEP-2.12] You should avoid using 'sed -i' for substitutions, please use 'ynh_replace_string' instead")
+ yield Warning("You should avoid using 'sed -i' for substitutions, please use 'ynh_replace_string' instead")
+
+ @test()
+ def sudo(self):
if self.containsregex(r"sudo \w"): # \w is here to not match sudo -u, legit use because ynh_exec_as not official yet...
- print_warning(
- "[YEP-2.12] You should not need to use 'sudo', the script is being run as root. "
+ yield Warning(
+ "You should not need to use 'sudo', the self is being run as root. "
"(If you need to run a command using a specific user, use 'ynh_exec_as')"
)
+ @test()
+ def chmod777(self):
if self.containsregex(r"chmod .*777") or self.containsregex(r'chmod .*o\+w'):
- print_warning(
+ yield Warning(
"DO NOT use chmod 777 or chmod o+w that gives write permission to every users on the system !!! If you have permission issues, just make sure that the owner and/or group owner is right ..."
)
+ @test()
+ def random(self):
if self.contains("dd if=/dev/urandom") or self.contains("openssl rand"):
- print_warning(
- "Instead of 'dd if=/dev/urandom' or 'openssl rand', "
- "you might want to use ynh_string_random"
+ yield Error(
+ "Instead of 'dd if=/dev/urandom' or 'openssl rand', you should use ynh_string_random"
)
- if self.contains("systemctl restart nginx") or self.contains("service nginx restart"):
- print_error(
- "Restarting nginx is quite dangerous (especially for web installs) "
- "and should be avoided at all cost. Use 'reload' instead."
- )
-
- if self.name == "install" and not self.contains("ynh_print_info") and not self.contains("ynh_script_progression"):
- print_warning(
+ @test(only=["install"])
+ def progression(self):
+ if not self.contains("ynh_print_info") and not self.contains("ynh_script_progression"):
+ yield Warning(
"Please add a few messages for the user, to explain what is going on "
"(in friendly, not-too-technical terms) during the installation. "
"You can use 'ynh_print_info' or 'ynh_script_progression' for this."
)
- if self.name == "install" and self.containsregex(r"^\w+\=\$\{?[0-9]"):
- print_error(
- "Do not fetch arguments from manifest using variable=$N (e.g."
- " domain=$1 ...) Instead, use name=$YNH_APP_ARG_NAME"
- )
+ @test()
+ def progression_time(self):
- if self.name == "install":
- if self.contains("/etc/apt/sources.list") \
- or (os.path.exists(self.app_path + "/scripts/_common.sh") and "/etc/apt/sources.list" in open(self.app_path+"/scripts/_common.sh").read() and "ynh_add_repo" not in open(self.app_path+"/scripts/_common.sh").read()):
- print_error(
- "[YEP-3.7] Manually messing with apt's sources.lists is strongly discouraged "
- "and should be avoided. Please use ynh_install_extra_app_dependencies is you "
- "need to install dependencies from a custom apt repo."
- )
+ # Usage of ynh_script_prorgression with --time or --weight=1 all over the place...
+ if self.containsregex(r"ynh_script_progression.*--time"):
+ yield Warning("Using ynh_script_progression --time should only be for calibrating the weight (c.f. --weight). It's not meant to be kept for production versions.")
- if self.name in ["install", "_common.sh"]:
- if self.containsregex("dependencies.*php-"):
- print_warning("You should avoid having dependencies like 'php-foobar'. Instead, specify the exact version you want like 'php7.0-foobar'. Otherwise, the *wrong* version of the dependency may be installed if sury is also installed. Note that for Stretch/Buster/Bullseye/... transition, Yunohost will automatically patch your file so there's no need to care about that.")
+ @test(ignore=["_common.sh", "backup"])
+ def progression_meaningful_weights(self):
- if self.name == "backup":
- if self.containsregex("^ynh_systemd_action"):
- print_warning("Unless you really have a good reason to do so, starting/stopping services during backup has no benefit and leads to unecessary service interruptions when creating backups... As a 'reminder': apart from possibly database dumps (which usually do not require the service to be stopped) or other super-specific action, running the backup script is only a *declaration* of what needs to be backuped. The real copy and archive creation happens *after* the backup script is ran.")
+ def weight(line):
+ match = re.search(r"ynh_script_progression.*--weight=([0-9]+)", ' '.join(line))
+ if match:
+ try:
+ return int(match.groups()[0])
+ except:
+ return -1
+ else:
+ return 1
+ script_progress = [line for line in self.lines if "ynh_script_progression" in line]
+ weights = [weight(line) for line in script_progress]
+
+ if not weights:
+ return
+
+ if len(weights) > 3 and statistics.stdev(weights) > 50:
+ yield Warning("To have a meaningful progress bar, try to keep the weights in the same range of values, for example [1,10], or [10,100] ... otherwise, if you have super-huge weight differentes, the progress bar rendering will be completely dominated by one or two steps... If these steps are really long, just try to indicated in the message that this will take a while.")
+
+ @test(only=["install", "_common.sh"])
+ def php_deps(self):
+ if self.containsregex("dependencies.*php-"):
+ yield Warning("You should avoid having dependencies like 'php-foobar'. Instead, specify the exact version you want like 'php7.0-foobar'. Otherwise, the *wrong* version of the dependency may be installed if sury is also installed. Note that for Stretch/Buster/Bullseye/... transition, Yunohost will automatically patch your file so there's no need to care about that.")
+
+ @test(only=["backup"])
+ def systemd_during_backup(self):
+ if self.containsregex("^ynh_systemd_action"):
+ yield Warning("Unless you really have a good reason to do so, starting/stopping services during backup has no benefit and leads to unecessary service interruptions when creating backups... As a 'reminder': apart from possibly database dumps (which usually do not require the service to be stopped) or other super-specific action, running the backup script is only a *declaration* of what needs to be backuped. The real copy and archive creation happens *after* the backup script is ran.")
+
+ @test()
+ def helpers_sourcing_after_official(self):
helpers_after_official = subprocess.check_output("head -n 30 '%s' | grep -A 10 '^ *source */usr/share/yunohost/helpers' | grep '^ *source' | tail -n +2" % self.path, shell=True).decode("utf-8")
helpers_after_official = helpers_after_official.replace("source", "").replace(" ", "").strip()
if helpers_after_official:
helpers_after_official = helpers_after_official.split("\n")
- print_warning("Please avoid sourcing additional helpers after the official helpers (in this case file %s)" % ", ".join(helpers_after_official))
+ yield Warning("Please avoid sourcing additional helpers after the official helpers (in this case file %s)" % ", ".join(helpers_after_official))
- if self.name in ["backup", "restore"]:
- if self.contains("source _common.sh") or self.contains("source ./_common.sh"):
- print_warning("In the context of backup and restore script, you should load _common.sh with \"source ../settings/scripts/_common.sh\"")
-
- # Usage of ynh_script_prorgression with --time or --weight=1 all over the place...
- if self.containsregex(r"ynh_script_progression.*--time"):
- print_warning("Using ynh_script_progression --time should only be for calibrating the weight (c.f. --weight). It's not meant to be kept for production versions.")
- if self.containsregex(r"ynh_script_progression.*--weight=1") \
- and not self.containsregex(r"ynh_script_progression.*--weight=([^1]|[1-9][0-9]+)"):
- print_warning("Having only '--weight=1' for ynh_script_progression is useless... Either calibrate the weights with --time once, or don't put any --weight at all.")
+ @test(only=["backup", "restore"])
+ def helpers_sourcing_backuprestore(self):
+ if self.contains("source _common.sh") or self.contains("source ./_common.sh"):
+ yield Warning("In the context of backup and restore script, you should load _common.sh with \"source ../settings/scripts/_common.sh\"")
def main():
@@ -935,9 +1169,13 @@ def main():
App(app_path).analyze()
if output == "json":
- print(json.dumps({"warnings": warnings, "errors": errors}, indent=4))
+ print(json.dumps({"warnings": [test for test, report in tests_reports if isinstance(report, Warning)],
+ "errors": [test for test, report in tests_reports if isinstance(report, Error)]}, indent=4))
else:
- if len(errors) > 0:
+ errors = [report for _, report in tests_reports if isinstance(report, Error)]
+ warnings = [report for _, report in tests_reports if isinstance(report, Warning)]
+ if errors:
+ print("Uhoh there are some errors to be fixed :(")
sys.exit(1)
elif len(warnings) > 3:
print("Still some warnings to be fixed :s")