From: Peter Rabbitson <ribasushi@cpan.org>
Date: Fri, 6 Mar 2015 16:08:17 +0000 (+0100)
Subject: Rewire OptDeps to not attempt any module loads under missing envvars
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=250d9e552fcad5491aec100f2fbf6a4c63edbb3c;p=dbsrgits%2FDBIx-Class-Historic.git

Rewire OptDeps to not attempt any module loads under missing envvars

The thinking is that if some envvars are missing, it is of secondary concern
to provide the exact list of missing module reqs to the user, while the
downside can be rather severe in some cases (broken DBD::Oracle on windows is
particularly obnoxious). Thus the logic for req_missing_for switched to only
look for "definitely missing" modules via a simple @INC-walk whenever it is
already clear that the req group failed
---

diff --git a/lib/DBIx/Class/Optional/Dependencies.pm b/lib/DBIx/Class/Optional/Dependencies.pm
index 56f04dc..d1d8bba 100644
--- a/lib/DBIx/Class/Optional/Dependencies.pm
+++ b/lib/DBIx/Class/Optional/Dependencies.pm
@@ -770,7 +770,11 @@ sub req_missing_for {
   my ($self, $groups) = @_;
 
   my $reqs = $self->_groups_to_reqs($groups);
-  my $mods_missing = $self->modreq_missing_for($groups);
+
+  my $mods_missing = $reqs->{missing_envvars}
+    ? $self->_list_physically_missing_modules( $reqs->{modreqs} )
+    : $self->modreq_missing_for($groups)
+  ;
 
   return '' if
     ! $mods_missing
@@ -1085,7 +1089,7 @@ sub _groups_to_reqs {
 }
 
 
-# this method tries to load specified modreqs and returns a hashref of
+# this method tries to find/load specified modreqs and returns a hashref of
 # module/loaderror pairs for anything that failed
 sub _errorlist_for_modreqs {
   # args supposedly already went through _groups_to_reqs and are therefore sanitized
@@ -1110,6 +1114,36 @@ sub _errorlist_for_modreqs {
   $ret;
 }
 
+# Unlike the above DO NOT try to load anything
+# This is executed when some needed envvars are not available
+# which in turn means a module load will never be reached anyway
+# This is important because some modules (especially DBDs) can be
+# *really* fickle when a require() is attempted, with pretty confusing
+# side-effects (especially on windows)
+sub _list_physically_missing_modules {
+  my ($self, $modreqs) = @_;
+
+  # in case there is a coderef in @INC there is nothing we can definitively prove
+  # so short circuit directly
+  return '' if grep { length ref $_ } @INC;
+
+  my @definitely_missing;
+  for my $mod (keys %$modreqs) {
+    (my $fn = $mod . '.pm') =~ s|::|/|g;
+
+    push @definitely_missing, $mod unless grep
+      # this should work on any combination of slashes
+      { $_ and -d $_ and -f "$_/$fn" and -r "$_/$fn" }
+      @INC
+    ;
+  }
+
+  join ' ', map
+    { $modreqs->{$_} ? qq("$_~>=$modreqs->{$_}") : $_ }
+    sort { lc($a) cmp lc($b) } @definitely_missing
+  ;
+}
+
 
 # This is to be called by the author only (automatically in Makefile.PL)
 sub _gen_pod {
diff --git a/t/00describe_environment.t b/t/00describe_environment.t
index 32345ca..8abb7b9 100644
--- a/t/00describe_environment.t
+++ b/t/00describe_environment.t
@@ -132,7 +132,12 @@ req_mod $_ for sort
   { ($load_weights->{$b}||0) <=> ($load_weights->{$a}||0) }
   keys %{
     DBIx::Class::Optional::Dependencies->req_list_for([
-      keys %{DBIx::Class::Optional::Dependencies->req_group_list}
+      grep
+        # some DBDs are notoriously problematic to load
+        # hence only show stuff based on test_rdbms which will
+        # take into account necessary ENVs
+        { $_ !~ /^rdbms_/ }
+        keys %{DBIx::Class::Optional::Dependencies->req_group_list}
     ])
   }
 ;
diff --git a/xt/extra/internals/optional_deps.t b/xt/extra/internals/optional_deps.t
index bf342f9..9cef633 100644
--- a/xt/extra/internals/optional_deps.t
+++ b/xt/extra/internals/optional_deps.t
@@ -42,7 +42,7 @@ is_deeply (
 {
 
 # make module loading impossible, regardless of actual libpath contents
-  local @INC = (sub { confess('Optional Dep Test') } );
+  local @INC;
 
 # basic test using the deploy target
   for ('deploy', ['deploy']) {
@@ -78,8 +78,8 @@ is_deeply (
 
     like (
       DBIx::Class::Optional::Dependencies->modreq_errorlist_for ($_)->{'SQL::Translator'},
-      qr/Optional Dep Test/,
-      'custom exception found in errorlist',
+      qr|\QCan't locate SQL/Translator.pm|,
+      'correct "unable to locate"  exception found in errorlist',
     );
 
     #make it so module appears loaded