Build universal installer for Windows

[dlon]

Add windows-installer crate which is used to pack the arm64 and x64 installers into a single executable when --universal is passed to build.sh.

Fix DES-1178.

---

This change is 

[linear]

DES-1178 Build Windows universal installer

[MarkusPettersson98]

Reviewed 9 of 11 files at r1.
Reviewable status: 8 of 11 files reviewed, 4 unresolved discussions (waiting on @dlon)

---

.github/workflows/rust-unused-dependencies.yml line 62 at r1 (raw file):

      - name: Check for unused dependencies
        shell: bash
        run: source env.sh && cargo udeps --workspace --exclude windows-installer

Why does windows-installer has to be excluded from cargo udeps? If it contains unused Is it because it is not listed as a default-member in the workspace Cargo.toml? 😊

Code quote:

      run: source env.sh && cargo udeps --workspace --exclude windows-installer

---

build.sh line 90 at r1 (raw file):

    # folder even when it matches you local machine, as opposed to just the target folder.
    # This causes the cached build not to get used when later running e.g.
    # 'cargo run --bin mullvad --shell-completions'.

This comment is still macOS-specific 😊

Code quote:

    # Universal macOS builds package targets for both aarch64-apple-darwin and x86_64-apple-darwin.
    # We leave the target corresponding to the host machine empty to avoid rebuilding multiple times.
    # When the --target flag is provided to cargo it always puts the build in the target/$ENV_TARGET
    # folder even when it matches you local machine, as opposed to just the target folder.
    # This causes the cached build not to get used when later running e.g.
    # 'cargo run --bin mullvad --shell-completions'.

---

build.sh line 397 at r1 (raw file):

        sign_win "dist/MullvadVPN-${PRODUCT_VERSION}.exe"
    fi
fi

⛏️ Could we get rid of the signing-branch here by simply moving this if-block before the preceding "sign installer on Windows" if-block?

# pack universal installer on Windows
if [[ "$UNIVERSAL" == "true" && "$(uname -s)" == "MINGW"* ]]; then
    ./scripts/pack-universal-win.sh \
        --x64-installer "$SCRIPT_DIR/dist/"*"$PRODUCT_VERSION"_x64.exe \
        --arm64-installer "$SCRIPT_DIR/dist/"*"$PRODUCT_VERSION"_arm64.exe
fi

# sign installer on Windows
if [[ "$SIGN" == "true" && "$(uname -s)" == "MINGW"* ]]; then
    for installer_path in dist/*"$PRODUCT_VERSION"*.exe; do
        log_info "Signing $installer_path"
        sign_win "$installer_path"
    done
fi

.. or does the individual installers need to be signed before packing them? 😊

Code quote:

# pack universal installer on Windows
if [[ "$UNIVERSAL" == "true" && "$(uname -s)" == "MINGW"* ]]; then
    ./scripts/pack-universal-win.sh \
        --x64-installer "$SCRIPT_DIR/dist/"*"$PRODUCT_VERSION"_x64.exe \
        --arm64-installer "$SCRIPT_DIR/dist/"*"$PRODUCT_VERSION"_arm64.exe
    if [[ "$SIGN" == "true" ]]; then
        sign_win "dist/MullvadVPN-${PRODUCT_VERSION}.exe"
    fi
fi

---

Cargo.toml line 48 at r1 (raw file):

    "windows-installer",
]
default-members = [

⛏️ Should we add a comment stating the purpose of default-members, i.e. that some packages does not always need to be built? 😊

Code quote:

default-members = [

[dlon]

Reviewable status: 4 of 11 files reviewed, 3 unresolved discussions (waiting on @MarkusPettersson98)

---

build.sh line 90 at r1 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

This comment is still macOS-specific 😊

Done.

---

build.sh line 397 at r1 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

⛏️ Could we get rid of the signing-branch here by simply moving this if-block before the preceding "sign installer on Windows" if-block?

# pack universal installer on Windows
if [[ "$UNIVERSAL" == "true" && "$(uname -s)" == "MINGW"* ]]; then
    ./scripts/pack-universal-win.sh \
        --x64-installer "$SCRIPT_DIR/dist/"*"$PRODUCT_VERSION"_x64.exe \
        --arm64-installer "$SCRIPT_DIR/dist/"*"$PRODUCT_VERSION"_arm64.exe
fi

# sign installer on Windows
if [[ "$SIGN" == "true" && "$(uname -s)" == "MINGW"* ]]; then
    for installer_path in dist/*"$PRODUCT_VERSION"*.exe; do
        log_info "Signing $installer_path"
        sign_win "$installer_path"
    done
fi

.. or does the individual installers need to be signed before packing them? 😊

We need to sign the embedded installers before building the outer one

---

.github/workflows/rust-unused-dependencies.yml line 62 at r1 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

Why does windows-installer has to be excluded from cargo udeps? If it contains unused Is it because it is not listed as a default-member in the workspace Cargo.toml? 😊

It won't compile for targets other than x86_64 windows.

[MarkusPettersson98]

Reviewed 2 of 11 files at r1, 1 of 1 files at r2, 3 of 4 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dlon)

---

build.sh line 397 at r1 (raw file):

Previously, dlon (David Lönnhager) wrote…

We need to sign the embedded installers before building the outer one

Aight aight

---

windows-installer/src/main.rs line 3 at r4 (raw file):

//! Universal Windows installer which contains both an x86 installer package and an ARM package.
//! This can only be built for x86 Windows. This is because the installer must run on both x86 and
//! ARM64, and x86 binaries can run on ARM64, but not vice versa.

⛏️ Would you mind enabling #![warn(clippy::undocumented_unsafe_blocks)] for this crate? It might help us just a little bit when we eventually vet all use of unsafe 😊

[MarkusPettersson98]

Reviewed 7 of 7 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved

[raksooo]

Reviewed 1 of 11 files at r1, 1 of 1 files at r2, 1 of 7 files at r5.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @dlon)

---

desktop/packages/mullvad-vpn/tasks/distribution.js line 282 at r5 (raw file):

    return {
      ...config,
      files: [...config.files],

Since you're not adding any files, this line should be possible to remove.

---

desktop/packages/mullvad-vpn/tasks/distribution.js line 285 at r5 (raw file):

      win: {
        ...config.win,
        extraResources: [...config.win.extraResources],

This line as well.

---

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

    fi
    if [[ "$(uname -s)" == "MINGW"* ]]; then
        if [[ -d "$BUILD_DIR/windows-installer" ]]; then

Could you add a comment about why this is here and when it can be removed?

---

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

    case "$(uname -s)" in
        MINGW*|MSYS_NT*)
            if [[ ! -d "$BUILD_DIR/windows-installer" ]]; then

Can't we just remove these 4 lines that build the arm installer? We don't need to be backwards compatible with arm builds.

---

scripts/pack-universal-win.sh line 1 at r5 (raw file):

#!/usr/bin/env bash

Maybe we should put this in desktop? How about creating a desktop/scripts directory?

[dlon]

Reviewable status: 9 of 12 files reviewed, 4 unresolved discussions (waiting on @MarkusPettersson98 and @raksooo)

---

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

Previously, raksooo (Oskar Nyberg) wrote…

Could you add a comment about why this is here and when it can be removed?

Done.

---

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

Previously, raksooo (Oskar Nyberg) wrote…

Can't we just remove these 4 lines that build the arm installer? We don't need to be backwards compatible with arm builds.

Done.

---

desktop/packages/mullvad-vpn/tasks/distribution.js line 282 at r5 (raw file):

Previously, raksooo (Oskar Nyberg) wrote…

Since you're not adding any files, this line should be possible to remove.

Something fails when building twice unless these lists are copied. I assume they're being modified by builder.build(...).

---

desktop/packages/mullvad-vpn/tasks/distribution.js line 285 at r5 (raw file):

Previously, raksooo (Oskar Nyberg) wrote…

This line as well.

Something fails when building twice unless these lists are copied. I assume they're being modified by builder.build(...).

---

scripts/pack-universal-win.sh line 1 at r5 (raw file):

Previously, raksooo (Oskar Nyberg) wrote…

Maybe we should put this in desktop? How about creating a desktop/scripts directory?

Done.

[raksooo]

Reviewable status: 9 of 12 files reviewed, 2 unresolved discussions (waiting on @dlon and @MarkusPettersson98)

---

desktop/packages/mullvad-vpn/tasks/distribution.js line 282 at r5 (raw file):

Previously, dlon (David Lönnhager) wrote…

Something fails when building twice unless these lists are copied. I assume they're being modified by builder.build(...).

Interesting! Maybe we could wrap config in a function which is called each time to have a fresh copy each time? I feel like someone will try to clean this up at some point and have to realize the same thing again.

[dlon]

Reviewable status: 8 of 12 files reviewed, 2 unresolved discussions (waiting on @MarkusPettersson98 and @raksooo)

---

desktop/packages/mullvad-vpn/tasks/distribution.js line 282 at r5 (raw file):

Previously, raksooo (Oskar Nyberg) wrote…

Interesting! Maybe we could wrap config in a function which is called each time to have a fresh copy each time? I feel like someone will try to clean this up at some point and have to realize the same thing again.

Done.

[raksooo]

Reviewed 1 of 1 files at r7.
Reviewable status: 9 of 12 files reviewed, all discussions resolved (waiting on @MarkusPettersson98)

---

desktop/packages/mullvad-vpn/tasks/distribution.js line 282 at r5 (raw file):

Previously, dlon (David Lönnhager) wrote…

Done.

Nice!

[MarkusPettersson98]

Reviewed 3 of 3 files at r6, 1 of 1 files at r7, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved