Skip to content

chore: add utils/check-file-permissions script #9558

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

paulbalandan
Copy link
Member

@paulbalandan paulbalandan commented May 14, 2025

Description
A pure Bash alternative to utils/check_permission_x.php that won't rely on composer update.
Replaces #9556

Failing test: https://github.com/codeigniter4/CodeIgniter4/actions/runs/15022857097/job/42216167612?pr=9558
image

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@paulbalandan paulbalandan added the github_actions Pull requests that update Github_actions code label May 14, 2025
Copy link
Collaborator

@ddevsr ddevsr left a comment

Choose a reason for hiding this comment

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

Thankyou👍

@michalsn
Copy link
Member

I'm confused - there was an issue with the PHP version earlier, but now there's no issue with the Bash version? Wasn't the problem related to the checkout action?

Also, you're referring to composer update, but I don't see any reference to that in the action logs. Am I missing something?

@paulbalandan
Copy link
Member Author

Here's the Github Action error: https://github.com/codeigniter4/CodeIgniter4/actions/runs/15002007798/job/42151150139
Although it throws an error, it exited successfully so we did not catch that in the pipeline.

Basically, the PHP scripts requires system/Test/bootstrap.php so as to autoload things. However, apparently, the autoloader does not look at system/ThirdParty/ to locate some interface (meaning it looks at vendor/). You can verify this by deleting the vendor/ then running php utils/check_permissions_x.php. The previous PR basically attempts to composer update the workspace so it will have the vendor/ dir present but that would be a waste of resources in my opinion.

@michalsn
Copy link
Member

Thanks for the explanation. It looks like we missed one spot when upgrading the Escaper. The new interface should also be added here: https://github.com/codeigniter4/CodeIgniter4/blob/develop/system/Config/AutoloadConfig.php#L110 - otherwise, users who don’t use composer and download the ZIP package will encounter this error as well.

@michalsn
Copy link
Member

It’s unfortunate that PHP errors don’t cause the github action to fail. Still, I’m not sure we should drop the current script - this is probably the only place where we "test" things without using composer. So even though the action doesn’t fail with a proper message, it still helped us catch the issue. It won't be the case with a bash script.

@paulbalandan
Copy link
Member Author

Thanks for spotting the issue with AutoloadConfig. I understand the concern and will close this after.

After looking at the trace, it seems our reliance on system/Test/bootstrap.php is the likely culprit. In the past, we are using the test bootstrap as it provides a more complete autoloading compared to system/bootstrap.php. That's why almost all of our non-test PHP scripts are requiring system/Test/bootstrap.php. However, the downside, as it turns out, is the environment is automatically set to testing, and in testing environment, the exception handler always exit gracefully.

With the presence of the Boot class, I'm thinking we can create a util_bootstrap.php (or whatever filename) that will be the bootstrap point for all non-test related things. What do you think?

@paulbalandan
Copy link
Member Author

The changes here will be moved to that bootstrap resulting in clearer errors:

diff --git a/utils/check_permission_x.php b/utils/check_permission_x.php
index 84f4b00a1b..073d0740ca 100644
--- a/utils/check_permission_x.php
+++ b/utils/check_permission_x.php
@@ -13,13 +13,19 @@ declare(strict_types=1);
 
 namespace Utils;
 
-require __DIR__ . '/../system/Test/bootstrap.php';
+require __DIR__ . '/../app/Config/Paths.php';
+require __DIR__ . '/../system/Boot.php';
 
+use CodeIgniter\Boot;
 use CodeIgniter\CLI\CLI;
+use Config\Paths;
 use RecursiveDirectoryIterator;
 use RecursiveIteratorIterator;
 use RuntimeException;
 
+define('FCPATH', __DIR__ . '/../public/');
+Boot::bootWeb(new Paths());
+
 function findExecutableFiles($dir)
 {
     $execFileList = [
php utils/check_permission_x.php

[Error]
Interface "Laminas\Escaper\EscaperInterface" not found
at SYSTEMPATH/ThirdParty/Escaper/Escaper.php:33

Backtrace:
  1    SYSTEMPATH/Autoloader/Autoloader.php:316
       include_once()

  2    SYSTEMPATH/Autoloader/Autoloader.php:258
       CodeIgniter\Autoloader\Autoloader()->includeFile('/Users/paul/Workspace/CodeIgniter4/system/ThirdParty/Escaper/Escaper.php')

  3    SYSTEMPATH/Common.php:443
       CodeIgniter\Autoloader\Autoloader()->loadClassmap('Laminas\\Escaper\\Escaper')

  4    SYSTEMPATH/Router/RouteCollection.php:1447
       esc('/')

  5    SYSTEMPATH/Router/RouteCollection.php:1052
       CodeIgniter\Router\RouteCollection()->create('GET', '/', 'Home::index', null)

  6    APPPATH/Config/Routes.php:8
       CodeIgniter\Router\RouteCollection()->get('/', 'Home::index')

  7    SYSTEMPATH/Router/RouteCollection.php:339
       require('/Users/paul/Workspace/CodeIgniter4/app/Config/Routes.php')

  8    SYSTEMPATH/CodeIgniter.php:821
       CodeIgniter\Router\RouteCollection()->loadRoutes()

  9    SYSTEMPATH/CodeIgniter.php:455
       CodeIgniter\CodeIgniter()->tryToRouteIt(null)

 10    SYSTEMPATH/CodeIgniter.php:354
       CodeIgniter\CodeIgniter()->handleRequest(null, Object(Config\Cache), false)

 11    SYSTEMPATH/Boot.php:334
       CodeIgniter\CodeIgniter()->run()

 12    SYSTEMPATH/Boot.php:67
       CodeIgniter\Boot::runCodeIgniter(Object(CodeIgniter\CodeIgniter))

 13    ROOTPATH/utils/check_permission_x.php:27
       CodeIgniter\Boot::bootWeb(Object(Config\Paths))

@michalsn
Copy link
Member

With the presence of the Boot class, I'm thinking we can create a util_bootstrap.php (or whatever filename) that will be the bootstrap point for all non-test related things. What do you think?

Sounds good to me. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
github_actions Pull requests that update Github_actions code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants