[GH-ISSUE #304] Drop scripts/config_gen.php #263

Closed
opened 2026-02-25 21:34:35 +03:00 by kerem · 5 comments
Owner

Originally created by @iredmail on GitHub (Nov 20, 2018).
Original GitHub issue: https://github.com/cypht-org/cypht/issues/304

Originally assigned to: @jasonmunro on GitHub.

Dear Jason,

I checked what scripts/config_gen.php does. Since i have more experience with Roundcube, i’d like to compare cypht with Roundcube, and share my thoughts below.

I understand dropping scripts/config_gen.php may leads to big changes in other php files, but i believe it's better drop it as early as possible, and use php file as config file directly.

check_php(): Check PHP for correct support

  • We can list all required php modules in Cypht installation tuturial.
  • This can be a one-time check before/during cypht installation. if we already have required php modules installed, no need to check it again and again.

parse_module_ini_files(): include module ini files in the main config

Roundcube uses php file as config file, this is very easy for solution integrator and sysadmin:

  • it’s easy to read settings in other php files.
  • no need to convert .ini to other format.
  • When sysadmin updates one parameter, new value works right after he/she quits the text editor.
  • It avoids extra command and workflow like running command config_gen.php to generate a final config file. This saves admin's time, but most importantly, it doesn’t require admin to remember extra procedure to update a parameter. The easier the better, the simpler the better.
  • it’s easy to load extra config files with php directive like require_once at the end of main config file. as a solution provider, we pre-define some settings in main config file, also has a require_once directive at the bottom of main config file to load admin’s custom config file. this way we can achieve fearless upgrade.

build_config(): Entry point into the configuration process

Maybe move PHP function existence checks to check_php()? But my purpose is dropping this whole file directly. haha

site settings

Why not simply check and load extra config file like <site>.ini (what i really mean is modular config file <site>.inc.php)?

Benefits of config file generated by config_gen.php?

  • What's the benefit of generating config file with config_gen.php? better performance or any advantages?
  • Do we really need to care about these benefits?

Roundcube uses php files as config files directly, i didn't see any performance issue at all.

Originally created by @iredmail on GitHub (Nov 20, 2018). Original GitHub issue: https://github.com/cypht-org/cypht/issues/304 Originally assigned to: @jasonmunro on GitHub. Dear Jason, I checked what `scripts/config_gen.php` does. Since i have more experience with Roundcube, i’d like to compare cypht with Roundcube, and share my thoughts below. I understand dropping `scripts/config_gen.php` may leads to big changes in other php files, but i believe it's better drop it as early as possible, and use php file as config file directly. ## `check_php()`: Check PHP for correct support - We can list all required php modules in Cypht installation tuturial. - This can be a one-time check before/during cypht installation. if we already have required php modules installed, no need to check it again and again. ## `parse_module_ini_files()`: include module ini files in the main config Roundcube uses php file as config file, this is very easy for solution integrator and sysadmin: - it’s easy to read settings in other php files. - no need to convert .ini to other format. - When sysadmin updates one parameter, new value works right after he/she quits the text editor. - It avoids extra command and workflow like running command `config_gen.php` to generate a final config file. This saves admin's time, but most importantly, it doesn’t require admin to remember extra procedure to update a parameter. The easier the better, the simpler the better. - it’s easy to load extra config files with php directive like `require_once` at the end of main config file. as a solution provider, we pre-define some settings in main config file, also has a `require_once` directive at the bottom of main config file to load admin’s custom config file. this way we can achieve fearless upgrade. ## `build_config()`: Entry point into the configuration process Maybe move PHP function existence checks to `check_php()`? But my purpose is dropping this whole file directly. haha ## site settings Why not simply check and load extra config file like `<site>.ini` (what i really mean is modular config file `<site>.inc.php`)? ## Benefits of config file generated by `config_gen.php`? - What's the benefit of generating config file with `config_gen.php`? better performance or any advantages? - Do we really need to care about these benefits? Roundcube uses php files as config files directly, i didn't see any performance issue at all.
kerem 2026-02-25 21:34:35 +03:00
  • closed this issue
  • added the
    framework
    label
Author
Owner

@jasonmunro commented on GitHub (Nov 20, 2018):

@iheinrich I appreciate the feedback and your thoughts on this. You are not the only one who does not like the config_gen script :) It would be a very large change to try to remove it, and there would be a performance penalty. The most important thing it does is build a map of modules to page ids, so it calculates a map of assigned modules to the request identifier and stores that in the generated output. Removing this step would require calculating this on each request which I would very much prefer not to do. It's also a large data structure that would be difficult to hard code into a php config file.

Another thing the script does is prepare the production site and combine and minify browser assets like javascript and css includes. I can use cypht on my phone over 3G and the performance is excellent. The entire Cypht app (depending on what modules you have activated and what options enabled) is on average less than 50KB to the browser with only a single js and single css include.

One of Cypht's design goals is to be lightweight on the server and the client, and the config gen script is an important part of what makes that happen. As I said, less than 50K sent to the browser on a typical page load, and on the server a Cypht request on average uses less than 3 MB of memory. I understand this might not be important for your use case(s), but it is for mine :) I'm not saying there could not be a better way, and if someone is interested in doing the work and submitting a PR I would definitely consider it, however at this time I am not going to remove the script.

I usually deploy the latest Cypht code several times a week and it's not hard to automate away the steps to do so in an update.sh type script such that I only have to run a single command to do the update.

<!-- gh-comment-id:440366680 --> @jasonmunro commented on GitHub (Nov 20, 2018): @iheinrich I appreciate the feedback and your thoughts on this. You are not the only one who does not like the config_gen script :) It would be a very large change to try to remove it, and there would be a performance penalty. The most important thing it does is build a map of modules to page ids, so it calculates a map of assigned modules to the request identifier and stores that in the generated output. Removing this step would require calculating this on each request which I would very much prefer not to do. It's also a large data structure that would be difficult to hard code into a php config file. Another thing the script does is prepare the production site and combine and minify browser assets like javascript and css includes. I can use cypht on my phone over 3G and the performance is excellent. The entire Cypht app (depending on what modules you have activated and what options enabled) is on average less than 50KB to the browser with only a single js and single css include. One of Cypht's design goals is to be lightweight on the server and the client, and the config gen script is an important part of what makes that happen. As I said, less than 50K sent to the browser on a typical page load, and on the server a Cypht request on average uses less than 3 MB of memory. I understand this might not be important for your use case(s), but it is for mine :) I'm not saying there could not be a better way, and if someone is interested in doing the work and submitting a PR I would definitely consider it, however at this time I am not going to remove the script. I usually deploy the latest Cypht code several times a week and it's not hard to automate away the steps to do so in an update.sh type script such that I only have to run a single command to do the update.
Author
Owner

@ulfgebhardt commented on GitHub (Nov 21, 2018):

Regarding minifying css and js files:

I do the following:

  1. Collect all css/js files >here
  2. Generate an Identifier for each file used >here
  3. Generate a Hash for all Identifiers combined >here
  4. Check Cache >here
  5. OnMiss: Minify them >here
  6. OnMiss: Cache them into a folder >here
  7. Serve single file for each js/css >here

This way the minify process takes place only once and on the fly, but generates an overhead for the collecting, identifying of files and hashing of the resulting string.

<!-- gh-comment-id:440629241 --> @ulfgebhardt commented on GitHub (Nov 21, 2018): Regarding minifying css and js files: I do the following: 1. Collect all css/js files [>here](https://github.com/webcraftmedia/system/blob/master/sai/page/default_page.php#L99) 2. Generate an Identifier for each file used [>here](https://github.com/webcraftmedia/system/blob/master/cache/cache_js.php#L50) 3. Generate a Hash for all Identifiers combined [>here](https://github.com/webcraftmedia/system/blob/master/cache/cache_js.php#L51) 4. Check Cache [>here](https://github.com/webcraftmedia/system/blob/master/cache/cache_js.php#L79) 5. OnMiss: Minify them [>here](https://github.com/webcraftmedia/system/blob/master/cache/cache_js.php#L80) 6. OnMiss: Cache them into a folder [>here](https://github.com/webcraftmedia/system/blob/master/cache/cache.php#L53) 7. Serve single file for each js/css [>here](https://github.com/webcraftmedia/system/blob/master/cache/cache_js.php#L85) This way the minify process takes place only once and on the fly, but generates an overhead for the collecting, identifying of files and hashing of the resulting string.
Author
Owner

@zeigerpuppy commented on GitHub (Nov 26, 2018):

Thanks @jasonmunro for the explanation. The main advantage that I see in cypht over other webclients is it's speed and consistency in the UI. The handling of folders is also excellent.
As a sysadmin, I don't find the build script to be such a big issue; deployment usually involves a script or two anyway.
I'm also a huge fan of the use of a build process if it means a smaller attack surface (I would hazard a wager that cypht is more secure-by-design than roundcube!).

<!-- gh-comment-id:441684831 --> @zeigerpuppy commented on GitHub (Nov 26, 2018): Thanks @jasonmunro for the explanation. The main advantage that I see in cypht over other webclients is it's speed and consistency in the UI. The handling of folders is also excellent. As a sysadmin, I don't find the build script to be such a big issue; deployment usually involves a script or two anyway. I'm also a huge fan of the use of a build process if it means a smaller attack surface (I would hazard a wager that cypht is more secure-by-design than roundcube!).
Author
Owner

@jasonmunro commented on GitHub (Nov 27, 2018):

@zeigerpuppy thanks for the feedback. You are correct that the build process reduces the attack surface as well. With the exception of assets required by module sets (only 2 come to mind, the SMTP set for HTML and Markdown formatting, and the themes css files), only 3 files are inside the document root when running Cypht in production mode: the auto-generated index.php file, and the combined site.js and site.css files.

We don't use any writable directories in the document root as that gives attackers a handy target for code injection (I'm honestly surprised by how many web apps allow this), but we also get some extra protection by keeping 99% of the php code outside the document root. I have seen many exploits over the years that started by loading a php file directly in the browser that was not intended to be used that way.

<!-- gh-comment-id:442186537 --> @jasonmunro commented on GitHub (Nov 27, 2018): @zeigerpuppy thanks for the feedback. You are correct that the build process reduces the attack surface as well. With the exception of assets required by module sets (only 2 come to mind, the SMTP set for HTML and Markdown formatting, and the themes css files), only 3 files are inside the document root when running Cypht in production mode: the auto-generated index.php file, and the combined site.js and site.css files. We don't use any writable directories in the document root as that gives attackers a handy target for code injection (I'm honestly surprised by how many web apps allow this), but we also get some extra protection by keeping 99% of the php code outside the document root. I have seen many exploits over the years that started by loading a php file directly in the browser that was not intended to be used that way.
Author
Owner

@jasonmunro commented on GitHub (Feb 13, 2019):

I appreciate this feedback, and completely understand the reasoning behind the request, but at this time I am not going to remove the build process. Thanks to everyone for the feedback. I'm going to close this issue as a part of just general housekeeping.

<!-- gh-comment-id:463058956 --> @jasonmunro commented on GitHub (Feb 13, 2019): I appreciate this feedback, and completely understand the reasoning behind the request, but at this time I am not going to remove the build process. Thanks to everyone for the feedback. I'm going to close this issue as a part of just general housekeeping.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/cypht#263
No description provided.