Skip to content

Conversation

@kyteinsky
Copy link
Contributor

No description provided.

@kyteinsky kyteinsky requested a review from juliusknorr January 11, 2024 10:55
@kyteinsky kyteinsky marked this pull request as draft January 11, 2024 11:01
@kyteinsky
Copy link
Contributor Author

nextcloud/ocp:28 is not installable with deeplcom/deepl-php due to psr/container's incompatible version, so used NC 26 for now. The app works with NC 28 but maybe a problem in the future.

@juliusknorr
Copy link
Member

nextcloud/ocp:28 is not installable with deeplcom/deepl-php due to psr/container's incompatible version, so used NC 26 for now. The app works with NC 28 but maybe a problem in the future.

I think the main issue here is that psalm pulls in symfony components that demand an older version of psr/container. The best would probably be to isolate psalm as a dependency either by using psalm/phar (e.g. https://github.com/nextcloud/deck/pull/4947/files#diff-d2ab9925cad7eac58e0ff4cc0d251a937ecf49e4b6bf57f8b95aab76648a9d34R19-R36) or bamarni/composer-bin-plugin (e.g. nextcloud/server#28507)

@kyteinsky
Copy link
Contributor Author

It is actually deeplcom/deepl-php that is uninstallable with ocp>26 when php is set to 8.0 or 7.4.
Is removing the config to set the php version in composer.json bad practice?

		"platform": {
			"php": "8.0"
		}

@juliusknorr
Copy link
Member

Then it picks up the current php version when installing, so you could also pin it to that one if the packages pulled in work fine with 7.4-8.2. Not sure if that doesn't bring more problems.

But when I checked with composer why psr/continer it clearly pointed to psalm.

@kyteinsky
Copy link
Contributor Author

Then it picks up the current php version when installing, so you could also pin it to that one if the packages pulled in work fine with 7.4-8.2. Not sure if that doesn't bring more problems.

Thanks for this, pinning 8.1 for the deepl package and ocp seems to work for the problematic symfony packages.
I will add psalm also in a separate bin ✌️ ✌️

Signed-off-by: Anupam Kumar <[email protected]>
@kyteinsky kyteinsky marked this pull request as ready for review January 12, 2024 13:45
@kyteinsky kyteinsky merged commit e55648a into main Jan 12, 2024
@delete-merged-branch delete-merged-branch bot deleted the maintenance branch January 12, 2024 13:51
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.

3 participants