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

Fix wrong negation in Glob::glob() introduced with 3.6.3 #46

Merged
merged 3 commits into from
Dec 29, 2021

Conversation

driehle
Copy link
Contributor

@driehle driehle commented Dec 29, 2021

Q A
Documentation no
Bugfix yes
BC Break no
New Feature no
RFC no
QA no

Summary

This PR fixes #45.

Description

Currently, all CI actions all executed using Ubuntu (latest). In Glob.php there is a distinction of cases if the current distribution supports GLOB_BRACE or not. If it does systemGlob() is used, otherwise fallbackGlob() is used.

Now, fallbackGlob() just handles GLOB_BRACE and otherwise relies on systemGlob(). If there is an error like the one introduced in #43, then it can happen that such error is overlooked, because the underlying distribution (Ubuntu) supports GLOB_BRACE. Consequently, CI actions should not only be executed on Ubuntu, but also on a distribution not supporting GLOB_BRACE, like Alpine linux.

This PR adds a CI action running on Alpine Linux. PHPUnit on Alpine Linux brings 4 errors and 1 failure (see output):

There were 4 errors:

1) LaminasTest\Stdlib\StringWrapper\IconvTest::testConvert with data set #1 ('ascii', 'utf-8', 'abc', 'abc')
iconv(): Wrong encoding, conversion from "ASCII" to "UTF-8//IGNORE" is not allowed

/__w/laminas-stdlib/laminas-stdlib/src/StringWrapper/Iconv.php:300
/__w/laminas-stdlib/laminas-stdlib/test/StringWrapper/CommonStringWrapperTest.php:153
/__w/laminas-stdlib/laminas-stdlib/vendor/phpunit/phpunit/phpunit:61

2) LaminasTest\Stdlib\StringWrapper\IconvTest::testConvert with data set #2 ('utf-8', 'ascii', 'abc', 'abc')
iconv(): Wrong encoding, conversion from "UTF-8" to "ASCII//IGNORE" is not allowed

/__w/laminas-stdlib/laminas-stdlib/src/StringWrapper/Iconv.php:300
/__w/laminas-stdlib/laminas-stdlib/test/StringWrapper/CommonStringWrapperTest.php:153
/__w/laminas-stdlib/laminas-stdlib/vendor/phpunit/phpunit/phpunit:61

3) LaminasTest\Stdlib\StringWrapper\IconvTest::testConvert with data set #3 ('utf-8', 'iso-8859-15', '€', '�')
iconv(): Wrong encoding, conversion from "UTF-8" to "ISO-8859-15//IGNORE" is not allowed

/__w/laminas-stdlib/laminas-stdlib/src/StringWrapper/Iconv.php:300
/__w/laminas-stdlib/laminas-stdlib/test/StringWrapper/CommonStringWrapperTest.php:153
/__w/laminas-stdlib/laminas-stdlib/vendor/phpunit/phpunit/phpunit:61

4) LaminasTest\Stdlib\StringWrapper\IconvTest::testConvert with data set #4 ('utf-8', 'iso-8859-16', '€', '�')
iconv(): Wrong encoding, conversion from "UTF-8" to "ISO-8859-16//IGNORE" is not allowed

/__w/laminas-stdlib/laminas-stdlib/src/StringWrapper/Iconv.php:300
/__w/laminas-stdlib/laminas-stdlib/test/StringWrapper/CommonStringWrapperTest.php:153
/__w/laminas-stdlib/laminas-stdlib/vendor/phpunit/phpunit/phpunit:61

--

There was 1 failure:

1) LaminasTest\Stdlib\GlobTest::testPatterns with data set #0 ('{{,*.}alph,{,*.}bet}a', array('alpha', 'eta.alpha', 'zeta.alpha', 'beta', 'eta.beta', 'zeta.beta'))
Failed asserting that actual size 0 matches expected size 6.

/__w/laminas-stdlib/laminas-stdlib/test/GlobTest.php:69
/__w/laminas-stdlib/laminas-stdlib/vendor/phpunit/phpunit/phpunit:61

The four errors about iconv() are because of a well-known issue with ext-iconv on Alpine Linux and not the fault of this package. I have added a check that skips IconvTest on Alpine Linux. See the following links for further information on this issue:

The one failure is exactly the issue introduced in #43 with a wrong negation. That is fixed with the last commit of this PR.

@driehle driehle force-pushed the hotfix/glob-brace branch 13 times, most recently from 023e20b to 52d8400 Compare December 29, 2021 11:46
@Ocramius Ocramius linked an issue Dec 29, 2021 that may be closed by this pull request
@driehle driehle force-pushed the hotfix/glob-brace branch 5 times, most recently from b7604d8 to 6695cc7 Compare December 29, 2021 12:55
@driehle driehle force-pushed the hotfix/glob-brace branch 2 times, most recently from ae8d6fd to b4b6d46 Compare December 29, 2021 13:01
@driehle driehle changed the title added test case for forceFallback Fix wrong negation in Glob::glob() introduced with 3.6.2 Dec 29, 2021
@driehle driehle marked this pull request as ready for review December 29, 2021 13:12
@driehle driehle changed the title Fix wrong negation in Glob::glob() introduced with 3.6.2 Fix wrong negation in Glob::glob() introduced with 3.6.3 Dec 29, 2021
@snapshotpl snapshotpl added the Bug Something isn't working label Dec 29, 2021
@snapshotpl snapshotpl added this to the 3.6.4 milestone Dec 29, 2021
@snapshotpl snapshotpl self-assigned this Dec 29, 2021
@snapshotpl snapshotpl merged commit 830a36d into laminas:3.6.x Dec 29, 2021
@driehle driehle deleted the hotfix/glob-brace branch December 29, 2021 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BC break in 3.6.3 with sorting paths
2 participants