dubious logic in package 'uninstall' function

[Sergei Steshenko]

Hello,

I am trying to understand what is the intended functionality of 'uninstall' 
function located in

octave-3.6.1/share/octave/3.6.1/m/pkg/pkg.m

file.

The function code with line numbers is below.

First about the 'global_install' flag - from analyzing full contents of the 
'pkg.m' file I came to the conclusion that if the flag set, packages are 
supposed to be installed into Octave tree and not under user's home directory.


If so, when packages are to be uninstalled, and if 'global_install' is set, 
packages are to be uninstalled from Octave tree, not from under user's home 
tree, correct ? If this statement is not correct, then what is the meaning of 
'global_install' ?

Looking at the code I see:


    965   if (global_install)
    966     installed_pkgs_lst = {local_packages{:}, global_packages{:}};
    967   else
    968     installed_pkgs_lst = local_packages;
    969   endif


and line #966 looks somewhat suspicious - because local (not inside Octave 
tree) packages are involved.

OTOH, local packages may depend on (some) global ones, so maybe line #966 is 
not that bad.


However, as I look further down, my suspicions grow.


If I understand correctly, 'delete_idx' is a list of package numeric indexes, 
and this list represents the packages to be deleted.


My suspicions grow really big looking at this:


    980   ## Are all the packages that should be uninstalled already installed?
    981   if (length (delete_idx) != length (pkgnames))
    982     if (global_install)
    983       ## Try again for a locally installed package.
    984       installed_pkgs_lst = local_packages;
    985
    986       num_packages = length (installed_pkgs_lst);
    987       delete_idx = [];
    988       for i = 1:num_packages
    989         cur_name = installed_pkgs_lst{i}.name;
    990         if (any (strcmp (cur_name, pkgnames)))
    991           delete_idx(end+1) = i;
    992         endif
    993       endfor
    994       if (length (delete_idx) != length (pkgnames))
    995         ## FIXME: We should have a better error message.
    996         warning ("some of the packages you want to uninstall are not 
installed");
    997       endif
    998     else
    999       ## FIXME: We should have a better error message.
   1000       warning ("some of the packages you want to uninstall are not 
installed");
   1001     endif
   1002   endif

piece of code.


'if' statement on line #981 has its condition TRUE if not all packages that 
were requested to be deleted were found in 'installed_pkgs_lst'. The piece of 
code initially filling 'delete_idx' is located on lines #973..978.


Since we are considering the case of global_install == TRUE, _all_ (both global 
and local) packages are scanned - because of line #966. So, if a package to be 
uninstalled wasn't found in either local or global list, it won't be found 
anywhere. Period.


Still, this:


    984       installed_pkgs_lst = local_packages;


narrows the list of installed packages to locally installed ones (again, 
global_install is set to TRUE !), and the loop located on lines #988..993 _can_ 
fill 'delete_idx'.


If 'delete_idx' is filled, it is filled _only_ with indexes of locally 
installed packages - because of already mentioned line #984.

Later on these:


   1004   ## Compute the packages that will remain installed.
   1005   idx = setdiff (1:num_packages, delete_idx);
   1006   remaining_packages = {installed_pkgs_lst{idx}}


lines compute which packages are to stay, and since both 'num_packages' and 
'delete_idx' now originate from _local_ packages (see lines #984, #986, #991, 
the list of remaining packages is the one of _local_ packages.

So, if my conclusions are correct, with 'global_install' flag set to TRUE some 
_local_ (rather than global) packages will be uninstalled.


Am I missing here something ?


Thanks,
  Sergei.


    960 function uninstall (pkgnames, handle_deps, verbose, local_list,
    961                     global_list, global_install)
    962   ## Get the list of installed packages.
    963   [local_packages, global_packages] = installed_packages(local_list,
    964                                                          global_list);
    965   if (global_install)
    966     installed_pkgs_lst = {local_packages{:}, global_packages{:}};
    967   else
    968     installed_pkgs_lst = local_packages;
    969   endif
    970
    971   num_packages = length (installed_pkgs_lst);
    972   delete_idx = [];
    973   for i = 1:num_packages
    974     cur_name = installed_pkgs_lst{i}.name;
    975     if (any (strcmp (cur_name, pkgnames)))
    976       delete_idx(end+1) = i;
    977     endif
    978   endfor
    979
    980   ## Are all the packages that should be uninstalled already installed?
    981   if (length (delete_idx) != length (pkgnames))
    982     if (global_install)
    983       ## Try again for a locally installed package.
    984       installed_pkgs_lst = local_packages;
    985
    986       num_packages = length (installed_pkgs_lst);
    987       delete_idx = [];
    988       for i = 1:num_packages
    989         cur_name = installed_pkgs_lst{i}.name;
    990         if (any (strcmp (cur_name, pkgnames)))
    991           delete_idx(end+1) = i;
    992         endif
    993       endfor
    994       if (length (delete_idx) != length (pkgnames))
    995         ## FIXME: We should have a better error message.
    996         warning ("some of the packages you want to uninstall are not 
installed");
    997       endif
    998     else
    999       ## FIXME: We should have a better error message.
   1000       warning ("some of the packages you want to uninstall are not 
installed");
   1001     endif
   1002   endif
   1003
   1004   ## Compute the packages that will remain installed.
   1005   idx = setdiff (1:num_packages, delete_idx);
   1006   remaining_packages = {installed_pkgs_lst{idx}};
   1007
   1008   ## Check dependencies.
   1009   if (handle_deps)
   1010     error_text = "";
   1011     for i = 1:length (remaining_packages)
   1012       desc = remaining_packages{i};
   1013       bad_deps = get_unsatisfied_deps (desc, remaining_packages);
   1014
   1015       ## Will the uninstallation break any dependencies?
   1016       if (! isempty (bad_deps))
   1017         for i = 1:length (bad_deps)
   1018           dep = bad_deps{i};
   1019           error_text = cstrcat (error_text, " ", desc.name, " needs ",
   1020                                dep.package, " ", dep.operator, " ",
   1021                                dep.version, "\n");
   1022         endfor
   1023       endif
   1024     endfor
   1025
   1026     if (! isempty (error_text))
   1027       error ("the following dependencies where unsatisfied:\n  %s", 
error_text);
   1028     endif
   1029   endif

   1030
   1031   ## Delete the directories containing the packages.
   1032   for i = delete_idx
   1033     desc = installed_pkgs_lst{i};
   1034     ## If an 'on_uninstall.m' exist, call it!
   1035     if (exist (fullfile (desc.dir, "packinfo", "on_uninstall.m"), 
"file"))
   1036       wd = pwd ();
   1037       cd (fullfile (desc.dir, "packinfo"));
   1038       on_uninstall (desc);
   1039       cd (wd);
   1040     endif
   1041     ## Do the actual deletion.
   1042     if (desc.loaded)
   1043       rmpath (desc.dir);
   1044       if (exist (getarchdir (desc)))
   1045         rmpath (getarchdir (desc));
   1046       endif
   1047     endif
   1048     if (exist (desc.dir, "dir"))
   1049       [status, msg] = rm_rf (desc.dir);
   1050       if (status != 1)
   1051         error ("couldn't delete directory %s: %s", desc.dir, msg);
   1052       endif
   1053       [status, msg] = rm_rf (getarchdir (desc));
   1054       if (status != 1)
   1055         error ("couldn't delete directory %s: %s", getarchdir (desc), 
msg);
   1056       endif
   1057       if (dirempty (desc.archprefix))
   1058         rm_rf (desc.archprefix);
   1059       endif
   1060     else
   1061       warning ("directory %s previously lost", desc.dir);
   1062     endif
   1063   endfor
   1064
   1065   ## Write a new ~/.octave_packages.
   1066   if (global_install)
   1067     if (length (remaining_packages) == 0)
   1068       unlink (global_list);
   1069     else
   1070       global_packages = save_order (remaining_packages);
   1071       save (global_list, "global_packages");
   1072     endif
   1073   else
   1074     if (length (remaining_packages) == 0)
   1075       unlink (local_list);
   1076     else
   1077       local_packages = save_order (remaining_packages);
   1078       save (local_list, "local_packages");
   1079     endif
   1080   endif
   1081
   1082 endfunction