Re-implement packaging for GitHub workflows (Linux ARM) (#7369)

* Debian 12 ARM64 target

* Fixed comments

* Don't check for server name when exiting because of args

* Exit with success when using `--version`, `--help`, etc

* Test Linux installation

* Add Windows ARM builder

* Extract magic string

* Use proper arch name

* Swap arch order

* Fixed test

* Bootstrap Windows ARM runner

* Install winget

* Conditionally install winget

* Winget take 2

* Use shell: pwsh

* Try `powershell`

* Try cmd to install winget

* Only build distro once

* Use enum for clarity

* Remove shell

* Back out Windows ARM testing

* Attempt arch matrix

* Add Fedora arm64

* Label all -amd64

* Special case for zypper

* Fixed bad var name on Windows

* Add missing /bin ignore

* Disable GPG check on OpenSUSE

* Fixed typo

* Update ChangeLog

.github/workflows/ci.yml

@@ -35,6 +35,7 @@ jobs:
   windows:
     name: ${{ matrix.target.name }}
     runs-on: ${{ matrix.target.runs-on }}
+    container: ${{ matrix.target.container }}
     timeout-minutes: 20
 
     env:
@@ -43,9 +44,8 @@ jobs:
     strategy:
       matrix:
         target:
-          - name: windows-2022
+          - name: windows-2022-x64
             runs-on: windows-2022
-            arch: x64
 
     steps:
       - name: Checkout
@@ -97,7 +97,7 @@ jobs:
         with:
           use_github: ${{ env.UPLOAD_TO_GITHUB }}
           use_gdrive: ${{ env.UPLOAD_TO_GDRIVE }}
-          github-target-filename: "${{ env.SYNERGY_PACKAGE_PREFIX }}-windows-${{ matrix.target.name }}"
+          github-target-filename: "${{ env.SYNERGY_PACKAGE_PREFIX }}-${{ matrix.target.name }}"
           gdrive-target-base-dir: ${{ vars.GDRIVE_TARGET_BASE_DIR }}
           gdrive-secret-key: ${{ secrets.GOOGLE_DRIVE_KEY }}
           gdrive-parent-folder-id: ${{ secrets.GOOGLE_DRIVE_TECH_DRIVE }}
@@ -175,8 +175,8 @@ jobs:
   linux:
     name: linux-${{ matrix.distro.name }}
     runs-on: ${{ matrix.distro.runs-on }}
-    timeout-minutes: 10
     container: ${{ matrix.distro.container }}
+    timeout-minutes: 10
 
     env:
       # Prevent apt prompting for input.
@@ -185,50 +185,60 @@ jobs:
     strategy:
       matrix:
         distro:
-          - name: ubuntu-24.04
+          - name: ubuntu-24.04-amd64
             container: ubuntu:24.04
             runs-on: ubuntu-latest
             install-deps-apt: true
             extra-packages: true
 
-          - name: ubuntu-22.04
+          - name: ubuntu-22.04-amd64
             container: ubuntu:22.04
             runs-on: ubuntu-latest
             install-deps-apt: true
 
-          - name: debian-12
+          - name: debian-12-arm64
+            container: arm64v8/debian:12
+            runs-on: buildjet-4vcpu-ubuntu-2204-arm
+            install-deps-apt: true
+
+          - name: debian-12-amd64
             container: debian:12
             runs-on: ubuntu-latest
             install-deps-apt: true
 
-          - name: debian-11
+          - name: debian-11-amd64
             container: debian:11
             runs-on: ubuntu-latest
             install-deps-apt: true
             legacy-cmake: true
 
-          - name: fedora-40
+          - name: fedora-40-arm64
+            container: arm64v8/fedora:40
+            runs-on: buildjet-4vcpu-ubuntu-2204-arm
+            install-deps-dnf: true
+
+          - name: fedora-40-amd64
             container: fedora:40
             runs-on: ubuntu-latest
             install-deps-dnf: true
 
-          - name: fedora-39
+          - name: fedora-39-amd64
             container: fedora:39
             runs-on: ubuntu-latest
             install-deps-dnf: true
 
-          - name: opensuse
+          - name: opensuse-amd64
             container: opensuse/tumbleweed:latest
             runs-on: ubuntu-latest
             install-deps-zypper: true
 
-          - name: arch
+          - name: arch-amd64
             container: archlinux:latest
             runs-on: ubuntu-latest
             install-deps-pacman: true
             package-user: build
 
-          - name: manjaro
+          - name: manjaro-amd64
             container: manjarolinux/base:latest
             runs-on: ubuntu-latest
             install-deps-pacman: true

.gitignore

@@ -1,5 +1,6 @@
 # temp dirs created during build
 /build
+/bin
 /dist
 /deps
 /tmp

ChangeLog

@@ -40,6 +40,7 @@ Enhancements:
 - #7363 Schedule CI daily at 5am to detect code rot early
 - #7364 Format all source with Clang and introduce lint workflow
 - #7368 Make version check URL v1-specific and configurable
+- #7369 Re-implement packaging for GitHub workflows (Linux ARM)
 
 # 1.14.6
 

scripts/lib/linux.py

@@ -1,35 +1,48 @@
 import os, shutil, glob
 import lib.cmd_utils as cmd_utils
 import lib.env as env
+from enum import Enum, auto
+
+
+class PackageType(Enum):
+    DISTRO = auto()
+    TGZ = auto()
+    STGZ = auto()
+
 
 dist_dir = "dist"
 build_dir = "build"
+package_name = "synergy"
+test_cmd = "synergys --version"
 
 
-def package(filename_base, build_distro=True, build_tgz=False, build_stgz=False):
+def package(filename_base, package_type: PackageType):
 
-    extension, cmd = get_package_info(build_distro, build_tgz, build_stgz)
+    extension, cmd = get_package_info(package_type)
     run_package_cmd(cmd)
     package_filename = get_package_filename(extension)
     target_file = f"{filename_base}.{extension}"
-    copy_to_dist_dir(package_filename, target_file)
+    target_path = copy_to_dist_dir(package_filename, target_file)
+
+    if package_type == PackageType.DISTRO:
+        test_install(target_path)
 
 
-def get_package_info(build_distro, build_tgz, build_stgz):
+def get_package_info(package_type: PackageType):
 
     command = None
     cpack_generator = None
     file_extension = None
 
-    if build_tgz:
+    if package_type == PackageType.TGZ:
         cpack_generator = "TGZ"
         file_extension = "tar.gz"
 
-    elif build_stgz:
+    elif package_type == PackageType.STGZ:
         cpack_generator = "STGZ"
         file_extension = "sh"
 
-    elif build_distro:
+    elif package_type == PackageType.DISTRO:
 
         distro, distro_like, _distro_version = env.get_linux_distro()
         if not distro_like:
@@ -45,7 +58,7 @@ def get_package_info(build_distro, build_tgz, build_stgz):
             command = ["makepkg", "-s"]
             file_extension = "pkg.tar.zst"
         else:
-            raise RuntimeError(f"Linux distro not yet supported: {distro_like}")
+            raise RuntimeError(f"Linux distro not yet supported: {distro}")
 
     if not cpack_generator and not command:
         raise RuntimeError("No package generator or command found")
@@ -59,7 +72,9 @@ def get_package_info(build_distro, build_tgz, build_stgz):
 def run_package_cmd(command):
     package_user = env.get_env("LINUX_PACKAGE_USER", required=False)
     if package_user:
-        cmd_utils.run(["sudo", "chown", "-R", package_user, "build"], check=True)
+        cmd_utils.run(
+            ["sudo", "chown", "-R", package_user, "build"], check=True, print_cmd=True
+        )
         command = ["sudo", "-u", package_user] + command
 
     cwd = os.getcwd()
@@ -85,3 +100,47 @@ def copy_to_dist_dir(source_file, target_file):
     target_path = f"{dist_dir}/{target_file}"
     print(f"Copying to: {target_path}")
     shutil.copy(source_file, target_path)
+
+    return target_path
+
+
+def test_install(package_file):
+
+    distro, distro_like, _distro_version = env.get_linux_distro()
+    if not distro_like:
+        distro_like = distro
+
+    install_pre = None
+    remove_pre = None
+    if "debian" in distro_like:
+        install_pre = ["apt", "install", "-f", "-y"]
+        remove_pre = ["apt", "remove", "-y"]
+    elif "fedora" in distro_like:
+        install_pre = ["dnf", "install", "-y"]
+        remove_pre = ["dnf", "remove", "-y"]
+    elif "opensuse" in distro_like:
+        install_pre = ["zypper", "--no-gpg-checks", "install", "-y"]
+        remove_pre = ["zypper", "remove", "-y"]
+    elif "arch" in distro_like:
+        install_pre = ["pacman", "-U", "--noconfirm"]
+        remove_pre = ["pacman", "-R", "--noconfirm"]
+    else:
+        raise RuntimeError(f"Linux distro not yet supported: {distro}")
+
+    has_sudo = cmd_utils.has_command("sudo")
+    sudo = ["sudo"] if has_sudo else []
+
+    print("Testing installation...")
+    cmd_utils.run(
+        sudo + install_pre + [f"./{package_file}"],
+        check=True,
+        print_cmd=True,
+    )
+
+    try:
+        cmd_utils.run(test_cmd, shell=True, check=True, print_cmd=True)
+    except Exception:
+        raise RuntimeError("Unable to verify version")
+    finally:
+        cmd_utils.run(sudo + remove_pre + [package_name], check=True, print_cmd=True)
+        print("Installation test passed")

scripts/package.py

@@ -2,6 +2,7 @@
 
 import platform
 import lib.env as env
+from lib.linux import PackageType
 
 env_file = ".env"
 default_package_prefix = "synergy"
@@ -47,6 +48,11 @@ def get_filename_base(version, use_linux_distro=True):
 
         return f"{package_base}-{distro}-{machine}-{version}"
     else:
+        # some windows users get confused by 'amd64' and think it's 'arm64',
+        # so we'll use intel's 'x64' branding (even though it's wrong).
+        if machine == "amd64":
+            machine = "x64"
+
         return f"{package_base}-{os}-{machine}-{version}"
 
 
@@ -67,12 +73,12 @@ def linux_package(filename_base, version):
 
     extra_packages = env.get_env_bool("LINUX_EXTRA_PACKAGES", False)
 
-    linux.package(filename_base)
+    linux.package(filename_base, PackageType.DISTRO)
 
     if extra_packages:
         filename_base = get_filename_base(version, use_linux_distro=False)
-        linux.package(filename_base, build_tgz=True)
+        linux.package(filename_base, PackageType.TGZ)
-        linux.package(filename_base, build_stgz=True)
+        linux.package(filename_base, PackageType.STGZ)
 
 
 if __name__ == "__main__":

src/lib/synergy/ArgParser.cpp

@@ -119,7 +119,7 @@ bool ArgParser::parseClientArgs(lib::synergy::ClientArgs &args, int argc,
   }
 
   // exactly one non-option argument (server-address)
-  if (i == argc) {
+  if (i == argc && !args.m_shouldExitFail && !args.m_shouldExitOk) {
     LOG((CLOG_CRIT "%s: a server address or name is required" BYE, args.m_pname,
          args.m_pname));
     return false;
@@ -138,7 +138,7 @@ bool ArgParser::parsePlatformArg(lib::synergy::ArgsBase &argsBase,
 #if WINAPI_MSWINDOWS
   if (isArg(i, argc, argv, nullptr, "--service")) {
     LOG((CLOG_WARN "obsolete argument --service, use synergyd instead."));
-    argsBase.m_shouldExit = true;
+    argsBase.m_shouldExitFail = true;
   } else if (isArg(i, argc, argv, nullptr, "--exit-pause")) {
     argsBase.m_pauseOnExit = true;
   } else if (isArg(i, argc, argv, nullptr, "--stop-on-desk-switch")) {
@@ -220,12 +220,12 @@ bool ArgParser::parseGenericArgs(int argc, const char *const *argv, int &i) {
     if (m_app) {
       m_app->help();
     }
-    argsBase().m_shouldExit = true;
+    argsBase().m_shouldExitOk = true;
   } else if (isArg(i, argc, argv, nullptr, "--version")) {
     if (m_app) {
       m_app->version();
     }
-    argsBase().m_shouldExit = true;
+    argsBase().m_shouldExitOk = true;
   } else if (isArg(i, argc, argv, nullptr, "--no-tray")) {
     argsBase().m_disableTray = true;
   } else if (isArg(i, argc, argv, nullptr, "--ipc")) {
@@ -298,7 +298,7 @@ bool ArgParser::isArg(int argi, int argc, const char *const *argv,
     if (argi + minRequiredParameters >= argc) {
       LOG((CLOG_PRINT "%s: missing arguments for `%s'" BYE, argsBase().m_pname,
            argv[argi], argsBase().m_pname));
-      argsBase().m_shouldExit = true;
+      argsBase().m_shouldExitFail = true;
       return false;
     }
     return true;

src/lib/synergy/ArgsBase.h

@@ -36,35 +36,65 @@ public:
   /// @brief This sets the type of the derived class
   enum Type { kBase, kServer, kClient };
 
-  Type m_classType = kBase; /// @brief Stores what type of object this is
+  /// @brief Stores what type of object this is
+  Type m_classType = kBase;
 
-  bool m_daemon = true;          /// @brief Should run as a daemon
+  /// @brief Should run as a daemon
-  bool m_restartable = true;     /// @brief Should the app restart automatically
+  bool m_daemon = true;
-  bool m_noHooks = false;        /// @brief Should the app use hooks
-  const char *m_pname = nullptr; /// @brief The filename of the running process
-  const char *m_logFilter =
-      nullptr; /// @brief The logging level of the application
-  const char *m_logFile = nullptr; /// @brief The full path to the logfile
-  const char *m_display =
-      nullptr;                /// @brief Contains the X-Server display to use
-  String m_name;              /// @brief The name of the current computer
-  bool m_disableTray = false; /// @brief Should the app add a tray icon
-  bool m_enableIpc =
-      false; /// @brief Tell the client to talk through IPC to the daemon
-  bool m_enableDragDrop = false; /// @brief Should drag drop support be enabled
 
-  bool m_shouldExit =
+  /// @brief Should the app restart automatically
-      false; /// @brief Will cause the application to exit when set to true
+  bool m_restartable = true;
-  String m_synergyAddress; /// @brief Bind to this address
+
-  bool m_enableCrypto =
+  /// @brief Should the app use hooks
-      false; /// @brief Should the connections be TLS encrypted
+  bool m_noHooks = false;
-  String
+
-      m_profileDirectory; /// @brief The profile DIR to use for the application
+  /// @brief The filename of the running process
-  String m_pluginDirectory; /// @brief //TODO Plugins? Get set in ARCH but
+  const char *m_pname = nullptr;
-                            /// doesn't seem to get used
+
-  String m_tlsCertFile; /// @brief Contains the location of the TLS certificate
+  /// @brief The logging level of the application
-                        /// file
+  const char *m_logFilter = nullptr;
-  bool m_preventSleep = false; /// @brief Stop this computer from sleeping
+
+  /// @brief The full path to the logfile
+  const char *m_logFile = nullptr;
+
+  /// @brief Contains the X-Server display to use
+  const char *m_display = nullptr;
+
+  /// @brief The name of the current computer
+  String m_name;
+
+  /// @brief Should the app add a tray icon
+  bool m_disableTray = false;
+
+  /// @brief Tell the client to talk through IPC to the daemon
+  bool m_enableIpc = false;
+
+  /// @brief Should drag drop support be enabled
+  bool m_enableDragDrop = false;
+
+  /// @brief Will cause the application to exit with OK code when set to true
+  bool m_shouldExitOk = false;
+
+  /// @brief Will cause the application to exit with fail code when set to true
+  bool m_shouldExitFail = false;
+
+  /// @brief Bind to this address
+  String m_synergyAddress;
+
+  /// @brief Should the connections be TLS encrypted
+  bool m_enableCrypto = false;
+
+  /// @brief The dir to load settings from
+  String m_profileDirectory;
+
+  /// @brief The dir to load plugins from
+  String m_pluginDirectory;
+
+  /// @brief Contains the location of the TLS certificate file
+  String m_tlsCertFile;
+
+  /// @brief Stop this computer from sleeping
+  bool m_preventSleep = false;
 
 #if SYSAPI_WIN32
   bool m_debugServiceWait = false;

src/lib/synergy/ClientApp.cpp

@@ -75,8 +75,12 @@ void ClientApp::parseArgs(int argc, const char *const *argv) {
   ArgParser argParser(this);
   bool result = argParser.parseClientArgs(args(), argc, argv);
 
-  if (!result || args().m_shouldExit) {
+  if (!result || args().m_shouldExitOk || args().m_shouldExitFail) {
-    m_bye(kExitArgs);
+    if (args().m_shouldExitOk) {
+      m_bye(kExitSuccess);
+    } else {
+      m_bye(kExitArgs);
+    }
   } else {
     // save server address
     if (!args().m_serverAddress.empty()) {

src/lib/synergy/ServerApp.cpp

@@ -79,8 +79,12 @@ void ServerApp::parseArgs(int argc, const char *const *argv) {
   ArgParser argParser(this);
   bool result = argParser.parseServerArgs(args(), argc, argv);
 
-  if (!result || args().m_shouldExit) {
+  if (!result || args().m_shouldExitOk || args().m_shouldExitFail) {
-    m_bye(kExitArgs);
+    if (args().m_shouldExitOk) {
+      m_bye(kExitSuccess);
+    } else {
+      m_bye(kExitArgs);
+    }
   } else {
     if (!args().m_synergyAddress.empty()) {
       try {

src/test/unittests/synergy/ArgParserTests.cpp

@@ -55,7 +55,7 @@ TEST(ArgParserTests, isArg_missingArgs_returnFalse) {
   bool result = ArgParser::isArg(i, argc, argv, "-t", NULL, 1);
 
   EXPECT_FALSE(result);
-  EXPECT_EQ(true, argsBase.m_shouldExit);
+  EXPECT_EQ(true, argsBase.m_shouldExitFail);
 }
 
 TEST(ArgParserTests, searchDoubleQuotes_doubleQuotedArg_returnTrue) {
@@ -235,7 +235,7 @@ TEST(ArgParserTests, parseServerArgs_parses_each_category) {
                         "--res-w", "888"};
   EXPECT_TRUE(
       parser.parseServerArgs(args, sizeof(argv) / sizeof(argv[0]), argv));
-  EXPECT_EQ(args.m_shouldExit, true);
+  EXPECT_EQ(args.m_shouldExitOk, true);
 }
 
 TEST(ArgParserTests, parseClientArgs_parses_single_help) {
@@ -257,5 +257,5 @@ TEST(ArgParserTests, parseClientArgs_parses_single_help) {
                         "127.0.0.1"};
   EXPECT_TRUE(
       parser.parseClientArgs(args, sizeof(argv) / sizeof(argv[0]), argv));
-  EXPECT_EQ(args.m_shouldExit, true);
+  EXPECT_EQ(args.m_shouldExitOk, true);
 }