Bug #93296 for Sub-Identify: Split XS code into separate dist

Sun Feb 23 19:15:39 2014 haarg [...] haarg.org - Ticket created

Subject:

Split XS code into separate dist

It would be helpful for things like App::FatPacker and Object::Remote if the XS bits of this could be split out into a separate dist, so that Sub::Identify itself would be pure-perl. I can work on the changes needed if it is acceptable change.

Mon Feb 24 04:02:35 2014 RGARCIA [...] cpan.org - Correspondence added

On Sun Feb 23 19:15:39 2014, haarg wrote: Show quoted text

> It would be helpful for things like App::FatPacker and Object::Remote > if the XS bits of this could be split out into a separate dist, so > that Sub::Identify itself would be pure-perl. > > I can work on the changes needed if it is acceptable change.

You can skip loading the XS implementation by setting the environment variable PERL_SUB_IDENTIFY_PP to a true value before loading the module. This way you don't even need to ship the object files corresponding to the XS implementation. Is this not enough ?

Mon Feb 24 04:02:35 2014 The RT System itself - Status changed from 'new' to 'open'

Mon Feb 24 12:17:26 2014 ether [...] cpan.org - Cc ETHER added

Mon Feb 24 17:06:41 2014 haarg [...] haarg.org - Correspondence added

On Mon Feb 24 04:02:35 2014, RGARCIA wrote: Show quoted text

> On Sun Feb 23 19:15:39 2014, haarg wrote:

> > It would be helpful for things like App::FatPacker and Object::Remote > > if the XS bits of this could be split out into a separate dist, so > > that Sub::Identify itself would be pure-perl. > > > > I can work on the changes needed if it is acceptable change.

> > > You can skip loading the XS implementation by setting the environment > variable PERL_SUB_IDENTIFY_PP to a true value before loading the > module. This way you don't even need to ship the object files > corresponding to the XS implementation. Is this not enough ?

fatpacker and Object::Remote try to deal with these things automatically. What they care about isn't strictly if something is pure perl, but if it is architecture specific. So they exclude modules from arch lib dirs. Using module specific workarounds is certainly possible in most cases, but it would be much preferable if Sub::Identify was available directly as a pure-perl module.

Mon Feb 24 18:39:20 2014 ether [...] cpan.org - Correspondence added

Subject:	Re: [rt.cpan.org #93296] Split XS code into separate dist
Date:	Mon, 24 Feb 2014 15:39:09 -0800
To:	Graham Knop via RT <bug-Sub-Identify [...] rt.cpan.org>
From:	Karen Etheridge <ether [...] cpan.org>

On Mon, Feb 24, 2014 at 05:06:42PM -0500, Graham Knop via RT wrote: Show quoted text

> Using module specific workarounds is certainly possible in most cases, but it would be much preferable if Sub::Identify was available directly as a pure-perl module.

Module::Implementation is a good way of selecting between different backends based on availability e.g. see https://metacpan.org/source/ETHER/Class-Load-0.21/lib/Class/Load.pm#L20 (that's not my code; I'm just the most recent maintainer.)

Sat Mar 01 23:42:38 2014 ether [...] cpan.org - Correspondence added

On 2014-02-24 01:02:35, RGARCIA wrote: Show quoted text

> You can skip loading the XS implementation by setting the environment > variable PERL_SUB_IDENTIFY_PP to a true value before loading the > module. This way you don't even need to ship the object files > corresponding to the XS implementation. Is this not enough ?

How would you suggest we load this module from code (say another cpan module) that needs to work in a pure-perl environment but should also use the faster XS path when possible? The best thing I can think of is trying to load the module twice: if (not eval { require Sub::Identify }) { local $ENV{PERL_SUB_IDENTIFY_PP} = 1; require Sub::Identify; } But this is really gross, and doesn't even take imports into consideration. It would be much nicer if there were an ::XS module that could be loaded separately, or a dynamic selection of backends based on whether XS was available (e.g. Module::Implementation).

Sun Apr 20 08:36:38 2014 haarg [...] haarg.org - Correspondence added

I've implemented the XS split at https://github.com/rgs/Sub-Identify/pull/1 and https://github.com/haarg/Sub-Identify-XS.

Tue Jun 10 03:37:20 2014 haarg [...] haarg.org - Correspondence added

Has this change been considered at all?

Wed Sep 10 07:49:15 2014 RGARCIA [...] cpan.org - Correspondence added

I'd rather split pure-perl code into an unrelated distribution, so the default upgrade path keeps being XS.

Wed Sep 10 07:49:15 2014 RGARCIA [...] cpan.org - Status changed from 'open' to 'rejected'

Wed Sep 10 08:09:01 2014 ribasushi [...] leporine.io - Cc RIBASUSHI added

Wed Sep 10 12:55:37 2014 rt-cpan [...] trout.me.uk - Correspondence added

On Wed Sep 10 07:49:15 2014, RGARCIA wrote: Show quoted text

> I'd rather split pure-perl code into an unrelated distribution, so the > default upgrade path keeps being XS.

This is why the standard approach is to have the XS code auto-depped in if a compiler is detected, so the default upgrade path *does* keep being XS. Re-opening ticket since the reason for the rejection was a misunderstanding.

Wed Sep 10 12:55:38 2014 rt-cpan [...] trout.me.uk - Status changed from 'rejected' to 'open'

Thu Sep 11 07:15:55 2014 RGARCIA [...] cpan.org - Correspondence added

As mentioned elsewhere, this is not about build dependencies, but run-time dependencies. It is not a good practice to have a distribution silently change the behavior of other distributions when installed. More generally this is also why I stay away from Module::Pluggable or similar frameworks, at least not without writing proper monitoring or alerting when the installation assumptions are broken.

Thu Sep 11 07:15:58 2014 RGARCIA [...] cpan.org - Status changed from 'open' to 'rejected'

Thu Sep 11 14:03:53 2014 rt-cpan [...] trout.me.uk - Correspondence added

On Thu Sep 11 07:15:55 2014, RGARCIA wrote: Show quoted text

> As mentioned elsewhere, this is not about build dependencies, but run- > time dependencies. It is not a good practice to have a distribution > silently change the behavior of other distributions when installed. > More generally this is also why I stay away from Module::Pluggable or > similar frameworks, at least not without writing proper monitoring or > alerting when the installation assumptions are broken.

The presence or non-presence of a file already determines what gets loaded in this code. Given the XS version is an optimisation, I don't think "silently change the behaviour" is accurate. What we're proposing is making the fact that the XS implementation may or may not be available explicit at the distribution level, rather than as it currently is, a hard to introspect artefact of the build process.