Problem/Motivation
@chike reported in slack https://drupal.slack.com/archives/C7QJNEY3E/p1669230846802479
on shared hosting there are a bunch of symlinks. Such as
./.cpanel/ea-php-cli/.ea-php-cli.cache
./.cpanel/ea-php-cli/composer-sal/.ea-php-cli.cache
./.cpanel/ea-php-cli/drush/.ea-php-cli.cache
./.cpanel/ea-php-cli/drush/vendor/drush/drush/.ea-php-cli.cache
./.cpanel/ea-php-cli/public_html/.ea-php-cli.cache
./.cpanel/ea-php-cli/vendor/bin/.ea-php-cli.cache
./.cpanel/ea-php-cli/vendor/drush/drush/.ea-php-cli.cache
Steps to reproduce
Proposed resolution
We could add a `CollectIgnoredPathsEvent` subscriber that reads a config for list of paths to exclude.
This would make it much easier for situations like that assuming `./.cpanel` is not needed for the update.
Remaining tasks
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #47 | composerjson.txt | 5.92 KB | chike |
Issue fork automatic_updates-3323461
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
wim leersThere's not enough detail in the issue summary. What are the paths in the issue summary relative to?
Comment #3
wim leersPossibly naïve questions:
composer-installed packages. So … why are we not automatically ignoring all files that aren't part of a package, no matter whether they're a symlink or not? We could do that usingcomposer status -v(see #3319679: Assert known preconditions for test runs and fail early if unmet).Comment #4
chikeLet me explain.
I am using Greengeeks shared hosting. Greengeeks, like most host companies use cPanel to manage the host plans. I installed 'automatic_updates' and got like 3 error messages which I fixed but got stuck with the one about symlinks.
From the help module I got some info how to fix the symlinks issue which I followed the steps,
I ran
composer require drupal/core-vendor-hardeningI added
"composer-exit-on-patch-failure": trueto composer.jsonI also added
to composer.json
Symlinks coming from 'drush/drush' and 'grasmash/yaml-expander' were solved but many more symlinks remained. When I checked I found the following list,
The folder structure of the site is,
As shown, 'vendor' is on root and Drupal lives inside 'public_html'.
And there are a bunch of other folders on the root added by the host provider.
I had Drush 8 on the site which I installed earlier outside vendor folder but afterwards I removed it and re-installed Drush with 'composer require drush/drush'.
Comment #5
wim leersThanks for the detailed additions, @chike, very much appreciated!
I think the most pragmatic way forward here is for one the Automatic Updates maintainers to get a one-month account with Green Geeks so we can observe and debug ourselves. Curious to hear what @tedbow thinks :)
In any case, I think this is critical: you're absolutely right that cPanel is very widely adopted. We should ensure that Automatic Updates works at least in those prior to getting into Drupal core as alpha experimental.
Automatic Updates may very well work on our development environments and pass automated tests, but that doesn't say much about the real world. This is a great example of that. Another is #3252299: Reliably support cweagans/composer-patches in Package Manager & Automatic Updates: validate stage, which @chike also encountered here. That issue is in the D10 alpha experimental roadmap. I believe this one should be too.
Comment #6
tedbow@Wim Leers re #3
Unfortunately even though something is not in the install path of a package does not mean it is not managed by Composer. For instance our
drupal/core-composer-scaffoldplugin puts index.php and other files in their places and when a core update happens these files might be updated. But if we looked for the path of these under any of theinstall_path's of the packages that Composer they would not be present. Of course we ship withdrupal/core-composer-scaffoldso we could special case these files.The problem is if we special case these files then we implicitly declaring that no other composer plugin can act like
drupal/core-composer-scaffoldand manage files outside it'sinstall_pathbecause we would not know about the files it manages.So by default package_manager has been including everything inside the Composer project expect things that explicitly excluded. We exclude paths we know should excluded like the Sqlite db file or the files folder, see the classes under package_manager/src/PathExcluder. Basically we did this because we determined there is no 100% sure way of knowing what is managed by composer. If we accidentally exclude files that are managed by composer these files will not get updated if a new version of package updates these. Since we don't know the purpose of the files we might miss it is probably best to assume they are critical.
Of course this does lead to problem in this issue.
I think there are few way to solve this
This would also mean we would miss any other file that are managed by composer in the same way the files managed by
drupal/core-composer-scaffold. Of course we could make an API for packages to let us know what files they manage outside of their install_path but that kind of defeats the purpose of using Composer because it allows us to use packages that are not drupal aware.drupal/core-composer-scaffoldbecause all it's files are under web/docroot. Then any other Composer plugin that managed files outside of install_path would be ok if they only managed files under the web root and vendorThis would probably solve the current use case but we don't 100% know there is not a Composer package being used that would manage files somewhere outside of the these folders. Also even if there is not such a package now we don't know that one might installed later and there would no way. of knowing when such as package is installed.
Comment #7
wim leersCan I summarize #6 as It's more complicated because of https://getcomposer.org/doc/articles/plugins.md?
We clearly cannot support arbitrary composer plugins, because we cannot know what their logic does.
But I don't see why we can not special case
extra.drupal-scaffold.file-mapping, and add explicit support for it? Similarly, supportingcomposer/installersseems reasonable. (Even if we restrict it to only thedrupal-*installers.)But any other
composer-pluginwe would not support. Which to me opens a question I've had in the back of my head for a while: how do we know that certain composer plugins will not break AU? For example, #3252299: Reliably support cweagans/composer-patches in Package Manager & Automatic Updates: validate stage exists to deal withcweagans/composer-patchesspecifically, but what about other composer plugins?AFAICT what we really need is a list of "known to be compatible" of composer plugins. Most will not need special attention, but some might. Of the 3 that Drupal core ships with,
drupal/core-composer-scaffoldneeds special attention.cweagans/composer-patchesclearly needs special attention, and we have no choice but to support.composer/package-versions-deprecateddoes not. Great, now we have 5 "known to be compatible" composer plugins. But then there's others that we may not need to bother with supporting, for examplephpstan/extension-installer.Once we have this, we could inform the user that "oh, sorry, you're using a composer plugin that we don't know if it breaks Automatic Updates/Package Manager, so we'd rather be safe than sorry: you won't be able to use AU/PM. Create a feature request to add support for it."
Isn't that the only way we can be truly confident about AU/PM working reliably?
Consider the above my addition to your list 🤓
Not assigning to @tedbow to also get feedback from @TravisCarden.
Comment #8
chikeThis being an issue with the Package Manager also affects Project Browser. I enabled Allow installing via UI (experimental) for Project Browser and visiting /admin/modules/browse flashed the error message,
And the Add and Install button wouldn't show up.
Comment #9
wim leersThanks for the report, @chike! Unfortunately I can't prioritize this higher than 😅 But I 100% agree this is important to resolve!
Comment #10
chikeSince this also affects Project Browser, is there need to reference this issue over there?
Comment #11
tedbow@Wim Leers re #7.
Yeah I think that would be great if we can just not support project that have unknown composer plugins. I guess I was unsure how common have other plugins would be. I was assuming it would be pretty common but perhaps I am wrong. Or maybe it won't be common for simpler sites
If I search for "composer-plugin-api" in composer.json files of vendor for my drupal clone I only see five. If I then
composer require drush/drushwhich adds more dependencies and do the same search I don't get any new results.If did only support know plugins maybe we should have a way for sites to set an
allow-listof plugins in settings.php that way if people want to write their own plugins or use certain ones they could do that. Because a lot of plugins are not going manage files outside the package directory itself. For instance our own https://github.com/php-tuf/composer-integration would not affect any such filesComment #12
wim leersI think 95% of sites will use the same dozen or so composer plugins.
So an approach where
drupal/package_managerhas an embedded list of "known safe/compatible composer plugins" andsettings.php(at their own risk) to not have to wait for additional composer plugins to be added to/ship with the embedded list in point 1 seems reasonable 👍Comment #13
tedbowNeeds 2 potential follow-ups
Comment #14
wim leers#13.1 now has the follow-up created: #3331168: Limit trusted Composer plugins to a known list, allow user to add more — thanks @tedbow! 👍
Comment #15
tedbow#13.2 follow-up #3331310: Exclude unknown paths in project base: only allow vendor + web root + whatever drupal/core-composer-scaffold allows
Comment #16
tedbowComment #17
wim leersThis is at a minimum postponed on #3331168: Limit trusted Composer plugins to a known list, allow user to add more and #3331310: Exclude unknown paths in project base: only allow vendor + web root + whatever drupal/core-composer-scaffold allows. Clarifying.
Comment #18
wim leers#3331168: Limit trusted Composer plugins to a known list, allow user to add more is fixed.
#3331310: Exclude unknown paths in project base: only allow vendor + web root + whatever drupal/core-composer-scaffold allows is the only thing that remains.
Comment #19
wim leers#3331310: Exclude unknown paths in project base: only allow vendor + web root + whatever drupal/core-composer-scaffold allows landed, which means this should be fixed now!
As soon as @tedbow has tagged a new
2.7release, we should ask @chike to update and report back 😊Comment #20
chikeI am waiting to do that.
I have been following the work. It's amazing.
Comment #21
wim leersCool to know you've been following along! 😊
Comment #22
chikeYes I have been following along and admiring how the work was going. I wish I knew enough programming to contribute, but maybe someday....maybe.
Comment #23
tedbowComment #24
tedbow@chike the 8.x-2.7 release is out if you want to try it
Comment #25
wim leersPer #24 — over to you, @chike! 😊
Comment #26
chikeUpdated to 2.7 and site throws WSOD with message,
TypeError: array_map(): Argument #2 ($array) must be of type array, null given in array_map() (line 130 of /home/project-root/public_html/modules/contrib/automatic_updates/package_manager/src/Validator/ComposerPluginsValidator.php)Comment #27
tedbow@chike thanks for testing
Sorry about that I think that is our fault. I created #3341927: No update hook for additional_trusted_composer_plugins setting which I think is the problem
I credited you on that issue
Comment #28
wim leers#26: you did not run
yoursite.com/update.php— i.e. you forgot to apply database updates 😅Comment #29
chikeI forgot to update the database, that solved the issue. I am sorry that caused you unnecessary trouble.
But the symlinks are still there.
It's a long list of them,
Comment #30
wim leersThe symlinks won't disappear. Automatic Updates/Package Manager didn't put them there!
What matters, is that these no longer prevent you from using Automatic Updates. If you visit the status report, do you still get error messages for Automatic Updates/Package Manager?
Comment #31
tedbowBesides #30 if you goto the update form at `
/admin/reports/updates/update` is the update button available? Or course if you are the on the latest patch version of you drupal core minor then there would be no version to update to.If the Update button is not there does it show you any errors?
Comment #32
chikeFor #30 update readiness checks is still failing,
For #31 the update button is not visible and I have pending core updates. I get the message,
Comment #33
wim leersHm … 🤔
This suggests that #3331310: Exclude unknown paths in project base: only allow vendor + web root + whatever drupal/core-composer-scaffold allows is not actually finding all the files in the project root … I think we should create a test that simulates @chike's environment, which roughly looks like this:
→
UnknownPathExcludershould add/home/project-root/.cagefs/symlink-hereto the collection of ignored paths!Assigning to @kunal.sachdev since he did 99% of the work on #3331310: Exclude unknown paths in project base: only allow vendor + web root + whatever drupal/core-composer-scaffold allows, so he'll be the most proficient at writing this test 😊
Comment #34
chikeIf you want to test on a shared hosting server you can purchase a shared hosting plan from Greengeeks (which I am using) Once activated you will find that your web-root is a folder called public_html (this is what servers managed with cPanel software always have as web-root by default) so you should make a modified install to place Drupal inside public_html as against the default web folder. The folder containing public_html will then be your project-root. The /home is part of the absolute path for the host account but never accessible.
Comment #37
kunal.sachdev commentedComment #38
wim leers@kunal.sachdev Next time, please push up a test-only commit first, so we can see it fail. Then we can see it pass after you push the fix. That makes it crystal clear that the test catches the problem and the fix fixes it 🤓
I did that locally: I reverted the fix, kept only the test additions, and it indeed fails:
Comment #39
chikeI applied MR 709 patch, cleared caches, reran readiness checks and yet same thing, site does not pass.
Comment #40
phenaproximaBack to the ol' drawing board, based on #39. 😭 Keep fighting the good fight! 💪
Comment #41
tedbow@kunal.sachdev I would suggest making local composer project and adding some symlinks like is describing in this issue. It would be good to see if you saw the same thing outside of tests.
Comment #42
kunal.sachdev commentedI made a local composer project with Drupal core 10.0.0 and created a symlink. I updated my site to 10.0.3 successfully using automatic updates.
Comment #43
chikeCould you try with web-root named differently from web.
Comment #44
wim leers#42: Can you show exactly what the directory structure was that you tested with? That's easy to achieve if you use
tree(brew install tree):should give you something like:
Use that to make it easy for us to understand what exactly you're testing :)
That, plus
find . -type lof course.Comment #45
kunal.sachdev commentedand
Comment #46
tedbow@chike can you upload the composer.json file at the root of your project? Thanks
Comment #47
chikeHere.
Comment #48
kunal.sachdev commentedSo I tried it again with the following directory structure :
and
and
I updated my site to 10.0.3 successfully using automatic updates.
Comment #49
wim leers@kunal.sachdev's manual testing in #48 suggests that it should work fine for @chike too … 🤔
@chike In #39 you said you had applied the patch for this MR. Are you sure it was truly applied? A small mistake is easy to make :) How did you apply it exactly? Using
cweagans/composer-patchesor some other mechanism?Comment #50
chikeIn #47 I posted my composer.json so you would see how I added the MR patch.
I tried AU in another D9 site and got this message:
On this second site I am using Asset Packagist to install libraries and wanted the libraries installed in libraries folder and not vendor so I added https://github.com/oomphinc/composer-installers-extender to be able to do that. This plugin should be popular so good for AU to exclude it.
Then I tried on a new D10 site I installed on a different shared hosting company and on this site I got no mention of symlinks, only one blocker as in the message I got;
I listed the symlinks on the site and got:
Note this line: find: `./.nc_plugin': Permission denied. From little Googling it's like .nc_plugin is troublesome. Here is info on it as on the website of the host company, Namecheap where I uploaded the D10 site, https://www.namecheap.com/support/knowledgebase/article.aspx/10294/29/ho...
So on two D9 sites I got symlinks blocking AU and on one D10 site it didn't mention symlinks
@kunal.sachdev could you make a test using D9?
Comment #51
wim leersWith #3319507: Add symlink support to Composer Stager 2.0, require that version, and simplify UX & tests accordingly committed, the
3.0.xbranch should simply not complain about this at all!Comment #52
phenaproxima