Seasocks now compiles in Windows with c++17 and VS2019

[sjk7]

I have been as careful as I can about the changes so as to hopefully not break the Linux compile. At least on WSL2 Linux, this code compiles and all tests pass.

Build process is exactly the same for Windows (mkdir build && cdbuild followed by cmake ../) as it is in Linux; except in the resulting build directory there will be Visual Studio solution files that will enable seasocks to be built and debugged in visual studio. Please note I only tried the static library builds.

Steve

[codecov[bot]]

Codecov Report

Merging #159 (95f20dc) into master (910a363) will decrease coverage by 0.02%. The diff coverage is 35.93%.

@@            Coverage Diff             @@
##           master     #159      +/-   ##
==========================================
- Coverage   35.89%   35.86%   -0.03%     
==========================================
  Files          52       53       +1     
  Lines        2279     2303      +24     
==========================================
+ Hits          818      826       +8     
- Misses       1461     1477      +16     

Impacted Files	Coverage Δ
src/main/c/HybiPacketDecoder.cpp	78.12% <ø> (ø)
src/main/c/internal/HeaderMap.h	71.42% <0.00%> (ø)
src/main/c/internal/RaiiFd.h	0.00% <ø> (ø)
src/main/c/seasocks/Connection.h	16.66% <0.00%> (ø)
src/main/c/seasocks/Request.h	100.00% <ø> (ø)
src/main/c/seasocks/Server.h	0.00% <0.00%> (ø)
src/main/c/seasocks/StrCompare.h	0.00% <0.00%> (ø)
src/main/c/seasocks/WebSocket.h	25.00% <ø> (ø)
src/main/c/seasocks/ZlibContext.cpp	3.27% <0.00%> (ø)
src/main/c/Connection.cpp	19.87% <5.55%> (-0.13%)	:arrow_down:
... and 4 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 910a363...95f20dc. Read the comment docs.

[offa]

@sjk7 Feel free to add the CMakeSettings.json to .gitignore, hereby it's ignored by git and wont get back into a commit.

[madebr]

Add this to .github/workflows/ci.yml to get CI working:

  build_msvc:
    runs-on: windows-latest
    strategy:
      matrix:
        build_config:
          - {args: "-DDEFLATE_SUPPORT=OFF", config: "Debug"}
    steps:
      - uses: actions/checkout@main
      - uses: actions/setup-python@v2
        with:
          python-version: '3.x'
      - name: Build
        run: |
            cmake . -Bbuild ${{ matrix.build_config.args }}
            cmake --build build --parallel --config ${{ matrix.build_config.config }}
      - name: Test
        run: |
            cd build
            cmake --build . --target test --config ${{ matrix.build_config.config }}
            ctest -D ExperimentalBuild -C ${{ matrix.build_config.config }}
            ctest -D ExperimentalMemCheck -C ${{ matrix.build_config.config }}
            # FIXME: use github actions for coverage: https://github.com/marketplace/actions/codecov

[sjk7]

@sjk7 Feel free to add the CMakeSettings.json to .gitignore, hereby it's ignored by git and wont get back into a commit.

Thanks, have done so.

[sjk7]

I was pretty sure it originally came from some open-sourced MS code, but of course, now I cannot find the link!

:-(

Anyhow, I think it may be a case of using compiler intrinsics. This might not go down too well with the mingw crowd, thou?

On Tue, Oct 26, 2021 at 10:04 PM Matt Godbolt @.***> wrote:

@.**** commented on this pull request.

In win32/win_byteswap.h https://github.com/mattgodbolt/seasocks/pull/159#discussion_r736920326:

@@ -0,0 +1,22 @@ +#pragma once

It's probably best if we rewrite this or find an equivalent licenseable version. You're right, I hadn't picked up this was just some random code on the internet.

It's an easy enough thing to re-do, I think :)

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/mattgodbolt/seasocks/pull/159#discussion_r736920326, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFZJ7FOWYMNGN2S2NAJVTOLUI4JVRANCNFSM5GNF6LWQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[sjk7]

I'm not at the dev machine, but this is relevant: https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/byteswap-uint64-byteswap-ulong-byteswap-ushort?view=msvc-160

On Wed, Oct 27, 2021 at 8:36 PM r webby @.***> wrote:

I was pretty sure it originally came from some open-sourced MS code, but of course, now I cannot find the link!

:-(

Anyhow, I think it may be a case of using compiler intrinsics. This might not go down too well with the mingw crowd, thou?

On Tue, Oct 26, 2021 at 10:04 PM Matt Godbolt @.***> wrote:

@.**** commented on this pull request.

In win32/win_byteswap.h https://github.com/mattgodbolt/seasocks/pull/159#discussion_r736920326:

@@ -0,0 +1,22 @@ +#pragma once

It's probably best if we rewrite this or find an equivalent licenseable version. You're right, I hadn't picked up this was just some random code on the internet.

It's an easy enough thing to re-do, I think :)

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/mattgodbolt/seasocks/pull/159#discussion_r736920326, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFZJ7FOWYMNGN2S2NAJVTOLUI4JVRANCNFSM5GNF6LWQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[mattgodbolt]

I was pretty sure it originally came from some open-sourced MS code, but of course, now I cannot find the link! :-( Anyhow, I think it may be a case of using compiler intrinsics. This might not go down too well with the mingw crowd, thou?

There's no need to use compiler intrinsics, but as you've spotted, there are intrinsics for Windows. It's a pain to write but I actually use a byteswap as an example of the kind of thing you can write and get the compiler to optimise :D

[sjk7]

"power moderator" may mean something like "someone who really knows what they are doing". LOL

On Thu, Oct 28, 2021 at 1:49 PM Matt Godbolt @.***> wrote:

@.**** commented on this pull request.

In src/app/c/serve.cpp https://github.com/mattgodbolt/seasocks/pull/159#discussion_r738355178:

@@ -29,7 +29,12 @@

include

include

+#ifdef _WIN32

I'm not sure what a "power moderator" is. I'd like this too (and nearly suggested it) but I'd rather do that as a second clean-up PR.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/mattgodbolt/seasocks/pull/159#discussion_r738355178, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFZJ7FMZ4IMEUIRHG5UTRTLUJFBE7ANCNFSM5GNF6LWQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[mattgodbolt]

Thanks for the extra tests!

[mattgodbolt]

Will merge this evening if nobody has any further comments. Thanks again for your patience!

[offa]

Looks good, all main issues are addressed and the rest can be cleaned-up later. :+1:

Awesome work @sjk7 !

[sjk7]

Thanks! Glad to be of service!

On Fri, Oct 29, 2021 at 5:42 PM offa @.***> wrote:

Looks good, all main issues are addressed and the rest can be cleaned-up later. 👍

Awesome work @sjk7 https://github.com/sjk7 !

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mattgodbolt/seasocks/pull/159#issuecomment-954890447, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFZJ7FPX7RZPBGOLV2OGM6TUJLFJBANCNFSM5GNF6LWQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[mattgodbolt]

Sorry I hadn't merged this earlier! Yay!