On Tue, Jan 26, 2021 at 05:23:30PM -0700, Aaron Bieber wrote:
> Hi,
>
> Recently a program was found that caused breakage in 'portgen go'. The
> breakage was two fold:
>
> 1) https://proxy.golang.org/qvl.io/promplot/@latest returns unexpected
> results. This caused portgen to bomb out.
> 2) Even it 1) had worked, the logic in 'get_ver_info' was broken and it
> picked the wrong version.
>
> This diff changes things so we first try to get our version from
> '/@latest'. If that fails, we call `get_ver_info` which pulls down the
> full list of versions from the '/@v/list' endpoint.
>
> Thanks to afresh1 for showing me the neat '//=' perl trick!
>
> Tested with:
> portgen go qvl.io/promplot
>
> as well as a number of other ports.
>
> OK? Cluesticks?
>
This seems OK to me, a couple of comments though, but it's up to you
whether you change anything.
> diff 6a862af059a42a1899fe9a62461b2acfc0f8eedc /usr/ports
> blob - 89f2c7297409ef9d54fd1bdde73cf1829c742ff3
> file + infrastructure/lib/OpenBSD/PortGen/Port/Go.pm
> --- infrastructure/lib/OpenBSD/PortGen/Port/Go.pm
> +++ infrastructure/lib/OpenBSD/PortGen/Port/Go.pm
> @@ -67,12 +67,9 @@ sub _go_determine_name
> # Some modules end in "v1" or "v2", if we find one of these, we need
> # to set PKGNAME to something up a level
> my ( $self, $module ) = @_;
> - my $json = $self->get_json( $self->_go_mod_normalize($module) .
> '/@latest' );
>
> - if ($json->{Version} =~ m/incompatible/) {
> - my $msg = "${module} $json->{Version} is incompatible with Go
> modules.";
> - croak $msg;
> - }
Do you still need to check for "incompatible" someplace?
> + my $json = eval { $self->get_json( $self->_go_mod_normalize($module) .
> '/@latest' ) };
Should this eval'd check for `@latest` be in `get_ver_info`?
If not, why not?
Perhaps `get_ver_info` should be:
sub get_ver_info {
my ($self, $module) = @_;
return $self->{ver_info} if $self->{ver_info};
my $ver_info = do { local $@; eval { local $SIG{__DIE__};
$self->_get_latest_ver_info($module) } };
unless ($version) {
my $version = $self->_get_versions($module)->[0];
croak("Unable to find a version for $module")
unless $version;
$ver_info = { Module => $module, Version => $version };
}
return $self->{ver_info} = $ver_info;
}
> + $json //= $self->get_ver_info($module);
>
> if ($module =~ m/v\d$/) {
> $json->{Name} = ( split '/', $module )[-2];
> @@ -81,6 +78,7 @@ sub _go_determine_name
> }
>
> $json->{Module} = $module;
> + $self->{version_info} = $json;
>
> return $json;
> }
> @@ -90,6 +88,7 @@ sub get_dist_info
> my ( $self, $module ) = @_;
>
> my $json = $self->_go_determine_name($module);
> +
> my ($dist, $mods) = $self->_go_mod_info($json);
> $json->{License} = $self->_go_lic_info($module);
>
> @@ -133,7 +132,7 @@ sub _go_mod_info
>
> my $mod = $self->get($self->_go_mod_normalize($json->{Module}) .
> "/\@v/$json->{Version}.mod");
> my ($module) = $mod =~ /\bmodule\s+(.*?)\s/;
> -
> +
> unless ( $json->{Module} eq $module ) {
> my $msg = "Module $json->{Module} doesn't match $module";
> croak $msg;
> @@ -213,6 +212,10 @@ sub _go_mod_normalize
> sub get_ver_info
> {
> my ( $self, $module ) = @_;
> +
> + # We already ran, skip re-running.
> + return $self->{version_info} if defined $self->{version_info};
> +
> my $version_list = $self->get( $module . '/@v/list' );
> my $version = "v0.0.0";
> my $ret;
> @@ -227,13 +230,15 @@ sub get_ver_info
> for my $v ( @parts ) {
> my $a =
> OpenBSD::PackageName::version->from_string($version);
> my $b = OpenBSD::PackageName::version->from_string($v);
> - if ($a->compare($b)) {
> + if ($a->compare($b) == -1) {
> $version = $v;
> }
> }
I think this Schwartzian transform is a bit easier for me to understand
what is going on, but either way this is a good bugfix.
Index: lib/OpenBSD/PortGen/Port/Go.pm
===================================================================
RCS file: /cvs/ports/infrastructure/lib/OpenBSD/PortGen/Port/Go.pm,v
retrieving revision 1.7
diff -u -p -r1.7 Go.pm
--- lib/OpenBSD/PortGen/Port/Go.pm 16 Jan 2021 23:38:13 -0000 1.7
+++ lib/OpenBSD/PortGen/Port/Go.pm 27 Jan 2021 02:42:30 -0000
@@ -223,14 +223,10 @@ sub get_ver_info
if ($version_list eq "") {
$ret = $self->get_json( $self->_go_mod_normalize($module) .
'/@latest' );
} else {
- my @parts = split("\n", $version_list);
- for my $v ( @parts ) {
- my $a =
OpenBSD::PackageName::version->from_string($version);
- my $b = OpenBSD::PackageName::version->from_string($v);
- if ($a->compare($b)) {
- $version = $v;
- }
- }
+ ($version) = map { $_->[0] }
+ sort { $b->[1]->compare($a->[1]) }
+ map { [ $_, OpenBSD::PackageName::version->from_string($_) }
+ split("\n", $version_list);
$ret = { Module => $module, Version => $version };
}
> $ret = { Module => $module, Version => $version };
> }
>
> + $self->{version_info} = $ret;
> +
> return $ret;
> }
>
--
andrew - http://afresh1.com
A printer consists of three main parts:
the case, the jammed paper tray and the blinking red light.
