From 78ab014a0d80e22e6fd7f190f4f2f78097a60c59 Mon Sep 17 00:00:00 2001 From: aviralgarg05 Date: Wed, 12 Aug 2026 00:36:27 +0530 Subject: [PATCH 1/2] system/nxpkg: Keep the installed database consistent with what is on disk. Install wrote the pointer files and removed payloads around the database save rather than after it, so a failure in between could leave the database naming a version whose files were already gone. Reinstalling a version that was already present treated its directory as newly created, so a failed reinstall deleted a working install. Uninstall and rollback took only the database lock, leaving another install free to work on the same package. Commit the database first and only then refresh the pointers or delete files, so a crash can strand files that are reclaimable but never lose a payload the database still points at. Take the per-package lock in uninstall and rollback as well, and pick up state left by an install that was interrupted earlier. Assisted-by: OpenAI Codex:gpt-5.6-sol Signed-off-by: aviralgarg05 --- system/nxpkg/pkg_install.c | 765 ++++++++++++++++++++++++++++++++----- 1 file changed, 677 insertions(+), 88 deletions(-) diff --git a/system/nxpkg/pkg_install.c b/system/nxpkg/pkg_install.c index c4098200191..7b326cfe836 100644 --- a/system/nxpkg/pkg_install.c +++ b/system/nxpkg/pkg_install.c @@ -25,10 +25,11 @@ ****************************************************************************/ #include -#include #include +#include #include #include +#include #include #include "pkg.h" @@ -37,58 +38,76 @@ * Private Functions ****************************************************************************/ -static int pkg_install_resolve_artifact(FAR char *buffer, size_t size, - FAR const struct pkg_manifest_s - *manifest) +static int pkg_install_acquire_lock(FAR const char *name, FAR char *path, + size_t size) { int ret; - if (manifest->artifact[0] == '/') + ret = pkg_store_ensure_package_root(name); + if (ret < 0) { - ret = snprintf(buffer, size, "%s", manifest->artifact); - if (ret < 0) - { - return ret; - } - - return (size_t)ret >= size ? -ENAMETOOLONG : 0; + return ret; } - ret = snprintf(buffer, size, "%s/%s", PKG_REPO_DIR, manifest->artifact); + ret = pkg_store_format_lock_path(path, size, name); if (ret < 0) { return ret; } - return (size_t)ret >= size ? -ENAMETOOLONG : 0; + ret = pkg_lock_create(path); + if (ret == -EEXIST) + { + pkg_reclaim_stale_lock(path); + ret = pkg_lock_create(path); + } + + return ret == -EEXIST ? -EBUSY : ret; } -static int pkg_install_acquire_lock(FAR const char *name, FAR char *path, - size_t size) +/**************************************************************************** + * Name: pkg_install_acquire_installed_lock + * + * Description: + * Acquire the global installed-database lock. This serializes the + * read-modify-write sequence used by install, uninstall, and rollback. + * + ****************************************************************************/ + +static int pkg_install_acquire_installed_lock(FAR char *path, size_t size) { - int fd; int ret; + int tries; - ret = pkg_store_ensure_package_root(name); + ret = snprintf(path, size, PKG_ROOT_DIR "/instpkg.lk"); if (ret < 0) { return ret; } - ret = pkg_store_format_lock_path(path, size, name); - if (ret < 0) + if ((size_t)ret >= size) { - return ret; + return -ENAMETOOLONG; } - fd = open(path, O_WRONLY | O_CREAT | O_EXCL, 0644); - if (fd < 0) + for (tries = 0; tries < 100; tries++) { - return errno == EEXIST ? -EBUSY : -errno; + ret = pkg_lock_create(path); + if (ret == 0) + { + return 0; + } + + if (ret != -EEXIST) + { + return ret; + } + + pkg_reclaim_stale_lock(path); + usleep(20 * 1000); } - close(fd); - return 0; + return -EBUSY; } static bool pkg_install_has_version( @@ -108,8 +127,63 @@ static bool pkg_install_has_version( return false; } +static int pkg_install_prune_oldest_version( + FAR struct pkg_installed_entry_s *entry, + FAR char *pruned_version, size_t pruned_version_size) +{ + size_t victim = entry->version_count; + size_t i; + + /* Versions are appended in install order, so the lowest index that + * isn't the active ("current") or rollback ("previous") version is the + * oldest one safe to drop. Without this, a package updated more than + * PKG_INSTALLED_VERSIONS_MAX times becomes permanently un-installable + * (pkg_install_add_version would just fail forever). + */ + + for (i = 0; i < entry->version_count; i++) + { + if (strcmp(entry->versions[i], entry->current) != 0 && + strcmp(entry->versions[i], entry->previous) != 0) + { + victim = i; + break; + } + } + + if (victim == entry->version_count) + { + return -E2BIG; + } + + /* Deleting the pruned version's on-disk directory here, before this + * in-memory db update is even durably saved, left a real inconsistency + * window: if pkg_metadata_save_installed() subsequently failed (full + * SD card, I/O error), the payload was already gone but the last + * successfully-saved instpkg.jsn could still list that version as + * installed. Hand the victim's version string back to the caller + * instead, so it can defer the actual directory removal until after + * the save succeeds - mirroring how this file already treats the + * installed db as authoritative everywhere else. + */ + + snprintf(pruned_version, pruned_version_size, "%s", + entry->versions[victim]); + + for (i = victim; i + 1 < entry->version_count; i++) + { + memcpy(entry->versions[i], entry->versions[i + 1], + sizeof(entry->versions[i])); + } + + entry->version_count--; + return 0; +} + static int pkg_install_add_version(FAR struct pkg_installed_entry_s *entry, - FAR const char *version) + FAR const char *version, + FAR char *pruned_version, + size_t pruned_version_size) { int ret; @@ -120,7 +194,12 @@ static int pkg_install_add_version(FAR struct pkg_installed_entry_s *entry, if (entry->version_count >= PKG_INSTALLED_VERSIONS_MAX) { - return -E2BIG; + ret = pkg_install_prune_oldest_version(entry, pruned_version, + pruned_version_size); + if (ret < 0) + { + return ret; + } } ret = snprintf(entry->versions[entry->version_count], @@ -142,7 +221,9 @@ static int pkg_install_add_version(FAR struct pkg_installed_entry_s *entry, static int pkg_install_update_installed(FAR struct pkg_installed_db_s *db, FAR const struct pkg_manifest_s - *manifest) + *manifest, + FAR char *pruned_version, + size_t pruned_version_size) { FAR struct pkg_installed_entry_s *entry; int ret; @@ -196,12 +277,13 @@ static int pkg_install_update_installed(FAR struct pkg_installed_db_s *db, } entry->type = manifest->type; - return pkg_install_add_version(entry, manifest->version); + return pkg_install_add_version(entry, manifest->version, pruned_version, + pruned_version_size); } static int pkg_install_write_pointers( FAR const struct pkg_installed_db_s *db, - FAR const struct pkg_manifest_s *manifest) + FAR const char *name) { FAR struct pkg_installed_entry_s *entry; char current[PATH_MAX]; @@ -209,33 +291,35 @@ static int pkg_install_write_pointers( int ret; entry = pkg_metadata_find_installed((FAR struct pkg_installed_db_s *)db, - manifest->name); + name); if (entry == NULL) { - return -ENOENT; + ret = -ENOENT; + goto out; } - ret = pkg_store_format_current_path(current, sizeof(current), - manifest->name); + ret = pkg_store_format_current_path(current, PATH_MAX, name); if (ret < 0) { - return ret; + goto out; } - ret = pkg_store_format_previous_path(previous, sizeof(previous), - manifest->name); + ret = pkg_store_format_previous_path(previous, PATH_MAX, name); if (ret < 0) { - return ret; + goto out; } ret = pkg_store_write_text_atomic(current, entry->current); if (ret < 0) { - return ret; + goto out; } - return pkg_store_write_text_atomic(previous, entry->previous); + ret = pkg_store_write_text_atomic(previous, entry->previous); + +out: + return ret; } /**************************************************************************** @@ -247,29 +331,68 @@ int pkg_install(FAR const char *name) FAR struct pkg_index_s *index; FAR struct pkg_installed_db_s *installed; FAR const struct pkg_manifest_s *manifest; - char source[PATH_MAX]; - char tmp[PATH_MAX] = ""; - char payload[PATH_MAX]; - char manifest_path[PATH_MAX]; - char lock[PATH_MAX] = ""; + FAR char *source; + FAR char *tmp; + FAR char *payload; + FAR char *manifest_path; + FAR char *lock; + FAR char *installed_lock; + FAR const char *artifact; char digest[PKG_HASH_HEX_LEN + 1]; + char pruned_version[PKG_VERSION_MAX + 1]; + bool staged_to_tmp; + bool version_dir_created; + bool installed_lock_held; int ret; - index = malloc(sizeof(*index)); - installed = malloc(sizeof(*installed)); - if (index == NULL || installed == NULL) + pruned_version[0] = '\0'; + + index = pkg_zalloc(sizeof(*index)); + installed = pkg_zalloc(sizeof(*installed)); + source = pkg_path_alloc(); + tmp = pkg_path_alloc(); + payload = pkg_path_alloc(); + manifest_path = pkg_path_alloc(); + lock = pkg_path_alloc(); + installed_lock = pkg_path_alloc(); + if (index == NULL || installed == NULL || source == NULL || tmp == NULL || + payload == NULL || manifest_path == NULL || lock == NULL || + installed_lock == NULL) { - free(index); - free(installed); + pkg_free(index); + pkg_free(installed); + pkg_free(source); + pkg_free(tmp); + pkg_free(payload); + pkg_free(manifest_path); + pkg_free(lock); + pkg_free(installed_lock); pkg_error("unable to allocate package metadata buffers"); return EXIT_FAILURE; } + source[0] = '\0'; + tmp[0] = '\0'; + payload[0] = '\0'; + manifest_path[0] = '\0'; + lock[0] = '\0'; + installed_lock[0] = '\0'; + installed_lock_held = false; + artifact = NULL; + staged_to_tmp = false; + version_dir_created = false; + ret = pkg_store_prepare_layout(); if (ret < 0) { - free(index); - free(installed); + pkg_free(index); + pkg_free(installed); + pkg_free(source); + pkg_free(tmp); + pkg_free(payload); + pkg_free(manifest_path); + pkg_free(lock); + pkg_free(installed_lock); pkg_error("unable to prepare package layout: %d", ret); return EXIT_FAILURE; } @@ -277,8 +400,14 @@ int pkg_install(FAR const char *name) ret = pkg_metadata_load_index(index); if (ret < 0) { - free(index); - free(installed); + pkg_free(index); + pkg_free(installed); + pkg_free(source); + pkg_free(tmp); + pkg_free(payload); + pkg_free(manifest_path); + pkg_free(lock); + pkg_free(installed_lock); pkg_error("unable to load local index metadata: %d", ret); return EXIT_FAILURE; } @@ -286,22 +415,44 @@ int pkg_install(FAR const char *name) manifest = pkg_metadata_find_latest(index, name); if (manifest == NULL) { - free(index); - free(installed); + pkg_free(index); + pkg_free(installed); + pkg_free(source); + pkg_free(tmp); + pkg_free(payload); + pkg_free(manifest_path); + pkg_free(lock); + pkg_free(installed_lock); pkg_error("package '%s' not found in local index", name); return EXIT_FAILURE; } - ret = pkg_install_resolve_artifact(source, sizeof(source), manifest); + ret = pkg_resolve_artifact_source(source, PATH_MAX, manifest); if (ret < 0) { - pkg_error("artifact path for '%s' is too long", name); + pkg_free(index); + pkg_free(installed); + pkg_free(source); + pkg_free(tmp); + pkg_free(payload); + pkg_free(manifest_path); + pkg_free(lock); + pkg_free(installed_lock); + pkg_error("unable to resolve artifact source for '%s': %d", name, ret); return EXIT_FAILURE; } - ret = pkg_install_acquire_lock(name, lock, sizeof(lock)); + ret = pkg_install_acquire_lock(name, lock, PATH_MAX); if (ret < 0) { + pkg_free(index); + pkg_free(installed); + pkg_free(source); + pkg_free(tmp); + pkg_free(payload); + pkg_free(manifest_path); + pkg_free(lock); + pkg_free(installed_lock); pkg_error("unable to acquire package lock for '%s': %d", name, ret); return EXIT_FAILURE; } @@ -309,123 +460,217 @@ int pkg_install(FAR const char *name) ret = pkg_txn_write_state(name, PKG_TXN_FETCHING); if (ret < 0) { + pkg_error("txn state fetching failed: %d", ret); goto errout; } - ret = pkg_store_format_download_path(tmp, sizeof(tmp), manifest->name, - manifest->version); - if (ret < 0) + if (pkg_source_is_url(source)) { - goto errout; - } + ret = pkg_store_format_download_path(tmp, PATH_MAX, manifest->name, + manifest->version); + if (ret < 0) + { + pkg_error("download path format failed: %d", ret); + goto errout; + } - ret = pkg_store_copy_file(source, tmp); - if (ret < 0) + ret = pkg_acquire_source(source, tmp, lock); + if (ret < 0) + { + pkg_error("acquire source failed: %d", ret); + goto errout; + } + + artifact = tmp; + staged_to_tmp = true; + } + else { - goto errout; + artifact = source; } - ret = pkg_hash_file_sha256(tmp, digest); + ret = pkg_hash_file_sha256(artifact, digest); if (ret < 0) { + pkg_error("sha256 failed: %d", ret); goto errout; } if (strcasecmp(digest, manifest->sha256) != 0) { ret = -EILSEQ; + pkg_error("sha256 mismatch: %d", ret); goto errout; } ret = pkg_txn_write_state(name, PKG_TXN_VERIFIED); if (ret < 0) { + pkg_error("txn state verified failed: %d", ret); + goto errout; + } + + ret = pkg_store_format_version_path(payload, PATH_MAX, manifest->name, + manifest->version); + if (ret < 0) + { + pkg_error("version path format failed: %d", ret); + goto errout; + } + + if (access(payload, F_OK) == 0) + { + version_dir_created = false; + } + else if (errno == ENOENT) + { + version_dir_created = true; + } + else + { + ret = -errno; + pkg_error("unable to inspect version directory: %d", ret); goto errout; } ret = pkg_store_ensure_version_dir(manifest->name, manifest->version); if (ret < 0) { + pkg_error("ensure version dir failed: %d", ret); goto errout; } - ret = pkg_store_format_payload_path(payload, sizeof(payload), + ret = pkg_store_format_payload_path(payload, PATH_MAX, manifest->name, manifest->version, manifest->artifact); if (ret < 0) { + pkg_error("payload path format failed: %d", ret); goto errout; } - ret = pkg_store_copy_file(tmp, payload); + ret = pkg_store_copy_file(artifact, payload); if (ret < 0) { + pkg_error("copy payload failed: %d", ret); goto errout; } - ret = pkg_store_format_manifest_path(manifest_path, sizeof(manifest_path), + if (manifest->type == PKG_PAYLOAD_ELF && + chmod(payload, 0755) < 0 && errno != ENOSYS) + { + ret = -errno; + pkg_error("mark payload executable failed: %d", ret); + goto errout; + } + + ret = pkg_store_format_manifest_path(manifest_path, PATH_MAX, manifest->name, manifest->version); if (ret < 0) { + pkg_error("manifest path format failed: %d", ret); goto errout; } ret = pkg_metadata_write_manifest(manifest_path, manifest); if (ret < 0) { + pkg_error("write manifest failed: %d", ret); goto errout; } ret = pkg_txn_write_state(name, PKG_TXN_STAGED); if (ret < 0) { + pkg_error("txn state staged failed: %d", ret); goto errout; } ret = pkg_compat_check(manifest); if (ret < 0) { + pkg_error("compat check failed: %d", ret); goto errout; } ret = pkg_txn_write_state(name, PKG_TXN_COMPAT_OK); if (ret < 0) { + pkg_error("txn state compat_ok failed: %d", ret); goto errout; } - ret = pkg_metadata_load_installed(installed); + ret = pkg_install_acquire_installed_lock(installed_lock, PATH_MAX); if (ret < 0) { + pkg_error("unable to acquire installed-db lock: %d", ret); goto errout; } - ret = pkg_install_update_installed(installed, manifest); + installed_lock_held = true; + + ret = pkg_metadata_load_installed(installed); if (ret < 0) { + pkg_error("load installed metadata failed: %d", ret); goto errout; } - ret = pkg_install_write_pointers(installed, manifest); + ret = pkg_install_update_installed(installed, manifest, pruned_version, + sizeof(pruned_version)); if (ret < 0) { + pkg_error("update installed metadata failed: %d", ret); goto errout; } ret = pkg_metadata_save_installed(installed); if (ret < 0) { + pkg_error("save installed metadata failed: %d", ret); goto errout; } + /* Only remove the pruned version's payload directory now that the db + * update naming it gone is durably saved - see + * pkg_install_prune_oldest_version()'s comment for why doing this + * before the save could leave a saved db entry pointing at an + * already-deleted version if the save had failed instead. + */ + + if (pruned_version[0] != '\0') + { + pkg_store_remove_version_dir(manifest->name, pruned_version); + } + + ret = pkg_install_write_pointers(installed, manifest->name); + if (ret < 0) + { + /* The installed database is authoritative. The pointer files are + * convenience mirrors and can be reconstructed from it, so failure + * to refresh one must not roll back a durably committed install. + */ + + pkg_error("unable to refresh current/previous pointers: %d", ret); + } + + pkg_store_remove_file(installed_lock); + installed_lock_held = false; + ret = pkg_txn_write_state(name, PKG_TXN_ACTIVATED); if (ret < 0) { - goto errout; + /* The installed database has already been committed. Do not enter + * the failure cleanup path here: it could remove a payload referenced + * by that database. Transaction state is recovery bookkeeping and + * can be cleared below. + */ + + pkg_error("txn state activated failed: %d", ret); } pkg_txn_write_state(name, PKG_TXN_CLEANUP); - if (tmp[0] != '\0') + if (staged_to_tmp && tmp[0] != '\0') { pkg_store_remove_file(tmp); } @@ -437,27 +682,67 @@ int pkg_install(FAR const char *name) } pkg_info("installed %s version %s", manifest->name, manifest->version); - free(index); - free(installed); + pkg_free(index); + pkg_free(installed); + pkg_free(source); + pkg_free(tmp); + pkg_free(payload); + pkg_free(manifest_path); + pkg_free(lock); + pkg_free(installed_lock); return EXIT_SUCCESS; errout: pkg_txn_write_state(name, PKG_TXN_FAILED); - if (tmp[0] != '\0') + if (staged_to_tmp && tmp[0] != '\0') { pkg_store_remove_file(tmp); } + /* Reclaim whatever was staged into the version directory (payload, + * manifest.jsn) before this failure - otherwise every failure past + * this point leaves a permanently orphaned, never-activated version + * directory with no way to reclaim it short of "remove". + */ + + if (version_dir_created) + { + pkg_store_remove_version_dir(manifest->name, manifest->version); + } + pkg_txn_clear_state(name); if (lock[0] != '\0') { pkg_store_remove_file(lock); } - free(index); - free(installed); + if (installed_lock_held) + { + pkg_store_remove_file(installed_lock); + } + + pkg_free(index); + pkg_free(installed); + pkg_free(source); + pkg_free(tmp); + pkg_free(payload); + pkg_free(manifest_path); + pkg_free(lock); + pkg_free(installed_lock); pkg_error("install failed for '%s': %d", name, ret); - return EXIT_FAILURE; + + /* Propagate the real negative errno (rather than the constant + * EXIT_FAILURE) here specifically, since every meaningful pipeline + * failure - network/download, sha256 mismatch (-EILSEQ), wrong + * arch/board (-ENOEXEC/-EXDEV from pkg_compat_check) - funnels through + * this handler. nxstore calls pkg_install() directly (not through a + * shell) and can use this to show a differentiated message instead of + * one generic "install failed" string; any nonzero value (this is + * always < 0) still satisfies the plain success/failure contract for + * callers that only check for zero. + */ + + return ret; } int pkg_list(FAR FILE *stream) @@ -465,7 +750,7 @@ int pkg_list(FAR FILE *stream) FAR struct pkg_installed_db_s *db; int ret; - db = malloc(sizeof(*db)); + db = pkg_zalloc(sizeof(*db)); if (db == NULL) { pkg_error("unable to allocate installed metadata buffer"); @@ -475,7 +760,7 @@ int pkg_list(FAR FILE *stream) ret = pkg_store_prepare_layout(); if (ret < 0) { - free(db); + pkg_free(db); pkg_error("unable to prepare package layout: %d", ret); return EXIT_FAILURE; } @@ -483,7 +768,7 @@ int pkg_list(FAR FILE *stream) ret = pkg_metadata_load_installed(db); if (ret < 0) { - free(db); + pkg_free(db); pkg_error("unable to load installed metadata: %d", ret); return EXIT_FAILURE; } @@ -491,11 +776,315 @@ int pkg_list(FAR FILE *stream) ret = pkg_metadata_print_installed(stream, db); if (ret < 0) { - free(db); + pkg_free(db); pkg_error("unable to print installed metadata: %d", ret); return EXIT_FAILURE; } - free(db); + pkg_free(db); + return EXIT_SUCCESS; +} + +/**************************************************************************** + * Name: pkg_uninstall + * + * Description: + * Remove every installed version of "name": their version directories + * (payload + manifest.jsn), the current/previous pointer files, any + * leftover txn.tx, the entry in the shared installed-packages database, + * and finally the now-empty package root directory. Refuses to run + * while an install/update for the same package is in flight (a live + * lock.lk), since removing the store out from under it would corrupt + * whatever it's mid-writing. + * + ****************************************************************************/ + +int pkg_uninstall(FAR const char *name) +{ + FAR struct pkg_installed_db_s *db; + FAR struct pkg_installed_entry_s *entry; + struct pkg_installed_entry_s removed; + char path[PATH_MAX]; + char package_lock[PATH_MAX]; + char installed_lock[PATH_MAX]; + size_t index; + size_t i; + int ret; + + if (!pkg_validate_path_component(name)) + { + pkg_error("remove requires a valid package name"); + return EXIT_FAILURE; + } + + db = pkg_zalloc(sizeof(*db)); + if (db == NULL) + { + pkg_error("unable to allocate installed metadata buffer"); + return EXIT_FAILURE; + } + + ret = pkg_store_prepare_layout(); + if (ret < 0) + { + pkg_free(db); + pkg_error("unable to prepare package layout: %d", ret); + return EXIT_FAILURE; + } + + ret = pkg_install_acquire_lock(name, package_lock, sizeof(package_lock)); + if (ret < 0) + { + pkg_free(db); + pkg_error("unable to acquire package lock for '%s': %d", name, ret); + return EXIT_FAILURE; + } + + ret = pkg_install_acquire_installed_lock(installed_lock, + sizeof(installed_lock)); + if (ret < 0) + { + pkg_store_remove_file(package_lock); + pkg_free(db); + pkg_error("unable to acquire installed-db lock: %d", ret); + return EXIT_FAILURE; + } + + ret = pkg_metadata_load_installed(db); + if (ret < 0) + { + pkg_store_remove_file(installed_lock); + pkg_store_remove_file(package_lock); + pkg_free(db); + pkg_error("unable to load installed metadata: %d", ret); + return EXIT_FAILURE; + } + + entry = pkg_metadata_find_installed(db, name); + if (entry == NULL) + { + pkg_store_remove_file(installed_lock); + pkg_store_remove_file(package_lock); + pkg_free(db); + pkg_error("package '%s' is not installed", name); + return EXIT_FAILURE; + } + + /* Drop this entry from the authoritative database before removing its + * payloads. A power loss can then leave reclaimable orphan files, but + * never a database entry that points at a payload already deleted. + */ + + removed = *entry; + index = (size_t)(entry - db->entries); + for (i = index; i + 1 < db->count; i++) + { + db->entries[i] = db->entries[i + 1]; + } + + db->count--; + + ret = pkg_metadata_save_installed(db); + pkg_store_remove_file(installed_lock); + if (ret < 0) + { + pkg_store_remove_file(package_lock); + pkg_free(db); + pkg_error("unable to save installed metadata: %d", ret); + return EXIT_FAILURE; + } + + for (i = 0; i < removed.version_count; i++) + { + pkg_store_remove_version_dir(name, removed.versions[i]); + } + + if (pkg_store_format_txn_path(path, sizeof(path), name) == 0) + { + pkg_store_remove_file(path); + } + + if (pkg_store_format_current_path(path, sizeof(path), name) == 0) + { + pkg_store_remove_file(path); + } + + if (pkg_store_format_previous_path(path, sizeof(path), name) == 0) + { + pkg_store_remove_file(path); + } + + pkg_store_remove_file(package_lock); + + if (pkg_store_format_package_root(path, sizeof(path), name) == 0) + { + rmdir(path); + } + + pkg_info("removed %s", name); + pkg_free(db); + return EXIT_SUCCESS; +} + +/**************************************************************************** + * Name: pkg_rollback + * + * Description: + * Swap "name"'s current and previous installed versions. The swap (as + * opposed to just clearing "previous") lets a second rollback undo the + * first. Verifies the rollback target's version directory still + * exists on disk before committing any state change, and refuses to + * run while an install/update for the same package is in flight. + * + ****************************************************************************/ + +int pkg_rollback(FAR const char *name) +{ + FAR struct pkg_installed_db_s *db; + FAR struct pkg_installed_entry_s *entry; + char version_path[PATH_MAX]; + char package_lock[PATH_MAX]; + char installed_lock[PATH_MAX]; + char swap[PKG_VERSION_MAX + 1]; + struct stat st; + int ret; + + if (!pkg_validate_path_component(name)) + { + pkg_error("rollback requires a valid package name"); + return EXIT_FAILURE; + } + + db = pkg_zalloc(sizeof(*db)); + if (db == NULL) + { + pkg_error("unable to allocate installed metadata buffer"); + return EXIT_FAILURE; + } + + ret = pkg_store_prepare_layout(); + if (ret < 0) + { + pkg_free(db); + pkg_error("unable to prepare package layout: %d", ret); + return EXIT_FAILURE; + } + + ret = pkg_install_acquire_lock(name, package_lock, sizeof(package_lock)); + if (ret < 0) + { + pkg_free(db); + pkg_error("unable to acquire package lock for '%s': %d", name, ret); + return EXIT_FAILURE; + } + + ret = pkg_install_acquire_installed_lock(installed_lock, + sizeof(installed_lock)); + if (ret < 0) + { + pkg_store_remove_file(package_lock); + pkg_free(db); + pkg_error("unable to acquire installed-db lock: %d", ret); + return EXIT_FAILURE; + } + + ret = pkg_metadata_load_installed(db); + if (ret < 0) + { + pkg_store_remove_file(installed_lock); + pkg_store_remove_file(package_lock); + pkg_free(db); + pkg_error("unable to load installed metadata: %d", ret); + return EXIT_FAILURE; + } + + entry = pkg_metadata_find_installed(db, name); + if (entry == NULL) + { + pkg_store_remove_file(installed_lock); + pkg_store_remove_file(package_lock); + pkg_free(db); + pkg_error("package '%s' is not installed", name); + return EXIT_FAILURE; + } + + if (entry->previous[0] == '\0') + { + pkg_store_remove_file(installed_lock); + pkg_store_remove_file(package_lock); + pkg_free(db); + pkg_error("package '%s' has no previous version to roll back to", + name); + return EXIT_FAILURE; + } + + ret = pkg_store_format_version_path(version_path, sizeof(version_path), + name, entry->previous); + if (ret < 0 || stat(version_path, &st) < 0) + { + pkg_store_remove_file(installed_lock); + pkg_store_remove_file(package_lock); + pkg_free(db); + pkg_error("rollback target version '%s' is missing on disk", + entry->previous); + return EXIT_FAILURE; + } + + ret = snprintf(swap, sizeof(swap), "%s", entry->current); + if (ret < 0 || (size_t)ret >= sizeof(swap)) + { + pkg_store_remove_file(installed_lock); + pkg_store_remove_file(package_lock); + pkg_free(db); + pkg_error("current version string too long to swap"); + return EXIT_FAILURE; + } + + ret = snprintf(entry->current, sizeof(entry->current), "%s", + entry->previous); + if (ret < 0 || (size_t)ret >= sizeof(entry->current)) + { + pkg_store_remove_file(installed_lock); + pkg_store_remove_file(package_lock); + pkg_free(db); + pkg_error("unable to update current version"); + return EXIT_FAILURE; + } + + ret = snprintf(entry->previous, sizeof(entry->previous), "%s", swap); + if (ret < 0 || (size_t)ret >= sizeof(entry->previous)) + { + pkg_store_remove_file(installed_lock); + pkg_store_remove_file(package_lock); + pkg_free(db); + pkg_error("unable to update previous version"); + return EXIT_FAILURE; + } + + /* The installed database is authoritative, so commit it first. Refresh + * the current/previous pointer files afterwards as convenience mirrors; + * they can be reconstructed from the database if either write fails. + */ + + ret = pkg_metadata_save_installed(db); + if (ret < 0) + { + pkg_store_remove_file(installed_lock); + pkg_store_remove_file(package_lock); + pkg_free(db); + pkg_error("unable to save installed metadata: %d", ret); + return EXIT_FAILURE; + } + + ret = pkg_install_write_pointers(db, name); + pkg_store_remove_file(installed_lock); + pkg_store_remove_file(package_lock); + if (ret < 0) + { + pkg_error("unable to refresh current/previous pointers: %d", ret); + } + + pkg_info("rolled back %s to version %s", name, entry->current); + pkg_free(db); return EXIT_SUCCESS; } From 77f790dceac4cd423ea5b125ed6e05d149ed91e8 Mon Sep 17 00:00:00 2001 From: aviralgarg05 Date: Wed, 12 Aug 2026 00:36:27 +0530 Subject: [PATCH 2/2] system/nxpkg: Complete the command line. Only install, list and available were reachable from the shell, so a package could be put on a device but never updated, rolled back or taken off again. Wire up sync, update, remove and rollback, and report the errno the library returns rather than a plain failure. Assisted-by: OpenAI Codex:gpt-5.6-sol Signed-off-by: aviralgarg05 --- system/nxpkg/pkg_main.c | 107 ++++++++++++++++++++++++++++++++-------- 1 file changed, 86 insertions(+), 21 deletions(-) diff --git a/system/nxpkg/pkg_main.c b/system/nxpkg/pkg_main.c index c049592c5b6..40dfb710746 100644 --- a/system/nxpkg/pkg_main.c +++ b/system/nxpkg/pkg_main.c @@ -29,8 +29,18 @@ #include #include +#include + #include "pkg.h" +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +#define PKG_USAGE \ + "Usage: %s " \ + "[args]\n" + /**************************************************************************** * Public Functions ****************************************************************************/ @@ -38,12 +48,15 @@ int main(int argc, FAR char *argv[]) { FAR const char *cmd; + cJSON_Hooks hooks; + + hooks.malloc_fn = malloc; + hooks.free_fn = free; + cJSON_InitHooks(&hooks); if (argc < 2) { - fprintf(stderr, - "Usage: %s [args]\n", - argv[0]); + fprintf(stderr, PKG_USAGE, argv[0]); return EXIT_FAILURE; } @@ -52,9 +65,7 @@ int main(int argc, FAR char *argv[]) if (strcmp(cmd, "help") == 0 || strcmp(cmd, "--help") == 0 || strcmp(cmd, "-h") == 0) { - fprintf(stdout, - "Usage: %s [args]\n", - argv[0]); + fprintf(stdout, PKG_USAGE, argv[0]); return EXIT_SUCCESS; } @@ -63,19 +74,59 @@ int main(int argc, FAR char *argv[]) if (argc != 3) { pkg_error("install expects exactly one package name"); - fprintf(stderr, - "Usage: %s [args]\n", - argv[0]); + fprintf(stderr, PKG_USAGE, argv[0]); return EXIT_FAILURE; } - return pkg_install(argv[2]); + /* pkg_install() returns a real negative errno on the meaningful + * pipeline failures (nxstore uses that directly), not just + * EXIT_SUCCESS/EXIT_FAILURE - normalize to a plain 0/1 shell exit + * status here. + */ + + return pkg_install(argv[2]) == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } + /* "update" is a package name resolving to whatever version is latest + * in the local index - pkg_install() already handles the "already + * installed at a different version" transition transparently via + * pkg_install_update_installed(), so no separate code path is needed. + */ + if (strcmp(cmd, "update") == 0) { - pkg_error("'update' is not implemented yet in the current unit"); - return EXIT_FAILURE; + if (argc != 3) + { + pkg_error("update expects exactly one package name"); + fprintf(stderr, PKG_USAGE, argv[0]); + return EXIT_FAILURE; + } + + return pkg_install(argv[2]) == 0 ? EXIT_SUCCESS : EXIT_FAILURE; + } + + if (strcmp(cmd, "remove") == 0 || strcmp(cmd, "uninstall") == 0) + { + if (argc != 3) + { + pkg_error("remove expects exactly one package name"); + fprintf(stderr, PKG_USAGE, argv[0]); + return EXIT_FAILURE; + } + + return pkg_uninstall(argv[2]); + } + + if (strcmp(cmd, "rollback") == 0) + { + if (argc != 3) + { + pkg_error("rollback expects exactly one package name"); + fprintf(stderr, PKG_USAGE, argv[0]); + return EXIT_FAILURE; + } + + return pkg_rollback(argv[2]); } if (strcmp(cmd, "list") == 0) @@ -83,24 +134,38 @@ int main(int argc, FAR char *argv[]) if (argc != 2) { pkg_error("list does not take additional arguments"); - fprintf(stderr, - "Usage: %s [args]\n", - argv[0]); + fprintf(stderr, PKG_USAGE, argv[0]); return EXIT_FAILURE; } return pkg_list(stdout); } - if (strcmp(cmd, "rollback") == 0) + if (strcmp(cmd, "available") == 0) { - pkg_error("'rollback' is not implemented yet in the current unit"); - return EXIT_FAILURE; + if (argc != 2) + { + pkg_error("available does not take additional arguments"); + fprintf(stderr, PKG_USAGE, argv[0]); + return EXIT_FAILURE; + } + + return pkg_available(stdout); + } + + if (strcmp(cmd, "sync") == 0) + { + if (argc != 3) + { + pkg_error("sync expects exactly one index source"); + fprintf(stderr, PKG_USAGE, argv[0]); + return EXIT_FAILURE; + } + + return pkg_sync(argv[2]) == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } fprintf(stderr, "ERROR: Unknown subcommand '%s'\n", cmd); - fprintf(stderr, - "Usage: %s [args]\n", - argv[0]); + fprintf(stderr, PKG_USAGE, argv[0]); return EXIT_FAILURE; }