diff --git a/package_linter.py b/package_linter.py index c5edcb9..799c24c 100755 --- a/package_linter.py +++ b/package_linter.py @@ -17,6 +17,16 @@ return_code = 0 # 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' @@ -99,7 +109,10 @@ class App(): self.check_source_management() self.check_manifest() - for script in self.scripts.values(): + # Copypasta of lines from __init__ instead of using + # self.script.values() because dict are unordered until python 3.7 + scripts = ["install", "remove", "upgrade", "backup", "restore"] + for script in [self.scripts[s] for s in scripts]: if script.exists: script.analyze() @@ -238,7 +251,7 @@ class App(): try: with open(manifest, encoding='utf-8') as data_file: - manifest = json.loads(data_file.read()) + 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.") @@ -333,7 +346,7 @@ class App(): if isinstance(descr, dict): descr = descr.get("en", None) - if descr is None or descr == manifest.get("name", None): + if descr is None or descr == "" or descr == manifest.get("name", None): print_warning( "[YEP-1.9] You should write a good description of the app, " "at least in english (1 line is enough)." @@ -357,15 +370,16 @@ class App(): print_error( "[YEP-2.1] \"multi_instance\" field must be boolean type values 'true' or 'false' and not string type") - if "services" in manifest: - services = ("nginx", "mysql", "uwsgi", "metronome", - "php5-fpm", "php7.0-fpm", "php-fpm", - "postfix", "dovecot", "rspamd") + 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 services: - # FIXME : wtf is it supposed to mean ... - print_warning("[YEP-2.1] " + service + " service may not exist") + if service not in known_services: + if 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"]: @@ -392,12 +406,34 @@ class App(): "No need to specify the choices list yourself." % argument["name"] ) + if argument["name"] == "is_public" and "help" not in argument.keys(): + print_warning( + "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' + ' }') + + if "url" in manifest and manifest["url"].endswith("_ynh"): print_warning( "'url' is not meant to be the url of the yunohost package, " "but rather the website or repo of the upstream app itself..." ) + yunohost_version_req = manifest.get("requirements", {}).get("yunohost", None) + if yunohost_version_req: + major_version = yunohost_version_req.split()[-1] + if major_version.startswith("2"): + print_warning( + "YunoHost version requirement is still 2.x ... Good job if " + "it does still work on Jessie !... But are you really sure " + "about that ;) ? be careful that many new helpers you might " + "already be playing with are only available on 3.x..." + ) + + class Script(): @@ -421,6 +457,7 @@ class Script(): lines = '\n'.join(lines).replace("\\\n", "").split("\n") for line in lines: + try: line = shlex.split(line, True) yield line @@ -434,7 +471,7 @@ class Script(): For instance, "app setting" is contained in "yunohost app setting $app ..." """ return any(command in line - for line in [ ' '.join(line) for line in self.lines]) + for line in [' '.join(line) for line in self.lines]) def analyze(self): @@ -444,6 +481,7 @@ class Script(): self.check_set_usage() self.check_helper_usage_dependencies() self.check_deprecated_practices() + self.check_source_common() def check_verifications_done_before_modifying_system(self): """ @@ -559,6 +597,12 @@ class Script(): "You can use 'ynh_print_info' or 'ynh_script_progression' for this." ) + def check_source_common(self): + + if self.name in ["backup", "restore"]: + if self.contains("source _common.sh") or self.contains("source ./_common.sh"): + print_error("In the context of backup and restore script, you should load _common.sh with \"source ../settings/scripts/_common.sh\"") + def main(): if len(sys.argv) != 2: @@ -568,6 +612,7 @@ def main(): app_path = sys.argv[1] header(app_path) App(app_path).analyze() + sys.exit(return_code)