My strategy for dealing with problems in other people's modules has been to patch them, send the patch to the author, and replace it when the author releases a version with the patch. Sometimes I wait a long time, sometimes it's the next day, but patching seems a lot more reliable to me than whatever symbol table magic Sub::Override is doing.

If you need to change the code, patching it directly is the wrong thing to do. What happens if I don't override and instead patch? Many (probably most) programmers work for companies where testing is simply not done. I can say "just run your tests", but the reality is, I want to make things workable for people regardless of whether or not they run tests. There are also companies which have learned the value of testing but still have plenty of code which doesn't yet have tests. Again, the strategy of an external patch which breaks when it needs to is safer than a silent failure on upgrade. Also, I know plenty of people who couldn't patch XS code if it's the XS which is buggy.

As for waiting for a patch to be applied, that becomes problematic when working with code that hundreds of people use, many system administrators might or might notice and a bundle of programmers could possibly rely on. If I wait over a year for a patch but everyone who knows about this has left in the interim, having code mysteriously break on upgrade is far worse than having it clearly designed to break due to an external patch. I agree that Sub::Override is not an optimal solution, but I'm not always able to control when a module is going to get updated and in large companies, sometimes you need a better failsafe mechanism.

Cheers,
Ovid

New address of my CGI Course.

I thought you were talking about your specific case, where I assumed you will have tests.

We package our installs with specific CPAN sources, so we never risk accidentally upgrading something. When we patch something, it's essentially the same as what you're doing -- an in-house change that works with one known version. The in-house code is marked with a different dist name than the CPAN dist. It would take a willful act to replace it with a different version.

If this is the case, your runtime overriding solution seems a bit fragile. In this case, if the original author/authors are unresponsive, the best you can do is to fork off from their work and use your forked modules in the code you deliver. You could pack the patched modules in your work, or give something to the community releasing the patched module (this is up to you and to your willness to cope with bug fixes in your forked module).

Yes, we all know what forking means, but you're probably thinking about forks in really active projects, where at a certain point different people would like to go in different directions. This does not seem the case of almost-dead modules IMHO.

If the original author(s?) come back to life, then, you can either decide to put your code back to their modules in your next release, or just stick to your already-working code. This probably depends on how much confidence you give to their rebirth, or how many bugs were solved in their patched version. But, again, you keep control over what you deliver, and don't get catched by accidental changes. Last, but not least, you don't get blamed if your Sub::Override doesn't work for that nasty corner case you didn't think of.

Flavio
perl -ple'$_=reverse' <<<ti.xittelop@oivalf

Don't fool yourself.

If the version method is called, the replace will die loudly if the version does not match. Thus, an external patch to a module will fail if that module is upgraded. This seems much safer than our current alternatives. Is this a reasonable approach or am I missing something?

If you can detect the broken functionality programatically - maybe only apply the patch in this instance?

Adrian (hoping it's not Test::Class that's causing the problems :-)

No, it's not Test::Class I'm carping about. Most of the time it's features I've wanted in Test::Class, not bug fixes.

As for detecting the broken functionality programmatically, the only thing I thought of was using B::Deparse to get the source code of the subroutine and create a digest for it after and compare that with the new sub. That involves embedding the digest and recreating if necessary. Also, just because the source code doesn't change doesn't mean the bug wasn't fixed elsewhere.

If I'm misunderstanding you or you have a better approach which doesn't involve running the code, please let me know. (Hmm, creating local test suites targeting external modules?)

Cheers,
Ovid

New address of my CGI Course.

If I'm misunderstanding you or you have a better approach which doesn't involve running the code, please let me know. (Hmm, creating local test suites targeting external modules?)

What I was thinking off was something like this. If Bar::foo(N) has a bug where it dies where N=0, when it should just return -1, then I might only apply my patch where eval { Bar::foo(0) } == -1 is false.

Of course this relies on whatever broken behaviour we're looking for being cheap to test for with no side effects. Make vague sense?

Good point.

Are there many modules out there that:

1. Have unresponsive maintaners and
2. Are still actively developed?

In other words, if the module you're working on is so forlorn and forgotten that no one will respond to you, you probably don't need to worry about them changing it and breaking your override thing.

Am I wrong/missing something/snorting glue?

There is an important issue which goes beyond Ovid's elegant solution. There should be a way to take over deceased modules. Text::CSV comes to mind. Is there any group who would be willing to arbitrate here? -- I would propose that a module could only be taken if the original developer does not reply for a certain period, say three months or six months. Is this a runner or are there insuperable copyright issues?

-- Anthony Staines

I don't know of a survey that would authoritatively answer that question, but modules do frequently change hands after the original developer loses either the motivation or the time to maintain them. There's often some latency between the onset of neglect and the adoption by a new maintainer, so you can't know in advance what the future of a neglected module might be.

I think that if you can't adopt the existing CPAN module yourself, the most bulletproof solution is to roll your own.

If an author is genuinely unresponsive, asking the modules@cpan mailing list to transfer ownership to yourself can work. That's how I took over Data::Compare, whose original author seems to have fallen off the face of the planet.

In answer to your question, except for my confusion above, yes, this strikes me as a nicely balanced solution for the many cases where it isn't easy or reliable to detect the specific flaw.

Yes, you're right. version should die. However, I also realized two problems. One, some modules don't have versions (shame on them) or they get released without a version upgrade (this is common when you have a distribution where only the main module's version changes). As a result, what I should really do is this:

$override->verify(
    module => 'Broken::Module',
    verify => \&verification_callback,
    fatal  => $boolean_value,           # defaults to true
);
[download]

Cheers,
Ovid

New address of my CGI Course.

I can imagine circumstances where Broken::Module might not always be broken - say that Broken::Module::foo only fails some times. It may be that you (the user of Broken::Module) can identify when this happens, and provide test cases and workaround logic in the application.

If Broken::Module::foo is idempotent in intention and in implementation (i.e. you can call it as many times as you want with no side effects), it would make sense to call the real Broken::Module::foo first, and detect the exception or bad return value, and only then call the workaround code. Obviously this wouldn't work if Broken::Module::foo is transactional in nature.

Just a thought

Runtime is the wrong time to be checking such things. It simply should not be possible to accidently upgrade a dependancy.

If the decision is taken to upgrade one or more third party dependancies, this would usually be a part of a major release, and any breakage as a result of the upgrades would be detected long before this ever got anywhere near a production environment.

If you have to patch a dependancy locally, then it should get an unambiguous new name. Probably moving from the standard /lib path to your /in-house/ path.