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

Magento modules symlinked into vendor from outside of the Magento Basedir are not watched by this module #117

Open
gwharton opened this issue Aug 25, 2024 · 4 comments

Comments

@gwharton
Copy link

gwharton commented Aug 25, 2024

The

\Magento\Framework\Component\ComponentRegistrar->getPaths('module')

command returns paths outside of the magento basedir when modules are symlinked in by composer into vendor from outside of the magento basepath.

The removal of basepath from these modules path (who's path doesnt start with basepath) causes the path to be butchered by removing a fixed length string (length of basepath + 1) from the beginning of the path.

(defn- list-components-cmd
"Returns the PHP command to run to output a list of all Magento components of the given type.
Currently the base path is removed from the beginning of the paths, only to be added
again later in `list-component-dirs`. This doesn't make sense, except that I'm thinking
about allowing a different Magento base dir path for PHP commands to be specified by
the user. In that case the magento-basedir provided to this method would be a
different one than the one added back on later.
If I decide against this feature then I can remove this logic again and let PHP
return complete paths directly again.
Reference https://github.com/mage2tv/magento-cache-clean/issues/33#issuecomment-479791859"
[magento-basedir type]
(let [composer-autoload (str magento-basedir "vendor/autoload.php")]
(when-not (fs/exists? composer-autoload)
(throw (ex-info (str "Composer autoload.php not found: " composer-autoload) {})))
(unescape-php-var-on-win-os
(str "php -r "
"\"require '" composer-autoload "'; "
"\\$bp = strlen(dirname(dirname(realpath('" composer-autoload "')))) + 1; "
"foreach ((new \\Magento\\Framework\\Component\\ComponentRegistrar)->getPaths('" type "') as \\$m) "
"echo substr(\\$m, \\$bp).PHP_EOL;\""))))

@Vinai
Copy link
Contributor

Vinai commented Aug 26, 2024

Thanks for the issue. As stated in the comment linked from the function doc, if the method doesn’t return a relative path, it causes issues with instances in a virtualized environment, if the cleaner is run outside the virtual environment, and the path to the base dir differs in the container and outside.

I currently see no easy way to change that while still supporting that kind of scenario.

@gwharton
Copy link
Author

Yeah, I can see the dilema. Its unfortunate as symlinking in external modules into vendor, is core composer functionality, and a common development use case.

@gwharton
Copy link
Author

Workaround is to ensure that third party/external/shared modules are checkout out under the magento base root, e.g <magentobaseroot>/ext/<vendor>/<module> and then the file based repository path set to ext// in composer.json

The downside being if the modules are shared between several projects, you have to have multiple copies checked out.

@gwharton
Copy link
Author

If the default logic cannot be changed, then perhaps a command line argument could be the way forward to allow both schemes to coexist? Something to ponder over.

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

No branches or pull requests

2 participants