Problem/Motivation
symfony/filesystem is currently a dev-dependency of Drupal core, which allows its use during the development of core, but not during actual site development. The component is not included in either the tarball or composer versions of Drupal
This issue aims to promote the dependency out of dev status, making those components available to parts of core Drupal developers may use in their projects.
Issues/Projects Impacted
#3364885: Allow themes to use a starterkit.yml file so it is easier to use the theme generator aims to use symfony/filesystem to mirror entire theme directories using a well-tested method rather than a custom recursive copy function. Additionally, filesystem is useful in renaming files as we change all instances of theme A's machine name and label in file names over to theme B's analogs
The automatic updates initiative also makes use of the filesystem component, though it seems their dependency structure is going through php-tuf/composer-stager rather than Drupal core.
Background
In slack, @catch says:
The main thing with both is that they've got overlap, but not interchangeable, with subsystems we've already got in core, so ideally we'd identify where that overlap is and isn't, and what should and shouldn't be converted etc. - it doesn't need to be fully done but it'd be good to have a start considering the last time this was looked at was ten years ago for finder.
Also that's probably the only thing, the normal dependency evaluation stuff is covered by 'it's another Symfony component' so it's just making sure we know where the lines are between it and existing code. We have the same overlap issues with the translation component (which we've been careful to avoid dependencies on) and other code.
If the answer is 'no we shouldn't use any of it', that's fine too but it should go in a change record or something.
Next Steps
- Need someone with PHP expertise evaluate symfony/filesystem for additional potential usage outside of its use for starterkit
- If there are no additional potential usages, we need to make sure that's reflected in the change record
Comments
Comment #2
longwaveI worry here about the crossover with the Drupal core filesystem service - we already provide copy, delete, mkdir, rmdir, tempnam and other methods, but Symfony also brings these in. Although, perhaps it is time to determine whether we should lean on Symfony components here and retire some of our custom code instead.
Comment #3
andy-blumComment #4
catchI'm not sure which automatic updates issue it was discussed in, but apparently the problem with replacing Drupal methods with the Symfony equivalent is lack of support for stream wrappers, so if we bring it in as-is, it can prevent adding new custom code but can't replace much or any existing code. Haven't looked at this myself though.
Comment #5
andy-blumUpdated issue summary
Comment #6
wim leersThis is blocking #3364885: Allow themes to use a starterkit.yml file so it is easier to use the theme generator, so matching issue metadata.
@andy-blum Why a new issue? The existing issues have many more followers, and are likely to proceed faster. 😅
@catch & @longwave Related Automatic Updates issues:
Comment #7
longwaveDiscussed this with @catch. The theme generator tool is a one-shot CLI script run by a developer, so can it just depend on
drupal/core-dev? The command can check for the existence of the correct classes when it is run, and fail with a message telling the user that they have to installdrupal/core-devto be able to use it. We have some precedent in run-tests.sh which is shipped with core but that requires PHPUnit to be installed to work.Comment #8
andy-blumThere are two primary arguments I have against requiring core-dev as a solution to this issue:
Comment #9
catchWe really need to retire tarballs (except the ones that composer downloads) - automatic updates/package manager should make them redundant for pretty much any user.
This is an interesting point but I'm not sure what I think about it - however does it need to actually block the other issues around this? Feels like we could change the level of the dependency independently of deciding to use it.
Comment #10
dave reidPersonally, as a module and distribution profile maintainer who maintains a lot of scaffolding tools, I've had to add the symfony/filesystem and symfony/finder as not-dev dependencies myself to the projects I work on. In fact, I found it odd that Drupal Core didn't provide these as dependencies already. I would support bringing them in and with the acknowledgement that yes, it is confusing that there is both a Drupal filesystem service and a Symfony filesystem service,
I would support adding the dependency for all, and then separately follow-up for D11 to see how we could reduce and switch more things over to the Symfony version. But I don't think the latter should be a hard blocker to solving this for the starterkit generator, which we desperately need as an install profile with a "starterkit" base theme.
We do not want drupal/core-dev as a dependency for anything, we have encountered issues bringing that into consumer sites because of the composer/composer dependency.
Comment #11
mherchelIMO this is a bad idea. It creates bad developer experience for front-end developers who are not familiar with modern PHP / Composer (which there are many).
An example use case is a front-end developer who gets up and running using lando or DDEV, by running an automated script. Installing additional composer dependencies seems onerous after already having to run a command to generate the theme.
Comment #12
alexpottThe question is do we expect this CLI tool to be able to run when you've used the core-recommended composer project. This template does not include dev dependencies (@longwave pointed this out to me). I think we do expect this tool to be available in this case. We could consider adding dev dependencies to core-recommended but this will get complex fast and there were reasons why we didn't include them.
I think given core-recommended exists and that drush uses both of the packages we want to add - the easy choice is to add the packages to the non dev dependencies.
Comment #13
andy-blumOpened #3395586: Add Symfony's Filesystem and Finder components to core for the work following this dependency evaluation.
Comment #14
alexpottComment #15
alexpottWe discussed this in Lille and @catch and @longwave agreed to add the components as regular dependencies.