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

Add some native return types #874

Merged
merged 1 commit into from Apr 30, 2022
Merged

Add some native return types #874

merged 1 commit into from Apr 30, 2022

Conversation

PhilETaylor
Copy link
Contributor

Symfony version(s) affected

symfony/flex: v1.18.5

Description

Composer version 2.3-dev (2.3-dev+c9baeda95b2da59a8c2d0bb7350b18d009aa891b) 2022-02-21 12:57:32

Symfony Project with symfony/flex: v1.18.5

PHP Fatal error:  Declaration of Symfony\Flex\ParallelDownloader::copy($originUrl, $fileUrl, $fileName, $progress = true, $options = []) must be compatible with Composer\Util\RemoteFilesystem::copy($originUrl, $fileUrl, $fileName, $progress = true, $options = []): bool in /Users/phil/Sites/example.com/vendor/symfony/flex/src/ParallelDownloader.php on line 132

Fatal error: Declaration of Symfony\Flex\ParallelDownloader::copy($originUrl, $fileUrl, $fileName, $progress = true, $options = []) must be compatible with Composer\Util\RemoteFilesystem::copy($originUrl, $fileUrl, $fileName, $progress = true, $options = []): bool in /Users/phil/Sites/example.com/vendor/symfony/flex/src/ParallelDownloader.php on line 132

How to reproduce


~/Sites/symfony-test composer -V
Composer version 2.3-dev (2.3-dev+c9baeda95b2da59a8c2d0bb7350b18d009aa891b) 2022-02-21 12:57:32
~/Sites/symfony-test composer selfup --snapshot
You are already using the latest available Composer version c9baeda95b2da59a8c2d0bb7350b18d009aa891b (snapshot channel).
~/Sites/symfony-test composer require symfony/flex 1.18.5
./composer.json has been created
Running composer update symfony/flex
Loading composer repositories with package information
Updating dependencies
Lock file operations: 1 install, 0 updates, 0 removals
  - Locking symfony/flex (v1.18.5)
Writing lock file
Installing dependencies from lock file (including require-dev)
Package operations: 1 install, 0 updates, 0 removals
  - Installing symfony/flex (v1.18.5): Extracting archive
symfony/flex contains a Composer plugin which is currently not in your allow-plugins config. See https://getcomposer.org/allow-plugins
Do you trust "symfony/flex" to execute code and wish to enable it now? (writes "allow-plugins" to composer.json) [y,n,d,?] y
PHP Fatal error:  Declaration of Symfony\Flex\ParallelDownloader::copy($originUrl, $fileUrl, $fileName, $progress = true, $options = []) must be compatible with Composer\Util\RemoteFilesystem::copy($originUrl, $fileUrl, $fileName, $progress = true, $options = []): bool in /Users/phil/Sites/symfony-test/vendor/symfony/flex/src/ParallelDownloader.php on line 132

Fatal error: Declaration of Symfony\Flex\ParallelDownloader::copy($originUrl, $fileUrl, $fileName, $progress = true, $options = []) must be compatible with Composer\Util\RemoteFilesystem::copy($originUrl, $fileUrl, $fileName, $progress = true, $options = []): bool in /Users/phil/Sites/symfony-test/vendor/symfony/flex/src/ParallelDownloader.php on line 132
~/Sites/symfony-test



Possible Solution

:bool ?

Additional Context

I appreciate Im ahead of the game again running bleeding edge composer :)

@PhilETaylor
Copy link
Contributor Author

Not sure how to fix the unit tests as they seem to be running Locking composer/composer (1.10.x-dev d4b29e9)

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Feb 21, 2022

They are running with both composer 1 and composer 2, which flex supports.

@PhilETaylor
Copy link
Contributor Author

I suck. The unit tests pass locally, just not in GitHub...

Screen Shot 2022-02-21 at 18 57 06

@nicolas-grekas
Copy link
Member

You can ignore these warnings!
But there are other failures ;)

@Seldaek
Copy link
Member

Seldaek commented Feb 22, 2022

Thanks for testing snapshots @PhilETaylor ! I reverted the changes as part of composer/composer#10561 because my intent here is to move the Composer code forward as much as I can but not to break things massively..

That said, it's still great if types get added here, so that one day Composer can add them too.

@PhilETaylor
Copy link
Contributor Author

PhilETaylor commented Feb 25, 2022

@Seldaek Hi there... I run snapshot updated daily "for fun" on my Mac. I appreciate it can sometimes be broken - but I do try to contribute when I can, and not complain or look for support on issues. Im not here looking for support but to help.

If I read correctly, this PR is no longer needed because the changes in composer are being reverted?

(I could not resolve the issues anyway, as the unit tests pass locally, but not in the symfony/flex GitHub actions and I ran out of time to debug that - cause probably by my lack of understanding and experience)

@PhilETaylor
Copy link
Contributor Author

I can confirm using Composer snapshot af995c30384a846a998a4dacb860a02d50400b16 and flex causes no issues so I'll close this for now.

@Seldaek
Copy link
Member

Seldaek commented Feb 25, 2022

@PhilETaylor it's not needed anymore but I think it'd still be nice to add return types so that one day we may add these to Composer too. I didn't want to break all currently installed flex versions right now tho.

So if you do have time to clean up and finalize the PR it'd be cool but if not I can't blame you ;)

@nicolas-grekas nicolas-grekas changed the title type hint return to be compatible with composer 2.3-dev Add some native return types Apr 30, 2022
@nicolas-grekas
Copy link
Member

Thank you @PhilETaylor.

@nicolas-grekas nicolas-grekas merged commit 64d2f3b into symfony:1.x Apr 30, 2022
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

3 participants