Re: [Pkgutil-users] patch to support non-standard VERSION entries
Status: Beta
Brought to you by:
bonivart
|
From: <ma...@pr...> - 2010-07-26 10:47:39
|
> On Sat 24/07/10 2:40 PM , Peter Bonivart bon...@op... sent:
>> On Fri, Jul 23, 2010 at 12:24 PM, ma...@pr... wrote:
>> Can I suggest that the following patch is accepted
>> into the next release of pkgutil?
<snip>
>
> Since It's a numerical comparison it could be considered a bug not
> cleaning up the values so I have checked in your change, it will be in
> 2.1 released soon. Thank you.
>
> --
> /peter
Thanks Peter.
Investigating support for non-CSW packages further, we could do with some more changes in pkgutil. Presently, pkgutil will only ever look at the REV component of a VERSION string when determining which package is newer. This is fine for CSW packages, where there is a standard that is strictly followed for the formatting of the VERSION string in pkginfo. However, in order for us to support non-CSW packages in our internal repo, from a variety of third party vendors, we need to have a slightly looser interpretation of package version to cater for packages where the REV component is the sub-version (i.e. version takes precedence) or where the REV component is entirely missing.
Here are some examples of what we needed:
VERSION=5.0.8 is newer than VERSION=5.0.7
VERSION=2.0,REV=16 is newer than VERSION=1.0,REV=18
I've thought hard about how to support this in pkgutil without breaking CSW packages, and came to the conclusion that the best way was to add a new option in pkgutil.conf that would add this needed functionality, which by default would be switched off.
While working on this, I also spotted and fixed a bug in the verscmp subroutine, where this check:
if (! $p2list[$i]) {
$update = 1;
last;
}
Should have said:
if ($i >= scalar(@p2list)) {
$update = 1;
last;
}
To avoid an error message about a non-initialised array element, and also to avoid the scenario where 5.0.8 would not be greater than 5.0.7 because of the zero in the second position.
By the way, I may have more suggested changes for you in the next few days. When do you plan to release 2.1?
The patches below also incorporate the patch I sent on Friday, so you will probably need to back that one out first. The intention is that to enable this functionality, both of the following configuration options need to be set:
noncsw=true
pkgcmpstyle=1
Here is a patch for pkgutil.conf:
*** pkgutil.conf Mon Jul 26 11:29:34 2010
--- pkgutil.conf.new Mon Jul 26 11:10:45 2010
***************
*** 63,68 ****
--- 63,78 ----
# Default: 0
pkgliststyle=1
+ # Style of package comparison used when checking for newer packages. 0 tells
+ # pkgutil to compare only the REV component of the VERSION string, and this is
+ # the correct behaviour for CSW packages. If you want to support non-CSW
+ # packages that do not always have a REV component, setting this to 1 will
+ # request that pkgutil compares the main VERSION string first, and then it
+ # will only look at the REV component if the main VERSION string is identical.
+ # The non-CSW behaviour can only be enabled when noncsw is set to true.
+ # Default: 0
+ pkgcmpstyle=1
+
# How to handle soft errors from hooks that are called. A hook that exits
# with code 1 will cause the pkgutil to stop. If this value is true,
# exit code 2 will also cause pkgutil to stop. Exit code 2 is a non-fatal
Here is the patch for pkgutil (quite a big one, sorry about that):
*** 2.1b1/pkgutil Sat Jul 17 15:44:53 2010
--- pkgutil-2.1b1-edited Mon Jul 26 11:22:10 2010
***************
*** 39,44 ****
--- 39,45 ----
my $use_gpg = 0;
my $noncsw = 0;
my $pkgliststyle = 0;
+ my $pkgcmpstyle = 0;
my $maxpkglist = 100000;
my @mirror;
my $defaultmirror = ("http://ibiblio.org/pub/packages/solaris/opencsw/current");
***************
*** 476,481 ****
--- 477,483 ----
"use_gpg" => "",
"use_md5" => "",
"pkgliststyle" => "",
+ "pkgcmpstyle" => "",
"maxpkglist" => "",
"noncsw" => "",
"stop_on_hook_soft_error" => "",
***************
*** 539,544 ****
--- 541,547 ----
}
$pkgliststyle = $config{pkgliststyle} if defined $config{pkgliststyle};
+ $pkgcmpstyle = $config{pkgcmpstyle} if defined $config{pkgcmpstyle};
$maxpkglist = $config{maxpkglist} if $config{maxpkglist};
$use_md5 = $use_gpg = $noncsw = 0;
$use_md5 = 1 if ($config{use_md5} eq "true");
***************
*** 1432,1438 ****
print "mirror\t\t\t" . ((scalar(@{$config{mirror}})) ? join("\n\t\t", @{$config{mirror}}) : "not set") . "\n\t\t\t(default: $defaultmirror)\n";
print "noncsw\t\t\t" . (($noncsw) ? $noncsw : "false") . " (default: false)\n";
print "pkgaddopts\t\t" . (($config{pkgaddopts}) ? $config{pkgaddopts} : "not set") . " (default: none)\n";
! print "pkgliststyle\t\t$pkgliststyle (default: 0)\n";
print "root_path\t\t" . (($config{root_path}) ? $config{root_path} : "not set") . " (default: /)\n";
print "stop_on_hook_soft_error\t" . (($config{stop_on_hook_soft_error}) ? $config{stop_on_hook_soft_error} : "not set") . " (default: false)\n";
print "use_gpg\t\t\t" . (($use_gpg) ? "true" : "false") . " (default: false)\n";
--- 1435,1442 ----
print "mirror\t\t\t" . ((scalar(@{$config{mirror}})) ? join("\n\t\t", @{$config{mirror}}) : "not set") . "\n\t\t\t(default: $defaultmirror)\n";
print "noncsw\t\t\t" . (($noncsw) ? $noncsw : "false") . " (default: false)\n";
print "pkgaddopts\t\t" . (($config{pkgaddopts}) ? $config{pkgaddopts} : "not set") . " (default: none)\n";
! print "pkgliststyle\t\t" . (($config{pkgliststyle}) ? $config{pkgliststyle} : "not set") . " (default: 0)\n";
! print "pkgcmpstyle\t\t" . (($config{pkgcmpstyle}) ? $config{pkgcmpstyle} : "not set") . " (default: 0)\n";
print "root_path\t\t" . (($config{root_path}) ? $config{root_path} : "not set") . " (default: /)\n";
print "stop_on_hook_soft_error\t" . (($config{stop_on_hook_soft_error}) ? $config{stop_on_hook_soft_error} : "not set") . " (default: false)\n";
print "use_gpg\t\t\t" . (($use_gpg) ? "true" : "false") . " (default: false)\n";
***************
*** 1732,1738 ****
# Comparison of two package versions as per
# http://pkgutil.net/get-install-and-configure#toc8
# As per cmp or <=>, -1, 0, or 1 if p1 is less than, equal to or greater than p2
! # Note that if neither has a REV code, -1 is *ALWAYS* returned.
# p1rev, p2rev - versions to compare
sub verscmp {
my($p1rev,$p2rev) = @_; # crev (new), irev (old), then 1 => upgrade
--- 1736,1743 ----
# Comparison of two package versions as per
# http://pkgutil.net/get-install-and-configure#toc8
# As per cmp or <=>, -1, 0, or 1 if p1 is less than, equal to or greater than p2
! # Note that if neither has a REV code, -1 is *ALWAYS* returned, unless
! # noncsw is true and pkgcmpstyle is 1.
# p1rev, p2rev - versions to compare
sub verscmp {
my($p1rev,$p2rev) = @_; # crev (new), irev (old), then 1 => upgrade
***************
*** 1758,1766 ****
}
}
! # 3rd case: installed REV, catalog REV => if newer, upgrade
if (! $skip) {
if ($p1rev =~ /REV=/ && $p2rev =~ /REV=/) {
if ($p1rev =~ /rev=/) {
($p1tmp) = ($p1rev =~ /REV=(.+)_/);
} else {
--- 1763,1814 ----
}
}
! # 3rd case: check main VERSION string => if newer, upgrade
! # (only if noncsw is true and pkgcmpstyle is 1)
! if (! $skip && $noncsw && $pkgcmpstyle) {
! print STDERR "Comparing VERSION strings\n" if $debug;
! $p1tmp = $p1rev;
! $p1tmp =~ s/,REV=.*$//;
! $p2tmp = $p2rev;
! $p2tmp =~ s/,REV=.*$//;
!
! if ($p1tmp ne $p2tmp) {
! # VERSION component is not equal
! @p1list = split(/\./,$p1tmp);
! @p2list = split(/\./,$p2tmp);
!
! for (my $i = 0; $i < scalar(@p1list); $i++) {
! if ($i >= scalar(@p2list)) {
! $update = 1;
! last;
! }
! $p1list[$i] =~ s/[^0-9]//g;
! $p2list[$i] =~ s/[^0-9]//g;
!
! if ($p1list[$i] != $p2list[$i]) {
! if ($p1list[$i] > $p2list[$i]) {
! $update = 1;
! } else {
! $update = -1;
! }
! print STDERR "$i $p1list[$i] $p2list[$i] $update\n" if $debug;
! last;
! }
! print STDERR "$i $p1list[$i] $p2list[$i] $update\n" if $debug;
! }
! $update = -1 if (! $update && scalar(@p2list) > scalar(@p1list));
! $skip = 1;
! } elsif ($p1rev !~ /REV=/ && $p2rev !~ /REV=/) {
! # VERSION component is equal and there is not REV component
! $update = 0;
! $skip = 1;
! }
! }
!
! # 4th case: installed REV, catalog REV => if newer, upgrade
if (! $skip) {
if ($p1rev =~ /REV=/ && $p2rev =~ /REV=/) {
+ print STDERR "Comparing REV strings\n" if $debug;
if ($p1rev =~ /rev=/) {
($p1tmp) = ($p1rev =~ /REV=(.+)_/);
} else {
***************
*** 1776,1785 ****
@p2list = split(/\./,$p2tmp);
for (my $i = 0; $i < scalar(@p1list); $i++) {
! if (! $p2list[$i]) {
$update = 1;
last;
}
if ($p1list[$i] != $p2list[$i]) {
if ($p1list[$i] > $p2list[$i]) {
$update = 1;
--- 1824,1836 ----
@p2list = split(/\./,$p2tmp);
for (my $i = 0; $i < scalar(@p1list); $i++) {
! if ($i >= scalar(@p2list)) {
$update = 1;
last;
}
+ $p1list[$i] =~ s/[^0-9]//g;
+ $p2list[$i] =~ s/[^0-9]//g;
+
if ($p1list[$i] != $p2list[$i]) {
if ($p1list[$i] > $p2list[$i]) {
$update = 1;
***************
*** 1797,1803 ****
}
}
! # 4th case: installed no REV, catalog no REV => upgrade
if (! $skip) {
$update = 1 if ($p1rev !~ /REV=/ && $p2rev !~ /REV=/);
}
--- 1848,1854 ----
}
}
! # 5th case: installed no REV, catalog no REV => upgrade
if (! $skip) {
$update = 1 if ($p1rev !~ /REV=/ && $p2rev !~ /REV=/);
}
***************
*** 2042,2047 ****
--- 2093,2102 ----
Style of package list when installing/upgrading packages. 0 is the one used since the first version of pkgutil, it's space efficient but less readable. 1 is the one introduced in v1.7, it's one package per line which is easier to read.
+ C<pkgcmpstyle>
+
+ Style of package comparison used when checking for newer packages. 0 tells pkgutil to compare only the REV component of the VERSION string, and this is the correct behaviour for CSW packages. If you want to support non-CSW packages that do not always have a REV component, setting this to 1 will request that pkgutil compares the main VERSION string first, and then it will only look at the REV component if the main VERSION string is identical. The non-CSW behaviour can only be enabled when noncsw is set to true.
+
C<root_path>
Set alternate root path (-R with pkg commands). Default is /.
I hope you get the patches ok and are happy to accept them. I'm happy to discuss alternative solutions.
Thanks & regards,
Mark Bannister.
|