Test Project Browser on Pantheon

Add "Make sure to actually install a few modules" step, add link.

Update Issue Summary "Remaining tasks", linking to parent issue.

I'm not as sure as with the other platforms (Acquia, Amazee, Platform.sh) but I think it's the same story here that the file system is not writeable, so Project Browser can't be used to add modules in the UI.

I'm not as sure as with the other platforms (Acquia, Amazee, Platform.sh) but I think it's the same story here that the file system is not writeable, so Project Browser can't be used to add modules in the UI.

It is true that it is not possible to write to the code directory in the production environment on Pantheon; however, in development environments, it is possible to use "on-server development mode" (aka "SFTP mode") to make the code directory writable. In this environment, Project Browser would be theoretically usable, and we do have customers who would like to use it.

However, another limitation is that it appears that Project Browser does not work with Drupal sites that use Path repositories, or that have composer update hooks or composer install hooks. Using Project Browser with a site created from a Pantheon repository results in error messages such as:

Could not scan for classes inside "upstream-configuration/scripts/ComposerScripts.php" which does not appear to be a file nor a folder
Class DrupalComposerManaged\ComposerScripts is not autoloadable, can not call pre-update-cmd script

and:

In PathRepository.php line 163:
                                                                                      
The `url` supplied for the path (upstream-configuration) repository does not exist 

I did not inspect the sources for Project Browser, but if I copy just the composer.json file into an empty directory and run composer update, then I am able to reproduce these same error messages exactly. This leads me to believe that Project Browser is doing its update operations in some sort of sandbox environment that it sets up by copying just composer.json, and perhaps some other initial files, into an empty directory, and moves the results back to the main site if the operation succeeds.

Would the Project Browser project be amenable to adding support for path repositories? In a Pantheon upstream, the following declaration can be found:

    "repositories": {
        "upstream-configuration": {
            "type": "path",
            "url": "upstream-configuration"
        },
        "drupal": {
            "type": "composer",
            "url": "https://packages.drupal.org/8"
        }
    },

The second, of course, is the standard Drupal packagist location; the former is the Pantheon path repository. If one copies composer.json and the entire contents of the upstream-configuration directory into an empty directory, then a composer update or composer require in that location works fine, without any error messages.

Proposed resolution:

When Project Browser is setting up its sandbox, after copying the composer.json file, it should parse it and examine the items in the repositories element. For any item found with type path, then the directory name stipulated by the url element should be copied from the source project into the sandbox, if it exists in the source.

If this enhancement was done, then Pantheon sites should work with the Project Browser. If the maintainers of this project would be amenable to accepting and maintaining such an enhancement, Pantheon could potentially write and test a merge request to provide it.

Greg you’ve pretty much hit the nail on the head with regard to how it’s working.

The area of interest you’re describing is in Package Manager, which is now in Drupal core.

Automatic Updates and Project Browser both rely on Package Manager to handle the sandboxing and Composer operations.

I’m not sure if PM would accept a request for this. If I were in charge I would! I will bring Adam into this thread and see what he thinks, as he was largely responsible for the package manager code.

So Package Manager itself has no opinion about path repositories or not.

What it does have an opinion on is unrecognized files and directories in the project root. It will, by default, exclude them from the sandbox, because it has no way of knowing what those files are for, or what kind of damage it might accidentally do by touching them. By being conservative, it's trying to avoid being destructive.

If the path repositories are located in the project root -- which, based on #10, they are -- they don't get copied into the sandbox, and then Composer freaks out when it runs in the sandbox because it can't find those path repositories.

Package Manager has a configuration flag to change this. Set this and see if you are successful:

drush config:set package_manager.settings include_unknown_files_in_project_root true

Thanks very much for the pointers; I'll look into it next week and see if I can learn more.

@greg.1.anderson, I tried a few things locally and I think you have found an honest bug in Package Manager.

If you step through the code I linked with a path repository whose path is upstream-configuration, you'll find that it eventually is converted to . by dirname($path, substr_count($path, '/') ?: 1).

Then, a little while later, that . is swatted away by array_diff($files_in_project_root, $always_include, ['.', '..']).

So basically - Package Manager is correctly detecting that the path repository should be included. But since the path repository is at the top level of the project, its gets excluded anyway by that array_diff() call.

The correct fix here is probably to remove that last argument from the array_diff() call. This needs to happen in core Package Manager, then be backported to contrib. Can you open an issue for that, linking to this comment?

@greg.1.anderson I am curious about the customers who want to use this in development environments, since it wouldn't be deployable to production environments. Do you know if they are planning it use it just for experimentation/testing? Or is there another use case?

@pameela: why would it not be deployable? Either Project Browser / Package Manager could gracefully degrade in non-writable environments or the host can suggest/include config_split to exclude the UIs that are not going to work on prod environments or they can suggest/add UI affordances to tell users about the workflow to use (involving going to their hosting UI or embedding part of the hosting UI) for the least inconvenience.

@phenaproxima/@greg.1.anderson: wow, good finds, a core bugreport would be great with a fix :)

I wasn’t saying PB/PM itself isn’t deployable, it obviously is (and it does degrade gracefully). I’m asking about how modules added this way to a dev environment would be deployed to other environments. Generally I’d like to know more about the development workflow of the customers who want this.

@phenaproxima: Thanks for tracking that down! So, in short, it sounds like the upstream-configuration path in the path repository should have ended with a slash. Composer enforces this for autoload paths, but not path repositories, so this discrepancy was missed. We can fix this with an upstream update on the Pantheon side; a core fix to handle misconfigured path repositories would also be useful. I'm unlikely to have time to do it in the next couple of weeks; if anyone else picks this up, please link to it from here. I can, at a minimum, test it.

@pameeela: Pantheon has a feature that will commit changes made to code files in a development environment back to the repository. This can be done from the Pantheon dashboard, or via the Terminus cli. If a module needs configuration saved, those files can also be exported and committed to the repository in the same way. Once the files are in the repository, they can be promoted to the test and live environments, and a config import will restore the configuration. I haven't tried this workflow with the Project Browser yet, but if there is anything special about how it works that would make these steps not work, I am unaware of it.

@greg.1.anderson thank you, that makes sense and is very cool! I don't see why this wouldn't work with PB, since it manages changes via composer, so other than that it would just be config changes which are also covered.

@phenaproxima: OK, it took me a couple of months to circle back here. The problem was not the lack of a "/" at the end of the path, but rather that the path was at the root of the project. You said this clearly in #14, but I misread it. ;p Once I looked at the code, I figured out what you were saying.

Anyway, as an experiment, I moved the "upstream-configuration" directory inside of a "pantheon" directory, and that got us past the problems described in #10 quite nicely. I could submit a fix to allow path repositories at the top level in Project Browser, but doing so would only get us so far, because once the update gets past the ComposerScripts step, we run into a much larger problem: the update fails with a timeout, because the Pantheon CDN has a hard time limit of 59 seconds.

Is it worth pursuing a fix for this? I'm not super familiar with the Project Browser code, but I am guessing that the composer update step is a single batch step that runs Composer synchronously. It would be possible, in theory, to instead run a script in the background, and have it report progress by writing to a file. Each AJAX call could then just check on the progress of the script and return quickly. This is just a theory; I didn't investigate the code at all, or otherwise attempt to assess the feasibility of such a change, but it sounds like kind of a large effort. Is CDN timeout a frequent problem with Project Browser? It looks like Acquia has a default timeout of 100 seconds, but this can be increased. (Pantheon will not adjust CDN timeout limits.) Is this an area that's worth pursuing, or is the complexity of working around short timeouts not worth the effort?

I think the above issue isn't actually a timeout, but is caused by something else. I'm still investigating.

Oh, the automatic_updates module has its own copy of the package_manager module! Ouch! I guess that's to make it easier to allow each version of the automatic_updates module to be compatible with a wider range of Drupal core versions.

Package Manager originated in Automatic Updates, which is where the discrepancy is coming from. The 4.x branch of Automatic Updates doesn't contain Package Manager, and relies on core's version.

@phenaproxima: Great, makes sense. I confirmed that the secondary problem is not caused by a timeout, but is the result of a problem on the Pantheon side that we can fix. I'll submit an MR for the package_manager module in core and the 3.x branch of the automatic_updates module to fix the primary problem, hopefully early next week.

In short:

--- a/core/modules/package_manager/src/PathExcluder/UnknownPathExcluder.php
+++ b/core/modules/package_manager/src/PathExcluder/UnknownPathExcluder.php
@@ -126,7 +126,11 @@ private function getExcludedPaths(): array {
         // Strip off everything except the top-level directory name. For
         // example, if the repository path is `custom/module/foo`, always
         // include `custom`.
-        $always_include[] = dirname($path, substr_count($path, '/') ?: 1);
+        $parents = substr_count($path, '/');
+        if ($parents > 0) {
+          $path = dirname($path, $parents);
+        }
+        $always_include[] = $path;
       }
       catch (InvalidArgumentException) {
         // The repository path is not relative to the project root, so we don't