Re: Strategy for improving install success of Inline::CPP

[Sisyphus]

----- Original Message ----- 
From: "Patrick LeBoutillier" <patrick.leboutill...@gmail.com>
To: "Sisyphus" <sisyph...@optusnet.com.au>
Cc: "David Oswald" <daosw...@gmail.com>; <inline@perl.org>
Sent: Monday, December 05, 2011 3:30 AM
Subject: Re: Strategy for improving install success of Inline::CPP

Here's the patch:

==============
--- ori/Inline-0.48_01/Inline.pm        2011-04-11 
06:10:06.000000000 -0400
+++ Inline-0.48_01/Inline.pm    2011-12-03 16:48:50.000000000 -0500
@@ -728,22 +728,33 @@
      }
    }

-    $o->create_config_file($DIRECTORY)
-      if not -e File::Spec->catfile($DIRECTORY, $configuration_file);
-
-    open CONFIG, "< ".File::Spec->catfile($DIRECTORY, 
$configuration_file)
-      or croak M17_config_open_failed($DIRECTORY);
-    my $config = join '', <CONFIG>;
-    close CONFIG;
-
-    croak M62_invalid_config_file(File::Spec->catfile($DIRECTORY,
$configuration_file))
-      unless $config =~ /^version :/;
-    if(UNTAINT) {
-      warn "In Inline::check_config_file(): Blindly untainting Inline
configuration file information.\n" unless
$o->{CONFIG}{NO_UNTAINT_WARN};
-      ($config) = $config =~ /(.*)/s;
-    }
-
-    %config = Inline::denter->new()->undent($config);
+       my $load_cfg = sub {
+           $o->create_config_file($DIRECTORY)
+             if not -e File::Spec->catfile($DIRECTORY, 
$configuration_file);
+
+           open CONFIG, "< ".File::Spec->catfile($DIRECTORY,
$configuration_file)
+             or croak M17_config_open_failed($DIRECTORY);
+           my $config = join '', <CONFIG>;
+           close CONFIG;
+
+           croak
M62_invalid_config_file(File::Spec->catfile($DIRECTORY,
$configuration_file))
+             unless $config =~ /^version :/;
+           if(UNTAINT) {
+             warn "In Inline::check_config_file(): Blindly untainting
Inline configuration file information.\n" unless
$o->{CONFIG}{NO_UNTAINT_WARN};
+             ($config) = $config =~ /(.*)/s;
+           }
+
+           %config = Inline::denter->new()->undent($config);
+       } ;
+
+       $load_cfg->() ;
+       if (! defined $config{languages}->{$o->{API}{language_id}}){
+        my $unlink = unlink(File::Spec->catfile($DIRECTORY,
$configuration_file));
+        if(!$unlink) {warn "Failed to remove config file"}
+        else {warn "config file removed\n" if $o->{CONFIG}{_TESTING}}
+               $load_cfg->() ;
+       }
+
    $Inline::languages = $config{languages};

    {
=============

I ran the Inline test suite an it passed, but I'm not quite sure how
to exactly reproduce the problem.
Rob/David can you give it a try?

Thanks Patrick - I think that should be fine.

One simple test (which I'll incorporate into the test suite) is to simply 
run the following code and check that we see the warning "config file 
removed":

#### bogus.pl ####
use warnings;

use Inline Config =>
   _TESTING => 1;

use Inline Bogus => <<'EOB';

foo(){}

EOB
################

With the patch included, that runs as expected and desired for me - we get 
the "config file removed" warning as the first output, which indicates that 
things are running as planned.
(Of course, we then also get the M19 error message, which is also to be 
expected for that script.)

However, it's not so easy to check that the *actual* problem is fixed. As a 
rough check:

1) Run an Inline::CPP script (that sets the "_TESTING" config option)
2) Remove all references to cpp, Cplusplus, etc. from the generated config 
file
3) Re-run the same Inline::CPP script - at which point we should see the 
"config file removed" warning, followed by the successful running of the 
script. (And if we re-examine the config file we should also find that the 
references to cpp, etc are back again.)

I haven't yet done this test myself - need to get some sleep first.

This test would not be so easy to incorporate into the Inline test suite ... 
and I'm presently thinking that it won't be.

Instead, we would just rely on the absence of any M19 errors in the 
Inline::CPP/Inline::Python cpan tester reports to demonstrate that the fix 
is working.

I'll run out a new devel release as soon as I can (ie later today, 
localtime) and, all being well, follow it up in a few days time with 
Inline-0.49.

The 'REWRITE_CONFIG_FILE' option is probably now a bit redundant, but I 
think I'll leave it there in case it's handy. (It doesn't add any 
significant overhead to anything.)

If this fix works as anticipated, then I think the M19 error message needs 
to be rewritten slightly (and I'll do just that later on today) - as there 
should now be no point in manually deleting the config file. If removal of 
that file didn't work the first time, then it's unlikely to work at a second 
attempt.

Cheers,
Rob