pkg: Rework downloading package metadata

Passing the '-refresh' option to 'pkg update' etc was previously causing a fresh
download of metadata from the server for each installed package. This changeset
fixes that behavior so that passing '-refresh' only causes one new download
of metadata, irrespective of the number of installed packages.

All changes in directory 'scripts/pkg/':

* pkg.m: Preload package metadata, call get_validated_pkg_list() directly.

* private/get_pkg_info.m: Remove function calls to update package metadata,
  change calling form, update documentation to match.

* private/get_validated_pkg_list.m: Rework logic to save metadata to cache.

* private/search_packages.m: Change calling form, update documentation to match.

Author: dasergatskov

scripts/pkg/pkg.m

@@ -525,17 +525,24 @@
         error ("pkg: search action requires at least one search term or '-all'");
       endif
 
+      ## Pre-load package database (respecting force_refresh flag)
+      get_validated_pkg_list (force_refresh, verbose);
+
       if (nargout)
-        local_packages = search_packages (files, want_all_packages, force_refresh, verbose);
+        local_packages = search_packages (files, want_all_packages, verbose);
       else
-        search_packages (files, want_all_packages, force_refresh, verbose);
+        search_packages (files, want_all_packages, verbose);
       endif
 
     case "install"
       if (isempty (files))
         error ("pkg: install action requires at least one filename");
       endif
 
+      ## Pre-load package database once (respecting force_refresh flag)
+      ## This ensures package name lookups don't trigger repeated downloads
+      get_validated_pkg_list (force_refresh, verbose);
+
       local_files = {};
       tmp_dir = tempname ();
       unwind_protect
@@ -577,7 +584,7 @@
             ## manually, just in case the package author chooses zip
             ## or any other archive format? Or will all packages always
             ## be required to give .tar.gz?
-            [v, url] = get_pkg_info (file, force_refresh, verbose);
+            [v, url] = get_pkg_info (file, verbose);
             tmp_file = tempname (tmp_dir, [file "-" v "-"]);
             tmp_file = [tmp_file, ".tar.gz"];
             local_files{end+1} = tmp_file;  # so that it gets cleaned up
@@ -777,18 +784,25 @@
         installed_pkgs_lst = update_lst;
       endif
 
+      ## Pre-load package database once (respecting force_refresh flag)
+      ## This prevents repeated downloads in the loop below
+      get_validated_pkg_list (force_refresh, verbose);
+
       for i = 1:numel (installed_pkgs_lst)
         installed_pkg_name = installed_pkgs_lst{i}.name;
         installed_pkg_version = installed_pkgs_lst{i}.version;
         try
-          online_pkg_version = get_pkg_info (installed_pkg_name, force_refresh, verbose);
+          ## Use already-loaded package database
+          online_pkg_version = get_pkg_info (installed_pkg_name, verbose);
         catch
           warning ("pkg: package %s not found on Octave Packages - skipping update\n",
                    installed_pkg_name);
           online_pkg_version = "0";
         end_try_catch
         if (compare_versions (online_pkg_version, installed_pkg_version, ">"))
+          ## Pass options but exclude -refresh (we already refreshed once above)
           options_to_pass = varargin (strncmp (varargin, "-", 1));
+          options_to_pass(strcmp (options_to_pass, "-refresh")) = [];
           feval (@pkg, "install", options_to_pass{:}, installed_pkg_name);
         endif
       endfor

scripts/pkg/private/get_pkg_info.m

@@ -25,16 +25,13 @@
 
 ## -*- texinfo -*-
 ## @deftypefn  {} {[@var{ver}, @var{url}] =} get_pkg_info (@var{name})
-## @deftypefnx {} {[@var{ver}, @var{url}] =} get_pkg_info (@var{name}, @var{force_refresh})
-## @deftypefnx {} {[@var{ver}, @var{url}] =} get_pkg_info (@var{name}, @var{force_refresh}, @var{verbose})
+## @deftypefnx {} {[@var{ver}, @var{url}] =} get_pkg_info (@var{name}, @var{verbose})
 ## Return the current version and URL of the Octave package @var{name}.
 ##
-## If @var{force_refresh} is true, always download fresh data from the server.
-##
 ## If @var{verbose} is true, print diagnostic messages.
 ## @end deftypefn
 
-function [ver, url] = get_pkg_info (name, force_refresh = false, verbose = false)
+function [ver, url] = get_pkg_info (name, verbose = false)
 
   ## Verify that name is valid.
   if (! (ischar (name) && rows (name) == 1 && ndims (name) == 2))
@@ -45,7 +42,7 @@
 
   name = lower (name);
 
-  __pkg__ = get_validated_pkg_list (force_refresh, verbose);
+  __pkg__ = get_validated_pkg_list (false, verbose);
   pkgnames = fieldnames (__pkg__);  # all the different packages
 
   if (any (strcmp (pkgnames, name)))  # named package does exist

scripts/pkg/private/get_validated_pkg_list.m

@@ -218,6 +218,8 @@
   endif
 
   ## Download fresh data from server
+  downloaded_fresh_data = false;
+
   if (verbose || force_refresh)
     printf ("pkg: downloading latest package database from packages.octave.org...\n");
     if (! verbose)
@@ -234,7 +236,7 @@
   if (! succ)
     ## Primary failed, try backup
     if (verbose)
-      printf ("pkg: primary repository unavailable, trying backup...\n");
+      printf ("pkg: primary repository unavailable, trying backup from gnu-octave.github.io...\n");
     endif
     [list, succ] = urlread (backup_url);
   endif
@@ -272,6 +274,11 @@
     else
       error ("pkg: could not read URL and no cache available, please verify internet connection");
     endif
+  else
+    ## Download succeeded - capture timestamp
+    ## This reflects actual data freshness, not when we started trying
+    download_time = now ();
+    downloaded_fresh_data = true;
   endif
 
   if (verbose)
@@ -289,22 +296,24 @@
     printf ("pkg: successfully parsed %d packages\n", numel (fieldnames (__pkg__)));
   endif
 
-  ## Save to cache with timestamp in filename
-  try
-    ## Create cache directory if it doesn't exist
-    if (! exist (cache_dir, "dir"))
-      if (verbose)
-        printf ("pkg: creating cache directory: %s\n", cache_dir);
-      endif
-      [success, msg] = mkdir (cache_dir);
-      if (! success)
-        warning ("pkg: could not create cache directory: %s", msg);
+  ## Save to cache with timestamp in filename (only if we downloaded fresh data)
+  if (downloaded_fresh_data)
+    try
+      ## Create cache directory if it doesn't exist
+      if (! exist (cache_dir, "dir"))
+        if (verbose)
+          printf ("pkg: creating cache directory: %s\n", cache_dir);
+        endif
+        [success, msg] = mkdir (cache_dir);
+        if (! success)
+          warning ("pkg: could not create cache directory: %s", msg);
+        endif
       endif
-    endif
 
     ## Generate timestamped filename using datestr
     ## Format: packages_yyyymmddHHMM.json
-    timestamp_str = datestr (now (), "yyyymmddHHMM");
+    ## Use download_time, not current time, so timestamp reflects data age
+    timestamp_str = datestr (download_time, "yyyymmddHHMM");
     new_cache_file = fullfile (cache_dir, ["packages_" timestamp_str ".json"]);
 
     if (verbose)
@@ -346,12 +355,13 @@
           end_try_catch
         endfor
       endif
-    else
-      warning ("pkg: could not write to cache file");
-    endif
-  catch
-    warning ("pkg: error updating cache: %s", lasterr ());
-  end_try_catch
+      else
+        warning ("pkg: could not write to cache file");
+      endif
+    catch
+      warning ("pkg: error updating cache: %s", lasterr ());
+    end_try_catch
+  endif  ## if (downloaded_fresh_data)
 
   retval = __pkg__;
 

scripts/pkg/private/search_packages.m

@@ -25,8 +25,7 @@
 
 ## -*- texinfo -*-
 ## @deftypefn {} {@var{retval} =} search_packages (@var{searchterms}, @var{allpackages})
-## @deftypefnx {} {@var{retval} =} search_packages (@var{searchterms}, @var{allpackages}, @var{force_refresh})
-## @deftypefnx {} {@var{retval} =} search_packages (@var{searchterms}, @var{allpackages}, @var{force_refresh}, @var{verbose})
+## @deftypefnx {} {@var{retval} =} search_packages (@var{searchterms}, @var{allpackages}, @var{verbose})
 ## Search for all packages on the Octave Packages repository whose
 ## descriptions include @var{searchterms}, then either display the search
 ## results with brief descriptions, or return the list of matching packages
@@ -36,14 +35,12 @@
 ## If @var{allpackages} is true, then return / display all packages,
 ## with no filtering for @var{searchterms}.
 ##
-## If @var{force_refresh} is true, always download fresh data from the server.
-##
 ## If @var{verbose} is true, print diagnostic messages.
 ## @end deftypefn
 
-function retval = search_packages (searchterms, allpackages, force_refresh = false, verbose = false)
+function retval = search_packages (searchterms, allpackages, verbose = false)
 
-  __pkg__ = get_validated_pkg_list (force_refresh, verbose);
+  __pkg__ = get_validated_pkg_list (false, verbose);
 
   pkgnames = fieldnames (__pkg__);