EPrints dies with unhelpful error message if Time::HiRes is not installed #265

Closed
MrkGrgsn opened this Issue Aug 21, 2014 · 3 comments

Comments

Projects
None yet
2 participants
@MrkGrgsn
Contributor

MrkGrgsn commented Aug 21, 2014

If Time::HiRes is not installed EPrints fails with a rather obtuse error message:

$ bin/epadmin
Undefined subroutine &EPrints::SystemSettings::load_config_file called at /eprints/eprints/eprints-core/bin/../perl_lib/EPrints/Config.pm line 150.
Compilation failed in require at bin/epadmin line 250.
BEGIN failed--compilation aborted at bin/epadmin line 250.

Time::HiRes is used within an eval in EPrints::Config::_bootstrap() which injects load_config_file() into EPrints::SystemSettings so the call on line 150 fails.

Removing the use Time::HiRes; line allows EPrints to run and it's not clear to me why it's required at all.

@MrkGrgsn

This comment has been minimized.

Show comment
Hide comment
@MrkGrgsn

MrkGrgsn Aug 21, 2014

Contributor

There are a couple of possible options for dealing with this:

  1. Add more useful error handling wherever eval $_bootstrap(...) is called
  2. Remove use Time::HiRes;

Option 1 will work, is safe and perhaps should be there as a good practice. Removing the dependency would be nice too but I'm unsure if this is safe to do.

Contributor

MrkGrgsn commented Aug 21, 2014

There are a couple of possible options for dealing with this:

  1. Add more useful error handling wherever eval $_bootstrap(...) is called
  2. Remove use Time::HiRes;

Option 1 will work, is safe and perhaps should be there as a good practice. Removing the dependency would be nice too but I'm unsure if this is safe to do.

@MrkGrgsn

This comment has been minimized.

Show comment
Hide comment
@MrkGrgsn

MrkGrgsn Aug 21, 2014

Contributor

Patch for option 1. Adds error handling and ensures package code returned by _bootstrap() returns true from correct execution:

diff --git a/perl_lib/EPrints/Config.pm b/perl_lib/EPrints/Config.pm
index ffcea88..b261cf3 100644
--- a/perl_lib/EPrints/Config.pm
+++ b/perl_lib/EPrints/Config.pm
@@ -140,7 +140,7 @@ sub load_system_config
                ${"EPrints::SystemSettings::config"} = $SYSTEMCONF;
        }

-       eval &_bootstrap( "EPrints::SystemSettings" );
+       eval &_bootstrap( "EPrints::SystemSettings" ) or die $@;

        # we want to sort by filename because we interleave files from default and
        # custom locations
@@ -269,7 +269,7 @@ sub load_repository_config_module
                ${"EPrints::Config::${id}::config"} = $info;
        }

-       eval &_bootstrap( "EPrints::Config::".$id );
+       eval &_bootstrap( "EPrints::Config::".$id ) or die $@;

        # we want to sort by filename because we interleave files from default and
        # custom locations
@@ -317,6 +317,8 @@ sub load_config_file
 }
 }

+1;
+
 EOP
 }
Contributor

MrkGrgsn commented Aug 21, 2014

Patch for option 1. Adds error handling and ensures package code returned by _bootstrap() returns true from correct execution:

diff --git a/perl_lib/EPrints/Config.pm b/perl_lib/EPrints/Config.pm
index ffcea88..b261cf3 100644
--- a/perl_lib/EPrints/Config.pm
+++ b/perl_lib/EPrints/Config.pm
@@ -140,7 +140,7 @@ sub load_system_config
                ${"EPrints::SystemSettings::config"} = $SYSTEMCONF;
        }

-       eval &_bootstrap( "EPrints::SystemSettings" );
+       eval &_bootstrap( "EPrints::SystemSettings" ) or die $@;

        # we want to sort by filename because we interleave files from default and
        # custom locations
@@ -269,7 +269,7 @@ sub load_repository_config_module
                ${"EPrints::Config::${id}::config"} = $info;
        }

-       eval &_bootstrap( "EPrints::Config::".$id );
+       eval &_bootstrap( "EPrints::Config::".$id ) or die $@;

        # we want to sort by filename because we interleave files from default and
        # custom locations
@@ -317,6 +317,8 @@ sub load_config_file
 }
 }

+1;
+
 EOP
 }

jiadiyao added a commit that referenced this issue Mar 17, 2015

fix issue 265 #265 EPrints dies with unhelpful error message if Time:…
…:HiRes is not installed. code provided by MrkGrgsn
@jiadiyao

This comment has been minimized.

Show comment
Hide comment
@jiadiyao

jiadiyao Mar 17, 2015

Contributor

fixed 12d2b19

Contributor

jiadiyao commented Mar 17, 2015

fixed 12d2b19

@jiadiyao jiadiyao closed this Mar 17, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment