octave-maintainers
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

pkg Patches


From: Bill Denney
Subject: pkg Patches
Date: Sun, 08 Oct 2006 12:52:49 -0400
User-agent: Thunderbird 1.5.0.7 (Windows/20060909)

So, I was working on packaging object graphics, and while doing that, I found a bug in the package script where it was not showing an error correctly. I went through and fixed the errors given (most of the patch is just removing \n from the end of each error string).

I also rewrote the compare_versions function and pulled it out of pkg (I think it's generally useful, perhaps in the strings directory). It can now compare arbitrarily long version numbers with strings appended using any boolean operator. I'm not including this as a patch because I'm not sure how to make a patch via cvs after a first change, and I know that it's best to keep separate changes separate. Before applying this, please remove the compare_versions function from within pkg.

Bill

2006-10-08  Bill Denney  <address@hidden>
scripts/Changelog:

   *pkg/pkg.m: fix error messages

scripts/Changelog:

*strings/compare_versions.m: New function to compare version numbers as strings with any boolean operators
Index: pkg.m
===================================================================
RCS file: /cvs/octave/scripts/pkg/pkg.m,v
retrieving revision 1.20
diff -u -r1.20 pkg.m
--- pkg.m       4 Oct 2006 20:38:06 -0000       1.20
+++ pkg.m       8 Oct 2006 16:11:30 -0000
@@ -129,21 +129,21 @@
             elseif (nargout == 2)
                 [local_packages, global_packages] = installed_packages();
             else
-                error("Too many output arguments requested.\n");
+                error("Too many output arguments requested.");
             endif
         case "install"
             if (length(files) == 0)
-                error("You must specify at least one filename when calling 
'pkg install'\n");
+                error("You must specify at least one filename when calling 
'pkg install'");
             endif
             install(files, deps, prefix);
         case "uninstall"
             if (length(files) == 0)
-                error("You must specify at least one package when calling 'pkg 
uninstall'\n");
+                error("You must specify at least one package when calling 'pkg 
uninstall'");
             endif
             uninstall(files, deps);
         case "load"
             if (length(files) == 0)
-                error("You must specify at least one package or 'all' when 
calling 'pkg load'\n");
+                error("You must specify at least one package or 'all' when 
calling 'pkg load'");
             endif
             load_packages(files, deps);
         case "prefix"
@@ -158,7 +158,7 @@
                 error("You must specify a prefix directory, or request an 
output arguement");
             endif
         otherwise
-            error("You must specify a valid action for 'pkg'. See 'help pkg' 
for details\n");
+            error("You must specify a valid action for 'pkg'. See 'help pkg' 
for details");
     endswitch
 endfunction
 
@@ -175,7 +175,7 @@
         warning("Creating installation directory %s", prefix);
         [status, msg] = mkdir(prefix);
         if (status != 1)
-            error("Could not create installation directory: %s\n", msg);
+            error("Could not create installation directory: %s", msg);
         endif
     endif
 
@@ -202,7 +202,7 @@
             tmpdirs{end+1} = tmpdir;
             [status, msg] = mkdir(tmpdir);
             if (status != 1)
-                error("Couldn't create temporary directory: %s\n", msg);
+                error("Couldn't create temporary directory: %s", msg);
             endif
 
             ## Uncompress the package
@@ -211,7 +211,7 @@
             ## Get the name of the directories produced by tar
             [dirlist, err, msg] = readdir(tmpdir);
             if (err)
-                error("Couldn't read directory produced by tar: %s\n", msg);
+                error("Couldn't read directory produced by tar: %s", msg);
             endif
 
            if (length(dirlist) > 3)
@@ -233,8 +233,7 @@
                [dummy, nm] = fileparts(tgz); 
                if ((length(nm) >= length(desc.name)) &&
                    ! strcmp(desc.name,nm(1:length(desc.name))))
-                 error(["Package name '",desc.name,"' doesn't correspond",
-                        "to its filename '",nm,"'"]);
+                 error("Package name '%s' doesn't correspond to its filename 
'%s'", desc.name, nm);
                endif
 
                 ## Set default installation directory
@@ -367,9 +366,9 @@
             rm_rf(descriptions{i}.dir);
         endfor
         if (global_install)
-            error("Couldn't append to %s: %s\n", global_list, 
lasterr()(8:end));
+            error("Couldn't append to %s: %s", global_list, lasterr()(8:end));
         else
-            error("Couldn't append to %s: %s\n", local_list, lasterr()(8:end));
+            error("Couldn't append to %s: %s", local_list, lasterr()(8:end));
         endif
     end_try_catch
 
@@ -402,7 +401,7 @@
     else
         installed_packages = local_packages;
     endif
-    
+
     num_packages = length(installed_packages);
     delete_idx = [];
     for i = 1:num_packages
@@ -411,11 +410,11 @@
             delete_idx(end+1) = i;
         endif
     endfor
-    
+
     ## Are all the packages that should be uninstalled already installed?
     if (length(delete_idx) != length(pkgnames))
         # XXX: We should have a better error message
-        error("Some of the packages you want to uninstall are not 
installed.\n");
+        error("Some of the packages you want to uninstall are not installed.");
     endif
 
     ## Compute the packages that will remain installed
@@ -424,7 +423,6 @@
     
     ## Check dependencies
     if (handle_deps)
-        ok = true;
         error_text = "";
         for i = 1:length(remaining_packages)
             desc = remaining_packages{i};
@@ -432,7 +430,6 @@
             
             ## Will the uninstallation break any dependencies?
             if (!isempty(bad_deps))
-                ok = false;
                 for i = 1:length(bad_deps)
                     dep = bad_deps{i};
                     error_text = [error_text "  " desc.name " needs " ...
@@ -442,7 +439,7 @@
             endif
         endfor
 
-        if (!ok)
+        if (! isempty(error_text))
             error("The following dependencies where unsatisfied:\n  %s", 
error_text);
         endif
     endif
@@ -459,7 +456,7 @@
                 cd(wd);
             catch
                 # XXX: Should this rather be an error?
-                warning("The 'on_uninstall' script returned the following 
error: %s", lasterr);
+                warning("The 'on_uninstall' script retsurned the following 
error: %s", lasterr);
                 cd(wd);
             end_try_catch
         endif
@@ -467,7 +464,7 @@
         rmpath(desc.dir);
         [status, msg] = rm_rf(desc.dir);
         if (status != 1)
-            error("Couldn't delete directory %s: %s\n", desc.dir, msg);
+            error("Couldn't delete directory %s: %s", desc.dir, msg);
         endif
     endfor
 
@@ -504,7 +501,7 @@
             cd(wd);
         catch
             cd(wd);
-            error("The pre-install function returned the following error: 
%s\n", lasterr);
+            error("The pre-install function returned the following error: %s", 
lasterr);
         end_try_catch
     endif
 
@@ -513,7 +510,7 @@
         [status, msg] = mkdir([packdir "inst"]);
         if (status != 1)
             rm_rf(desc.dir);
-            error("The 'inst' directory did not exist and could not be 
created: %s\n", msg);
+            error("The 'inst' directory did not exist and could not be 
created: %s", msg);
         endif
     endif
 endfunction
@@ -527,7 +524,7 @@
             [status, output] = system(["cd " src " ;./configure --prefix=" 
desc.dir]);
             if (status != 0)
                 rm_rf(desc.dir);
-                error("The configure script returned the following error: 
%s\n", output);
+                error("The configure script returned the following error: %s", 
output);
             endif
         endif
 
@@ -536,13 +533,13 @@
             [status, output] = system(["export INSTALLDIR=" desc.dir "; make 
-C " src]);
             if (status != 0)
                 rm_rf(desc.dir);
-                error("'make' returned the following error: %s\n", output);
+                error("'make' returned the following error: %s", output);
             endif
             %# make install
             %[status, output] = system(["export INSTALLDIR=" desc.dir "; make 
install -C " src]);
             %if (status != 0)
             %    rm_rf(desc.dir);
-            %    error("'make install' returned the following error: %s\n", 
output);
+            %    error("'make install' returned the following error: %s", 
output);
             %endif
         endif
 
@@ -551,7 +548,7 @@
         if (exist(files, "file"))
             [fid, msg] = fopen(files, "r");
             if (fid < 0)
-                error("Couldn't open %s: %s\n", files, msg);
+                error("Couldn't open %s: %s", files, msg);
             endif
             filenames = char(fread(fid))';
             filenames = strrep(filenames, "\n", " ");
@@ -574,7 +571,7 @@
         endif
         if (status != 0)
             rm_rf(desc.dir);
-            error("Couldn't copy files from 'src' to 'inst': %s\n", output);
+            error("Couldn't copy files from 'src' to 'inst': %s", output);
         endif
     endif
 endfunction
@@ -647,7 +644,7 @@
     if (! exist (desc.dir, "dir"))
       [status, output] = mkdir (desc.dir);
       if (status != 1)
-       error("Couldn't create installation directory %s : %s\n", 
+       error("Couldn't create installation directory %s : %s", 
              desc.dir, output);
       endif
     endif
@@ -657,7 +654,7 @@
       [status, output] = system(["cp -R " packdir "inst/* " desc.dir]);
       if (status != 0)
           rm_rf(desc.dir);
-          error("Couldn't copy files to the installation directory\n");
+          error("Couldn't copy files to the installation directory");
       endif
     endif
 
@@ -666,21 +663,21 @@
     [status, msg] = mkdir (packinfo);
     if (status != 1)
         rm_rf(desc.dir);
-        error("Couldn't create packinfo directory: %s\n", msg);
+        error("Couldn't create packinfo directory: %s", msg);
     endif
 
     ## Copy DESCRIPTION
     [status, output] = system(["cp " packdir "DESCRIPTION " packinfo]);
     if (status != 0)
        rm_rf(desc.dir);
-       error("Couldn't copy DESCRIPTION: %s\n", output);
+       error("Couldn't copy DESCRIPTION: %s", output);
     endif
 
     ## Copy COPYING
     [status, output] = system(["cp " packdir "COPYING " packinfo]);
     if (status != 0)
        rm_rf(desc.dir);
-       error("Couldn't copy COPYING: %s\n", output);
+       error("Couldn't copy COPYING: %s", output);
     endif
 
     ## Is there an INDEX file to copy or should we generate one?
@@ -688,7 +685,7 @@
         [status, output] = system(["cp " packdir "INDEX " packinfo]);
         if (status != 0)
             rm_rf(desc.dir);
-            error("Couldn't copy INDEX file: %s\n", output);
+            error("Couldn't copy INDEX file: %s", output);
         endif
     else
         try
@@ -704,7 +701,7 @@
         [status, output] = system(["cp " packdir "on_uninstall.m " packinfo]);
         if (status != 0)
             rm_rf(desc.dir);
-            error("Couldn't copy on_uninstall.m: %s\n", output);
+            error("Couldn't copy on_uninstall.m: %s", output);
         endif
     endif
 
@@ -730,7 +727,7 @@
         catch
             cd(wd);
             rm_rf(desc.dir);
-            error("The post_install function returned the following error: 
%s\n", lasterr);
+            error("The post_install function returned the following error: 
%s", lasterr);
         end_try_catch
     endif
 endfunction
@@ -752,9 +749,9 @@
 
 ## This function parses the DESCRIPTION file
 function desc = get_description(filename)
-    fid = fopen(filename, "r");
+    [fid, msg] = fopen(filename, "r");
     if (fid == -1)
-        error("The DESCRIPTION file %s could not be read\n", filename);
+        error("The DESCRIPTION file %s could not be read: %s", filename, msg);
     endif
 
     desc = struct();
@@ -780,7 +777,7 @@
                 value   = strip(line(colon+1:end));
                 if (length(value) == 0)
                     fclose(fid);
-                    error("The keyword %s have empty value\n", 
desc.keywords{end});
+                    error("The keyword %s have empty value", 
desc.keywords{end});
                 endif
                 desc.(keyword) = value;
             endif
@@ -794,7 +791,7 @@
                      "author", "maintainer", "description"};
     for f = needed_fields
         if (!isfield(desc, f{1}))
-            error("Description is missing needed field %s\n", f{1});
+            error("Description is missing needed field %s", f{1});
         endif
     endfor
     desc.version = fix_version(desc.version);
@@ -826,7 +823,7 @@
             return
         endif
     endif
-    error("Bad version string: %s\n", v);
+    error("Bad version string: %s", v);
 endfunction
 
 ## Makes sure the depends field is of the right format.
@@ -860,7 +857,7 @@
             endif
             operator = parts{idx(1)};
             if (!any(strcmp(operator, {">=", "<=", "=="}))) ## XXX: I belive 
we also support ">" and "<" 
-                error("Unsupported operator: %s\n", operator);
+                error("Unsupported operator: %s", operator);
             endif
             version  = fix_version(parts{idx(2)});
              
@@ -917,7 +914,7 @@
     ## Get names of functions in dir
     [files, err, msg] = readdir(dir);
     if (err)
-        error("Couldn't read directory %s: %s\n", dir, msg);
+        error("Couldn't read directory %s: %s", dir, msg);
     endif
     
     functions = {};
@@ -933,17 +930,17 @@
     
     ## Does desc have a categories field?
     if (!isfield(desc, "categories"))
-        error("The DESCRIPTION file must have a Categories field, when no 
INDEX file is given.\n");
+        error("The DESCRIPTION file must have a Categories field, when no 
INDEX file is given.");
     endif
     categories = split_by(desc.categories, ",");
     if (length(categories) < 1)
-        error("The Category field is empty.\n");
+        error("The Category field is empty.");
     endif
     
     ## Write INDEX
     fid = fopen(INDEX, "w");
     if (fid == -1)
-        error("Couldn't open %s for writing.\n", INDEX);
+        error("Couldn't open %s for writing.", INDEX);
     endif
     fprintf(fid, "%s >> %s\n", desc.name, desc.title);
     fprintf(fid, "%s\n", categories{1});
@@ -985,12 +982,14 @@
 ## Example: v1="0.1.0", v2="0.0.9", operator=">=" => true
 ## TODO: This function is very long and complex! Can't it be simplified?
 function out = compare_versions(v1, v2, operator)
+
+
     dot1 = find(v1 == ".");
     dot2 = find(v2 == ".");
     if (length(dot1) != 2 || length(dot2) != 2)
         error("Given version strings are not valid: %s %s", v1, v2);
     endif
-    
+
     major1 = str2num( v1( 1:dot1(1)-1 ) );
     minor1 = str2num( v1( dot1(1)+1:dot1(2)-1 ) );
     rev1   = str2num( v1( dot1(2)+1:end ) );
@@ -998,14 +997,14 @@
     minor2 = str2num( v2( dot2(1)+1:dot2(2)-1 ) );
     rev2   = str2num( v2( dot2(2)+1:end) );
     mmr = [sign(major1-major2), sign(minor1-minor2), sign(rev1-rev2)];
-    
+
     if (length(operator) > 1 && operator(2) == "=" && !any(mmr))
         out = 1;
         return;
     elseif (operator(1) == "<")
         mmr = -mmr;
     endif
-    
+
     ## Now we ony need to decide if v1 > v2
     if (mmr(1) == 1)
         out = 1;
@@ -1111,7 +1110,7 @@
         for i = 1:length(files)
             idx = strcmp(pnames, files{i});
             if (!any(idx))
-                error("Package %s is not installed\n", files{i});
+                error("Package %s is not installed", files{i});
             endif
             dirs{end+1} = pdirs{idx};
             if (handle_deps)
## Copyright (C) 2006  Bill Denney  <address@hidden>
##
## This program is free software; you can redistribute it and/or modify
## it under the terms of the GNU General Public License as published by
## the Free Software Foundation; either version 2 of the License, or
## (at your option) any later version.
##
## This program is distributed in the hope that it will be useful,
## but WITHOUT ANY WARRANTY; without even the implied warranty of
## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
## GNU General Public License for more details.
##
## You should have received a copy of the GNU General Public License
## along with this program; if not, write to the Free Software
## Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA

## -*- texinfo -*-
## @deftypefn {Command} compare_versions(@var{v1}, @var{v2}, @var{operator})
## Compares to version strings using the given @var{operator}.
##
## This function assumes that versions @var{v1} and @var{v2} are
## arbitrarily long strings made of numeric and period characters
## possibly followed by an arbitrary string (e.g. "1.2.3", "0.3",
## "0.1.2+", or "1.2.3.4-test1").
##
## The version is first split into the numeric and the character parts
## then the parts are padded to be the same length (i.e. "1.1" would be
## padded to be like "1.1.0" when being compared with "1.1.1", and
## separately, the character parts of the strings are padded with
## nulls).
##
## The operator can be any logical operator from the set
##
## @itemize @bullet
## @item
## "=="
## equal
## @item
## "<"
## less than
## @item
## "<="
## less than or equal to
## @item
## ">"
## greater than
## @item
## ">="
## greater than or equal to
## @item
## "!="
## not equal
## @item
## "~="
## not equal
## @end itemize
##
## Note that version "1.1-test2" would compare as greater than
## "1.1-test10". Also, since the numeric part is compared first, "a"
## compares less than "1a" because the second string starts with a
## numeric part even though double("a") is greater than double("1").
## @end deftypefn

## TODO?: This allows a single equal sign "=" to indicate equality, do
## we want to require a double equal since that is the boolean operator?

function out = compare_versions(v1, v2, operator)

  ## make sure that the version numbers are valid
  if ~ (ischar (v1) && ischar (v2))
    error ("Both version numbers must be strings");
  elseif (size (v1, 1) ~= 1) || (size (v2, 1) ~= 1)
    error ("Version numbers must be a single row")
  endif

  ## check and make sure that the operator is valid
  if (~ ischar (operator))
    error("Operator must be a character string");
  elseif (numel (operator) > 2)
    error("Operator cannot be more than 2 characters long");
  endif

  ## trim off any character data that is not part of a normal version
  ## number
  numbers = "0123456789.";
  v1firstchar = find(~ ismember(v1, numbers), 1);
  v2firstchar = find(~ ismember(v2, numbers), 1);
  if ~ isempty (v1firstchar)
    v1c = v1(v1firstchar:length(v1));
    v1nochar = v1(1:v1firstchar-1);
  else
    v1c = "";
    v1nochar = v1;
  endif
  if ~ isempty (v2firstchar)
    v2c = v2(v2firstchar:length(v2));
    v2nochar = v2(1:v2firstchar-1);
  else
    v2c = "";
    v2nochar = v2;
  endif

  v1n = str2num (split (v1nochar, '.'));
  v2n = str2num (split (v2nochar, '.'));
  if ((isempty (v1n) && isempty (v1c)) || (isempty (v2n) && isempty(v2c)))
    error ("Given version strings are not valid: %s %s", v1, v2);
  endif

  ## assume that any additional elements would be 0 if one is longer
  ## than the other
  maxnumlen = max ([length(v1n) length(v2n)]);
  if (length (v1n) < maxnumlen)
    v1n(length(v1n)+1:maxnumlen) = 0;
  endif
  if (length (v2n) < maxnumlen)
    v2n(length(v2n)+1:maxnumlen) = 0;
  endif

  ## assume that any additional character elements would be 0 if one is
  ## longer than the other
  maxcharlen = max ([length(v1c) length(v2c)]);
  if (length (v1c) < maxcharlen)
    v1c(length(v1c)+1:maxcharlen) = "\0";
  endif
  if (length (v2c) < maxcharlen)
    v2c(length(v2c)+1:maxcharlen) = "\0";
  endif

  ## determine the operator
  if any (ismember (operator, "="))
    equal_op = true;
  else
    equal_op = false;
  end
  if any (ismember (operator, "~!"))
    not_op = true;
  else
    not_op = false;
  endif
  if any (ismember (operator, "<"))
    lt_op = true;
  else
    lt_op = false;
  endif
  if any (ismember (operator, ">"))
    gt_op = true;
  else
    gt_op = false;
  endif

  ## make sure that we don't have conflicting operators
  if (gt_op && lt_op)
    error("Operator cannot contain both greater and less than symbols");
  elseif ((gt_op || lt_op) && not_op)
    error("Operator cannot contain not and greater than or less than symbols");
  endif

  ## compare the versions (making sure that they're the same shape)
  vcmp = v1n(:) - v2n(:);
  vcmp = [vcmp; (v1c - v2c)(:)];
  if (lt_op)
    ## so that we only need to check for the output being greater than 1
    vcmp = -vcmp;
  endif
  firstdiff = find(vcmp != 0, 1);

  if isempty (firstdiff)
    ## they're equal
    out = equal_op;
  elseif (lt_op || gt_op)
    ## they're correctly less than or greater than
    out = (vcmp(firstdiff) > 0);
  else
    ## they're not correctly less than or greater than, and they're not equal
    out = false;
  endif

  ## reverse the output if not is given
  out = xor (not_op, out);
endfunction

## tests
## test both equality symbols
%!assert(compare_versions("1", "1", "="), true)
## test arbitrarily long equality
%!assert(compare_versions("1.1.0.0.0", "1.1", "=="), true)
%!assert(compare_versions("1", "1.1", "<"), true)
%!assert(compare_versions("1.1", "1.1", "<="), true)
%!assert(compare_versions("1.1", "1.1.1", "<="), true)
%!assert(compare_versions("1.23", "1.24", "=<"), true)
## test different length numbers
%!assert(compare_versions("23.2000", "23.1", ">"), true)
%!assert(compare_versions("0.0.2", "0.0.1", ">="), true)
%!assert(compare_versions("0.2", "0.0.100", "=>"), true)
%!assert(compare_versions("0.1", "0.2", "!="), true)
%!assert(compare_versions("0.1", "0.2", "~="), true)

## test alphanumeric strings
%!assert(compare_versions("1a", "1b", "<"), true)
%!assert(compare_versions("a", "1", "<"), true)
%!assert(compare_versions("1a", "1b", ">"), false)
%!assert(compare_versions("a", "1", ">"), false)
%!assert(compare_versions("1.1.0a", "1.1.0b", "=="), false)
%!assert(compare_versions("1.1.0a", "1.1.0b", "!="), true)
%!assert(compare_versions("1.1.0test", "1.1.0b", "=="), false)
%!assert(compare_versions("1.1.0test", "1.1.0test", "=="), true)

## make sure that it won't just give true output
%!assert(compare_versions("1", "0", "="), false)
## test arbitrarily long equality
%!assert(compare_versions("1.1.1.0.0", "1.1", "=="), false)
%!assert(compare_versions("1.1", "1", "<"), false)
%!assert(compare_versions("2", "1.1", "<="), false)
%!assert(compare_versions("1.1.1", "1.1", "<="), false)
%!assert(compare_versions("1.25", "1.24", "=<"), false)
## test different length numbers
%!assert(compare_versions("23.2", "23.100", ">"), false)
%!assert(compare_versions("0.0.0.2", "0.0.1", ">="), false)
%!assert(compare_versions("0.0.20", "0.10.2", "=>"), false)
%!assert(compare_versions("0.1", "0.1", "!="), false)
%!assert(compare_versions("0.1", "0.1", "~="), false)

## FIXME: how do we check to make sure that it gives errors when it
## should

reply via email to

[Prev in Thread] Current Thread [Next in Thread]