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.

Sign up for GitHub

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

Jump to bottom

Don't install symfony/console via composer #636

Merged
merged 1 commit into from
Feb 22, 2020
Merged

Don't install symfony/console via composer #636

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@
"symfony/service-contracts": "1.1.8"
},
"replace": {
"guzzlehttp/guzzle": "*"
"guzzlehttp/guzzle": "*",
"symfony/console": "*"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've never user replace but from https://getcomposer.org/doc/04-schema.md#replace it sounds a bit dangerous. Like you're telling composer that this package replaces guzzle and symfony/console. And I assume your intention is that the app shall fine and use the ones from server, right? So this can work if the composer here assumes the same guzzle version as in Nextcloud, but might fail hard otherwise, right? Like here composer assumes guzzle x is available, but in reality version y is shipped.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the libs here can also work with httplug instead of guzzle directly, you may also try https://packagist.org/packages/christophwurst/nextcloud-http-client

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. Using replace is tricky. symfony/console is luckily only a false positive. feed-io does not depend on symfony/console (actually it does but only for some cli scripts not used by news). guzzlehttp/guzzle maintains a stable api for a while. It seems that all guzzle versions shipped from Nextcloud 16 to Nextcloud 19 do work with feed-io.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

guzzlehttp/guzzle maintains a stable api for a while. It seems that all guzzle versions shipped from Nextcloud 16 to Nextcloud 19 do work with feed-io.

I've seen issues with Guzzle in twofactor_gateway when we shipped our own version. It was caused by _idn_uri_convert or similar. It was somewhat related to guzzle/guzzle#2600 (comment) but I do not remember the details.

},
"autoload": {
"psr-4": {
Expand Down