-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
build/test issues on WSL #18258
Comments
Complete support for bash on windows is not a 2.0 goal |
marked as test bug as the failures we currently have are in tests. the other issues are external to WSL |
51 tests are currently failing in WSL 1803. Attached the logs as txt.
System.Net.Http.Unit.Tests.txt |
What is left to do here:
Marking as 3.0. |
Remaining list, I took out the GetSetTimes ones that @Anipik fixed.
|
Changing milestone to future. Would be great to get test failures to zero but not a high-prio. |
WSL2 runs on a fully supported Linux kernel which should behave like all other distros. I believe we don't need this work here anymore. nit: we should remove the existing WSL conditions in source code. |
We still have tests disabled agains this issue. |
The remaining ones are all networking except 1. Moving to networking area. We'd welcome anyone who would like to take the attributes off and see which now pass in WSL2.
|
@danmosemsft - do let @craigloewen-msft know if you need any assistance etc. from WSL |
It's @bitcrazed himself 😃 Sure, although I don't plan to look at this in the near future - these are just test failures, they may not be failing because of issues that would actually impact a customer. These attributes were all added on WSL1. First task here is for a volunteer to just remove the attributes and see how many pass on WSL2. |
@danmosemsft I'll try that with WSL2 during next week or so and will report what I find. |
@danmosemsft looking at the method private static bool GetIsWindowsSubsystemForLinux()
{
// https://github.com/Microsoft/BashOnWindows/issues/423#issuecomment-221627364
if (IsLinux)
{
const string versionFile = "/proc/version";
if (File.Exists(versionFile))
{
string s = File.ReadAllText(versionFile);
if (s.Contains("Microsoft") || s.Contains("WSL"))
{
return true;
}
}
}
return false;
} on WSL2 (Ubuntu 18.04), I am seeing the content of the file is
Which will make the detection always fail. note that in another word, setting the attribute will have the same effect as removing it when running on WSL2. |
@tarekgh I guess we need to decide whether we want to have a separate condition for WSL1 vs WSL2. It would be nice if tests run clean on both but WSL2 is more interesting I think. |
I ran all the tests and looks they are passing. I didn't remove the attributes manually because is not really affecting the run and I am not seeing any test skipped because of this attribute. I am seeing the run of some of the networking tests (e.g. |
I don't think we need to build on or for WSL1. We shall triage bug reports from official releases and prioritize them accordingly. (and the priority may be low) |
Yep, WSL2 is not considered as "WSL" in our tests (the conditional attribute is not hit and these tests are executed). AFAIK, we don't have any tests disabled against it, the list above is for WSL1. We have also already decided in some issues that we're not making changes just to support WSL1 (e.g. #26730) Lastly, WSL2 is not without its issues, e.g. microsoft/WSL#4322 hit in here #687 |
If the goal is simply "make sure developers working on our libraries can get a clean test run on WSL2" then if I understand @tarekgh correctly, there is nothing to do, except remove the attributes (purely for tidiness) |
Right, in addition to closing all WSL1 issues. I don't know if there are any specific build/test scripts handle WSL1 though. @ViktorHofer may know better about that. |
No there's is no infra related to WSL1 except the PlatformDetection.IsWindowsSubsystem property. FWIW I think Santi did some networking work in the past specifically for WSL1 in the product. |
@danmosemsft I'd personally keep the attributes until WSL2 is officially out. If you get rid of them now, the only thing it will do is to make test failing on WSL1. We're not running them anywhere, it's just for people working on WSL1, who usually do not want to do Insider Preview. And we might see some issues pop up due to it. Though we should be clear on not supporting WSL1 and decide to what extent we will support WSL2 and be unified in this towards community. |
Either seems reasonable |
I'm wondering if it would make sense to update the name to *1 for clarity as well as add some explicit notes to building instructions to avoid wasted time for people who may try on WSL1 ... and fail. |
@wfurt That's actually a great idea. We might need detection for WSL2 in the future and this would make it easier and more clear. |
I think we don't want this detection. users can run |
Sure we could do that or what I prefer, wait another month until Win10 2004 is released and remove the property and all the attributes with it. |
So let's wait for official WSL2 rollout, then get rid of the |
WSL2 is generally available now, right? Should we address this issue now? There are a bunch of tests in System.Net.Sockets that are disabled via |
Those conditions don't apply on WSL2 anyway so it shouldn't be a concern but as we don't support WSL1 officially we should just get rid off them. |
I'm running WSL2 and I saw that these tests were skipped. So it looks to me like the conditions apply to WSL2 as well. |
runtime/src/libraries/Common/tests/TestUtilities/System/PlatformDetection.Windows.cs Lines 110 to 123 in 3846a9c
This could possibly misfire for Azure cloud linux https://en.wikipedia.org/wiki/CBL-Mariner My suggestion would be to either bound |
We don't support WSL1 both from an infra and product code perspective and should just remove these attributes entirely. |
So here's the proc/version string on my Ubuntu WSL2 instance: The condition applies to WSL2 as well because the code checks for "WSL". I will submit a PR to remove this check and the attributes entirely. |
WSL1 isn't a supported build or execution environment for dotnet/runtime. Because of that removing the checks and platform detection for it as that code path isn't necessary for WSL2 anymore as it contains a fully featured kernel which behaves just a like a usual Linux distro. Fixes dotnet#18258
WSL1 isn't a supported build or execution environment for dotnet/runtime. Because of that removing the checks and platform detection for it as that code path isn't necessary for WSL2 anymore as it contains a fully featured kernel which behaves just a like a usual Linux distro. Fixes #18258
Tracking current issues blocking CoreFx testing on Ubuntu on Windows:
(Note that basically nothing works prior to rs_preview build 14905, which introduced "restartable syscall" support.)
rs_preview build 14936 has a known bug that causes a BSOD when running our networking tests.
Still need investigation:
The text was updated successfully, but these errors were encountered: