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

[Config] Disable default alphabet sorting in glob function due of unstable sort #33998

Merged
merged 1 commit into from Oct 30, 2019

Conversation

@hurricane-voronin
Copy link
Contributor

hurricane-voronin commented Oct 16, 2019

…table sort

Q A
Branch? 3.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #33990
License MIT
Doc PR no

\Symfony\Component\Config\Resource\GlobResource::getIterator loads files using glob not it the stable sorting, e.g several files: doctrine.yml and doctrine_mongodb.yaml in config/packages folder.
On requests these files come(randomly) in a different order, which leads to reinitialization of symfony kernel in dev environment. It's a little bit annoying and takes a lot of time in a common :(

@hurricane-voronin hurricane-voronin force-pushed the hurricane-voronin:glob_sort_33990_3.4 branch from 41c11aa to c5fbcc1 Oct 16, 2019
@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Oct 16, 2019
@nicolas-grekas nicolas-grekas changed the title [Config] Disable default alphabet sorting in glob function due of uns… [Config] Disable default alphabet sorting in glob function due of unstable sort Oct 16, 2019
@hurricane-voronin hurricane-voronin force-pushed the hurricane-voronin:glob_sort_33990_3.4 branch from c5fbcc1 to cf7c751 Oct 16, 2019
@stof

This comment has been minimized.

Copy link
Member

stof commented Oct 16, 2019

isn't disabled sorting even more likely to produce different orders ?

@hurricane-voronin hurricane-voronin force-pushed the hurricane-voronin:glob_sort_33990_3.4 branch from cf7c751 to 7a2ed47 Oct 16, 2019
@hurricane-voronin

This comment has been minimized.

Copy link
Contributor Author

hurricane-voronin commented Oct 16, 2019

isn't disabled sorting even more likely to produce different orders ?

Not able to reproduce issues with disabled sort.

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Oct 16, 2019

Would you be able to debug two different lists when sorting is enabled? Maybe that will allow us to better understand what's going on.

@hurricane-voronin

This comment has been minimized.

Copy link
Contributor Author

hurricane-voronin commented Oct 16, 2019

Would you be able to debug two different lists when sorting is enabled? Maybe that will allow us to better understand what's going on.

Well, what exactly do you need? May I miss some information to provide?

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Oct 16, 2019

Two differently ordered list of files while the same input is provided would be nice.

@hurricane-voronin

This comment has been minimized.

Copy link
Contributor Author

hurricane-voronin commented Oct 16, 2019

Two differently ordered list of files while the same input is provided would be nice.

Oh, sure, with the order I've got randomly 2 lists:

Array( [0] => /my_path/symfony/config/packages/doctrine_mongodb.yaml [1] => /my_path/symfony/config/packages/doctrine.yaml [2] => /my_path/symfony/config/packages/framework.yaml [3] => /my_path/symfony/config/packages/httplug.yaml [4] => /my_path/symfony/config/packages/jms_serializer.yaml [5] => /my_path/symfony/config/packages/knp_snappy.yaml [6] => /my_path/symfony/config/packages/monolog.yaml [7] => /my_path/symfony/config/packages/rabbitmq.yaml [8] => /my_path/symfony/config/packages/routing.yaml [9] => /my_path/symfony/config/packages/security.yaml [10] => /my_path/symfony/config/packages/sensio_framework_extra.yaml [11] => /my_path/symfony/config/packages/sentry.yaml [12] => /my_path/symfony/config/packages/static_content.yaml [13] => /my_path/symfony/config/packages/twig.yaml [14] => /my_path/symfony/config/packages/workflow.yaml )

and

Array( [0] => /my_path/symfony/config/packages/doctrine.yaml [1] => /my_path/symfony/config/packages/doctrine_mongodb.yaml [2] => /my_path/symfony/config/packages/framework.yaml [3] => /my_path/symfony/config/packages/httplug.yaml [4] => /my_path/symfony/config/packages/jms_serializer.yaml [5] => /my_path/symfony/config/packages/knp_snappy.yaml [6] => /my_path/symfony/config/packages/monolog.yaml [7] => /my_path/symfony/config/packages/rabbitmq.yaml [8] => /my_path/symfony/config/packages/routing.yaml [9] => /my_path/symfony/config/packages/security.yaml [10] => /my_path/symfony/config/packages/sensio_framework_extra.yaml [11] => /my_path/symfony/config/packages/sentry.yaml [12] => /my_path/symfony/config/packages/static_content.yaml [13] => /my_path/symfony/config/packages/twig.yaml [14] => /my_path/symfony/config/packages/workflow.yaml )

I guess that the problem is with an underscore in doctrine yamls.

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Oct 16, 2019

ok thanks
can you dump setlocale(LC_COLLATE, '0') in both cases, please?

@hurricane-voronin

This comment has been minimized.

Copy link
Contributor Author

hurricane-voronin commented Oct 16, 2019

Got "en_US.UTF-8" and "C" for first and second lists appropriate

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Oct 16, 2019

Thanks, that's why.
Then I'd suggest to always sort() the array returned by glob() and always pass GLOB_NOSORT. Btw, no need for the "defined" check for it (GLOB_BRACE is special).
Can you please survey if other places in the code could need the same kind of patch?

@hurricane-voronin hurricane-voronin force-pushed the hurricane-voronin:glob_sort_33990_3.4 branch 2 times, most recently from da40a9f to 809e68e Oct 16, 2019
@hurricane-voronin hurricane-voronin force-pushed the hurricane-voronin:glob_sort_33990_3.4 branch from 809e68e to 49d06a6 Oct 16, 2019
@nicolas-grekas nicolas-grekas force-pushed the hurricane-voronin:glob_sort_33990_3.4 branch from b690db5 to 3bed024 Oct 30, 2019
@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Oct 30, 2019

Thank you @hurricane-voronin.

nicolas-grekas added a commit that referenced this pull request Oct 30, 2019
… due of unstable sort (hurricane-voronin)

This PR was squashed before being merged into the 3.4 branch.

Discussion
----------

[Config] Disable default alphabet sorting in glob function due of unstable sort

…table sort

| Q             | A
| ------------- | ---
| Branch?       | 3.4  <!-- see below -->
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | Fix #33990  <!-- prefix each issue number with "Fix #", if any -->
| License       | MIT
| Doc PR        | no <!-- required for new features -->

`\Symfony\Component\Config\Resource\GlobResource::getIterator` loads files using `glob` not it the stable sorting, e.g several files: `doctrine.yml` and `doctrine_mongodb.yaml` in `config/packages` folder.
On requests these files come(randomly) in a different order, which leads to reinitialization of symfony kernel in `dev` environment. It's a little bit annoying and takes a lot of time in a common :(

<!--
Additionally (see https://symfony.com/roadmap):
 - Always add tests and ensure they pass.
 - Never break backward compatibility (see https://symfony.com/bc).
 - Bug fixes must be submitted against the lowest maintained branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too.)
 - Features and deprecations must be submitted against branch 4.4.
 - Legacy code removals go to the master branch.
-->

Commits
-------

3bed024 [Config] Disable default alphabet sorting in glob function due of unstable sort
@nicolas-grekas nicolas-grekas merged commit 3bed024 into symfony:3.4 Oct 30, 2019
1 of 3 checks passed
1 of 3 checks passed
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
fabbot.io Your code looks good.
Details
@fabpot fabpot mentioned this pull request Nov 1, 2019
@fabpot fabpot mentioned this pull request Nov 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.