Re: [portgen go] try harder to determine the version

2021-01-30 Thread Aaron Bieber


Andrew Hewus Fresh writes:

> On Sat, Jan 30, 2021 at 01:20:25PM -0700, Aaron Bieber wrote:
>> 
>> Aaron Bieber writes:
>> 
>> > Andrew Hewus Fresh writes:
>> >
>> >> 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?
>> >
>> > As mentioned in irc - this is leftover and we don't actually hit the
>> > condition anyway.
>> >>
>> >>
>> >>> +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?
>> >>
>> 
>> Here is a version that moves all the version checks into
>> get_ver_info. Really it just removes the duplicate I had >.>
>> 
>
>> diff 9ae2711eb3404621b05283a43f79a65b656ddf47 /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,8 @@ 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;
>> -}
>> +my $json = $self->get_ver_info($module);
>>  
>>  if ($module =~ m/v\d$/) {
>>  $json->{Name}   = ( split '/', $module )[-2];
>> @@ -90,6 +86,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 +130,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,28 +210,36 @@ sub _go_mod_normalize
>>  sub get_ver_info
>>  {
>>  my ( $self, $module ) = @_;
>> -my $version_list = $self->get( $module . '/@v/list' );
>> +
>> +# We already ran, skip re-running.
>> +return $self->{version_info} if defined $self->{version_info};
>> +
>> +my $json;
>> +my $version_list = eval { $self->get( $module . '/@v/list' ) };
>
> My understanding of go version numbers is that "0" is not a valid
> version, which means that all valid versions will be truthy.  If that
> understanding is wrong, then this code is probably wrong, but if true
> that means you don't need the `//= ""` because you can just test
> $version_list in boolean context directly.
>
> Here's an untested suggestion of how I'd write get_ver_info.
> (this also avoids re-parsing what we think is the latest version
> multiple times)
>
>   my $version_l

Re: [portgen go] try harder to determine the version

2021-01-30 Thread Andrew Hewus Fresh
On Sat, Jan 30, 2021 at 01:20:25PM -0700, Aaron Bieber wrote:
> 
> Aaron Bieber writes:
> 
> > Andrew Hewus Fresh writes:
> >
> >> 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?
> >
> > As mentioned in irc - this is leftover and we don't actually hit the
> > condition anyway.
> >>
> >>
> >>> + 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?
> >>
> 
> Here is a version that moves all the version checks into
> get_ver_info. Really it just removes the duplicate I had >.>
> 

> diff 9ae2711eb3404621b05283a43f79a65b656ddf47 /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,8 @@ 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;
> - }
> + my $json = $self->get_ver_info($module);
>  
>   if ($module =~ m/v\d$/) {
>   $json->{Name}   = ( split '/', $module )[-2];
> @@ -90,6 +86,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 +130,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,28 +210,36 @@ sub _go_mod_normalize
>  sub get_ver_info
>  {
>   my ( $self, $module ) = @_;
> - my $version_list = $self->get( $module . '/@v/list' );
> +
> + # We already ran, skip re-running.
> + return $self->{version_info} if defined $self->{version_info};
> +
> + my $json;
> + my $version_list = eval { $self->get( $module . '/@v/list' ) };

My understanding of go version numbers is that "0" is not a valid
version, which means that all valid versions will be truthy.  If that
understanding is wrong, then this code is probably wrong, but if true
that means you don't need the `//= ""` because you can just test
$version_list in boolean context directly.

Here's an untested suggestion of how I'd write get_ver_info.
(this also avoids re-parsing what we think is the latest version
multiple times)

my $version_list = do { local $@; eval { local $SIG{__DIE__};
$self->get( $module . '/@v/list' ) } };

my $version_info;
if ($version_list) {
my %v = ( o => 
OpenBSD::PackageName::ver

Re: [portgen go] try harder to determine the version

2021-01-30 Thread Aaron Bieber

Aaron Bieber writes:

> Andrew Hewus Fresh writes:
>
>> 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?
>
> As mentioned in irc - this is leftover and we don't actually hit the
> condition anyway.
>>
>>
>>> +   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?
>>

Here is a version that moves all the version checks into
get_ver_info. Really it just removes the duplicate I had >.>

diff 9ae2711eb3404621b05283a43f79a65b656ddf47 /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,8 @@ 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;
-	}
+	my $json = $self->get_ver_info($module);
 
 	if ($module =~ m/v\d$/) {
 		$json->{Name}   = ( split '/', $module )[-2];
@@ -90,6 +86,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 +130,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,28 +210,36 @@ sub _go_mod_normalize
 sub get_ver_info
 {
 	my ( $self, $module ) = @_;
-	my $version_list = $self->get( $module . '/@v/list' );
+
+	# We already ran, skip re-running.
+	return $self->{version_info} if defined $self->{version_info};
+
+	my $json;
+	my $version_list = eval { $self->get( $module . '/@v/list' ) };
 	my $version = "v0.0.0";
-	my $ret;
 
+	$version_list //= "";
+
 	# If list isn't populated, it's likely that upstream doesn't follow
 	# semver, this means we will have to fallback to @latest to get the
 	# version information.
 	if ($version_list eq "") {
-		$ret = $self->get_json( $self->_go_mod_normalize($module) . '/@latest' );
+		$json = $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)) {
+			if ($a->compare($b) == -1) {
 $version = $v;
 			}
 		}
-		$ret = { Module => $module, Version => $version };
+		$json = { Module => $module, Version => $version };
 	}
 
-	return $ret;
+	$self->{version_info} = $json;
+
+	return $json;
 }
 
 sub name_new_port


Re: [portgen go] try harder to determine the version

2021-01-27 Thread Aaron Bieber


Andrew Hewus Fresh writes:

> 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?

As mentioned in irc - this is leftover and we don't actually hit the
condition anyway.
>
>
>> +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?
>

Since I need to know the version to get the distfiles - I have to do the
check early on in get_dist_info - then later portgen calls
get_ver_info. Currently I am caching the version in
$self->{version_info} and short circuiting get_ver_info if it's set.

I actually want the failure to happen in get_ver_info, because at that
point we really can't determine what we need to download.

Hope that makes sense :D

> 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

Re: [portgen go] try harder to determine the version

2021-01-26 Thread Andrew Hewus Fresh
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 -  1.7
+++ lib/OpenBSD/PortGen/Port/Go.pm  27 Jan 2021 02:42:30 -
@@ -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:

[portgen go] try harder to determine the version

2021-01-26 Thread Aaron Bieber
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?

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;
-	}
+	my $json = eval { $self->get_json( $self->_go_mod_normalize($module) . '/@latest' ) };
+	$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;
 			}
 		}
 		$ret = { Module => $module, Version => $version };
 	}
 
+	$self->{version_info} = $ret;
+
 	return $ret;
 }