[DrupalCon] Automatic Updates beta test result

The temporary directory write issue in the container. It seems like the automatic_updates module tried to create a directory in the /tmp directory in the container and it was not able to do that. As a workaround, I changed the temporary directory for the Drupal site in settings.php using

$settings['file_temp_path'] = 'sites/default/files/tmp';

Oooh, this is a good find.

I suspect we probably will need to add some kind of check in Package Manager to ensure that the temporary directory (or "staging root" in Package Manager's jargon) is writable, to prevent folks from running into this error without recourse.

However, what confuses about your fix is this: as far as I know, Package Manager never uses file_temp_path to create its staging area. It normally delegates to FileSystem::getOsTemporaryDirectory(), which doesn't use file_temp_path. So I don't know how it's possible that your change to settings.php worked around the problem. 🤔

This needs further triage, but I suspect we have a new stable blocker here.

• First, we need to update Composer Stager to 0.4.0. (That's being done in #3277211: Update Composer Stager to 0.5.0)
• Secondly, we need to always use FileSystemInterface::getTempDirectory() to determine the staging root. Right now there's a discrepancy between ProcessFactory::getComposerHomePath() and Stage::getStagingRoot().
• Third, we need to implement a precondition (or similar) which ensures that the staging directory can never be inside the active directory. I think Composer Stager 0.4.0 might already have this in place, but we need to ensure it does, and expose it as a pre-create validator/readiness check.

I'm not going to change the issue status or tags right now, but based on my reading of @AjitS's top-notch bug report, I think that's what this boils down to: the staging root being inside the active directory is a no-go, and we're not doing enough to guard against that.

Will need @tedbow to review 3rd point to continue with issue.

Third, we need to implement a precondition (or similar) which ensures that the staging directory can never be inside the active directory. I think Composer Stager 0.4.0 might already have this in place, but we need to ensure it does, and expose it as a pre-create validator/readiness check.

No, Composer Stager does not ensure that the staging directory is not inside the active directory because it explicitly supports (and tests) that scenario. See RsyncFileSyncer.php#L73-L76 and PhpFileSyncer.php#L104-L109, c.f. EndToEndFunctionalTestCase.php#L276-L300.

@TravisCarden thanks for clearing that up.
re @phenaproxima's question as #2

it does ultimately use file_temp_path or at least we have test coverage for that case in \Drupal\Tests\package_manager\Kernel\StageTest::testGetStageDirectory

It looks like \Drupal\Core\File\FileSystemInterface::getTempDirectory() will use this. See docs

   * If the path is not set, it will fall back to the OS-specific default if
   * set, otherwise a directory under the public files directory. It will then
   * set this as the configured directory.

also see implementation

so it looks like it could be under docroot or anywhere

From #3

First, we need to update Composer Stager to 0.4.0. (That's being done in #3277211: Update Composer Stager to 0.5.0)

We are not on beta so this is done

Secondly, we need to always use FileSystemInterface::getTempDirectory() to determine the staging root. Right now there's a discrepancy between ProcessFactory::getComposerHomePath() and Stage::getStagingRoot().

this was fixed in #3278411: Always use FileSystemInterface::getTempDirectory() to get the temporary directory

Third, we need to implement a precondition (or similar) which ensures that the staging directory can never be inside the active directory. I think Composer Stager 0.4.0 might already have this in place, but we need to ensure it does, and expose it as a pre-create validator/readiness check.

re #6 it seems like composer stager supports this. Of course we don't to support this if we think it will lead to other problems

I suspect we probably will need to add some kind of check in Package Manager to ensure that the temporary directory (or "staging root" in Package Manager's jargon) is writable, to prevent folks from running into this error without recourse.

So we might want an issue to make this check.

create follow-ups
#3299093: Add validation that staging directory is writable
#3299094: Prevent staging areas that nested in the active Composer project directory

Credited @AjitS on both. Thanks for testing and finding this issues