Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #4935: Replace webmozart/path-util with symfony/filesystem #5264

Merged
merged 3 commits into from
Oct 11, 2022

Conversation

ossobuffo
Copy link
Contributor

symfony/filesystem as of v5.4 is pretty much a drop-in replacement for webmozart/path-util, at least for all the drush use-cases.

@weitzman
Copy link
Member

Thanks for getting this going. Looks like consolidation/site-alias has a hidden dependency on webmozart. Lets fix that too. We can get that merged and released quickly.

Also note that you may see one security related test failure in drush thats unrelated to this PR.

@ossobuffo
Copy link
Contributor Author

@weitzman weitzman merged commit 888bea7 into drush-ops:11.x Oct 11, 2022
@effulgentsia
Copy link

This broke compatibility with both the Drupal 9 and Drupal 10 versions of drupal/core-dev, because the former requires symfony/filesystem: ^4.4 and the latter requires symfony/filesystem: ^6.1. See also drupal/core-dev#3.

@ossobuffo
Copy link
Contributor Author

I would imagine we could fix this by changing drush's composer.json filesystem reference to this:

"symfony/filesystem": "^4.4 || ^5.4 || ^6.1",

but this would require more testing.

@weitzman
Copy link
Member

Ouch. That approach won’t work for 4 because path functionality is not in file system v4. Looks like we gotta revert this.

@Pasqualle
Copy link
Contributor

It would be nice to allow symfony/filesystem version 6.

Can we just remove symfony/filesystem dependency for drush, as it is already required by consolidation/site-alias (by both consolidation/site-alias versions: ^3.1.6 and ^4).

$ composer why symfony/filesystem
chi-teck/drupal-code-generator 2.6.1    requires symfony/filesystem (^4.4 || ^5.1 || ^6)
consolidation/robo             3.0.10   requires symfony/filesystem (^4.4.9 || ^5 || ^6)
consolidation/self-update      2.0.5    requires symfony/filesystem (^2.5 || ^3 || ^4 || ^5 || ^6)
consolidation/site-alias       4.0.0    requires symfony/filesystem (^5.4 || ^6)
drush/drush                    11.x-dev requires symfony/filesystem (^4.4 || ^5.4)

$ composer why consolidation/site-alias
consolidation/site-process 5.1.1    requires consolidation/site-alias (^3 || ^4)
drush/drush                11.x-dev requires consolidation/site-alias (^3.1.6 || ^4)

https://packagist.org/packages/consolidation/site-alias#3.1.6
https://packagist.org/packages/consolidation/site-alias#4.0.0

@weitzman
Copy link
Member

It doesn’t make sense to me to to remove the dependency. We use it extensively. Happy to expand its constraint.

@Pasqualle
Copy link
Contributor

note: symfony/filesystem ^6.1 added in #5292

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants