search

openvswitch / ovs-issues

Issue tracker repo for Open vSwitch
10 stars 3 forks source link

Windows: include Windows build in github action? #209

Open fw2568 opened 3 years ago

fw2568 commented 3 years ago

Hello,

as the github "build and test" action currently doesn't build for Windows I have added this feature in this fork / branch: https://github.com/dbosoft/ovs/tree/pull-upstream/winbuild

However, didn't submitted this directly as PR because:

From my perspective I would advise to include at least the windows build in the github action. Afterwards I can review each failing test if it is failing due to missing feature or platform issue and submit further PRs to make testing for Windows finally working.

What do you think?

Best Regards, Frank

blp commented 3 years ago

We don't currently depend on the builds directly to merge PRs or patches (Github PRs are not the primary OVS workflow), so increasing the length of time that the builds take is not necessarily a problem.

The tests do sound like a problem. If there's a chance that an increasing number of tests can be made to work over time, then we could start by marking the tests that fail to be skipped on Windows, with the expectation that over time we would reduce the number that are skipped.

So, myself, I'm willing to take a PR, but I'd like to hear from others on this matter too.

igsilya commented 3 years ago

I think it's valuable to have Windows builds and tests in GHA. Yes, Github PRs are not the primary OVS workflow, however I'm typically waiting for the GHA run on my personal account before pushing changes to the main repository. Longer run will affect my workflow a bit, but I don't think that this should be a blocker. We have examples of recent windows build breakages due to lack of CI. It would be nice, though, to work on improvements in this area.

IIUC, current build process uses MSVC compiler, right? I see that AppVeyor also takes ~40 minutes per build, same as this GHA build, so the speed is not specific to particular environment. I remember reading some remarks about moving to clang for windows build, maybe that would help improve the build time? I don't know much about windows kernel parts (is it required to build them only with MSVC?), but if we can build userspace components faster by switching to a different compiler/linker that would be extremely useful.

fw2568 commented 3 years ago

Hi Ilya, thank you for your comment. Yes, same WF for me, too. I made some experiments with clang (included in VS 2019) and was not able to get this to work. Don't think that MSVC is to blame here, I suspect more make on mingw as also Testsuite runs terrible slow on Windows. I switched from MINGW to URT64 and that improved runtime up to 10 minutes, but that's still slow.

I think this something we have to life with at current state. The only alternative I see is to use some bot to trigger win builds on demand or for tags. But running tests also for all platform should be part of the build process.

Further thoughts?

Thanks, Frank

igsilya commented 3 years ago

@fw2568 , what is URT64? (Google doesn't give any clues)

I think that we should have build with failing tests skipped. Performance should not be a big problem here, and it's definitely good to have a CI. If it will be terribly slow or cause some other problems, we could switch it to 'scheduled' run or do something else later.

For the make... I'm wondering if meson+ninja might be any faster than automake in this scenario. But it will not be easy to check.

Also, some googling around performance issues with autotools on windows suggests to disable windows defender as it consumes a lot of resources because of checking of every spawned process. Maybe we can try to disable it with something like Add-MpPreference -ExclusionPath <path> ? I'm not sure if it's not disabled by default or if it's possible to disable it in GHA, though.

fw2568 commented 3 years ago

sorry typo, UCRT64: https://www.msys2.org/docs/environments/, UCRT should perform better then MINGW, in theory...

Yes I can confirm that impact of Virus Scanner on my local machine.

fw2568 commented 3 years ago

Currently I'm working on a patch for test 1907: monitor-cond-change with many sessions pending where the tests hit on wait handle limit for windows and logs so many errors in testsuite.log that it reachs 1GB logsize.

The test itself is still failing, but such things we have to solve first before we can enable testsuite for build. @igsilya but completly agree, we should accept a longer build now and maybe it is solved in a future GHA image, MS is also aware of some issues here: https://github.com/microsoft/Windows-Dev-Performance/issues/15

aserdean commented 3 years ago

The main drawback we had testing appveyor was the one hour build mark. Can we configure the build time in GHA?

@igsilya The kernel driver requires to be compiled using MSVC. I think William tried compiling the userspace using MINGW, that would help speedup the compilation because they could be done cross-platform. No one tried compiling the userspace on Windows using Clang AFAIR.

Beside any form of AV. The compilation is slow because spawning processes on Windows is slow w.r.t Linux/Unix. I'm not familiar with meson but from what I used the combination of using cmake + ninja + clang was the best.

I see that AppVeyor also takes ~40 minutes per build,

FWIW Appveyor does a bit more: compiles the userspace and the drivers for Win8, 8.1 and 10 (Debug + Release). Runs static code analysis over Win8, 8.1, 10; generates the MSI installer and a distribution package and generates the artifacts for it. GHA is a bit slower but if we can run tests that shouldn't be the issue.

@fw2568 feel free to disable the test: 1907: monitor-cond-change with many sessions pending

igsilya commented 3 years ago

The main drawback we had testing appveyor was the one hour build mark. Can we configure the build time in GHA?

Yes, it's configurable and the default limit is 6 hours: https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#jobsjob_idtimeout-minutes

Beside any form of AV. The compilation is slow because spawning processes on Windows is slow w.r.t Linux/Unix. I'm not familiar with meson but from what I used the combination of using cmake + ninja + clang was the best.

It seems that paths are already excluded from the realtime checking in GHA so, it's not the case. FOr the build systems, meson should be on par with cmake in performance, AFAICT. The main problem I see with a build system change is porting of unit tests from autotools. This would not be a pleasant experience.

FWIW Appveyor does a bit more: compiles the userspace and the drivers for Win8, 8.1 and 10 (Debug + Release). Runs static code analysis over Win8, 8.1, 10; generates the MSI installer and a distribution package and generates the artifacts for it.

We can, probably, have a separate job for this too in GHA.

BTW, AppVeyor suddenly started to fail while building the windows kernel driver: https://ci.appveyor.com/project/blp/ovs/builds/39596663/job/31v0ca9xa7q0h2s7#L2919 I suppose some part of environment changed. @aserdean , @fw2568 , if you can take a look that would be great.

fw2568 commented 3 years ago

BTW, AppVeyor suddenly started to fail while building the windows kernel driver: https://ci.appveyor.com/project/blp/ovs/builds/39596663/job/31v0ca9xa7q0h2s7#L2919 I suppose some part of environment changed. @aserdean , @fw2568 , if you can take a look that would be great.

@igsilya I'm aware of this issue as I had it on my own environment. Most probably AppVeyor has updated to VS 2019 16.10: https://github.com/dbosoft/ovs/commit/302d3e5b6d4ad03237a4dfe38ae01edf55f288a5

Didn't submitted this as PR as I still had hope that MS will fix this soon. I will submit it upstream later.

williamtu commented 3 years ago

@fw2568: thanks for working on this! Question: what's the benefits of using GitHub actions for windows build, compared to current Appveyor windows build? Is it because GitHub actions allow more time, 6 hours, so we can run the OVS unit tests?

btw, I went back to look at my previous patch about cross-compile OVS using mingw-gcc https://www.mail-archive.com/ovs-dev@openvswitch.org/msg40784.html it still has too many warnings not fixed yet and, although it's faster, I don't think cross-compile make things easier. William

fw2568 commented 3 years ago

Hi @williamtu ,

yes the general idea was to use same workflow - GHA - for all platforms. So windows unit tests are also run for each commit and so improve unit testing / general quality for Windows on the long run.

When I look at the current discussion I would suggest to proceed like this:

  1. include current MSVC build in PR / GHA build
  2. rework unit tests to make them run or skipped on Windows
  3. future: switch userspace build to cross-compiler

?

williamtu commented 3 years ago

@fw2568, thanks. agree with 1 -3. I'm wondering that if compiling the code takes such a long time (40min per build), then it's pretty painful for developer to work on adding new features to windows. Hopefully cross-compile can make things better.

williamtu commented 3 years ago

@fw2568 Sergey and I are working on meson build. any comments are welcome! https://mail.openvswitch.org/pipermail/ovs-dev/2021-August/386578.html https://mail.openvswitch.org/pipermail/ovs-dev/2021-August/386579.html