Fix "Invalid template file" error on Windows

[Flyingmana]

ensure both sides of the comparison use the same directory separator
previous implementation did cause Issues in a Windows environment

Issue: Windows was not able to load templates but did log a lot "Invalid template file..." lines

Manual testing scenarios (*)

1. have a magento setup running on native windows php
2. open any frontend page which uses templates

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds on Travis CI are green)

[m2-assistant]

Hi @Flyingmana. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento-engcom-team give me test instance - deploy test instance based on PR changes
• @magento-engcom-team give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

[magento-cicd2]

All committers have signed the CLA.

[orlangur]

Hi @Flyingmana, unfortunately we cannot accept this contribution as Windows is not a supported platform, please check #9492 (comment) for more details.

Also, you can check #19471 (comment) for an easiest solution to deploy under Windows 10.

[m2-assistant]

Hi @Flyingmana, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

[fooman]

@orlangur just because a fix is provided it doesn't mean that Windows is now a supported platform. If this fix improves Windows platform support and it doesn't cause any harm why not merge it?

[orlangur]

@fooman,

just because a fix is provided it doesn't mean that Windows is now a supported platform

I didn't say a word about it.

If this fix improves Windows platform support and it doesn't cause any harm why not merge it?

It causes harm actually and then we receive some complains that something is not working in unsupported OS (while it is not supposed to be working).

The only contribution which would be useful related to Windows is a proper check, similar to check for a proper PHP version, so that developers are not confused anymore.

Such pull requests must be closed and not merged without any exception:
#9492 (comment)
#13340 (comment)

The rule is simple: only contributions fixing real issues must be processed. If somebody is trying to use Magento 2 under Windows not the way I described earlier - then he is doing development wrong and this needs to be fixed.

I'm kindly ask you to not reopen this improper PR without agreement in Slack.
Cc: @sidolov @ishakhsuvarov

[m2-assistant]

Hi @Flyingmana, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

[rhoerr]

It causes harm actually and then we receive some complains that something is not working in unsupported OS (while it is not supposed to be working).

The only contribution which would be useful related to Windows is a proper check, similar to check for a proper PHP version, so that developers are not confused anymore.

I don't think I could disagree with this response more. You can very clearly say "WINDOWS IS NOT A SUPPORTED PLATFORM" and reject bug reports for it (just like I'm sure already happens for many browsers and devices) without actively blocking contribution and improvements that don't negatively impact the rest of the software.

I see the argument that code targeting an unsupported platform is bloat and possible maintenance or security problems down the line, but I don't see how that can justifiably be applied to this specific PR.

You can say 'that's policy' and that may be, but that doesn't mean the policy is good.

[orlangur]

Hey @rhoerr, the main idea is to not encourage improper development environments and do not waste time on such contributions as without Windows CI builds there will always be some issues and more developers trying to use Magento without proper environment setup.

This code is bloat, if somebody wants to maintain Windows compatibility for whatever reason it should be better done outside of Magento main repo as a separate module or even a fork like https://github.com/OpenMage/magento-lts.

[Vinai]

Not accepting this PR is a harmful decision. There are many developers who use windows. Often it is not their decision, but because of the constraints of their work environment. Making their life easier, even while Windows is not supported, should be a positive thing.
Magento makes it difficult enough to figure out a good development setup. Why make it harder?
I strongly urge the maintainers to reopen this PR!

[orlangur]

@Vinai last time this question was raised we all agreed that https://www.howtogeek.com/249966/how-to-install-and-use-the-linux-bash-shell-on-windows-10/ is a decent compromise (this is the link I referref initially in #22275 (comment)).

I don't see any new inputs to revise this decision.

What's the problem with using this approach?
What's the problem with maintaining such compatibility outside of official repo as Windows is OFFICIALLY not supported?

[ishakhsuvarov]

I assume we should keep this PR open while discussion is in active progress.

It's best to escalate this discussion to another level and make sure everyone may participate in this decision. This might be a great resource to start with: https://github.com/magento/architecture

[orlangur]

@ishakhsuvarov there is no reason to keep it open considering all the previous discussions I already referred above. In such case you can reopen all old Windows-related issues and pull requests.

[Flyingmana]

I may not support this decision, but I understand and accept the reasons for this strict line against any windows centered Issues and contributions.

But I want to say some words. I do most of my open source contributions as part of my free time. In my free time I use a setup which I feel comfortable with and which I do know good enough to solve Issues it causes myself. For a few years now this is Windows with a very basic setup.
If I cant work with a project in this way, I usually either decide to work with something else in my free time, or I do fork it. In my current case using something else is not an option, so forking it was already decided before it was mentioned.

[sidolov]

@Flyingmana , please, join our next architecture meeting next Wed, April 17 when we will discuss your case with Magento architects. Thank you for your involvement!

[orlangur]

@Flyingmana do you have Windows 10 in your hand? Maybe you would like https://www.howtogeek.com/249966/how-to-install-and-use-the-linux-bash-shell-on-windows-10/ approach.

My main concern here is that without an official Windows support (or strict OS check) we will always have unhappy developers complaining, some new incompatibilities introduced in each release as there is no appropriate testing done.

I really didn't like a situation with HHVM in the past "Magento is more or less compatible with but there is no support for HHVM". Any of two - official Windows support or official statement regarding incompatibility - seems to be working better in my eyes. Especially important here is to document #19471 (comment) seamless path for developers using Windows.

[Vinai]

@orlangur Responding to you previous response to me, the difference is that this is not an issue report but a PR, and even more a small and unobtrusive one.

It doesn’t introduce a maintenance burden, it only improves the experience of other devs.

I believe out of empathy with other developers a good judgement call in this situation would be to accept the PR.

[real34]

Would a PR that remove all Windows related code from the current codebase (e.g

magento2/app/code/Magento/Email/Model/Plugin/WindowsSmtpConfig.php

Line 54 in 0eb8677

if ($this->osInfo->isWindows()) {

) be accepted then?

[orlangur]

@real34 surely, there are some not so obvious places also with directory separator replacement. Those are remnants from times when Windows-related PRs were acceptable. Similar to HHVM-related code which is not relevant anymore, it would be nice to get rid of it (volunteers may even move it to a separate VendorName_WindowsDeveloper module if they wish so).

But first we need a proper error message displayed when somebody tries to install Magento under Windows.

[kalpmehta]

@orlangur Responding to you previous response to me, the difference is that this is not an issue report but a PR, and even more a small and unobtrusive one.

It doesn’t introduce a maintenance burden, it only improves the experience of other devs.

I believe out of empathy with other developers a good judgement call in this situation would be to accept the PR.

I am only commenting on that code change here, it might look a small and unobtrusive one. However, that code will require thorough testing given that method had some serious vulnerabilities in the past resulting in back to back RCE/LFIs :)

[fooman]

If we can't merge this to the mainline due to maintenance burden, security implications etc I suggest we come up with a way of centralising developer efforts to make Magento work on Windows that stays outside the realm of released / supported code. Reason I am advocating for this is things like
https://insights.stackoverflow.com/survey/2019#technology-_-developers-primary-operating-systems
Windows is by far the most popular developing platform out there.

Suggestions which I believe could be a middle ground while also more clearly communicating what is supported:

• Add an OS check to the main Magento release that prevents Magento from running on Windows (needs to be extendable) with a link to the next item
• Start new Community Project - Windows compatibility. Could have the form of a metapackage with all the fixes needed so final way of running on Windows would be composer require magento/windows-comp-metapackage
• Move all Windows specific code out of the main project into the above

[orlangur]

@fooman, please read #22275 (comment).

Windows is by far the most popular developing platform out there.

This has nothing to do with hacks like this one for native Windows support. There are a lot of options like Vagrant, Docker or Linux Subsystem.

Start new Community Project - Windows compatibility

There is nothing even close to a reason to have such compatibility. What would be really helpful for those Windows developers not able to setup their local environment properly is up-to-date project like https://github.com/magento/magento2devbox-web.

[Flyingmana]

Closing this myself now.

I dont see Docker (only available for Windows Pro) or WSL (still very experimental,incomplete,hacky,unstable) as proper replacement.

But the Announcement of WSL2 and what it covers shows where Windows is going in the long run and that I will just need to wait a bit more to have a good working environment.

[m2-assistant]

Hi @Flyingmana, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

[orlangur]

@Flyingmana could you add a bit more on what's wrong with WSL as it is currently?

Thanks for your attention to this PR.

[Flyingmana]

Let me see.

There is on one hand a large number of Issues open which seem to often relate to every-day tasks
https://github.com/microsoft/WSL/issues?page=4&q=is%3Aissue+is%3Aopen

Here is a PHP related one microsoft/WSL#3858
which if you follow the referenced Issue contains

This may be a bug in wsl. According to strace's log, the pipeline cannot trigger writable events.

It has no GPU support, which on the first thought may be irrelevant.
ElasticSearch seems to not be using it to soon. But for example everything machine learning related is.
There are some Redis modules which do I think.

I heard from multiple people, the Filesystem access is still quite slow and problematic for working with Magento

There is also this Blog posts, although some of the mentioned issues are related to Windows.
https://medium.com/@carlosbaraza/is-windows-an-option-for-developers-in-2019-6fafaee8ea03

I think most interesting are this 3 parts.

WSL can read Windows files, but Windows can’t read WSL files or bugs will happen if you try to do so.

Terminal emulators are buggy, slow or unpolished. I tried many terminal emulators and none of them work as well as iTerm2 for OSX. I ended up using ConEMU and Hyper, but none of them really satisfy me.

Bugs. There are many bugs related to WSL. For example, I encountered one running NVM (Node version manager), that sometimes would completely halt the initialization of the terminal session. I fixed it by delaying NVM load until I required Node. However, it took me a few hours to figure out what happened, debugging all the scripts running on my .zshrc.

[Flyingmana]

As of https://www.phoronix.com/scan.php?page=article&item=windows-10-wsl2&num=1 it seems, WSL2 is based on Hyper-V, which is as of https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/quick-start/enable-hyper-v not available in Win 10 Home

Update:
As of https://devblogs.microsoft.com/commandline/wsl-2-post-build-faq/

Does WSL 2 use Hyper-V? Will it be available on Windows 10 Home?
WSL 2 will be available on all SKUs where WSL is currently available, including Windows 10 Home.

The newest version of WSL uses Hyper-V architecture to enable its virtualization. This architecture will be available in an optional component that is a subset of the Hyper-V feature. This optional component will be available on all SKUs.