Adapt linter to support v2 packages

package_linter.py

@@ -1,6 +1,7 @@
 #!/usr/bin/env python3
 # -*- coding: utf8 -*-
 
+import toml
 import sys
 import os
 import re
@@ -176,6 +177,34 @@ official_helpers = {
     "ynh_compare_current_package_version": "3.8.0",
 }
 
+deprecated_helpers_in_v2 = [
+    ("ynh_clean_setup", "(?)"),
+    ("ynh_abort_if_errors", "nothing, handled by the core, just get rid of it"),
+    ("ynh_backup_before_upgrade", "nothing, handled by the core, just get rid of it"),
+    ("ynh_restore_upgradebackup", "nothing, handled by the core, just get rid of it"),
+    ("ynh_system_user_create", "the system_user resource"),
+    ("ynh_system_user_delete", "the system_user resource"),
+    ("ynh_webpath_register", "the permission resource"),
+    ("ynh_webpath_available", "the permission resource"),
+    ("ynh_permission_update", "the permission resource"),
+    ("ynh_permission_create", "the permission resource"),
+    ("ynh_permission_exists", "the permission resource"),
+    ("ynh_legacy_permissions_exists", "the permission resource"),
+    ("ynh_legacy_permissions_delete_all", "the permission resource"),
+    ("ynh_install_app_dependencies", "the apt resource"),
+    ("ynh_install_extra_app_dependencies", "the apt source"),
+    ("ynh_remove_app_dependencies", "the apt resource"),
+    ("ynh_psql_test_if_first_run", "the database resource"),
+    ("ynh_mysql_setup_db", "the database resource"),
+    ("ynh_psql_setup_db", "the database resource"),
+    ("ynh_mysql_remove_db", "the database resource"),
+    ("ynh_psql_remove_db", "the database resource"),
+    ("ynh_find_port", "the port resource"),
+    ("ynh_send_readme_to_admin", "the doc/notifications/post_install.md or post_upgrade.md mechanism")
+]
+
+# Default to 1, set to 2 automatically if a toml manifest is found
+app_packaging_format = None
 
 # ############################################################################
 #   Utilities
@@ -488,7 +517,6 @@ class App(TestSuite):
     @test()
     def mandatory_scripts(app):
         filenames = (
-            "manifest.json",
             "LICENSE",
             "README.md",
             "scripts/install",
@@ -509,6 +537,7 @@ class App(TestSuite):
 
     @test()
     def doc_dir(app):
+
         if not os.path.exists(app.path + "/doc"):
             yield Info(
                 """READMEs are to be automatically generated using https://github.com/YunoHost/apps/tree/master/tools/README-generator.
@@ -522,6 +551,41 @@ class App(TestSuite):
             if screenshots_size > 512000:
                 yield Info("Consider keeping the content of doc/screenshots under ~512Kb for better UI/UX once the screenshots will be integrated in the webadmin app's catalog (to be discussed with the team)")
 
+    @test()
+    def doc_dir_v2(app):
+
+        if app_packaging_format <= 1:
+            return
+
+        if not os.path.exists(app.path + "/doc/DESCRIPTION.md"):
+            yield Error("A DESCRIPTION.md is now mandatory in packaging v2 and is meant to contains an extensive description of what the app is and does. Consider also adding a '/doc/screenshots/' folder with a few screenshots of what the app looks like.")
+        elif os.system(fr'grep -inrq "Some long and extensive description\|lorem ipsum dolor sit amet\|Ut enim ad minim veniam" {app.path}/doc/DESCRIPTION.md') == 0:
+            yield Error("It looks like DESCRIPTION.md just contains placeholder texts")
+
+        if os.path.exists(app.path + "/doc/DISCLAIMER.md"):
+            yield Warning("""The whole thing about doc/DISCLAIMER.md is refactored again in v2 (sorry about that :/) to improve the UX - basically people shouldnt have to actively go read the READMEs to get those infos
+
+    You are encouraged to split its infos into:
+
+    - Integration-related infos (eg. LDAP/SSO support, arch support, resource usage, ...)
+        -> neant to go in the 'integration' section of the manifest.toml
+
+    - Antifeatures-related infos (eg. alpha/deprecated software, arbitrary limiations, ...)
+        -> these are now formalized using the 'antifeatures' mechanism in the app catalog directly : cf https://github.com/YunoHost/apps/blob/master/antifeatures.yml and the 'antifeatures' key in apps.json
+
+    - Important infos that the admin should be made aware of *before* or *after* the install
+        -> infos *before* the install are meant to go in 'doc/notifications/pre_install.md'
+        -> infos *after* the install are meant to go in 'doc/notifications/post_install.md' (mostly meant to replace ynh_send_readme_to_admin, typically tips about how to login for the first time on the app / finish the install, ...).
+        -> these will be shown to the admin before/after the install (and the post_install notif will also be available in the app info page)
+        -> note that in these files, the __FOOBAR__ syntax is supported and replaced with the corresponding 'foobar' setting.
+
+    - General admin-related infos (eg. how to access the admin interface of the app, how to install plugin, etc)
+        -> meant to go in 'doc/ADMIN.md' which shall be made available in the app info page in the webadmin after installation.
+        -> if relevant, you can also create custom doc page, just create 'doc/WHATEVER.MD' and this will correspond to a specific documentation tab in the webadmin.
+        -> note that in these files, the __FOOBAR__ syntax is supported and replaced with the corresponding 'foobar' setting.
+""")
+
+
     @test()
     def disclaimer_wording(app):
         if os.path.exists(app.path + "/doc"):
@@ -539,9 +603,16 @@ class App(TestSuite):
     @test()
     def change_url_script(app):
 
-        has_domain_arg = any(
+        if app_packaging_format <= 1:
-            a["name"] == "domain" for a in app.manifest["arguments"].get("install", [])
+            args = app.manifest["arguments"].get("install", [])
-        )
+        else:
+            keyandargs = app.manifest["install"]
+            for key, infos in keyandargs.items():
+                infos["name"] = key
+            args = keyandargs.values()
+
+        has_domain_arg = any(a["name"] == "domain" for a in args)
+
         if has_domain_arg and not file_exists(app.path + "/scripts/change_url"):
             yield Info(
                 "Consider adding a change_url script to support changing where the app can be reached"
@@ -677,9 +748,14 @@ class App(TestSuite):
         )
         custom_helpers = [c.split("__")[0] for c in custom_helpers]
 
+        if app_packaging_format <= 1:
             yunohost_version_req = (
                 app.manifest.get("requirements", {}).get("yunohost", "").strip(">= ")
             )
+        else:
+            yunohost_version_req = (
+                app.manifest.get("integration", {}).get("yunohost", "").strip(">= ")
+            )
 
         cmd = "grep -IhEro 'ynh_\w+' '%s/scripts'" % app.path
         helpers_used = (
@@ -711,6 +787,23 @@ class App(TestSuite):
                 )
                 yield Error(message) if major_diff else Warning(message)
 
+    @test()
+    def helpers_deprecated_in_v2(app):
+
+        if app_packaging_format <= 1:
+            return
+
+        cmd = f"grep -IhEro 'ynh_\w+' '{app.path}/scripts/install' '{app.path}/scripts/remove' '{app.path}/scripts/upgrade' '{app.path}/scripts/backup' '{app.path}/scripts/restore'"
+        helpers_used = (
+            subprocess.check_output(cmd, shell=True).decode("utf-8").strip().split("\n")
+        )
+        helpers_used = sorted(set(helpers_used))
+
+        deprecated_helpers_in_v2_ = {k: v for k, v in deprecated_helpers_in_v2}
+
+        for helper in [h for h in helpers_used if h in deprecated_helpers_in_v2_.keys()]:
+            yield Warning(f"Using helper {helper} is deprecated when using packaging v2 ... It is replaced by: {deprecated_helpers_in_v2_[helper]}")
+
     @test()
     def helper_consistency_apt_deps(app):
         """
@@ -929,13 +1022,20 @@ class Configurations(TestSuite):
 
         app = self.app
 
+        if app_packaging_format <= 1:
+            args = app.manifest["arguments"].get("install", [])
+        else:
+            keyandargs = app.manifest["install"]
+            for key, infos in keyandargs.items():
+                infos["name"] = key
+            args = keyandargs.values()
+
         check_process_file = app.path + "/check_process"
         if not file_exists(check_process_file):
             return
 
         has_is_public_arg = any(
-            a["name"] == "is_public"
+            a["name"] == "is_public" for a in args
-            for a in app.manifest["arguments"].get("install", [])
         )
         if has_is_public_arg:
             if (
@@ -955,7 +1055,7 @@ class Configurations(TestSuite):
                 )
 
         has_path_arg = any(
-            a["name"] == "path" for a in app.manifest["arguments"].get("install", [])
+            a["name"] == "path" for a in args
         )
         if has_path_arg:
             if (
@@ -966,7 +1066,7 @@ class Configurations(TestSuite):
                     "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 app.manifest.get("multi_instance") in [True, 1, "True", "true"] or app.manifest.get("integration", {}).get("multi_instance") == True:
             if (
                 os.system(r"grep -q '^\s*multi_instance=1' '%s'" % check_process_file)
                 != 0
@@ -1359,7 +1459,14 @@ class Manifest(TestSuite):
     def __init__(self, path):
 
         self.path = path
-        self.test_suite_name = "manifest.json"
+        self.test_suite_name = "manifest"
+
+        global app_packaging_format
+        manifest_path = os.path.join(path, "manifest.toml")
+        if os.path.exists(manifest_path):
+            app_packaging_format = 2
+        else:
+            app_packaging_format = 1
             manifest_path = os.path.join(path, "manifest.json")
             assert os.path.exists(manifest_path), "manifest.json is missing"
 
@@ -1373,12 +1480,14 @@ class Manifest(TestSuite):
                     dict_out[key] = val
             return dict_out
 
-        manifest_path = os.path.join(path, "manifest.json")
+        self.raw_manifest = open(manifest_path, encoding="utf-8").read()
-        raw_manifest = open(manifest_path, encoding="utf-8").read()
         try:
+            if app_packaging_format == 1:
                 self.manifest = json.loads(
-                raw_manifest, object_pairs_hook=check_for_duplicate_keys
+                    self.raw_manifest, object_pairs_hook=check_for_duplicate_keys
                 )
+            else:
+                self.manifest = toml.loads(self.raw_manifest)
         except Exception as e:
             print(
                 c.FAIL
@@ -1389,18 +1498,31 @@ class Manifest(TestSuite):
     @test()
     def mandatory_fields(self):
 
-        fields = (
+        base_fields = [
-            "name",
-            "id",
             "packaging_format",
+            "id",
+            "name",
             "description",
             "version",
+        ]
+
+        if app_packaging_format == 1:
+            fields = base_fields + [
                 "maintainer",
                 "requirements",
                 "multi_instance",
                 "services",
                 "arguments",
-        )
+            ]
+        else:
+            fields = base_fields + [
+                "maintainers",
+                "upstream",
+                "integration",
+                "install",
+                "resources",
+            ]
+
         missing_fields = [
             field for field in fields if field not in self.manifest.keys()
         ]
@@ -1410,6 +1532,7 @@ class Manifest(TestSuite):
                 "The following mandatory fields are missing: %s" % missing_fields
             )
 
+        if app_packaging_format == 1:
             fields = ("license", "url")
             missing_fields = [
                 field for field in fields if field not in self.manifest.keys()
@@ -1419,6 +1542,11 @@ class Manifest(TestSuite):
                 yield Error(
                     "The following mandatory fields are missing: %s" % missing_fields
                 )
+        else:
+            if "license" not in self.manifest.get("upstream"):
+                yield Error(
+                    "The license key in the upstream section is missing"
+                )
 
     @test()
     def upstream_fields(self):
@@ -1442,10 +1570,17 @@ class Manifest(TestSuite):
             if "example.com" in self.manifest["upstream"].get("demo", "") or "example.com" in self.manifest["upstream"].get("website", ""):
                 yield Warning("It seems like the upstream section still contains placeholder values such as 'example.com' ...")
 
+    @test()
+    def FIXMEs(self):
+        if "FIXME" in self.raw_manifest:
+            yield Warning("There are still some FIXMEs remaining in the manifest")
 
     @test()
     def yunohost_version_requirement(self):
 
+        if app_packaging_format >= 2:
+            return
+
         if not self.manifest.get("requirements", {}).get("yunohost", ""):
             yield Critical(
                 "You should add a YunoHost version requirement in the manifest"
@@ -1454,6 +1589,11 @@ class Manifest(TestSuite):
     @test()
     def yunohost_version_requirement_superold(app):
 
+        if app_packaging_format >= 2:
+            yunohost_version_req = (
+                app.manifest.get("integration", {}).get("yunohost", "").strip(">= ")
+            )
+        else:
             yunohost_version_req = (
                 app.manifest.get("requirements", {}).get("yunohost", "").strip(">= ")
             )
@@ -1471,21 +1611,61 @@ class Manifest(TestSuite):
     @test()
     def basic_fields_format(self):
 
-        if self.manifest.get("packaging_format") != 1:
+        if self.manifest.get("packaging_format") != app_packaging_format:
-            yield Error("packaging_format should be 1")
+            yield Error(f"packaging_format should be {app_packaging_format}")
         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 Error("The app name is too long")
 
+        if app_packaging_format <= 1:
+            if self.manifest.get("url", "").endswith("_ynh"):
+                yield Info(
+                    "'url' is not meant to be the URL of the YunoHost package, "
+                    "but rather the website or repo of the upstream app itself..."
+                )
+            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"
+                )
+            return
+
+        keys = {
+            "yunohost": (lambda v: isinstance(v, str) and re.fullmatch(r"^>=\s*[\d\.]+\d$", v), "Expected something like '>= 4.5.6'"),
+            "architectures": (lambda v: v == "all" or (isinstance(v, list) and all(subv in ["i386", "amd64", "armhf", "arm64"] for subv in v)), "'all' or a list of values in ['i386', 'amd64', 'armhf', 'arm64']"),
+            "multi_instance": (lambda v: isinstance(v, bool), "Expected a boolean (true or false, no quotes!)"),
+            "ldap": (lambda v: isinstance(v, bool) or v == "not_relevant", "Expected a boolean (true or false, no quotes!) or 'not_relevant'"),
+            "sso": (lambda v: isinstance(v, bool) or v == "not_relevant", "Expected a boolean (true or false, no quotes!) or 'not_relevant'"),
+            "disk": (lambda v: isinstance(v, str), "Expected a string"),
+            "ram": (lambda v: isinstance(v, dict) and isinstance(v.get("build"), str) and isinstance(v.get("runtime"), str), "Expected to find ram.build and ram.runtime with string values"),
+        }
+
+        for key, validator in keys.items():
+            if key not in self.manifest.get("integration", {}):
+                yield Error(f"Missing key in the integration section: {key}")
+            value = self.manifest["integration"][key]
+            if not validator[0](value):
+                yield Error(f"Error found with key {key} in the 'integration' section: {validator[1]}, got: {value}")
+
+        if not self.manifest.get("upstream", {}).get("license"):
+            yield Error("Missing 'license' key in the upstream section")
+
+
     @test()
     def license(self):
 
+        if app_packaging_format <= 1:
             if "license" not in self.manifest:
                 return
 
+            if "upstream" in self.manifest and "license" in self.manifest["upstream"] and self.manifest["upstream"]["license"] != self.manifest["license"]:
+                yield Warning("The content of 'license' in the 'upstream' block should be the same as 'license' (yes sorry, this is duplicate info, this is transitional for the manifest v2 ...)")
+
             # Turns out there may be multiple licenses... (c.f. Seafile)
             licenses = self.manifest["license"].split(",")
+        else:
+            # Turns out there may be multiple licenses... (c.f. Seafile)
+            licenses = self.manifest.get("upstream", {}).get("license", "").split(",")
 
         for license in licenses:
 
@@ -1506,8 +1686,6 @@ class Manifest(TestSuite):
                 )
                 return
 
-        if "upstream" in self.manifest and "license" in self.manifest["upstream"] and self.manifest["upstream"]["license"] != self.manifest["license"]:
-            yield Warning("The content of 'license' in the 'upstream' block should be the same as 'license' (yes sorry, this is duplicate info, this is transitional for the manifest v2 ...)")
 
     @test()
     def description(self):
@@ -1547,36 +1725,43 @@ class Manifest(TestSuite):
                 "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 Info(
-                "'url' is not meant to be the URL of the YunoHost package, "
-                "but rather the website or repo of the upstream app itself..."
-            )
-
     @test()
     def install_args(self):
 
         recognized_types = (
-            "domain",
+            "string",
+            "text",
+            "select",
+            "tags",
+            "email",
+            "url",
+            "date",
+            "time",
+            "color",
+            "password",
             "path",
             "boolean",
-            "password",
+            "domain",
             "user",
-            "string",
+            "group",
+            "number",
+            "range",
             "display_text",
+            "alert",
+            "markdown",
+            "file",
+            "app",
         )
 
-        for argument in self.manifest["arguments"].get("install", []):
+        if app_packaging_format <= 1:
+            args = self.manifest["arguments"].get("install", [])
+        else:
+            keyandargs = self.manifest["install"]
+            for key, infos in keyandargs.items():
+                infos["name"] = key
+            args = keyandargs.values()
+
+        for argument in args:
             if not isinstance(argument.get("optional", False), bool):
                 yield Warning(
                     "The key 'optional' value for setting %s should be a boolean (true or false)"
@@ -1593,6 +1778,10 @@ class Manifest(TestSuite):
                     "it probably doesn't behave as you expect? Choose among those instead: %s"
                     % (argument["type"], argument["name"], ", ".join(recognized_types))
                 )
+            elif argument["type"] == "display_text":
+                yield Info(
+                    "Question type 'display_text' is deprecated. You might want to use 'markdown' or 'alert' instead."
+                )
             elif argument["type"] == "boolean" and argument.get(
                 "default", True
             ) not in [True, False]:
@@ -1632,9 +1821,18 @@ class Manifest(TestSuite):
             ("admin", "user"),
             ("is_public", "boolean"),
             ("password", "password"),
+            ("init_main_permission", "group"),
         ]
 
-        for argument in self.manifest["arguments"].get("install", []):
+        if app_packaging_format <= 1:
+            args = self.manifest["arguments"].get("install", [])
+        else:
+            keyandargs = self.manifest["install"]
+            for key, infos in keyandargs.items():
+                infos["name"] = key
+            args = keyandargs.values()
+
+        for argument in args:
 
             if (
                 argument.get("ask")
@@ -1656,20 +1854,6 @@ class Manifest(TestSuite):
                     % argument.get("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 a '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'
-                    "    }"
-                )
-
-
 ########################################
 #  _____       _        _              #
 # /  __ \     | |      | |             #
@@ -1917,7 +2101,6 @@ class Script(TestSuite):
         lines = "\n".join(lines).replace("\\\n", "").split("\n")
 
         some_parsing_failed = False
-        some_parsing_failed_closing_quotation = False
 
         for line in lines:
 
@@ -1977,6 +2160,14 @@ class Script(TestSuite):
     @test()
     def error_handling(self):
 
+        if app_packaging_format == 2:
+            if self.contains("ynh_abort_if_errors") \
+            or self.contains("set -eu") \
+            or self.contains("set -u"):
+                yield Error("ynh_abort_if_errors or set -eu is now handled by YunoHost core in packaging v2, you should not have to add it to your script !")
+
+            return
+
         if self.name in ["backup", "remove", "_common.sh"]:
             present = (
                 self.contains("ynh_abort_if_errors")