Merge branch 'master' into check-for-path-traversal-issue

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:
+        if "services" in manifest and self.scripts["install"].exists:
-            services = ("nginx", "mysql", "uwsgi", "metronome",
+
+            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:
+                if service not in known_services:
-                    # FIXME : wtf is it supposed to mean ...
+                    if not self.scripts["install"].contains("yunohost service add %s" % service):
-                    print_warning("[YEP-2.1] " + service + " service may not exist")
+                        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
@@ -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)