Rename classes that reference a "stage" to be clearer and use the term "sandbox" instead

We discussed this issue at #3517306: Drupal Usability Meeting 2025-04-11. That issue will have a link to a recording of the meeting.

For the record, the attendees at the usability meeting were benjifisher, rkoller, and simohell.

We are not familiar with the code base and the various ways that "stage" is used, but we tried to suggest some guidelines for naming things.

First, we agree with the first suggestion in the Proposed Resolution. If adding "directory" is too long, then use "staging" or "staged", not "stage" in the context of the staging directory. For example: use StagingBase or StagingDirectoryBase instead of StageBase.

Second, use "phase" or "step" instead of "stage" when you can. Use "step" if it is possible to move back, or "phase" if only one direction is possible.

I hope those suggestions are helpful!

A few proposals:

• StageBase should become SandboxManagerBase, to more accurately describe what it does. Hopefully, this will completely disambiguate the concept of an object that manages the existence and lifecycle of a sandbox. Generically these will be referred to as "sandbox managers"
• "Sandbox" should refer to the temporary copy of the Drupal project, as a whole. This is more common term than "stage", but refers to the same concept, and is less likely to be confused with a verb.
• "Sandbox directory" specifically refers to the directory where the sandbox is located.
• "Create", "require", "apply", "post-apply", and "destroy" can be consistently referred to as "phases", e.g. "the create phase", "the apply phase", etc.

#5 sounds good to me

This is a huge scope of work. I'd like to propose that we split up the work here into two issues.

• In this one, which should block beta, we just rename the classes as appropriate, but let "stage" persist in comments. Since beta would mean we are API-stable, it makes sense to get this sorted out now.
• In another one, which can be stable blocking, we change all comments, but no runtime code whatsoever, to use the updated terminology.

I think this is ready for an initial review. It doesn't change any commentary, only runtime code.

• StageBase is now SandboxManagerBase.
• Events that have a $stage property now have a $sandboxManager property.
• getStageDirectory() and stageDirectoryExists() are now getSandboxDirectory() and sandboxDirectoryExists().
• Any parameters or local variables named $stage are now $sandbox_manager.
• Exceptions that began with "Stage" now start with "Sandbox" (e.g. StageException is now SandboxException). StageFailureMarkerException now has the less ambiguous name of FailureMarkerExistsException.
• The idea of "operations" vs. "phases" is still fuzzier than I'd like -- I think that create, apply, and destroy should be considered "phases", and anything that happens between create and apply should be considered an "operation". Having said that, PreOperationStageEvent was named years ago, before we understood that the "pre" events are central to validation. But, since the ability to log show-stopping errors is their only real superpower, I have renamed PreOperationStageEvent to SandboxValidationEvent to clarify its purpose.
• A couple of tests and test methods were renamed, but generally the tests were not altered.

Re-titling for clarity. @catch endorsed the two-issues idea in Slack.

Opened #3519799: Update all comments and documentation in Package Manager to refer to "sandboxes" and related terminology to do the documentation and comment changes.

While discussing this with @catch in Slack, I realized that RequireEventTrait could be better-named. There is nothing about it that specifically has anything to do with a composer require operation -- if, say, we implemented a composer remove capability in the future, and wanted to dispatch a pre- and post-event for that, then we'd want to also use something like RequireEventTrait, but it would be confusingly named for that scenario.

I've renamed it to the clearer and more accurate EventWithPackageListTrait. This could have been done in its own issue, but while we're getting everything ready to go with better permanent names in this one beta-blocking issue, I figured now was the easiest time to do it.

Adjusting credit.

@phenaproxima thanks for this. I think it is great improvement!

@catch thanks for the detailed review.

I still see a lot of variables that start with `$stage*` or are just `$stage` . and function names like `\Drupal\Tests\package_manager\Kernel\PackageManagerKernelTestBase::createStage`

Since this issue just deals with class names and the follow-up #3519799: Update all comments and documentation in Package Manager to refer to "sandboxes" and related terminology do we want to expand this issue update variable and function names or create a follow-up for that too?

I think renaming local variables and test methods can be done in a follow-up. I'm pretty sure Package Manager's tests aren't part of its API, and therefore we have some latitude to change them later. But I'll try get them done before 11.2 beta1 anyway.

Opened #3520506: Remove minor usages of "stage" from Package Manager's runtime code to do more clean-up.

I checked this out locally and searched for Stage in the package manager code base. There's still a lot of references. A lot are to the ComposerStager classes from the PHPTuf namespace and can be ignored. But there's still some class names and methods in runtime (non test) code that use Stage and not Sandbox. I think we should address at least the class names, constants and methods here. There are also some local variables. Not super fussed on those, that could be a low priority follow-up for a Novice - a lot of these are only in tests.

• \Drupal\package_manager\Validator\StagedDBUpdateValidator has a method checkForStagedDatabaseUpdates but it appears to be checking the sandbox for database updates. Should it also be renamed?
• \Drupal\package_manager\Validator\StageNotInActiveValidator is checking if a sandbox is part of the project root - should it be renamed too? It adds an error with text Stage directory is a subdirectory of the active directory. - that is neither a runtime class or a code comment so I'm not sure which phase that should be changed in - are we missing a third stage to this issue to update user facing strings or should we do that here?
• \Drupal\package_manager\ComposerInspector::validateExecutable has a variable $stage_dir should that now be $sandbox_dir
• Should we rename \Drupal\package_manager\SandboxManagerBase::TEMPSTORE_DESTROYED_STAGES_INFO_PREFIX?
• \Drupal\package_manager\SandboxManagerBase::rethrowAsStageException throws a SandboxException - should we rename that method too?
• \Drupal\package_manager\SandboxManagerBase::create has a variable $stage_dir that refers to a sandbox. So do \Drupal\package_manager\SandboxManagerBase::apply and \Drupal\package_manager\PathExcluder\SiteConfigurationExcluder::syncDefaultSiteDirectoryPermissions - rename?
• \Drupal\package_manager\SandboxManagerBase::require has a local function in variable with name $do_stage should that be renamed? Inside that function it has a variable $stage_dir that refers to a sandbox, rename?
• \Drupal\package_manager\Validator\ComposerPatchesValidator::validate has a number of local variables with stage in the name but that refer to sandboxes - rename? Same for \Drupal\package_manager\Validator\DuplicateInfoFileValidator::validate, \Drupal\package_manager\Validator\EnabledExtensionsValidator::validate and \Drupal\package_manager\Validator\LockFileValidator::validate about another half-dozen validator classes in that namespace.
• \Drupal\package_manager_test_api\ApiController::create has a local variable $stage that refers to a sandbox manager.
• \Drupal\package_manager_test_api\ApiController::__construct(stage) should this property be renamed? It contains a SandboxManager now
• Numerous other instances of local variables named $stage in test code that refer to sandboxes.

\Drupal\package_manager\Validator\StagedDBUpdateValidator has a method checkForStagedDatabaseUpdates but it appears to be checking the sandbox for database updates. Should it also be renamed?

Renamed to SandboxDatabaseUpdatesValidator, and its test-only version to TestSandboxDatabaseUpdatesValidator.

\Drupal\package_manager\Validator\StageNotInActiveValidator is checking if a sandbox is part of the project root - should it be renamed too? It adds an error with text Stage directory is a subdirectory of the active directory. - that is neither a runtime class or a code comment so I'm not sure which phase that should be changed in - are we missing a third stage to this issue to update user facing strings or should we do that here?

Renamed to SandboxDirectoryValidator, and changed the user-facing string.

The rest of your comments seem to be about local variables and test code, which will be handled in
#3520506: Remove minor usages of "stage" from Package Manager's runtime code. Same goes for anything with private visibility, since none of those things are part of our API.

Committed to 11.x - thanks!

Used some cli commands to knock up a rough change record https://www.drupal.org/node/3521441