pug-php / pug

Pug template engine for PHP
https://www.phug-lang.com
MIT License
387 stars 42 forks source link

read-only cache directory #226

Closed s4msung closed 4 years ago

s4msung commented 4 years ago

Hello, im trying to trim down a docker production build which is using pug-php/pug 3.2.0

What i would like to do:

Current state:

Thanks!

kylekatarnls commented 4 years ago

Hi,

Docker should has no impact.

Once you used cacheDirectory to create the PHP files, you can use Phug\Optimizer (see https://phug-lang.com/#usage) to load them skipping completely all the compiling steps of Phug (so you can safely remove pug templates and make your cache folder readonly).

I can't say much more as I have no idea of what you do in your build.

Note than we use file paths to match templates to cached files. So you must build them in the same machine and folder you read them on run-time, you can't create them then move or upload your cache directory.

s4msung commented 4 years ago

I totally overlooked the Phug\Optimizer thing while migrating, so that is now in place.

After some digging around, i resolved the slow pre caching (xdebug was enabled in the container which is used for generating the cache).

The read-only cache thing is still not working for me and im kinda confused how this works:

https://github.com/phug-php/phug/blob/master/src/Phug/Optimizer.php#L183

i don't see how $__pug_cache_file could be initialized elsewhere and it bails out with:

include(): Failed opening '' for inclusion (include_path='.:/usr/local/lib/php') in /var/www/vendor/phug/phug/src/Phug/Optimizer.php on line 227

Any suggestions? Is this a bug? should i provide more information?

s4msung commented 4 years ago

From what i can see, a read-only cache without source templates is not possible.

When the option up_to_date_check is false then it does not resolve anything, hence the empty import in the displayFile function.

I added this line:

$cachePath = rtrim($this->cacheDirectory, '\\/').DIRECTORY_SEPARATOR.$this->hashPrint($file).'.php';

after https://github.com/phug-php/phug/blob/master/src/Phug/Optimizer.php#L157 and it works for me. In my case, $file is already normalized, but for a real solution a CacheLocator should be provided which only does the normalization part and can handle a cache-only use-case.

kylekatarnls commented 4 years ago

i don't see how $__pug_cache_file could be initialized elsewhere and it bails out with

It's not, it's passed by reference toi be set from inside isExpired: https://github.com/phug-php/phug/blob/master/src/Phug/Optimizer.php#L155 So $__pug_cache_file is set here: https://github.com/phug-php/phug/blob/master/src/Phug/Optimizer.php#L166

And I agree it makes sense to also set this variable inside the if line 157.

I'm in a big process of converting Phug into a mono-repository with synchronized version and releasing the v 1.0 of all the packages. I will fix that right after that.

kylekatarnls commented 4 years ago

Here is the fix planned for phug/phug 1.0.1: https://github.com/phug-php/phug/pull/47

So using pug-php, a simple composer update will fix it once the version will be released.

kylekatarnls commented 4 years ago

1.0.1 released, please try to upgrade.

s4msung commented 4 years ago

i tried, but the version requirement of pug-php/pug prevents it:

  Problem 1
    - pug-php/pug 3.2.0 requires phug/phug ^0.3.0 -> satisfiable by phug/phug[0.3.0, 0.3.1, 0.3.2] but these conflict with your requirements or minimum-stability.
    - pug-php/pug 3.2.0 requires phug/phug ^0.3.0 -> satisfiable by phug/phug[0.3.0, 0.3.1, 0.3.2] but these conflict with your requirements or minimum-stability.
    - pug-php/pug 3.2.0 requires phug/phug ^0.3.0 -> satisfiable by phug/phug[0.3.0, 0.3.1, 0.3.2] but these conflict with your requirements or minimum-stability.
    - Installation request for pug-php/pug ^3.2 -> satisfiable by pug-php/pug[3.2.0].
kylekatarnls commented 4 years ago

Oh yes, I must publish a minor of pug-php/pug.

kylekatarnls commented 4 years ago

Published, you can remove phug/* dependencies from your composer.json and simply run composer update

s4msung commented 4 years ago

i upgraded with composer update and tested the caching. Now the checksums for the cached files are mismatching and i have no clue why. That worked before i upgraded with my small patch to Phug/Optimizer.

I checked, and both, the cacheDirectory() and Optimizer are using the same hash algo sha512/256.

Did something change in this regard?

kylekatarnls commented 4 years ago

Yes, you need to recompile the cache directory. I will do a showcase.

kylekatarnls commented 4 years ago

Here is a test to show how to use the CLI and the Optimize together: https://github.com/pug-php/pug/blob/master/tests/performance/cache.php#L75

s4msung commented 4 years ago

can you delete the *.pug files after caching, just to make sure the cached version is used?

kylekatarnls commented 4 years ago

Indeed, the test does not pass. I think it's due to the realpath call that returns false if the file does not exist, but if we don't use it, we can have mismatch of paths.

This means: it's not because files must exist than the cache files are not used. They are, .pug files are just used to calculate the path and find the cache path from the real path.

Maybe I can normalize the paths using a pure string calculation so the file does not need to exist.

kylekatarnls commented 4 years ago

The error stack trace go to the compiler and the optimizer purpose is to skip it completely. So it's worst than I thought.

kylekatarnls commented 4 years ago

'up_to_date_check ' with an extra space 🤦‍♂

s4msung commented 4 years ago

It's been a while, i hadn't much time to check back on this.

Was that the bug? Is it fixed now?

I'd like to test it again and hopefully get it working.

@kylekatarnls Btw, let me know with what you guys need help with. I'd be glad to dedicate some spare time to help out with anything to improve this project in the long term. Even if it's only just testing and finding weird bugs :D

kylekatarnls commented 4 years ago

As you may see, I sent commit to the PR #228 just yesterday and I'm working on it right now.

It's not that easy. Most of the work has to be done in phug/compiler and phug/renderer and we need to handle mutiple directories and multiple extensions, for example:

$pug = new Pug([
  'paths' => ['./vendor/views', './app/views'],
  'extensions' => ['.pug', '.view'],
]);

$pug->renderFile('my-page');

There, the compiler will try the following paths:

And returns the first one found.

So I'm considering multiple possibilities:

kylekatarnls commented 4 years ago

This requested a lot of changes but https://github.com/phug-php/phug/pull/49 should allow to remove view directories safely and even move the app directory once cached.

See the unit test: https://github.com/phug-php/phug/pull/49/files#diff-b946bb0d99b4dd58d4f5824a927b7c45R754

kylekatarnls commented 4 years ago

You can now update then safely remove the view directory after caching.