system/nxpkg: fix concurrency and versioning bugs found in review.

- pkg_sync(): index.jsn and repo.url were updated as two independent
  atomic writes with nothing serializing the pair against a second,
  concurrent pkg_sync() call - each write stayed internally
  consistent, but the pair didn't, so one sync's index could end up on
  disk next to a different sync's source URL. Add a dedicated sync
  lock (pkg_repo_acquire_sync_lock(), mirroring the existing installed-
  db lock's blocking-retry-with-stale-reclaim pattern) around the
  whole read-fetch-write sequence. Also renew the lock's mtime as data
  actually arrives (pkg_repo_sink(), via a new renew_lock_path field
  threaded through pkg_acquire_source()) rather than only stamping it
  once at acquire time - a lock acquired once and then measured
  against a fixed 10-minute staleness window could otherwise be
  reclaimed mid-download on a large-enough file over a slow-enough
  link, even though the download was still genuinely in progress.
  pkg_reclaim_stale_lock() (renamed from pkg_install_reclaim_stale_lock,
  made public) is shared between both lock kinds rather than
  duplicated.

- pkg_install_prune_oldest_version(): deleted the pruned version's
  on-disk payload directory before the updated installed database was
  even durably saved. If pkg_metadata_save_installed() subsequently
  failed, the payload was already gone but the last successfully-saved
  instpkg.jsn could still list that version as installed. The victim
  version is now handed back to the caller (threaded through
  pkg_install_add_version()/pkg_install_update_installed()) so
  pkg_install() can defer the actual directory removal until after the
  save succeeds.

- pkg_metadata_version_token_cmp(): two version tokens with equal
  numeric prefixes (e.g. "1a" and "1b", both parsing as 1) compared as
  equal instead of falling back to a lexical comparison of what
  follows the number, contradicting this function's own documented
  behavior and silently treating genuinely different versions as the
  same one. Compare the non-numeric remainder lexically when the
  numeric prefixes match instead of falling through.

Assisted-by: Claude:claude-sonnet-5
Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>

Author: aviralgarg05

system/nxpkg/pkg.h

@@ -259,7 +259,9 @@ int pkg_resolve_artifact_source(FAR char *buffer, size_t size,
                                 FAR const struct pkg_manifest_s *manifest);
 int pkg_resolve_icon_source(FAR char *buffer, size_t size,
                             FAR const struct pkg_manifest_s *manifest);
-int pkg_acquire_source(FAR const char *source, FAR const char *dest);
+int pkg_acquire_source(FAR const char *source, FAR const char *dest,
+                       FAR const char *renew_lock_path);
+void pkg_reclaim_stale_lock(FAR const char *path);
 int pkg_sync(FAR const char *source);
 int pkg_install(FAR const char *name);
 int pkg_uninstall(FAR const char *name);

system/nxpkg/pkg_install.c

@@ -40,39 +40,6 @@
  * Private Functions
  ****************************************************************************/
 
-/****************************************************************************
- * Name: pkg_install_reclaim_stale_lock
- *
- * Description:
- *   Remove "path" if it is old enough that it cannot belong to a
- *   still-running install (see PKG_LOCK_STALE_SECONDS).  Best-effort: any
- *   stat()/unlink() failure just falls through to the normal -EBUSY
- *   result, since a lock we can't inspect should be treated as held.
- *
- ****************************************************************************/
-
-static void pkg_install_reclaim_stale_lock(FAR const char *path)
-{
-  struct stat st;
-  time_t now;
-
-  if (stat(path, &st) < 0)
-    {
-      return;
-    }
-
-  now = time(NULL);
-  if (now < st.st_mtime ||
-      (now - st.st_mtime) < PKG_LOCK_STALE_SECONDS)
-    {
-      return;
-    }
-
-  pkg_error("reclaiming stale lock '%s' (age %ld s)",
-            path, (long)(now - st.st_mtime));
-  unlink(path);
-}
-
 static int pkg_install_acquire_lock(FAR const char *name, FAR char *path,
                                     size_t size)
 {
@@ -94,7 +61,7 @@ static int pkg_install_acquire_lock(FAR const char *name, FAR char *path,
   fd = open(path, O_WRONLY | O_CREAT | O_EXCL, 0644);
   if (fd < 0 && errno == EEXIST)
     {
-      pkg_install_reclaim_stale_lock(path);
+      pkg_reclaim_stale_lock(path);
       fd = open(path, O_WRONLY | O_CREAT | O_EXCL, 0644);
     }
 
@@ -176,7 +143,7 @@ static int pkg_install_acquire_installed_lock(FAR char *path, size_t size)
           return -errno;
         }
 
-      pkg_install_reclaim_stale_lock(path);
+      pkg_reclaim_stale_lock(path);
       usleep(20 * 1000);
     }
 
@@ -201,7 +168,8 @@ static bool pkg_install_has_version(
 }
 
 static int pkg_install_prune_oldest_version(
-              FAR struct pkg_installed_entry_s *entry)
+              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;
@@ -228,7 +196,19 @@ static int pkg_install_prune_oldest_version(
       return -E2BIG;
     }
 
-  pkg_store_remove_version_dir(entry->name, entry->versions[victim]);
+  /* 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++)
     {
@@ -241,7 +221,9 @@ static int pkg_install_prune_oldest_version(
 }
 
 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;
 
@@ -252,7 +234,8 @@ static int pkg_install_add_version(FAR struct pkg_installed_entry_s *entry,
 
   if (entry->version_count >= PKG_INSTALLED_VERSIONS_MAX)
     {
-      ret = pkg_install_prune_oldest_version(entry);
+      ret = pkg_install_prune_oldest_version(entry, pruned_version,
+                                             pruned_version_size);
       if (ret < 0)
         {
           return ret;
@@ -278,7 +261,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;
@@ -332,7 +317,8 @@ 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(
@@ -380,6 +366,42 @@ static int pkg_install_write_pointers(
  * Public Functions
  ****************************************************************************/
 
+/****************************************************************************
+ * Name: pkg_reclaim_stale_lock
+ *
+ * Description:
+ *   Remove "path" if it is old enough that it cannot belong to a still-
+ *   running holder (see PKG_LOCK_STALE_SECONDS).  Best-effort: any
+ *   stat()/unlink() failure just falls through to the normal -EBUSY
+ *   result, since a lock we can't inspect should be treated as held.
+ *   Generic across every lock file this package uses (per-package
+ *   install locks, the shared installed-db lock, pkg_repo.c's sync
+ *   lock) - not install-specific despite living in this file.
+ *
+ ****************************************************************************/
+
+void pkg_reclaim_stale_lock(FAR const char *path)
+{
+  struct stat st;
+  time_t now;
+
+  if (stat(path, &st) < 0)
+    {
+      return;
+    }
+
+  now = time(NULL);
+  if (now < st.st_mtime ||
+      (now - st.st_mtime) < PKG_LOCK_STALE_SECONDS)
+    {
+      return;
+    }
+
+  pkg_error("reclaiming stale lock '%s' (age %ld s)",
+            path, (long)(now - st.st_mtime));
+  unlink(path);
+}
+
 int pkg_install(FAR const char *name)
 {
   FAR struct pkg_index_s *index;
@@ -393,11 +415,14 @@ int pkg_install(FAR const char *name)
   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;
 
+  pruned_version[0] = '\0';
+
   index = pkg_zalloc(sizeof(*index));
   installed = pkg_zalloc(sizeof(*installed));
   source = pkg_path_alloc();
@@ -525,7 +550,7 @@ int pkg_install(FAR const char *name)
           goto errout;
         }
 
-      ret = pkg_acquire_source(source, tmp);
+      ret = pkg_acquire_source(source, tmp, lock);
       if (ret < 0)
         {
           pkg_error("acquire source failed: %d", ret);
@@ -667,7 +692,8 @@ int pkg_install(FAR const char *name)
       goto errout;
     }
 
-  ret = pkg_install_update_installed(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);
@@ -681,6 +707,18 @@ int pkg_install(FAR const char *name)
       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)
     {

system/nxpkg/pkg_metadata.c

@@ -404,6 +404,9 @@ static int pkg_metadata_version_token_cmp(FAR const char *lhs,
   long rightnum;
   FAR char *leftend;
   FAR char *rightend;
+  FAR const char *cmpleft;
+  FAR const char *cmpright;
+  int ret;
 
   leftnum = strtol(lhs, &leftend, 10);
   rightnum = strtol(rhs, &rightend, 10);
@@ -419,22 +422,36 @@ static int pkg_metadata_version_token_cmp(FAR const char *lhs,
         {
           return 1;
         }
+
+      /* Equal numeric prefixes don't make the tokens equal if what
+       * follows the number differs - e.g. "1a" and "1b" both parse as
+       * 1, but are documented to compare lexically by their non-numeric
+       * component.  Falling through to "equal" here (as this used to)
+       * silently treated any such pair as the same version, which is
+       * exactly backwards for tokens whose whole point is to be
+       * distinguishable.  Compare what's left after the numeric prefix
+       * - leftend/rightend - lexically instead of falling through.
+       */
+
+      cmpleft = leftend;
+      cmpright = rightend;
     }
   else
     {
-      int ret;
+      cmpleft = lhs;
+      cmpright = rhs;
+    }
 
-      ret = pkg_string_cmp(lhs, PKG_VERSION_MAX + 1,
-                           rhs, PKG_VERSION_MAX + 1);
-      if (ret < 0)
-        {
-          return -1;
-        }
+  ret = pkg_string_cmp(cmpleft, PKG_VERSION_MAX + 1,
+                       cmpright, PKG_VERSION_MAX + 1);
+  if (ret < 0)
+    {
+      return -1;
+    }
 
-      if (ret > 0)
-        {
-          return 1;
-        }
+  if (ret > 0)
+    {
+      return 1;
     }
 
   return 0;