On Sat, Apr 18, 2020 at 01:10:51AM +0200, Thomas L. wrote:
> On Fri, 17 Apr 2020 13:29:12 -0700
> Andrew Hewus Fresh <[email protected]> wrote:
> > Try this patch with `portgen ruby activesupport` and you'll see that
> > it it still doesn't find it.
> >
> > You would have to add the same logic in the name_new_ports sub as
> > the parent module looks for the *current* port being ported.
> 
> I moved the logic to name_new_ports, but `portgen ruby activesupport`
> still duplicates activesupport to mystuff (but with in the same
> directory as in /usr/ports). I don't think portgen checks whether the
> gem it is called with is already ported.

Hmm, when I did it, it worked for me.  Although I didn't actually try
your exact patch, but it was the same adding of `module_in_ports` to
name_new_port.



> > I also don't really like hard-coding the list of possible ruby flavors
> > here in PortGen as nobody will remember to update them.  I'm OK doing
> > it for "py" vs "py3" because a major Python version number change is
> > a big deal, a new Ruby 2.8 is significantly less so.  If we can find
> > a nice way to grab this list from ruby.port.mk, I would go for a
> > patch to fix the places that need it to use that.
> 
> I see. How about this?

This is really nice, although I see now that "is_standard_module" needs
some help, probably to require calling it with a ruby version, sigh.
Not really sure, I haven't done much with Ruby or the PortGen Ruby
stuff.

I do have some suggestions though, but overall excellent work!



> diff --git infrastructure/lib/OpenBSD/PortGen/Port/Ruby.pm 
> infrastructure/lib/OpenBSD/PortGen/Port/Ruby.pm
> index 1703fba5b89..bf9ff286467 100644
> --- infrastructure/lib/OpenBSD/PortGen/Port/Ruby.pm
> +++ infrastructure/lib/OpenBSD/PortGen/Port/Ruby.pm
> @@ -21,9 +21,19 @@ use warnings;
> 
>  use parent 'OpenBSD::PortGen::Port';
> 
> +use List::Util qw( reduce );
> +
>  use OpenBSD::PortGen::Dependency;
>  use OpenBSD::PortGen::Utils qw( module_in_ports );
> 
> +my ( $portsdir, @rubyversions );
> +
> +BEGIN {

I don't think this needs to be in a BEGIN block, see below.

> +     $portsdir = $ENV{PORTSDIR} || '/usr/ports';

There's a helper for this:

https://github.com/openbsd/ports/blob/master/infrastructure/lib/OpenBSD/PortGen/Utils.pm#L47

> +     @rubyversions = `ECHO_MSG=true make -C $portsdir/lang/ruby show=BINREV`;
> +     chomp for @rubyversions;

and a `$self->make_show` helper, although it doesn't support `-C` so
probably do need to do it yourself.  Or, do the `chdir` dance I do in
some other places.  Probably could use a refactor to do that better.

https://github.com/openbsd/ports/blob/master/infrastructure/lib/OpenBSD/PortGen/Port.pm#L514

> +}

I would probably do that with a lazy cache so it doesn't happen when
porting non-ruby modules:

my $ruby_versions; # cache of ruby versions
sub ruby_versions {
    return @{ $ruby_versions } if $ruby_versions;

    my @ruby_versions = do {
        my $ruby_dir = ports_dir() . '/lang/ruby';
        local $ENV{ECHO_MSG} = 'true';
        qx{make -C $ruby_dir show=BINREV};
    };

    chomp @ruby_versions;
    $ruby_versions = \@ruby_versions;

    return @ruby_versions;
}

> +
>  sub ecosystem_prefix
>  {
>       my $self = shift;
> @@ -57,8 +67,9 @@ sub name_new_port
>  {
>       my ( $self, $di ) = @_;
> 
> -     my $name = $self->SUPER::name_new_port(
> -         ref $di ? $di->{name} : $di );
> +     my $gem = ref $di ? $di->{name} : $di;
> +     my $name = reduce { $a || module_in_ports( $gem, "ruby$b-" ) } 0, 
> @rubyversions;

Then we could have:

    my $name = reduce { $a || module_in_ports($gem, "ruby$b-") } 0,
        $self->ruby_versions;

> +     $name = $self->SUPER::name_new_port( $gem ) unless $name;

Or:
        $name ||= $self->SUPER::name_new_port( $gem );

>       $name = "ruby/$name" unless $name =~ m{/};
> 
>       return $name;
> @@ -97,8 +108,7 @@ sub get_deps
> 
>               next if $self->is_standard_module($name);
> 
> -             my $port = module_in_ports( $name, 'ruby-' )
> -                 || $self->name_new_port($name);
> +             my $port = $self->name_new_port($name);

This part probably should happen in most of the Port::* modules.


l8rZ,
-- 
andrew - http://afresh1.com

The 3 great virtues of a programmer: Laziness, Impatience, and Hubris.
                      --Larry Wall

Reply via email to