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

Add experimental support for Windows ARM64 #6315

Merged
merged 1 commit into from
Aug 15, 2024
Merged

Conversation

dpaoliello
Copy link
Contributor

@dpaoliello dpaoliello commented Jun 3, 2024

Adds experimental support for Windows ARM64:

  • Add instructions on how to cross-compile for Windows ARM64.
  • Add sections to config.toml.
  • Update to version of maybenot that uses newer version of ring that supports Windows ARM64.
  • Ran cargo update.
  • Updated build scripts to support Windows ARM64 or be architecture agnostic.
  • Modified exception logging to support ARM64 context.
  • Added ARM64 configs to C++ projects.

NOTE: WireGuard is working, but OpenVPN is not.


This change is Reviewable

@dlon
Copy link
Member

dlon commented Jun 4, 2024

Thank you so much for these changes! We will try to review and test everything soon.

Copy link
Member

@dlon dlon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 17 of 21 files at r1, 1 of 3 files at r2, all commit messages.
Reviewable status: 18 of 21 files reviewed, 1 unresolved discussion (waiting on @dpaoliello)


gui/tasks/distribution.js line 405 at r2 (raw file):

  }
  // Use host architecture.
  return 'x64';

Assuming that the host is x64 won't work on arm. This might deserve a comment at the very least.

@dpaoliello
Copy link
Contributor Author

@dlon this PR is not yet ready for review: first we need the prebuilt aarch64-pc-windows-msvc binaries added to the dist-assets/binaries submodule. I'm assuming that's something that you or someone else from Mullvad needs to do so that the binaries are built in a trusted way (and, at the very least, the driver needs to be signed).

@dlon
Copy link
Member

dlon commented Aug 12, 2024

@dpaoliello The binaries are available on the arm64-binaries branch of mullvadvpn-app-binaries (but not yet merged, and apisocks5 is missing). So feel free to make sure that this PR is ready for review.

@dpaoliello
Copy link
Contributor Author

Working for me locally:
image

@dlon
Copy link
Member

dlon commented Aug 13, 2024

@dpaoliello It works great using Wireguard! We can get OpenVPN to work as well by building talpid_openvpn_plugin.dll for x64 instead of arm. I've added the arm build of wintun.dll to the branch in the binaries submodule.

@dpaoliello
Copy link
Contributor Author

dpaoliello commented Aug 13, 2024

@dpaoliello It works great using Wireguard! We can get OpenVPN to work as well by building talpid_openvpn_plugin.dll for x64 instead of arm. I've added the arm build of wintun.dll to the branch in the binaries submodule.

Yep, using x64 talpid_openvpn_plugin.dll worked.

The alternative is to get an ARM64 openvpn.exe - it looks like the last successful build of Mullvad's openvpn repo did include ARM64 Windows, but the artifacts are now expired: https://github.com/mullvad/openvpn/actions/runs/7058997763

@dlon
Copy link
Member

dlon commented Aug 13, 2024

We don't think it makes sense to block this PR on an ARM64 openvpn build. Let's include an x64 build of talpid_openvpn_plugin.dll and we'll take of it later.

We currently cross-compile openvpn using mingw on Linux, which can't target arm, and our branch doesn't even compile with MSVC. We'll likely switch soon, though.

@dpaoliello
Copy link
Contributor Author

Let's include an x64 build of talpid_openvpn_plugin.dll and we'll take of it later.

Done

Copy link
Member

@dlon dlon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 21 files at r1, 8 of 12 files at r4, 4 of 6 files at r5, all commit messages.
Reviewable status: 24 of 26 files reviewed, 1 unresolved discussion (waiting on @dpaoliello)


test/ci-runtests.sh line 102 at r1 (raw file):

            ;;
        windows*)
            echo "MullvadVPN-${version}_x64.exe"

The _x64 suffix is missing now from the function in test/scripts/test-utils.sh.

Copy link
Member

@dlon dlon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 24 of 26 files reviewed, 2 unresolved discussions (waiting on @dpaoliello)


ci/buildserver-build.sh line 228 at r5 (raw file):

        pushd "$artifact_dir"
        for original_file in MullvadVPN-*-dev-*{.deb,.rpm,.exe,.pkg}; do
            new_file=$(echo "$original_file" | sed -nE "s/^(MullvadVPN-.*-dev-.*)(_amd64\.deb|_x86_64\.rpm|_arm64\.deb|_aarch64\.rpm|\.exe|\.pkg)$/\1$version_suffix\2/p")

I believe $version_suffix should come before _x64 | _arm64.

@dlon
Copy link
Member

dlon commented Aug 14, 2024

Fixed the issues above and also added apisocks5.exe.

Copy link
Member

@dlon dlon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 6 files at r5, 9 of 9 files at r6, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link
Member

@raksooo raksooo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 11 of 21 files at r1, 2 of 12 files at r4, 1 of 6 files at r5, 5 of 9 files at r6, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@raksooo raksooo requested a review from dlon August 15, 2024 07:09
Copy link
Member

@raksooo raksooo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed all commit messages.
Reviewable status: 26 of 27 files reviewed, all discussions resolved (waiting on @dlon)

Copy link
Member

@raksooo raksooo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r7.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@dlon dlon merged commit dd48537 into mullvad:main Aug 15, 2024
55 checks passed
@dpaoliello dpaoliello deleted the arm64 branch August 15, 2024 15:19
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 this pull request may close these issues.

3 participants