WIP: Packaging v3 #2070

alexAubin · 2025-03-19T00:38:20Z

As foretold by The Prophecy

(NB: no need to try to review this, it's not nearly done, this is just the beginning, there's still a good 3 ~ 6 months of work ...)

TODO / Brain dump

…able to run app scripts snippets, in particular the future v3 app functions build/init etc

…p assets and files to keep during ynh_setup_source such that we can run it as non-root ... not super happy about it, it's a mess, we should probably find a simpler way to handle this under a common directory which becomes YNH_APP_BASEDIR..

…nd run the build() function from the app's scripts.sh instead of the install script (later we shall have a _pre_build_as_root, _post_build_as_root, the init phase, etc, gotta think about how to organize the code)

…ant as non-root because $app can't give a file to the www-data group ... we'll need to do this between build and init/migrate somehow

…a separate utils file

…m + if clause handling

src/app.py

src/tests/test_app_regenconf.py

+
+        super().__init__(*args, **kwargs)
+
+    def reload():


To fix the issue, we need to determine the intended behavior of the reload method:

If the method is meant to interact with the instance (e.g., access or modify instance attributes), the self parameter should be added as the first argument.

If the method is independent of the instance, it should be decorated with @staticmethod to clarify its intent.

Given the context, it is likely that reload is intended to be an instance method, as it is defined within a class that represents configurations. Therefore, the best fix is to add the self parameter to the method definition.

src/utils/configurations.py

src/utils/eval.py

+        ast.Eq: op.eq,
+        ast.NotEq: op.ne,
+    }
+    context["true"] = True


To fix the issue, replace the mutable default value ({}) of the context parameter in evaluate_simple_js_expression with None. Then, within the function, check if context is None and initialize it to an empty dictionary. This ensures that a new dictionary is created for each function call, avoiding unintended side effects caused by modifying a shared default value.

Steps to implement the fix:

Change the default value of the context parameter in evaluate_simple_js_expression from {} to None.

Add a check within evaluate_simple_js_expression to initialize context to an empty dictionary if it is None.

src/utils/eval.py

+        ast.NotEq: op.ne,
+    }
+    context["true"] = True
+    context["false"] = False


To fix the issue, we will replace the mutable default argument ({}) with None and initialize the dictionary within the function if context is None. This ensures that each call to the function gets a fresh dictionary, preventing unintended side effects from shared state.

Steps to fix:

Update the evaluate_simple_js_expression function definition to set context=None as the default value.

Add a check at the beginning of the function to initialize context as an empty dictionary if it is None.

src/utils/eval.py

+    }
+    context["true"] = True
+    context["false"] = False
+    context["null"] = None


To fix the issue, we will replace the mutable default value ({}) for the context parameter with a placeholder value (None). Inside the function, we will check if context is None and initialize it to an empty dictionary ({}) if necessary. This ensures that a new dictionary is created for each function call, avoiding the unintended sharing of state between calls.

Steps to fix:

Update the evaluate_simple_js_expression function definition to set the default value of context to None instead of {}.

Add a check at the beginning of the function to initialize context to an empty dictionary if it is None.

src/app.py

src/service.py

+    #if service not in services.keys():
+    #    raise YunohostValidationError("service_unknown", service=service)


To fix the issue, we need to either remove the commented-out code or reinstate it. Since the _run_service_command function already contains logic for handling services, reinstating the code would require validating its functionality and ensuring it integrates correctly with the existing logic. If the code is unnecessary, it should be removed entirely.

The best approach here is to remove the commented-out code, as there is no indication that it is required or functional. This will clean up the code and eliminate any confusion for future developers.

src/service.py

        # Remove the lock if one was given
        if need_lock and PID != 0:
            _remove_lock(PID)

+    if wait_until_pattern:
+        try:
+            out, err = watch_log_proc.communicate(timeout=timeout)


To fix the issue, we need to remove the unused variable out while ensuring that the communicate() method is still called to preserve any side effects it may have. Since the variable err is also unused, we can simplify the call to communicate() by omitting the unpacking of its return values entirely. This will make the code cleaner and avoid confusion about unused variables.

The changes will be made in the function where the issue occurs, specifically on line 691. No additional imports, methods, or definitions are required.

src/service.py

        # Remove the lock if one was given
        if need_lock and PID != 0:
            _remove_lock(PID)

+    if wait_until_pattern:
+        try:
+            out, err = watch_log_proc.communicate(timeout=timeout)


To fix the issue, we should remove the assignment to the unused variable err while preserving the assignment to out, as out is used later in the code. This can be achieved by modifying the communicate call to only capture the out variable. The communicate method returns a tuple, so we can directly assign the first element to out and ignore the second element (which corresponds to err). Alternatively, we can use a placeholder name like _ for the unused variable to explicitly indicate that it is not used.

src/service.py

@@ -760,9 +846,9 @@
    # such that /etc/yunohost/services.yml contains the minimal
    # changes with respect to the base conf

-    conf_base = yaml.safe_load(open(SERVICES_CONF_BASE)) or {}
+    conf_base: dict[str, ServiceInfos] = yaml.safe_load(open(SERVICES_CONF_BASE)) or {}


To fix the issue, replace the open(SERVICES_CONF_BASE) call with a with statement. This ensures the file is closed automatically when the block is exited, even if an exception occurs. The yaml.safe_load function can be used directly within the with block to read the file's contents.

…, it's too clumsy and the new automerge mechanism + extras conf mechanism should cover most of the usecases

…pattern appears in log, similar to what's already done in app systemd helpers

alexAubin added 🏗️ Major project Big decision work needed labels Mar 19, 2025

Base automatically changed from new-resources to dev March 19, 2025 13:53

alexAubin force-pushed the packaging-v3 branch from 98877be to d522087 Compare March 19, 2025 13:57

alexAubin force-pushed the packaging-v3 branch 4 times, most recently from da7ec5e to d659e58 Compare April 7, 2025 17:41

alexAubin force-pushed the packaging-v3 branch from d659e58 to 24cfbb9 Compare April 14, 2025 18:30

yalh76 mentioned this pull request Apr 23, 2025

Implement git in ynh_setup_source #2085

Open

alexAubin added 13 commits May 29, 2025 14:17

packagingv3: factorize/generalize _run_script from appresource to be …

8457c2f

…able to run app scripts snippets, in particular the future v3 app functions build/init etc

packagingv3: fix the way we run script as non-root in hook_exec_bash

e807ca4

packagingv3: _ynh_apply_default_permissions doesn't work / aint relev…

57ef3b8

…ant as non-root because $app can't give a file to the www-data group ... we'll need to do this between build and init/migrate somehow

packagingv3: tweaks to be able to run composer as non-root

a1f9e0d

packagingv3: Basis for new declarative configuration mechanism

f428d7d

packagingv3: Further implement the packagingv3 install flow

834fd57

packagingv3: add tests for new app configuration / regenconf mechanism

d2a2b3c

packagingv3: add new 'yunohost app regenconf' command

74caa0a

Simplify sym_to_oct / oct_to_sym utils

a2e617a

packagingv3: Move evaluate_simple_js_expression from configpanels to …

c46dba7

…a separate utils file

packagingv3: in configurations, add the 'exposed properties' mechanis…

47e5f3f

…m + if clause handling

alexAubin force-pushed the packaging-v3 branch from 4cee82c to 47e5f3f Compare May 29, 2025 12:17

github-advanced-security bot found potential problems May 29, 2025

View reviewed changes

alexAubin added 2 commits May 29, 2025 14:20

klass -> cls for class methods

9573edf

Generator -> Iterator for clarity

a0737bb

github-advanced-security bot found potential problems Jun 1, 2025

View reviewed changes

src/app.py Dismissed Show dismissed Hide dismissed

alexAubin force-pushed the packaging-v3 branch from e496e3d to fdb0ae9 Compare June 1, 2025 23:14

github-advanced-security bot found potential problems Jun 3, 2025

View reviewed changes

alexAubin added 9 commits June 4, 2025 03:34

packagingv3: implement logic and test for nginx conf management

e8c54d8

packagingv3: misc tweaks in utils + drop the '.d' mechanism for nginx…

7340f02

…, it's too clumsy and the new automerge mechanism + extras conf mechanism should cover most of the usecases

packagingv3: implement logic and test for php conf management

350f5ba

quality: Please the linter gods?

23cf807

services: add mechanism to _run_service_command to 'wait until' some …

09dbc0e

…pattern appears in log, similar to what's already done in app systemd helpers

service.py: add typing to all functions

78abcdf

packagingv3: implement logic and test for systemd conf management

2b3ffa8

Minor method signature change, var name wording

6cc8ea3

packagingv3: implement logic and test for fail2ban conf management

9a8acf8

alexAubin force-pushed the packaging-v3 branch from ce154ea to 9a8acf8 Compare June 4, 2025 01:35

@@ -54,3 +54,3 @@
-                def reload():
+                def reload(self):
                     pass

@@ -168,3 +168,5 @@
-            def evaluate_simple_js_expression(expr: str, context: dict[str, Any] = {}) -> bool:
+            def evaluate_simple_js_expression(expr: str, context: dict[str, Any] = None) -> bool:
+                if context is None:
+                    context = {}
                 if not expr.strip():

@@ -168,3 +168,5 @@
-            def evaluate_simple_js_expression(expr: str, context: dict[str, Any] = {}) -> bool:
+            def evaluate_simple_js_expression(expr: str, context: dict[str, Any] = None) -> bool:
+                if context is None:
+                    context = {}
                 if not expr.strip():

@@ -168,3 +168,5 @@
-            def evaluate_simple_js_expression(expr: str, context: dict[str, Any] = {}) -> bool:
+            def evaluate_simple_js_expression(expr: str, context: dict[str, Any] = None) -> bool:
+                if context is None:
+                    context = {}
                 if not expr.strip():

@@ -631,4 +631,3 @@
                 services = _get_services()
-                # if service not in services.keys():
-                #    raise YunohostValidationError("service_unknown", service=service)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

WIP: Packaging v3 #2070

WIP: Packaging v3 #2070

Uh oh!

alexAubin commented Mar 19, 2025 •

edited

Loading

Uh oh!

Uh oh!

Check notice

Copilot Autofix

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Check failure

Copilot Autofix

Check failure

Copilot Autofix

Check failure

Copilot Autofix

Uh oh!

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check warning

Copilot Autofix

Uh oh!

		#if service not in services.keys():
		# raise YunohostValidationError("service_unknown", service=service)

@@ -690,3 +690,3 @@
                     try:
-                        out, err = watch_log_proc.communicate(timeout=timeout)
+                        out, _ = watch_log_proc.communicate(timeout=timeout)
                     except subprocess.TimeoutExpired:

@@ -857,3 +857,4 @@
-                conf_base: dict[str, ServiceInfos] = yaml.safe_load(open(SERVICES_CONF_BASE)) or {}
+                with open(SERVICES_CONF_BASE, 'r') as file:
+                    conf_base: dict[str, ServiceInfos] = yaml.safe_load(file) or {}

Uh oh!

WIP: Packaging v3 #2070

Are you sure you want to change the base?

WIP: Packaging v3 #2070

Uh oh!

Conversation

alexAubin commented Mar 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TODO / Brain dump

Uh oh!

Uh oh!

Check notice

Copilot Autofix

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Check failure

Uh oh!

Copilot Autofix

Check failure

Uh oh!

Copilot Autofix

Check failure

Uh oh!

Copilot Autofix

Uh oh!

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check warning

Copilot Autofix

Uh oh!

alexAubin commented Mar 19, 2025 •

edited

Loading