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

Declare $searchForThis property #1689

Closed

Conversation

davejennings
Copy link

@davejennings davejennings commented Jan 9, 2023

Due to the deprecation of dynamic properties in PHP 8.2 (https://www.php.net/manual/en/migration82.deprecated.php#migration82.deprecated.core.dynamic-properties) any use of phing v2 results in:

$ vendor/bin/phing -v
Creation of dynamic property Phing::$searchForThis is deprecated

$searchForThis doesn't appear to need to be a class property as I can only see use in Phing::execute() but I've just followed suit with what's on the main branch for version 3.

@jawira jawira mentioned this pull request Jan 16, 2023
@williamdes
Copy link

williamdes commented Feb 5, 2023

I applied this onto 2.17.4 and there was more to fix:

[PHP Error] Return type of ConditionBase::getIterator() should either be compatible with IteratorAggregate::getIterator(): Traversable, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice [line 85 of /usr/share/php/phing/tasks/system/condition/ConditionBase.php]
[PHP Error] Return type of ConditionEnumeration::current() should either be compatible with Iterator::current(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice [line 369 of /usr/share/php/phing/tasks/system/condition/ConditionBase.php]
[PHP Error] Return type of ConditionEnumeration::next() should either be compatible with Iterator::next(): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice [line 379 of /usr/share/php/phing/tasks/system/condition/ConditionBase.php]
[PHP Error] Return type of ConditionEnumeration::key() should either be compatible with Iterator::key(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice [line 387 of /usr/share/php/phing/tasks/system/condition/ConditionBase.php]
[PHP Error] Return type of ConditionEnumeration::valid() should either be compatible with Iterator::valid(): bool, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice [line 364 of /usr/share/php/phing/tasks/system/condition/ConditionBase.php]
[PHP Error] Return type of ConditionEnumeration::rewind() should either be compatible with Iterator::rewind(): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice [line 392 of /usr/share/php/phing/tasks/system/condition/ConditionBase.php]
[PHP Error] Return type of IterableFileSet::getIterator() should either be compatible with IteratorAggregate::getIterator(): Traversable, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice [line 37 of /usr/share/php/phing/types/IterableFileSet.php]


[PHP Error] Creation of dynamic property TargetHandler::$context is deprecated [line 70 of /usr/share/php/phing/parser/TargetHandler.php]
[PHP Error] Creation of dynamic property ElementHandler::$configurator is deprecated [line 91 of /usr/share/php/phing/parser/ElementHandler.php]
[PHP Error] Creation of dynamic property ElementHandler::$configurator is deprecated [line 91 of /usr/share/php/phing/parser/ElementHandler.php]
[PHP Error] Creation of dynamic property ElementHandler::$configurator is deprecated [line 91 of /usr/share/php/phing/parser/ElementHandler.php]

Here are my patches for Debian (please copy them into this PR):

@mrook
Copy link
Member

mrook commented Feb 7, 2023

Hi, I'm reluctant to do (another) patch update of Phing 2.x. Are you able to use the latest release candidate for 3.0?

@williamdes
Copy link

williamdes commented Feb 7, 2023

Hi, I'm reluctant to do (another) patch update of Phing 2.x. Are you able to use the latest release candidate for 3.0?

since debian freezes the 12th I will not have the time to package 3.x
give I just saved phing right in time or it would have Benn excluded from debian as no update was done since ~5 years
see: https://tracker.debian.org/pkg/phing
I wrote some very light auto test: https://salsa.debian.org/php-team/pear/phing/-/tree/debian/latest/debian/tests
any more complex build.xml example is welcome

if you can include the php 8 fixes and make one more 2.x release that would be awesome, gpg signed would be even more awesome :)

I also edited the package.xml a bit: https://salsa.debian.org/php-team/pear/phing/-/blob/debian/latest/debian/patches/0003-Update-the-package-summary.patch

@williamdes
Copy link

⚠️ f466181#diff-74183ecaf5e202bbed461a9a4bb0e2b85baefac71c5e381231c5157762553ce5R39

this is a mistake of mine, you should not put this on classes, only methods

@davejennings
Copy link
Author

@williamdes: Apologies, I just took the patches and applied quickly. Cheers for your input and reviewing properly.

Copy link

@williamdes williamdes left a comment

Choose a reason for hiding this comment

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

all good, maybe there is more because my build.xml is not that complicated

@mrook
Copy link
Member

mrook commented Jul 6, 2023

Hi, closing this PR. I don't have time for another 2.x release, I will endeavor to make a final 3.0 release in the next two months or so.

@mrook mrook closed this Jul 6, 2023
@davejennings
Copy link
Author

Cheers for the update, @mrook. Final release for 3.0 would be great, ta. 👍

@davejennings davejennings deleted the oldstable branch July 6, 2023 07:43
@williamdes
Copy link

Was this merged somewhere?

@davejennings
Copy link
Author

Nothing merged from this pull request with the changes being specific to v2.

v3 had already addressed the original change which I just followed suit with before your feedback noting more problems. I assume those too have been dealt with or otherwise refactored on v3 but it's not something I've checked.

Cheers for your help on this PR. While it didn't get merged, the fork was useful for our purposes while we made time to migrate to v3.

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

Successfully merging this pull request may close these issues.

None yet

3 participants