Wrong + Right = Even Worse

In a legacy project at work, we have lots of scripts that should in fact be modules. But they declare no package, use the .pl extension and call each other via require. Originally, the full paths were used in require, which constituted one of the obstacles to run the project locally. I replaced all the occurrences of

require '/path/to/some/script.pl';
[download]

with

use lib '/path/to/some';
require 'script.pl';
[download]

and I was able to run some of the scripts locally (I had to add some paths to @INC).

After several days, we noticed some of the scripts failed. I inspected the code and discovered that there was one newer OO module that was written "correctly": it lived in a .pm file and declared a package. It used lib and only required the script names.

Combining "right" and "wrong" led to "even worse": The OO module required several .pl files. All the subroutines they declared were therefore created in the namespace of the module. Later, when a calling script required the same .pl file, it already existed in %INC, so it was not read again. No import was called anywhere, so the subroutines did not appear in the main:: package.

Here are sample files so you can try it yourself:

module.pm

package module;

use warnings;
use strict;

require 'required1.pl';
subroutine();

warn 'module INC: ', $INC{'required1.pl'};

__PACKAGE__
[download]

required1.pl

#!/usr/bin/perl
use warnings;
use strict;

warn "Loading 1";

sub subroutine {
    my $file = (caller)[1];
    $file =~ s=.*/==;
    warn 'subroutine defined in ', $file;
}

subroutine();

1;
[download]

script.pl

#!/usr/bin/perl
use warnings;
use strict;

use FindBin;
use lib $FindBin::Bin;

require 'required1.pl';
use module;
warn 'script INC ', $INC{'required1.pl'};

subroutine();
[download]

Update:

The Conclusion

We now have 2 options:

Revert to the original full-path style require with no possibility to run the project locally.
Refactor everything to proper modules. 100,000+ lines.

Update2:

I just had an idea: there is a third option. In the OO module, store %INC at the beginning, do all you need, then set %INC back to what it was.

my %_INC = %INC;

# ...

%INC = %_INC;
[download]

لսႽ† ᥲᥒ⚪⟊Ⴙᘓᖇ Ꮅᘓᖇ⎱ Ⴙᥲ𝇋ƙᘓᖇ

Comment on Wrong + Right = Even Worse Select or Download Code

Replies are listed 'Best First'.
Re: Wrong + Right = Even Worse by davido (Cardinal) on Apr 03, 2014 at 15:52 UTC
I haven't tested this, but can't you do this in your .pm module? `{ local %INC = .....; # Work, profit! } # %INC reverts. # Work on things that require an unadulterated %INC... 1; # Leave work by 5:30pm.` [download] It seems like this is mostly what you are accomplishing by storing %INC's value in a lexical, and then restoring it later, except for the issue of lexical vs dynamic scoping, which is why I mention I haven't tested. ;) Dave	[reply] [d/l]
Re: Wrong + Right = Even Worse by sundialsvc4 (Abbot) on Apr 03, 2014 at 14:24 UTC
I have encountered such garbage before, and, after a lot of bouncing back-and-forth as to what to do, the decision was ... to bite the bullet and fix the damn thing, once and for all. The project was of a comparable size and, as it turns out, perhaps even-worse than what you describe, because identical subroutine-names were used in the different required-files. (So, you absolutely could not predict what a particular subroutine-call would actually do.) It took about a month to knuckle down and do the conversion, including testing and so-forth, and along the way the entire team cursed the name and the family heritage of the hack that had done this. But the application also became more-or-less stable for the very first time in its life. I would probably make the same recommendation again. It is the lesser of two evils, but status-quo is not acceptable either.
Re: Wrong + Right = Even Worse by LanX (Saint) on Apr 03, 2014 at 22:44 UTC
I know its a hack but using virtually different path for imports in modules should fix it. Create softlinks on top of the path hierarchy like `/path_4pm/to/some/.` Like that you have a migration strategy if you decide to gradually change from pl to pm. untested intuition! HTH! :) Cheers Rolf ( addicted to the Perl Programming Language)	[reply] [d/l]
Re: Wrong + Right = Even Worse (do) by tye (Sage) on Apr 04, 2014 at 06:17 UTC
To load a non-module Perl file unconditionally, you should use do, not require. (Even better is to (eventually) convert things to real modules, of course.) - tye	[reply]
Re^2: Wrong + Right = Even Worse (do) by choroba (Cardinal) on Apr 04, 2014 at 07:34 UTC
That depends on what the pseudo-modules do. Most of them define subs and initialize global variables, so do wouldn't be much better - at least in the pseudo-modules. It might work in the OO module, though. لսႽ† ᥲᥒ⚪⟊Ⴙᘓᖇ Ꮅᘓᖇ⎱ Ⴙᥲ𝇋ƙᘓᖇ	[reply]
Re^3: Wrong + Right = Even Worse (do) by tye (Sage) on Apr 04, 2014 at 13:45 UTC
Most of them define subs and initialize global variables, so do wouldn't be much better The problem you reported was that one module was including non-modules in order to get those subroutines defined in its package and that broke other places that still wanted those subs defined in their package (main). do would fix the problem you reported. - tye	[reply]
Re^4: Wrong + Right = Even Worse (do) by choroba (Cardinal) on Apr 04, 2014 at 14:58 UTC
Re: Wrong + Right = Even Worse by tobyink (Canon) on Apr 03, 2014 at 22:01 UTC
Or in the OO module... `package Foo::Bar; use strict; use warnings; { package main; require "script1.pl"; require "script2.pl"; }; ...; 1;` [download] `use Moops; class Cow :rw { has name => (default => 'Ermintrude') }; say Cow->new->name`	[reply] [d/l]
Re^2: Wrong + Right = Even Worse by choroba (Cardinal) on Apr 03, 2014 at 22:20 UTC
But in that case I would have to change all calls in the OO module from `subroutine()` to `main::subroutine()`. لսႽ† ᥲᥒ⚪⟊Ⴙᘓᖇ Ꮅᘓᖇ⎱ Ⴙᥲ𝇋ƙᘓᖇ	[reply] [d/l] [select]
Re^3: Wrong + Right = Even Worse by tobyink (Canon) on Apr 03, 2014 at 22:41 UTC
Or `::subroutine()` (the `main` is optional). That seems simpler than changing everything else though, and less hacky than trying to fool `%INC` (which will probably lead you to a whole bunch of warnings in the `redefine` category if warnings are enabled). `use Moops; class Cow :rw { has name => (default => 'Ermintrude') }; say Cow->new->name`	[reply] [d/l] [select]
Re^4: Wrong + Right = Even Worse by choroba (Cardinal) on Apr 03, 2014 at 22:44 UTC
Re: Wrong + Right = Even Worse by LanX (Saint) on Apr 03, 2014 at 23:37 UTC
> In the OO module, store %INC at the beginning, do all you need, then set %INC back to what it was. Or the other way round, you create a loop inspecting the `%module::` stash to copy all imported subs into `%main::` At the end you need a big migration strategy which doesn't shoot you in the foot if you decide one day to migrate all "pseudo-modules" to real modules. Cheers Rolf ( addicted to the Perl Programming Language)	[reply] [d/l] [select]
Re: Wrong + Right = Even Worse by LanX (Saint) on Apr 04, 2014 at 14:45 UTC
> Originally, the full paths were used in require, which constituted one of the obstacles to run the project locally. You can also put a coderref-hook in front of @INC to catch and redirect all do and require. You can apply the same technique for a life monitoring of the dependencies by logging the caller. This might help you migrate the projects. Not sure about CPAN modules doing this, see Re: How to use @INC coderef hooks (perldoc wrong) and linked threads for demos. Cheers Rolf ( addicted to the Perl Programming Language)	[reply]
Re^2: Wrong + Right = Even Worse (@INC sub) by tye (Sage) on Apr 04, 2014 at 15:52 UTC
You can also put a coderref-hook in front of @INC to catch and redirect all do and require. Not "all". It won't catch a require for an already-loaded module. - tye	[reply]
Re^3: Wrong + Right = Even Worse (@INC sub) by LanX (Saint) on Apr 04, 2014 at 16:08 UTC
One would need a private shadow %MY_INC within the catch-sub to handle it and keeping %INC clean. I'm aware this would cause problems with code accessing %INC directly, but I somehow doubt this legacy code base ever attempts to do such things. IMHO good enough for debugging. Cheers Rolf ( addicted to the Perl Programming Language)	[reply]
Re: Wrong + Right = Even Worse by sundialsvc4 (Abbot) on Apr 04, 2014 at 13:46 UTC
As I suggested earlier ... you could be faced with a well-behaved just poorly-designed program, or you could be dealing with a monster. You need to find out which one it is, first. In a word: “can all these various bits be part of the same program at the same time?” As we all know, it’s not illegitimate for a well-designed modular program to load parts of itself dynamically. Modules like UNIVERSAL::require can do a lot to de-fang this process. However, there is a lot of old legacy-software out there which was built in the times when programs couldn’t be big. (The days of overlays and such.) These programs that couldn’t be big, also couldn’t be tested as if they were big. Fast-forward a couple of decades and guess what ... they can be big, and they’re not stable. Programs that have gone through this life-cycle are often fairly intractable. If this program, as I somewhat suspect, “evolved,” then the first step that I would take is that very thorough present-state assessment ... and probably lobby for some overhaul work at a near-future date. These types of programs are often very unknown. (And if my comments turn out to be nonsensically off-the-mark, well, mea culpa, but they are based on some pretty painful memories of similar software.)


Keep It Simple, Stupid
	PerlMonks