Prevent staging areas that nested in the active Composer project directory

Is this what you meant?

Is this still relevant?

Perhaps stating the obvious, but this will needs tests 🤓

We may want to use realpath() for the staging_root and project_root, I'm not sure if we will hit that edge case wher staging root is something like vfs:root/active/../stage which means stage is not inside of active but the validator would think that for now.

7 remarks — 6 nits, 1 critical one about test coverage.

Already looking good though! 👍

2 remarks on the MR. 99% done!

Perfect! 🤩

268a4cc8 addressed #20 👍

@tedbow: do we want add a kernel test that checks that all validators subscribing to PreCreateEvent also subscribe to StatusCheckEvent except for the few that we know don't need it? That would prevent regressions. Could be a nice & simple follow-up 😊

Yeah it fails because D9 doesn't support Symfony 5.4 (where the new class Symfony\Component\Filesystem\Path was introduced), probably need to make a helper method.

Error: Class 'Symfony\Component\Filesystem\Path' not found
😱

Looks like this does not exist in Drupal 9.4 or 9.5 …😬 which means we cannot use Path::canonicalize().

This is why:

$ git d 9.5.0 10.0.0 -- composer.json  | grep filesystem
-        "symfony/filesystem": "^4.4",
+        "symfony/filesystem": "^6.2",

🤔

That method was added in https://github.com/symfony/filesystem/commit/d5ca5f7080a759dcb65300a55d5.... I propose to copy it verbatim into our code and just remove that in #3321474: Adopt PHP 8.1-only capabilities such as constructor property promotion + drop BC layers, where we will require Drupal >=10 anyway 🤓

Do we also want to copy over the test ? , and i am disabling phpcs and cspell checks on the Path class.

Still need to add a @todo to be removed in 8.x-3.x

Nits only, fixed those, RTBC! 😊

Updated #3321474 for #28: #3321474-9: Adopt PHP 8.1-only capabilities such as constructor property promotion + drop BC layers.

+1 for @tedbow's remarks, and found one more edge case to worry about.

InvalidArgumentException: project_root and staging_root need to be absolute paths.

🤣

That's because now you've added proper validation, so the vfs:// test cases are no longer accepted. Until we move away from VFS entirely in #3328234: Improve test DX *and* confidence: stop using VFS, you'll need to change it to if (str_starts_with('vfs://'…) || Path::isAbsolute(…)) { … } 🤓

One last nit!

Ready! 🚢 Just have comment nits (mostly relating to #3328234: Improve test DX *and* confidence: stop using VFS), will apply those.