laminas / laminas-stdlib

SPL extensions, array utilities, error handlers, and more
https://docs.laminas.dev/laminas-stdlib/
BSD 3-Clause "New" or "Revised" License
190 stars 40 forks source link

BC break in 3.6.3 with sorting paths #45

Closed snapshotpl closed 2 years ago

snapshotpl commented 2 years ago

BC Break Report

Q A
Version 3.6.3

Summary

43 with fix for #15 introduces BC break. #17 explained it already.

Previous behavior

File paths didn't sort.

Current behavior

File paths are sort now.

snapshotpl commented 2 years ago

I have use Glob::glob in laminas-modulemanager context in config_glob_paths

Ocramius commented 2 years ago

Should we just revert?

snapshotpl commented 2 years ago

Not sure.

For me call fallbackGlob with $forceFallback and skip systemGlob should help.

https://github.com/laminas/laminas-stdlib/blob/3.7.x/src/Glob.php#L55

driehle commented 2 years ago

I can confirm this bug. Updating from laminas-stdlib 3.6.2 to 3.6.3 breaks my application. I am having a configuration file named ldap.global.php which seems not to be loaded anymore and then a service breaks, throwing an exception for missing its configuration.

Thanks, snapshotpl, for figuring out the cause!

By the way, I am using pretty much the default Laminas MVC skeleton with the following configuration in application.config.php:

        'config_glob_paths' => [
            realpath(__DIR__) . '/autoload/{{,*.}global,{,*.}local}.php',
        ],

... and the following in development.config.php(.dist):

        'config_glob_paths' => [
            realpath(__DIR__) . '/autoload/{,*.}{global,local}-development.php',
        ],

This is how the merged configuration looks like:

grafik

So far everything fine, I think, but ldap.global.php is only loaded with laminas-stdlib 3.6.2, not with 3.6.3.

snapshotpl commented 2 years ago

Looks like this will break most laminas app, so need to revert ASAP. @Ocramius ? @weierophinney ?

Ocramius commented 2 years ago

Send a revert patch 👍

naur1970 commented 2 years ago

It seems to be negation sign is forgotten here https://github.com/laminas/laminas-stdlib/blob/3.7.x/src/Glob.php#L112 if (! $flags & self::GLOB_BRACE) { was before if (self::flagsIsEqualTo($flags, self::GLOB_BRACE)) { is now

driehle commented 2 years ago

I can confirm that adding the missing negation sign fixes the broken loading of configuration files in my MVC application.

Ocramius commented 2 years ago

@driehle should I wait for your work on the patch, or revert?

driehle commented 2 years ago

@Ocramius I think a revert is not needed. Simply fixing the negation sign should be fine. What I am currently trying to to is setting up a CI job that runs with Alpine Linux, so that we have a failing test case.

I think that the current issue only appears on Alpine Linux, which doesn't provide GLOB_BRACE. At least my app runs on Alpine Linux. Maybe @snapshotpl and @naur1970 can confirm that for him the issue appears on Alpine Linux as well? Anyways, currently CI runs on Ubuntu (latest) only, which provides GLOB_BRACE. Hence, the fallback is never actually covered in CI which is likely why this error has not been discovered prior to merging.

naur1970 commented 2 years ago

Confirm! In my case the ploblem has manifested on Alpine Linux.

driehle commented 2 years ago

@Ocramius I think I got it all solved, see #46 for a patch.

snapshotpl commented 2 years ago

Yes, it appears on Alpine. Thanks for patch!

snapshotpl commented 2 years ago

FIxed in #46

weierophinney commented 2 years ago

Ah, that's interesting! Previously, the only OS that didn't provide GLOB_BRACE was IBM i - and we had workarounds for that (it's why we even had the functionality in here!).

Will look at the patch later today - thanks for doing the legwork!

driehle commented 2 years ago

Thanks to @naur1970, who found the actual cause!

Ocramius commented 2 years ago

Thanks for getting it released so quickly, y'all: stellar work! :muscle:

nynka commented 2 years ago

Good job guys! I was struggling with this for a while today. Thanks for fixing so quickly! 👍