Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Recompile container when translations directory changes#32708

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

@pierredup
Copy link
Contributor

QA
Branch?4.3
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets#32706
LicenseMIT
Doc PRN/A

The list of translation resources is cached by the container, so when adding a new translation file, the container needs to be recompiled otherwise the translator won't know about the new file.

TangMonk reacted with thumbs up emoji
@nicolas-grekasnicolas-grekas added this to the4.3 milestoneJul 24, 2019
@nicolas-grekas
Copy link
Member

nicolas-grekas commentedJul 24, 2019
edited
Loading

This partly reverts#28937 /cc@rpkamp
See also#27600 andphp-translation/symfony-bundle#198
We should be sure we go in the right direction, with the target being set by these issues.

@stof
Copy link
Member

Well,#28937 passed the scanned directories to the translator cache. But it won't automatically bring in the new resource files if the container itself is not rebuilt. So this revert is indeed necessary.

@rpkamp
Copy link
Contributor

rpkamp commentedJul 24, 2019
edited
Loading

so when adding a new translation file, the container needs to be recompiled otherwise the translator won't know about the new file.

New files within previously known directories should be fine. It seems to me the problem is newly added directories (like a new bundle, or a new directory added to thepaths of the translator configuration) do not trigger the container to recompile, thus the translator will also not be triggered to recompile.

It looks like#27600 is still solved with this PR (MessageCatalogue will still correctly report it's not fresh when a translation file was changed / added / removed), but the added benefit of#28937 that the container no longer needs to be recompiled when a translation is changed is gone (the container will report it's not fresh when any translation file was added / removed / changed).

@stof
Copy link
Member

@rpkamp this PR still does not rebuild the container when you change a file.

What we need based on the responsibility of each:

  1. rebuild the container cache when a translation folder or a translation file in it is added or removed, as the container is holding the list of resource files
  2. rebuild the translation cache when the list of resource file is changed (adding or removing a file in it)
  3. rebuild the translation cache when the one of the resource file is changed (editing translations)

Symfony 4.2 and older were doing 1 and 3 properly, but failing on 2.#28937 is adding the support for 2, but is breaking 1 (as it never rebuilds the container cache for such things). This PR is reintroducing 1 partially (it detects when you add or remove a translation folder, due to adding a bundle for instance, but not when you add a file in an existing folder, due to adding a new locale or catalogue in this folder).

@rpkamp
Copy link
Contributor

@rpkamp this PR still does not rebuild the container when you change a file.

As far as I can tell it does, because all directories that contain translation files are marked as DirectoryResource in the container, which are considered non fresh when the mtime changes, and that changes when the mtime of one of the files within that directory changes.

Which is fine, correctness over performance and all that, but it would be nice if we could have our cake and eat it to 🍰

@pierredup
Copy link
ContributorAuthor

One option around the issue is to deprecate the behaviour of caching the list of resources in the container and only passing the directories so that the translator is fully independent of the container cache. I created a POC at#32735 to discuss this option, but for now, this PR solves the other issues mentioned.

@nicolas-grekas
Copy link
Member

Thank you@pierredup.

@nicolas-grekasnicolas-grekas merged commit7f2e7e2 intosymfony:4.3Jul 30, 2019
nicolas-grekas added a commit that referenced this pull requestJul 30, 2019
…ierredup)This PR was merged into the 4.3 branch.Discussion----------Recompile container when translations directory changes| Q             | A| ------------- | ---| Branch?       | 4.3| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#32706| License       | MIT| Doc PR        | N/AThe list of translation resources is cached by the container, so when adding a new translation file, the container needs to be recompiled otherwise the translator won't know about the new file.Commits-------7f2e7e2 Recompile container when translations directory changes
@pierreduppierredup deleted the translation-cache-fix branchJuly 30, 2019 13:47
@fabpotfabpot mentioned this pull requestAug 26, 2019
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

4.3

Development

Successfully merging this pull request may close these issues.

5 participants

@pierredup@nicolas-grekas@stof@rpkamp@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp