search

ingydotnet / inline-c-pm

10 stars 19 forks source link

Inline::C's strong dependency on Inline #93

Closed rongrw closed 4 years ago

rongrw commented 5 years ago

Dear maintainers,

IIRC Inline 0.55 was the last version that included Inline::C in the Inline distribution. After that Inline::C was split from Inline and became its own distribution. This paved the way to make Inline::C more independent of Inline.

However, I feel that this independence is getting lost, and consequently backward compatibility with older versions of Inline is lost. Currently, to install the latest version of Inline::C (0.81), requires the latest version of Inline (0.83).

I'm currently developing Inline::F2003, an ILSM for modern Fortran. The module distribution is not yet released on CPAN, but is maintained on Sourceforge

Backward compatibility with older versions of Inline is an important design feature to me. In fact for the OS X platform I'm going as far back as Inline 0.48 on ActivePerl 5.12.4. Perl scripts using Inline::F2003 cannot execute without Inline::C. Now, if I wanted to use a more recent version of Inline::C (eg. 0.81), I would have to upgrade Inline itself, thus eliminating all backward compatibility testing for Inline::F2003 !

I certainly don't expect Inline::C to be backward compatible with Inline 0.48. However, I do think there should be some scope for Inline::C to execute on older versions of Inline. What do you guys think?

Cheers, Ron.

mohawk2 commented 5 years ago

As a toolchain person, I appreciate the importance of backwards compatibility in the abstract. However, what problem are you solving with this level of effort for your module?

rongrw commented 5 years ago

Thanks @mohawk2

Its not so much about what problem my module is attempting to address, but rather about being forced to update a software component when it isn't really necessary. For instance, does the latest version of Inline::C really require the latest version of Inline in order to work correctly? Or does the current version of Inline::C still happen to work just fine with Inline 0.80 for example?

mohawk2 commented 5 years ago

You may well have a point about Inline::C's needed dep on the very latest and shiniest Inline.

The only difficulty is from 09parser.t, which gives a different number of log-lines between Inline 0.80 and 0.83. If we adjusted that test, or made its expected lines conditional on the version of Inline, then the required Inline could indeed change to something lower.

rongrw commented 5 years ago

Yes, on Fedora Linux running ActivePerl 5.24.1, the current Inline::C test suite running against Inline 0.80 fails only a single test out of 161 tests! To me that clearly indicates that it still works with Inline 0.80.

Maybe we can all agree to set the minimum required version of Inline to 0.80, and ensure that any future enhancements to Inline::C are backward compatible with Inline 0.80? Just my thoughts.

mohawk2 commented 5 years ago

I think it would be a great idea to update 09parser.t with a conditional based on $Inline::VERSION for the relevant log-lines. Ron, want to make a PR?

perlpunk commented 5 years ago

Wasn't the point of the recent Inline.pm updates to make cpantesters for Inline::C pass? And now we go back to Inline.pm 0.80 because we know most cpantesters have probably Inline.pm 0.83 installed anyway?

mohawk2 commented 5 years ago

@perlpunk You're quite right.

@rongrw The older Inline was causing spurious failures of Inline::C (even though the bug was really in Inline), and I'm keen to avoid that kind of false negative. That has to override the back-compat you'd like to see for your module, although if you just require a less-than-latest Inline::C, you can still do that. The latest Inline::C genuinely needs the latest Inline, because that fixes bugs.

I do wonder though whether https://github.com/ingydotnet/inline-c-pm/pull/92 and https://github.com/ingydotnet/inline-pm/pull/75 are better solutions to that problem.

rongrw commented 5 years ago

@mohawk2 @perlpunk thank you for your views.

I'm actually not suggesting that we change anything with Inline. The current version works perfectly with the current version of Inline::C. Also, cpantesters should be testing the latest versions of the modules. I have no issue on those points.

The changes I'm proposing for Inline::C are very small. Simply to lower the required version of Inline from 0.83 to 0.80. BTW the module file C.pm currently loads Inline with:

use Inline 0.56;

which doesn't reflect the actual requirement.

So, for users who have Inline 0.80 installed, they will no longer get a pre-requisite warning, but when they run the test suite they will encounter one failure. I think that's OK though because all the other 160 tests will pass.

mohawk2 commented 5 years ago

If the required version of Inline is lowered, this will become a problem again, which is not acceptable:

The older Inline was causing spurious failures of Inline::C (even though the bug was really in Inline), and I'm keen to avoid that kind of false negative. That has to override the back-compat you'd like to see for your module[...]

Since the whole reason you raise this issue is because your F2003 module depends on Inline::C, why not start by lowering your module's requirement to an earlier Inline::C, thereby completely avoiding this issue?