Universal NSIS setup file for x64+arm64 (WoA) is broken

[quanglam2807]

Universal NSIS setup file for x64+arm64 (Windows on ARM) is broken. Instead of installing the app, it wipes out the whole program directory, and copies only the Uninstall ....exe file (tested on x64 devices).

Is this change ready? The PR that tried to revert this is closed: #4416 but when I tried to add arm64 support to my app, the universal-arch NSIS doesn't seem to work correctly on my x64 system. It only installs the Uninstall {...}.exe and ignores everything else.

Originally posted by @quanglam2807 in #4228 (comment)

[gnattu]

Current extractAppPackage.nsh is not correctly set, for example this part:

electron-builder/packages/app-builder-lib/templates/nsis/include/extractAppPackage.nsh

Lines 8 to 13 in aa3625d

	!ifdef APP_64
	!ifdef APP_ARM64
	StrCpy $packageArch "ARM64"
	!else
	StrCpy $packageArch "64"
	!endif

Only one of ARM64 or 64 will be set, if arm64 target is defined, then 64 will not be set afterwards.

This also casues arm64 targets not complie-able without setting x64 as target, as 64 is undefined without x64., then the script will only include ia32 target in the target file.

Also this part:

electron-builder/packages/app-builder-lib/templates/nsis/include/extractAppPackage.nsh

Lines 49 to 53 in aa3625d

	${if} ${IsNativeARM64}
	!insertmacro arm64_app_files
	${elseif} ${RunningX64}
	!insertmacro x64_app_files
	${else}

IsNativeARM64 will cause problems for devices not using arm CPU or running via emulation, in my test case, emulated x86 node on arm64 CPU, IsNativeARM64 seems to return false in this script and asking me to provide an x86_64 application. On the other hand, if IsNativeARM64 returns true, the x86_64 files will be added as the macro is not even inserted. This may explain the reason why the installation is wiped out the directory instead of copy files, because the only file is the uninstaller in this case.

[eengoron]

@mmaietta @lutzroeder @develar -- Are the any ideas here on how to fix this? Since we recently recently got universal builds for Apple Silicon working, this has become an issue on the Windows end.

Thanks!

[lutzroeder]

@eengoron @quanglam2807 can you have a look at #4416 and #4228 which included changes to these files. The commit was reverted as it included an NSIS version change that regressed other scenarios. @dangeredwolf might have some insights as well.

[quanglam2807]

@lutzroeder #4416 was not merged so #4228 changes remain intact and not reverted.

I think we should try to fix extractAppPackage.nsh as @gnattu mentioned above. Unlike macOS, electron-builder opts to use universal binary on Windows so there's no way to build independent arm64 and x64 setup files in one build process. I think we should also make this consistent with macOS by adding Arch.universal support to Windows but it's not a simple task.

[lutzroeder]

@quanglam2807 makes sense and agree that fixing extractAppPackage.nsh seems more urgent to unblock the scenario. Not sure the universal approach will work. My understanding is Windows can have x86+x64+arm64 as well? I'm not an expert reading NSIS scripts. Did you try the change @gnattu proposed?

[gnattu]

@lutzroeder What I did is a workaround for this situation so that we can have arm64 and x86+x64 nsis separated in two nsis files, but I did not figure out how to make a x86+x64+arm64 universal package.

[lutzroeder]

@gnattu yes, context was replying to @quanglam2807 relating to Mac support. For Mac --universal in it's current form means x64+arm64 which might not translate to Windows as there are 3 architectures that could go into a single target. Sounds like the right approach is to initially focus on a fix that allows merging all specified architectures into a single installer (fixing the x64 issue in the description) and look at architecture configuration options separately.

[mmaietta]

@lutzroeder @gnattu @quanglam2807 Can you try using this script instead?
extractAppPackage.nsh

!macro extractEmbeddedAppPackage
  !ifdef COMPRESS
    SetCompress off
  !endif

  Var /GLOBAL packageArch
  
  !insertmacro identify_package
  !insertmacro compute_files_for_current_arch

  !ifdef COMPRESS
    SetCompress "${COMPRESS}"
  !endif

  !insertmacro decompress
  !insertmacro custom_files_post_decompression
!macroend

!macro identify_package 
  !ifdef APP_32
    StrCpy $packageArch "32"
  !endif
  !ifdef APP_64
    ${if} ${RunningX64}
    ${OrIf} ${IsNativeARM64}
      StrCpy $packageArch "64"
    ${endif}
  !endif
  !ifdef APP_ARM64
    ${if} ${IsNativeARM64}
      StrCpy $packageArch "ARM64"
    ${endif}
  !endif
!macroend

!macro compute_files_for_current_arch
  ${if} $packageArch == "ARM64"
    !ifdef APP_ARM64
      !insertmacro arm64_app_files
    !endif
  ${elseif} $packageArch == "64"
    !ifdef APP_64
      !insertmacro x64_app_files
    !endif
  ${else}
    !ifdef APP_32
      !insertmacro ia32_app_files
    !endif
  ${endIf}
!macroend

!macro custom_files_post_decompression
  ${if} $packageArch == "ARM64"
    !ifmacrodef customFiles_arm64
      !insertmacro customFiles_arm64
    !endif
  ${elseif} $packageArch == "64"
    !ifmacrodef customFiles_x64
      !insertmacro customFiles_x64
    !endif
  ${else}
    !ifmacrodef customFiles_ia32
      !insertmacro customFiles_ia32
    !endif
  ${endIf}
!macroend

!macro arm64_app_files
  File /oname=$PLUGINSDIR\app-arm64.${COMPRESSION_METHOD} "${APP_ARM64}"
!macroend

!macro x64_app_files
  File /oname=$PLUGINSDIR\app-64.${COMPRESSION_METHOD} "${APP_64}"
!macroend

!macro ia32_app_files
  File /oname=$PLUGINSDIR\app-32.${COMPRESSION_METHOD} "${APP_32}"
!macroend

!macro decompress
  !ifdef ZIP_COMPRESSION
    nsisunz::Unzip "$PLUGINSDIR\app-$packageArch.zip" "$INSTDIR"
  !else
    Nsis7z::Extract "$PLUGINSDIR\app-$packageArch.7z"
  !endif
!macroend

[lutzroeder]

@mmaietta can you Zip and share extractAppPackage.nsh? The patch did not apply.

[mmaietta]

@lutzroeder updated the comment to be the exact script instead of a patch.

[lutzroeder]

@mmaietta works for x64+arm64 on both Windows x64 and Windows ARM. @gnattu can you try as well?

[matthiasg]

@mmaietta can make use of that script via some options (nsis/script or include) instead of patching electron-builder ?

[mmaietta]

@matthiasg you can include custom scripts via EB config:

    nsis: {
        include: "installer/win/nsis-installer.nsh"
    },

[matthiasg]

@mmaietta I tried that but it didn't work. It complained about duplicate macros. Also tried with script. Also tried to copy all existing scripts over and just overwriting etc.

[mmaietta]

@develar could you execute another pre-release cut to catch up with the latest changes on master please? 🙂

[mmaietta]

Merged in #5558 in v22.10.5 . Closing issue