Skip to content

Commit

Permalink
address potential arithmatic warnings in PHP 7.1+; related fuel/core#…
Browse files Browse the repository at this point in the history
  • Loading branch information
WanWizard committed Mar 6, 2017
1 parent a26d360 commit 30dcb1f
Showing 1 changed file with 15 additions and 9 deletions.
24 changes: 15 additions & 9 deletions classes/generate.php
Original file line number Diff line number Diff line change
Expand Up @@ -1771,20 +1771,26 @@ private static function _find_migration_number()
}

$files = new \GlobIterator($base_path .'migrations/*_*.php');

try
if ($files->count())
{
$migrations = array();
foreach($files as $file)
try
{
$migrations[] = $file->getPathname();
$migrations = array();
foreach($files as $file)
{
$migrations[] = $file->getPathname();
}
sort($migrations);
list($last) = explode('_', basename(end($migrations)));
}
catch (\LogicException $e)
{
throw new Exception("Unable to read existing migrations. Path does not exist, or you may have an 'open_basedir' defined");
}
sort($migrations);
list($last) = explode('_', basename(end($migrations)));
}
catch (\LogicException $e)
else
{
throw new Exception("Unable to read existing migrations. Path does not exist, or you may have an 'open_basedir' defined");
$last = 0;
}

return str_pad($last + 1, 3, '0', STR_PAD_LEFT);
Expand Down

5 comments on commit 30dcb1f

@itcom
Copy link

@itcom itcom commented on 30dcb1f Mar 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It occurs in PHP 5.4.16

Uncaught exception LogicException: 0 - The parent constructor was not called: the object is in an invalid state in /home/web/api19/fuel/packages/oil/classes/generate.php on line 2037

diff --git a/classes/generate.php b/classes/generate.php
index 4ba3aa5..5e3b769 100644
--- a/classes/generate.php
+++ b/classes/generate.php
@@ -2034,7 +2034,7 @@ CLASS;
                }

                $files = new \GlobIterator($base_path .'migrations/*_*.php');
-               if ($files->count())
+               if (count(iterator_to_array($files)))
                {
                        try
                        {

Please see the URL, HTH.
https://bugs.php.net/bug.php?id=55701

@WanWizard
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PHP 5.4 has been end of life for about 2,5 years now?

@itcom
Copy link

@itcom itcom commented on 30dcb1f Mar 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the market share of PHP 5.x is still high on the site,
I think that it is better to correspond.

https://w3techs.com/technologies/details/pl-php/all/all
https://w3techs.com/technologies/details/pl-php/5/all

In CentOS 7, the default is PHP 5.4.16.

@WanWizard
Copy link
Member Author

@WanWizard WanWizard commented on 30dcb1f Mar 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two issues with GlobIterator:

  • it throws a LogicException with message 'The parent constructor was not called' on its first operation when the glob expression doesn't match any file
  • It throws a LogicException on the first operation after the iteration completes, when the glob expression does match some files

According to the PHP team, both are not considered a bug, which I think might be open for debate.

It's probably best to just catch the LogicException, and take it count = 0 when that happens. I'll commit a fix, and scan the codebase for other uses of GlobIterator.

@WanWizard
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.