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

Upgrade to site-alias 4+ in order to remove abandoned dependency webmozart/path-util #5279

Closed
Defcon0 opened this issue Oct 17, 2022 · 11 comments · Fixed by #5281
Closed

Upgrade to site-alias 4+ in order to remove abandoned dependency webmozart/path-util #5279

Defcon0 opened this issue Oct 17, 2022 · 11 comments · Fixed by #5281

Comments

@Defcon0
Copy link

Defcon0 commented Oct 17, 2022

Hello,

at the moment, consolidation/site-alias is a dependency of drush:

https://github.com/drush-ops/drush/blob/11.x/composer.json#L41

For this reason, webmozart/path-util is a "sub dependency" of drush:

https://github.com/consolidation/site-alias/blob/3.1.7/src/HostPath.php#L6

In the latest site-alias the abandoned dependency has been replaced by symfony/filesystem:

https://github.com/consolidation/site-alias/blob/4.0.0/src/HostPath.php#L6

If we could upgrade to site-alias 4.0+, we could get rid of path-util which has been started here already:

#4935 (comment)

Thanks in advance!

Bye Defcon0

@weitzman
Copy link
Member

Yes, I think we forgot to allow site-alias:^4 in PR #5278

@mikemadison13
Copy link

mikemadison13 commented Oct 27, 2022

FYI @weitzman / @greg-1-anderson it looks like another conflict between drush and drupal/core-dev just cropped up. drupal/core-dev requires symfony/filesystem ^4.4 (and nothing in the 5.x range). i'm running into some issues with this on the 4.4.x, trying to test further (disregard previous comment, apologies missed that you still allow 4.4).

@mikemadison13
Copy link

so, it looks like this fix is only accessible if you are running the consolidation/site-alias ^4 version, which itself requires symfony/filesystem ^5 or ^6 (not ^4). so, while drush is still compatible (my bad, misspoke in previous post) you cannot use this fix without updating to a newer version of these libraries (which will make your project incompatible with drupal/core-dev). if you aren't using core-dev, no problem. if you are, you'll get stuck on the lower version of symfony/filesystem and consolidation/site-alias (so you'll still have webmozart in place).

@weitzman
Copy link
Member

Right. The private copy of the library is how we decided to support symfony 4 users.

@mikemadison13
Copy link

@weitzman i've been traversing the issues here trying to find the private copy of the library you've mentioned. could you please point me to where / how you did that? i'd love to adopt a similar approach on blt so we don't have to re-add webmozart/path-util.

@weitzman
Copy link
Member

The issue I'm thinking of is consolidation/site-alias#59.

@weitzman
Copy link
Member

weitzman commented Oct 28, 2022

I guess we didn't go with the private copy of library in the end. I misremembered that.

@mikemadison13
Copy link

mikemadison13 commented Oct 28, 2022

hm, ok. so how does it work for people still running symfony/filesystem 4.4? it seems to me it doesn't using only drush dependencies (based on my experience with blt). obviously if i re-add webmozart/path-util then i'm good, or if i can update to symfony/file-system 5.x then i'm fine. otherwise, i'm guessing there's no path to upgrade to drush 11.3.x using the drupal/core-dev plugin?

@weitzman
Copy link
Member

Yes, we did go with a private library for Symfony 4 users. My recall is diminishing. See https://github.com/drush-ops/drush/pull/5278/files

@greg-1-anderson
Copy link
Member

Yeah we have a copy of Symfony Filesystem's Path class, still residing in Symfony's namespace. We load it early if class_exists says it doesn't already exist. class_exist uses the autoloader, so it will find the Symfony version if it's in your vendor.

@mikemadison13
Copy link

thanks guys

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 a pull request may close this issue.

4 participants