Skip to content
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

Doesn't play nice with our Docker dev env #33

Closed
peterjaap opened this issue Apr 1, 2019 · 5 comments
Closed

Doesn't play nice with our Docker dev env #33

peterjaap opened this issue Apr 1, 2019 · 5 comments
Labels

Comments

@peterjaap
Copy link

peterjaap commented Apr 1, 2019

We use JeroenBoersma/docker-compose-development

When starting the cache-cleaner, this is what I get;

$ vendor/bin/cache-clean.js --watch                                         
Release 0.0.35 sponsored by https://www.mage2.tv

PHP Parse error:  syntax error, unexpected end of file in Command line code on line 1
[ERROR] Command failed: php -r "require '/home/peterjaap/development/workspace/client/magento2/vendor/autoload.php'; foreach ((new \Magento\Framework\Component\ComponentRegistrar)->getPaths('module') as \$m) echo \$m.PHP_EOL;"
PHP Parse error:  syntax error, unexpected end of file in Command line code on line 1

I guess this is because the absolute path isn't correct because PHP runs in a Docker container. The valid path in the container is /data/client/magento2/.....

I tried this;

$ vendor/bin/cache-clean.js --directory /data/client/magento2 --watch
Release 0.0.35 sponsored by https://www.mage2.tv

[ERROR] The Magento directory "/data/client/magento2" does not exist

But that errors out because the directory doesn't exist on the local system. I tried symlinking it, but it gives the same error (it probably checks for a directory and a symlink doesn't count).

When I copy the actual dir to that location on my local machine and run this, it doesn't work either;

$ vendor/bin/cache-clean.js --directory /data/client/magento2 --watch

PHP Parse error:  syntax error, unexpected end of file in Command line code on line 1
[ERROR] Command failed: php -r "require '/data/client/magento2/vendor/autoload.php'; foreach ((new \Magento\Framework\Component\ComponentRegistrar)->getPaths('module') as \$m) echo \$m.PHP_EOL;"
PHP Parse error:  syntax error, unexpected end of file in Command line code on line 1

I thought that might work because the script checks that the directory exists but then actually looks in the /home/peterjaap/development/workspace/client/magento2 dir through the container.

So we may have a few options to work around this;

  • Make all paths relative
  • Support symlinks
  • Add a flag to check the directory-does-not-exist warning

Thoughts?

@Vinai
Copy link
Contributor

Vinai commented Apr 1, 2019

Hey @peterjaap, happy to hear you are trying out the cache-clean tool!
There is a related issue that I'm working on (#31) which allows exporting the relevant Magento configuration (cache config, themes and modules) into a JSON file. If that file var/cache-clean-config.json exists, the utility will just use that file as input instead of shelling out to PHP.

I'm currently using a simple PHP script to generate the file.
Once it works, I'll add a separate optional Magento module that will automatically rewrite the file when new modules are added.

Here is the current first version of the script:

<?php declare(strict_types=1);

$magento_basedir = rtrim($argv[1] ?? '.', '/');

$magento_env = $magento_basedir . '/app/etc/env.php';
$composer_autoload = $magento_basedir . '/vendor/autoload.php';
$output_file = $magento_basedir . '/var/cache-clean-config.json';

if (in_array($magento_basedir, ['-h', '--help'], true)) {
    echo "Usage: {$argv[0]} [path/to/magento]\n";
    exit(1);
}

if (! file_exists($magento_env)) {
    fwrite(STDERR, "[ERROR] Unable to find configuration \"$magento_env\"\n");
    exit(1);
}

if (! file_exists($composer_autoload)) {
    fwrite(STDERR, "[ERROR] Unable to find composer autoload file \"$composer_autoload\"\n");
    exit(1);
}

require $composer_autoload;

$registrar = new \Magento\Framework\Component\ComponentRegistrar();

$config = [
   'app' => require $magento_env,
   'modules' => array_values($registrar->getPaths('module')),
   'themes' => array_values($registrar->getPaths('theme')),
];

if (! is_dir(dirname($output_file))) {
    mkdir(dirname($output_file), 0777, true);
}

file_put_contents(
    $output_file,
    json_encode($config, JSON_PRETTY_PRINT|JSON_UNESCAPED_SLASHES) . PHP_EOL
);

echo "Wrote configuration to $output_file\n";

Any thoughts on how it could be made more useful in your context?

The file would probably need to be generated by PHP inside the container.
Are you running the cache-clean.js utility on a regular node process on the host system? In that case it would need to be synchronized from the guest to the host system.
Alternatively I could add an option to dump the output to stdout. In that case you could run the script in docker and pipe the output to var/cache-clean-config.json on the host.

@Vinai Vinai added more-info-required Further information is requested docker labels Apr 1, 2019
@Vinai
Copy link
Contributor

Vinai commented Apr 2, 2019

Hi @peterjaap, I've implemented a prototype of the approach described above.
Maybe you can try it out and provide feedback when you have a chance? I would appreciate it very much!
More info can be found here: #31 (comment)

@Vinai Vinai removed the more-info-required Further information is requested label Apr 4, 2019
@Vinai
Copy link
Contributor

Vinai commented Apr 4, 2019

Note to self:

With the changes introduced in the scope of issue #31 the paths exported from PHP to the node watcher in the file are "relativized" in relation to the Magento base directory.
The node process watcher then prefixes the base-path it knows to the exported paths.

This allows for interoperability in virtualized environments where the PHP process basedir path is different from the node process basedir path.

When the process shells out to PHP to read the list of modules and themes, it still gets absolute paths.
If the same approach as with the config dump file where to be followed, the dump might not even be necessary in many cases.

On the downside, it makes the inline command that is passed to PHP in cache.config.php/list-components-cmd more complex, as the Magento basedir path has to be removed from each module and theme directory path.

After having a go at implementing this I realize there is a problem:
The node script builds the PHP to dump the config, including the call to require vendor/autoload.php. The path to this file is built using the base path the node process knows.
So unless I add an argument to the watcher that allows specifying a different base path for the PHP process this won't work.

Not sure if it's a good idea to do that, as too many options make a tool hard to use. I'll let this issue sit for a while to think about it, especially since there is a workable solution with the dump file in place soon.

I think it's worth making the adjustment though as it will benefit the developer experience with the watcher "just working" under more circumstances.

@Vinai
Copy link
Contributor

Vinai commented Apr 25, 2019

@peterjaap Even though I have closed issue #31, I still would very much appreciate getting your feedback on release 0.0.41, if you expose the varnish port to receive purge requests from the host system and try and tweak the config export JSON file according to your needs.
I'm pretty positive you will be able to have it functional without too much work.

@peterjaap
Copy link
Author

Works! 🎉

See JeroenBoersma/docker-compose-development#92

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants