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

[RFC] Review mergeFiles logic in case of missing file #874

Closed
mmenozzi opened this issue Dec 19, 2019 · 3 comments · Fixed by #2445
Closed

[RFC] Review mergeFiles logic in case of missing file #874

mmenozzi opened this issue Dec 19, 2019 · 3 comments · Fixed by #2445

Comments

@mmenozzi
Copy link
Contributor

Hi guys,
given this line in the Mage_Core_Helper_Data::mergeFiles() method:

if (!file_exists($file) || @filemtime($file) > $targetMtime) {

In case one of the source file doesn't exists on filesystem the merge is performed repeatedly at every request.

I agree that this should never happen because if a file is not on filesystem it should not be used by the application.

But if by mistake a CSS is removed from filesystem and the developer forgot to remove the related layout XML rule, we have other CSS files merged again and again at every request.

What do you think? It's worth to find a better way to handle merging in case of missing files or simply we don't want to "support" this kind of mistake.

@colinmollenhour
Copy link
Member

Good observation. I think it would be appropriate to not re-merge the files every request as you stated, and an error should be logged when a file is missing.

@mmenozzi
Copy link
Contributor Author

I agree.

@addison74
Copy link
Contributor

If this is an issue in OpenMage please create a PR for being tested. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants