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

fix Warning: "continue" targeting switch is equivalent to "break". Di… #80

Merged
merged 1 commit into from Dec 4, 2018

Conversation

Projects
None yet
7 participants
@andreybolonin
Contributor

andreybolonin commented Oct 27, 2018

…d you mean to use "continue 2"? in PEAR/PackageFile/v2/Validator.php on line 1933

fix Warning: "continue" targeting switch is equivalent to "break". Di…
…d you mean to use "continue 2"? in PEAR/PackageFile/v2/Validator.php on line 1933
@petk

This comment has been minimized.

Contributor

petk commented Nov 26, 2018

This one has been emitting warnings all over the places here too. Thanks for the patch... :)

@derickr

This comment has been minimized.

derickr commented Dec 4, 2018

Is somebody going to merge this? PHP 7.3 is really close now. /cc @mj

@ashnazg

This comment has been minimized.

Member

ashnazg commented Dec 4, 2018

I'll try to have a look at it today.

@kenguest

This comment has been minimized.

Member

kenguest commented Dec 4, 2018

@ashnazg ashnazg self-assigned this Dec 4, 2018

@ashnazg ashnazg self-requested a review Dec 4, 2018

@ashnazg

ashnazg approved these changes Dec 4, 2018

@ashnazg ashnazg merged commit 817c2ee into pear:master Dec 4, 2018

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
@ashnazg

This comment has been minimized.

Member

ashnazg commented Dec 4, 2018

I'll try to get a new release out tomorrow... simply ran out of time today.

@remicollet

This comment has been minimized.

Contributor

remicollet commented Dec 6, 2018

I really don't understand why this change...

The warning is raised for "continue", and have been fixed in 1.10.6 in pr #76

Switching from break to continue 2 is probably noop.

@remicollet

This comment has been minimized.

Contributor

remicollet commented Dec 6, 2018

@cmb69 ping

I think the issue is rather in PHP-7.3 sources which provides 1.10.5 in pear/install-pear-nozlib.phar

This need to be updated.

@petk

This comment has been minimized.

Contributor

petk commented Dec 6, 2018

I think remi is correct. Is it possible that we were seeing warnings for the previous release of 1.10.5? And that 1.10.6 hasnt been made available in PHP via download yet... So those warnings were already fixed just fixes not distributed to the phar download yet... sorry I haven't noticed this one before

@petk

This comment has been minimized.

Contributor

petk commented Dec 6, 2018

Otherwise worth noting that the pear/install-pear-nozlib.phar is installed from a remote location which is consequently provided by this repo https://github.com/pear/pearweb_phars and phar there has also been already updated to 1.10.7 so this will work ok as far as this warning and issue is concerned. Freshly built PHP versions will include that in them.

It goes ok through. (There are still some other warnings which are not related to this though).

Installing PEAR environment:      /testing-php-installation-7/usr/local/lib/php/
--2018-12-06 10:18:32--  https://pear.php.net/install-pear-nozlib.phar
Resolving pear.php.net (pear.php.net)... 109.203.101.62
Connecting to pear.php.net (pear.php.net)|109.203.101.62|:443... connected.
HTTP request sent, awaiting response... 200 OK
Length: 3612377 (3,4M) [text/plain]
Saving to: ‘pear/install-pear-nozlib.phar’

install-pear-nozlib.phar                           100%[================================================================================================================>]   3,44M  2,00MB/s    in 1,7s    

2018-12-06 10:18:34 (2,00 MB/s) - ‘pear/install-pear-nozlib.phar’ saved [3612377/3612377]

[PEAR] Archive_Tar    - installed: 1.4.3
[PEAR] Console_Getopt - installed: 1.4.1
[PEAR] Structures_Graph- installed: 1.1.1
[PEAR] XML_Util       - installed: 1.4.3
warning: pear/PEAR requires package "pear/Archive_Tar" (recommended version 1.4.3)
warning: pear/PEAR requires package "pear/Structures_Graph" (recommended version 1.1.1)
warning: pear/PEAR requires package "pear/Console_Getopt" (recommended version 1.4.1)
warning: pear/PEAR requires package "pear/XML_Util" (recommended version 1.4.2)
[PEAR] PEAR           - installed: 1.10.7
...
@cmb69

This comment has been minimized.

cmb69 commented Dec 6, 2018

I've just run makedist on a clean PHP-7.3.0 checkout, and got:

+ wget https://pear.php.net/install-pear-nozlib.phar -nd -P pear/
--2018-12-06 10:28:45--  https://pear.php.net/install-pear-nozlib.phar
Resolving pear.php.net (pear.php.net)... 109.203.101.62
Connecting to pear.php.net (pear.php.net)|109.203.101.62|:443... connected.
HTTP request sent, awaiting response... 200 OK
Length: 3612377 (3.4M) [text/plain]
Saving to: ‘pear/install-pear-nozlib.phar’

install-pear-nozlib 100%[===================>]   3.44M   566KB/s    in 6.2s

2018-12-06 10:28:51 (565 KB/s) - ‘pear/install-pear-nozlib.phar’ saved [3612377/3612377]

This installed PEAR-1.10.7. I'm pretty sure that the same download happened before I tagged php-7.3.0 two days ago, but obviously I got PEAR-1.10.5, even though 1.10.7 has been released in 2017, which looks wrong.

@petk

This comment has been minimized.

Contributor

petk commented Dec 6, 2018

I think the tags of https://github.com/pear/pearweb_phars/releases are not the same as tags of pear-core repository so the v1.10.8 in the pearweb_phars has a separate timeline... Pear 1.10.7 is now downloaded from the remote location.

@cmb69

This comment has been minimized.

cmb69 commented Dec 6, 2018

@petk Ah, that makes some sense. However, why didn't I get 1.10.6 two days ago?

@derickr

This comment has been minimized.

derickr commented Dec 6, 2018

It's cached locally as well.

@ashnazg

This comment has been minimized.

Member

ashnazg commented Dec 6, 2018

So, I need to revert this PR... and remove the pear-core-1.10.7 and pearweb_phars-1.10.8 releases altogether... is that what I'm hearing?

@ashnazg

This comment has been minimized.

Member

ashnazg commented Dec 6, 2018

(yes, the versions between pear-core and pearweb_phars did diverge, due to a bad phar release once... maybe back then I should have done a X.Y.Z.1 patch release instead of Z+1)

@petk

This comment has been minimized.

Contributor

petk commented Dec 6, 2018

Reverting this patch won't make any difference. Using break (previous preferred solution) or continue 2 (more C language familiar solution) are the same in this case.

https://wiki.php.net/rfc/continue_on_switch_deprecation

So, I think that everything is ok here now and reverting is not needed. Except, that current PHP 7.3 tarball (archive downloadable from php.net) has now PEAR 1.10.5 included which will still produce this warning. I think we'll need to wait for this one until PHP 7.3.1 release now or have a PHP build from the release branch PHP-7.3.0 from Git repository. When a new makedist will be executed the PEAR 1.10.7 will be downloaded.

@cmb69, yes, PEAR 1.10.6 hasn't been updated as a phar on that repo, that's why there is no PEAR 1.10.6 available for PHP releases to download directly.

@ashnazg

This comment has been minimized.

Member

ashnazg commented Dec 6, 2018

@petk which repo is missing something? Are you meaning pear-core or pearweb_phars on github?

@petk

This comment has been minimized.

Contributor

petk commented Dec 6, 2018

@ashnazg when PEAR 1.10.6 has been released, the pearweb_phars repository wasn't updated and PHP installer at that time was still downloading the PEAR 1.10.5. Now, it will download 1.10.7 so that's resolved now...

@cmb69

This comment has been minimized.

cmb69 commented Dec 6, 2018

I think we'll need to wait for this one until PHP 7.3.1 release now

Yes. Or users can workaround the warning with pear upgrade PEAR.

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