PerlAlien / Alien-Build

Build external dependencies for use in CPAN
16 stars 25 forks source link

Comments on the implementation of the prefer mechanism #308

Closed djerius closed 2 years ago

djerius commented 2 years ago

I'd like to add the equivalent of atleast_version and exact_version to share installs. I thought I could piggy back off of the existing Prefer::SortVersions plugin, but I'm finding the Prefer plugin setup a little awkward.

The simplest approach is to apply an around modifier to the prefer phase, but that modifier is not exposed by alienfile, just the before and after ones.

I could wrap Prefer::SortVersions with another plugin, as Prefer::GoodVersion does, but I won't get access to the version property, so I'll need to specify that in my plugin as well as in the Download plugin.

The only way of getting version and filter to be automatically passed to my plugin is via the Download plugin, but that doesn't allow specification of an alternate Prefer plugin class. It's either Prefer::SortVersions or a coderef, and the latter doesn't get access to those parameters except if I pass them via a closure. filter and version are properties of Download; there's no mechanism to pass arbitrary properties to the Prefer plugin.

What complicates things is that version is used by Download::Negotiate to trigger application of a decoder plugin, so it does need access to it.

One way around some of this would be to add a prefer_class property to choose the default Prefer class, and a prefer_args property which would pass additional properties when instantiating prefer_class.

And, if it's not a design choice, exposing the around modifier in an alienfile.

plicease commented 2 years ago

tl;dr feel free to use meta->around to apply an around modifier in an alienfile to prefer or any other phase. This is considered supported.

We can revisit the conspicuous absence of a naked around modifier in alinefile. But this was my reasoning: The around modifier felt a little more complicated than before and after, and I felt forcing an author to invoke the meta-> prefix indicates a warning that you are doing something a little more “advanced”. (same goes for anything that you can do with the meta object which is more or less anything that you can do with a plugin) It was never to discourage ever using the around modifier. Arguably before and after should also have the meta-> prefix, but this is where I happened to draw a line.

djerius commented 2 years ago

tl;dr feel free to use meta->around to apply an around modifier in an alienfile to prefer or any other phase. This is considered supported.

Unfortunately, meta->around results in

Can't locate object method "around" via package "Alien::Build::Meta" ...

This works:

alienfile::_add_modifier( around =>  prefer => sub { ...  } );
plicease commented 2 years ago

Unfortunately, meta->around results in

Can't locate object method "around" via package "Alien::Build::Meta" ...

My bad, the correct incantation is meta->around_hook. Example from Alien::FFI:

https://github.com/PerlFFI/Alien-FFI/blob/main/alienfile#L44-L57

Since this is a bit confusing I am adding a link from the alienfile.pm documentation to the section in Alien::Build that documents the set of methods that you can call on the meta object:

https://github.com/PerlAlien/Alien-Build/pull/310

This works:

alienfile::_add_modifier( around =>  prefer => sub { ...  } );

That is private don't use that :)

djerius commented 2 years ago

That is private don't use that :)

True! But it seems to do a lot of extra stuff that plain around_hook doesn't do. Seems a shame that before and after get the special treatment and around doesn't.

plicease commented 2 years ago

I think with the understanding that meta methods are allowed in an alienfile (as per #310) you can basically anything inside an alienfile that you can do in a plugin, though some of these things may be a bit more complicated and advanced. For now I think around_hook is the way to do this, but we may revisit in the future.