TgBot++: android_builder: Make PythonClass singleton

- Make a sperate fd for python script log
- Slience EBADF log
- And other improvements

src/android_builder/ArgumentBuilder.cpp

@@ -28,11 +28,12 @@ PyObject* ArgumentBuilder::build() {
         PyErr_Print();
         return nullptr;
     }
+    DLOG(INFO) << "Building arguments: size=" << argument_count;
     arguments_ref.reserve(argument_count);
     for (Py_ssize_t i = 0; i < argument_count; ++i) {
         PyObject* value = nullptr;
         std::visit(
-            [&value, &arguments_ref](auto&& arg) {
+            [&value, &arguments_ref, i](auto&& arg) {
                 using T = std::decay_t<decltype(arg)>;
                 if constexpr (std::is_same_v<T, long>) {
                     value = PyLong_FromLong(arg);
@@ -41,6 +42,7 @@ PyObject* ArgumentBuilder::build() {
                     value = PyUnicode_FromString(arg.c_str());
                     arguments_ref.emplace_back(value);
                 }
+                DLOG(INFO) << "Arguments[" << i << "]: " << arg;
             },
             arguments[i]);
         if (value == nullptr) {

src/android_builder/ConfigParsers.cpp

@@ -2,7 +2,7 @@
 
 #include <absl/log/log.h>
 
-constexpr bool parserDebug = false;
+constexpr bool parserDebug = true;
 
 // Name RomName LocalManifestUrl LocalManifestBranch device variant
 template <>
@@ -44,14 +44,17 @@ std::vector<BuildConfig> parse(std::ifstream data) {
             LOG(INFO) << "LocalManifestBranch: "
                       << config.local_manifest.branch;
             LOG(INFO) << "Device: " << config.device;
-            LOG(INFO) << "Variant: "
-                      << (config.variant == BuildConfig::Variant::kUser
-                              ? "User"
-                              : (config.variant ==
-                                         BuildConfig::Variant::kUserDebug
-                                     ? "UserDebug"
-                                     : "Eng"));
-            LOG(INFO) << "---------------------------";
+            switch (config.variant) {
+                case BuildConfig::Variant::kUser:
+                    LOG(INFO) << "Variant: User";
+                    break;
+                case BuildConfig::Variant::kUserDebug:
+                    LOG(INFO) << "Variant: UserDebug";
+                    break;
+                case BuildConfig::Variant::kEng:
+                    LOG(INFO) << "Variant: Eng";
+                    break;
+            }
             LOG(INFO) << "---------------------------";
         }
         configs.emplace_back(config);
@@ -72,6 +75,13 @@ std::vector<RomConfig> parse(std::ifstream data) {
         splitWithSpaces(line, items);
         if (items.size() != RomConfig::elem_size) {
             LOG(WARNING) << "Skipping invalid line: " << line;
+            if (parserDebug) {
+                LOG(INFO) << "Invalid rom config size: " << items.size();
+                int count = 0;
+                for (const auto& item : items) {
+                    LOG(INFO) << '[' << count << "]=" << item;
+                }
+            }
             continue;
         }
         rom.name = items[RomConfig::indexOf_name];

src/android_builder/ForkAndRun.cpp

@@ -1,34 +1,54 @@
 #include "ForkAndRun.hpp"
 
+#include <Python.h>
 #include <absl/log/log.h>
+#include <absl/log/log_sink_registry.h>
 #include <dlfcn.h>
 #include <internal/_FileDescriptor_posix.h>
 #include <sys/wait.h>
 
 #include <csignal>
 #include <cstdlib>
-#include <cstring>
+#include <string>
 #include <thread>
 
 bool ForkAndRun::execute() {
     Pipe stdout_pipe{};
     Pipe stderr_pipe{};
+    Pipe python_pipe{};
 
-    if (!stderr_pipe.pipe() || !stdout_pipe.pipe()) {
+    if (!stderr_pipe.pipe() || !stdout_pipe.pipe() || !python_pipe.pipe()) {
+        stderr_pipe.close();
+        stdout_pipe.close();
+        python_pipe.close();
         PLOG(ERROR) << "Failed to create pipes";
         return false;
     }
     pid_t pid = fork();
     if (pid == 0) {
+        FDLogSink sink;
+
+        absl::AddLogSink(&sink);
         dup2(stdout_pipe.writeEnd(), STDOUT_FILENO);
         dup2(stderr_pipe.writeEnd(), STDERR_FILENO);
         close(stdout_pipe.readEnd());
         close(stderr_pipe.readEnd());
-        if (runFunction()) {
-            _exit(EXIT_SUCCESS);
-        } else {
-            _exit(EXIT_FAILURE);
-        }
+        close(python_pipe.readEnd());
+
+        PyObject* os = PyImport_ImportModule("os");
+        PyObject* os_environ = PyObject_GetAttrString(os, "environ");
+        PyObject* value = PyUnicode_FromString(
+            std::to_string(python_pipe.writeEnd()).c_str());
+        PyMapping_SetItemString(os_environ, "PYTHON_LOG_FD", value);
+
+        // Clean up
+        Py_DECREF(value);
+        Py_DECREF(os_environ);
+        Py_DECREF(os);
+
+        int ret = runFunction() ? EXIT_SUCCESS : EXIT_FAILURE;
+        absl::RemoveLogSink(&sink);
+        _exit(ret);
     } else if (pid > 0) {
         Pipe program_termination_pipe{};
         bool breakIt = false;
@@ -37,6 +57,7 @@ bool ForkAndRun::execute() {
         childProcessId = pid;
         close(stdout_pipe.writeEnd());
         close(stderr_pipe.writeEnd());
+        close(python_pipe.writeEnd());
         program_termination_pipe.pipe();
 
         selector.add(
@@ -63,6 +84,18 @@ bool ForkAndRun::execute() {
                 }
             },
             Selector::Mode::READ);
+        selector.add(
+            python_pipe.readEnd(),
+            [&python_pipe] {
+                BufferType buf{};
+                ssize_t bytes_read =
+                    read(python_pipe.readEnd(), buf.data(), buf.size() - 1);
+                if (bytes_read >= 0) {
+                    printf("Python output: ");
+                    fputs(buf.data(), stdout);
+                }
+            },
+            Selector::Mode::READ);
         selector.add(
             program_termination_pipe.readEnd(),
             [program_termination_pipe, &breakIt]() { breakIt = true; },
@@ -93,6 +126,9 @@ bool ForkAndRun::execute() {
         pollThread.join();
 
         // Cleanup
+        selector.remove(stdout_pipe.readEnd());
+        selector.remove(stderr_pipe.readEnd());
+        selector.remove(program_termination_pipe.readEnd());
         program_termination_pipe.close();
         stderr_pipe.close();
         stdout_pipe.close();

src/android_builder/ForkAndRun.hpp

@@ -1,11 +1,39 @@
 #pragma once
 
 // Helper class to fork and run a subprocess with stdout/err
+#include <unistd.h>
+
 #include <array>
+#include <boost/algorithm/string/trim.hpp>
 #include <iostream>
+#include <string_view>
 
+#include "absl/log/log_entry.h"
+#include "absl/log/log_sink.h"
 #include "socket/selector/SelectorPosix.hpp"
 
+struct FDLogSink : public absl::LogSink {
+    void Send(const absl::LogEntry& logSink) override {
+        const auto message = logSink.text_message_with_prefix_and_newline();
+        constexpr std::string_view prefix = "SubProcess: ";
+        if (isWritable && logSink.log_severity() <= absl::LogSeverity::kWarning) {
+            write(stdout_fd, prefix.data(), prefix.size());
+            write(stdout_fd, message.data(), message.size());
+        }
+    }
+    explicit FDLogSink() : stdout_fd(::dup(STDOUT_FILENO)) {
+        if (stdout_fd < 0) {
+            PLOG(ERROR) << "Failed to duplicate stdout";
+            isWritable = false;
+        }
+    }
+    ~FDLogSink() override { ::close(stdout_fd); }
+
+   private:
+    int stdout_fd;
+    bool isWritable = true;
+};
+
 class ForkAndRun {
    public:
     virtual ~ForkAndRun() = default;
@@ -24,9 +52,7 @@ class ForkAndRun {
      *
      * @param buffer The buffer containing the new stdout data.
      */
-    virtual void onNewStdoutBuffer(BufferType& buffer) {
-        std::cout << buffer.data() << std::endl;
-    }
+    virtual void onNewStdoutBuffer(BufferType& buffer) {}
 
     /**
      * @brief Callback function for handling new stderr data.
@@ -38,7 +64,8 @@ class ForkAndRun {
      * @param buffer The buffer containing the new stderr data.
      */
     virtual void onNewStderrBuffer(BufferType& buffer) {
-        std::cerr << buffer.data() << std::endl;
+        std::string lines = buffer.data();
+        std::cerr << boost::trim_copy(lines) << std::endl;
     }
 
     /**
@@ -57,7 +84,9 @@ class ForkAndRun {
      *
      * @param signal The signal that caused the subprocess to exit.
      */
-    virtual void onSignal(int signal) {}
+    virtual void onSignal(int signal) {
+        onExit(signal);
+    }
 
     /**
      * @brief The function to be run in the subprocess.

src/android_builder/PythonClass.cpp

@@ -2,23 +2,35 @@
 
 #include <absl/log/log.h>
 
+#include <limits>
 #include <memory>
-#include <optional>
 
 namespace details {
 
 template <>
 bool convert<bool>(PyObject* value) {
+    if (!PyBool_Check(value)) {
+        LOG(ERROR) << "Invalid boolean value: expected boolean object";
+        return std::numeric_limits<bool>::min();
+    }
     return PyObject_IsTrue(value) == 1;
 }
 
 template <>
 long convert<long>(PyObject* value) {
+    if (!PyLong_Check(value)) {
+        LOG(ERROR) << "Invalid integer value: expected integer object";
+        return std::numeric_limits<long>::min();
+    }
     return PyLong_AsLong(value);
 }
 
 template <>
 double convert<double>(PyObject* value) {
+    if (!PyFloat_Check(value)) {
+        LOG(ERROR) << "Invalid float value: expected float object";
+        return std::numeric_limits<double>::quiet_NaN();
+    }
     return PyFloat_AsDouble(value);
 }
 

src/android_builder/PythonClass.hpp

@@ -36,9 +36,11 @@ class PythonClass : public std::enable_shared_from_this<PythonClass> {
     explicit PythonClass() = default;
 
    public:
+    using Ptr = std::shared_ptr<PythonClass>;
     static std::shared_ptr<PythonClass> get() {
         static PyInitHolder holder;
-        return std::make_shared<PythonClass>(PythonClass());
+        static auto instance = std::make_shared<PythonClass>(PythonClass());
+        return instance;
     }
 
     class ModuleHandle;
@@ -56,6 +58,7 @@ class PythonClass : public std::enable_shared_from_this<PythonClass> {
 
        public:
         friend class ModuleHandle;
+        using Ptr = std::shared_ptr<FunctionHandle>;
         std::string name;  // Logging purposes
         ~FunctionHandle() { Py_XDECREF(function); }
         FunctionHandle(FunctionHandle&& other) noexcept
@@ -65,7 +68,29 @@ class PythonClass : public std::enable_shared_from_this<PythonClass> {
             other.function = nullptr;
         }
 
-        // Manage the arg parameter lifetime yourself
+        /**
+         * Calls the specified Python function with the given arguments.
+         *
+         * @param args The arguments to be passed to the Python function.
+         * @param out A pointer to the variable where the result will be stored.
+         *            If the function returns a value, it will be converted to
+         * the specified type and stored in out. If the function does not return
+         * a value, out can be nullptr.
+         *
+         * @return true If the function call is successful and no error occurs.
+         *         false If an error occurs during the function call.
+         *
+         * @note The caller is responsible for managing the lifetime of the args
+         * parameter.
+         *
+         * @note If an error occurs during the function call, the error message
+         * will be printed to the standard error stream. The caller can retrieve
+         * the error message using PyErr_Print() or PyErr_Fetch().
+         *
+         * @see https://docs.python.org/3/c-api/object.html#c.Py_DECREF
+         * @see https://docs.python.org/3/c-api/exceptions.html#c.PyErr_Print
+         * @see https://docs.python.org/3/c-api/exceptions.html#c.PyErr_Fetch
+         */
         template <typename T>
         bool call(PyObject* args, T* out) {
             LOG(INFO) << "Calling Python function: module "
@@ -80,6 +105,8 @@ class PythonClass : public std::enable_shared_from_this<PythonClass> {
                 LOG(ERROR) << "Failed to call function";
                 return false;
             }
+            // If the function returned a value, convert it to the specified
+            // type and store it in out.
             if constexpr (!std::is_void_v<T>) {
                 T resultC = details::convert<T>(result);
                 if (PyErr_Occurred()) {
@@ -88,6 +115,7 @@ class PythonClass : public std::enable_shared_from_this<PythonClass> {
                     *out = resultC;
                 }
             }
+            DLOG(INFO) << "Call succeeded";
             // Decrement the reference count of the result
             Py_DECREF(result);
             return true;

src/android_builder/scripts/build_rom_utils.py

@@ -1,5 +1,8 @@
 import subprocess_utils
 import os
+import print
+
+print = print.print_fd
 
 def find_vendor_str() -> str:
     vendor_path = 'vendor/'

src/android_builder/scripts/print.py

@@ -0,0 +1,23 @@
+import os
+
+def print_fd(*args, **kwargs):
+    """
+    Prints the provided arguments to the file descriptor specified by the environment variable 'PYTHON_LOG_FD'.
+
+    Parameters:
+    *args: Variable length argument list. The arguments to be printed.
+    **kwargs: Arbitrary keyword arguments. The arguments to be passed to the print function.
+
+    Returns:
+    None
+
+    Note:
+    The file descriptor should be opened in write mode ('w') before using this function.
+    The file descriptor will be closed after printing the arguments.
+    """
+    print(*args, **kwargs)
+    if False: # Tmp disabled
+        logfd = int(os.environ['PYTHON_LOG_FD'])
+        out = open(logfd, 'w')
+        print(*args, file=out, **kwargs)
+        out.close()

src/android_builder/scripts/subprocess_utils.py

@@ -1,9 +1,12 @@
 import subprocess
 import sys
+import print
 
 testing = False
 testingRet = True
 
+print = print.print_fd
+
 def run_command(command: str) -> bool:
     print("Running command: %s" % command)
     sys.stdout.flush()