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

feat(nsis): allow disabling of building a universal installer #8607

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/early-deers-suffer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"app-builder-lib": minor
---

feat: allow disabling of building a universal windows installer
10 changes: 10 additions & 0 deletions packages/app-builder-lib/scheme.json
Original file line number Diff line number Diff line change
Expand Up @@ -3896,6 +3896,11 @@
"string"
]
},
"buildUniversalInstaller": {
"default": true,
"description": "Disable building an universal installer of the archs specified in the target configuration\n*Not supported for nsis-web*",
"type": "boolean"
},
"createDesktopShortcut": {
"default": true,
"description": "Whether to create desktop shortcut. Set to `always` if to recreate also on reinstall (even if removed by user).",
Expand Down Expand Up @@ -4196,6 +4201,11 @@
"string"
]
},
"buildUniversalInstaller": {
"default": true,
"description": "Disable building an universal installer of the archs specified in the target configuration\n*Not supported for nsis-web*",
"type": "boolean"
},
"createDesktopShortcut": {
"default": true,
"description": "Whether to create desktop shortcut. Set to `always` if to recreate also on reinstall (even if removed by user).",
Expand Down
55 changes: 38 additions & 17 deletions packages/app-builder-lib/src/targets/nsis/NsisTarget.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,18 @@
import BluebirdPromise from "bluebird-lst"
import { Arch, asArray, AsyncTaskManager, exec, executeAppBuilder, getPlatformIconFileName, InvalidConfigurationError, log, spawnAndWrite, use, getPath7za } from "builder-util"
import {
Arch,
asArray,
AsyncTaskManager,
exec,
executeAppBuilder,
getPlatformIconFileName,
InvalidConfigurationError,
log,
spawnAndWrite,
use,
getPath7za,
getArchSuffix,
} from "builder-util"
import { CURRENT_APP_INSTALLER_FILE_NAME, CURRENT_APP_PACKAGE_FILE_NAME, PackageFileInfo, UUID } from "builder-util-runtime"
import { exists, statOrNull, walk } from "builder-util"
import _debug from "debug"
Expand All @@ -9,7 +22,7 @@
import { getBinFromUrl } from "../../binDownload"
import { Target } from "../../core"
import { DesktopShortcutCreationPolicy, getEffectiveOptions } from "../../options/CommonWindowsInstallerConfiguration"
import { computeSafeArtifactNameIfNeeded, normalizeExt } from "../../platformPackager"
import { chooseNotNull, computeSafeArtifactNameIfNeeded, normalizeExt } from "../../platformPackager"
import { hashFile } from "../../util/hash"
import { isMacOsCatalina } from "../../util/macosVersion"
import { time } from "../../util/timer"
Expand Down Expand Up @@ -73,8 +86,16 @@
NsisTargetOptions.resolve(this.options)
}

get shouldBuildUniversalInstaller() {
const buildSeparateInstallers = this.options.buildUniversalInstaller === false
return !buildSeparateInstallers
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

protected buildUniversalInstaller() {
    return this.options.buildUniversalInstaller 
 }

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

buildUniversalInstaller is optional, so we must check against explicit false.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not a get function, so there shouldn't be any caching, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do we need to cache the result? It's just reading from options.

But I agree, it could simply be a get function. I'll update the PR


build(appOutDir: string, arch: Arch) {
this.archs.set(arch, appOutDir)
if (!this.shouldBuildUniversalInstaller) {
return this.buildInstaller(new Map<Arch, string>().set(arch, appOutDir))
}
return Promise.resolve()
}

Expand Down Expand Up @@ -117,18 +138,23 @@
}
}

protected get installerFilenamePattern(): string {
// tslint:disable:no-invalid-template-strings
return "${productName} " + (this.isPortable ? "" : "Setup ") + "${version}.${ext}"
protected installerFilenamePattern(primaryArch?: Arch | null, defaultArch?: string): string {
const setupText = this.isPortable ? "" : "Setup "
const archSuffix = !this.shouldBuildUniversalInstaller && primaryArch != null ? getArchSuffix(primaryArch, defaultArch) : ""

return "${productName} " + setupText + "${version}" + archSuffix + ".${ext}";

Check warning on line 145 in packages/app-builder-lib/src/targets/nsis/NsisTarget.ts

View workflow job for this annotation

GitHub Actions / test-linux (ArtifactPublisherTest,BuildTest,ExtraBuildTest,RepoSlugTest,binDownloadTest,configura...

Delete `;`

Check warning on line 145 in packages/app-builder-lib/src/targets/nsis/NsisTarget.ts

View workflow job for this annotation

GitHub Actions / test-linux (snapTest,debTest,fpmTest,protonTest)

Delete `;`
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

protected installerFilenamePattern(primaryArch?: Arch | null, defaultArch?: string): string {
  const setupText = this.isPortable ? "" : "Setup ";
  const archSuffix = !this.buildUniversalInstaller() && primaryArch != null 
    ? getArchSuffix(primaryArch, defaultArch) 
    : "";
  
  return "${productName} " + setupText + "${version}" + archSuffix + ".${ext}";
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Love it, I'll update the logic.


private get isPortable(): boolean {
return this.name === "portable"
}

async finishBuild(): Promise<any> {
if (!this.shouldBuildUniversalInstaller) {
return this.packageHelper.finishBuild()
}
try {
const { pattern } = this.packager.artifactPatternConfig(this.options, this.installerFilenamePattern)
const { pattern } = this.packager.artifactPatternConfig(this.options, this.installerFilenamePattern())
const builds = new Set([this.archs])
if (pattern.includes("${arch}") && this.archs.size > 1) {
;[...this.archs].forEach(([arch, appOutDir]) => builds.add(new Map().set(arch, appOutDir)))
Expand All @@ -147,14 +173,8 @@
const packager = this.packager
const appInfo = packager.appInfo
const options = this.options
const installerFilename = packager.expandArtifactNamePattern(
options,
"exe",
primaryArch,
this.installerFilenamePattern,
false,
this.packager.platformSpecificBuildOptions.defaultArch
)
const defaultArch = chooseNotNull(this.packager.platformSpecificBuildOptions.defaultArch, this.packager.config.defaultArch) ?? undefined
const installerFilename = packager.expandArtifactNamePattern(options, "exe", primaryArch, this.installerFilenamePattern(primaryArch, defaultArch), false, defaultArch)
const oneClick = options.oneClick !== false
const installerPath = path.join(this.outDir, installerFilename)

Expand Down Expand Up @@ -328,7 +348,7 @@
await this.executeMakensis(defines, commands, sharedHeader + (await this.computeFinalScript(script, true, archs)))
await Promise.all<any>([packager.sign(installerPath), defines.UNINSTALLER_OUT_FILE == null ? Promise.resolve() : unlink(defines.UNINSTALLER_OUT_FILE)])

const safeArtifactName = computeSafeArtifactNameIfNeeded(installerFilename, () => this.generateGitHubInstallerName())
const safeArtifactName = computeSafeArtifactNameIfNeeded(installerFilename, () => this.generateGitHubInstallerName(primaryArch, defaultArch))
let updateInfo: any
if (this.isWebInstaller) {
updateInfo = createNsisWebDifferentialUpdateInfo(installerPath, packageFiles)
Expand All @@ -351,10 +371,11 @@
})
}

protected generateGitHubInstallerName(): string {
protected generateGitHubInstallerName(primaryArch: Arch | null, defaultArch: string | undefined): string {
const appInfo = this.packager.appInfo
const classifier = appInfo.name.toLowerCase() === appInfo.name ? "setup-" : "Setup-"
return `${appInfo.name}-${this.isPortable ? "" : classifier}${appInfo.version}.exe`
const archSuffix = !this.shouldBuildUniversalInstaller && primaryArch != null ? getArchSuffix(primaryArch, defaultArch) : ""
return `${appInfo.name}-${this.isPortable ? "" : classifier}${appInfo.version}${archSuffix}.exe`
}

private get isUnicodeEnabled(): boolean {
Expand Down
11 changes: 9 additions & 2 deletions packages/app-builder-lib/src/targets/nsis/WebInstallerTarget.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Arch, log } from "builder-util"
import { computeDownloadUrl, getPublishConfigs, getPublishConfigsForUpdateInfo } from "../../publish/PublishManager"
import { WinPackager } from "../../winPackager"
import { NsisWebOptions } from "./nsisOptions"
Expand Down Expand Up @@ -35,8 +36,14 @@ export class WebInstallerTarget extends NsisTarget {
defines.APP_PACKAGE_URL = appPackageUrl
}

protected get installerFilenamePattern(): string {
// tslint:disable:no-invalid-template-strings
get shouldBuildUniversalInstaller() {
if (this.options.buildUniversalInstaller === false) {
log.warn({ buildUniversalInstaller: true }, "only universal builds are supported for nsis-web installers, overriding setting")
}
return true
}

protected installerFilenamePattern(_primaryArch?: Arch | null, _defaultArch?: string): string {
return "${productName} Web Setup ${version}.${ext}"
}

Expand Down
7 changes: 7 additions & 0 deletions packages/app-builder-lib/src/targets/nsis/nsisOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,13 @@ export interface NsisOptions extends CommonNsisOptions, CommonWindowsInstallerCo
* @default [".avi", ".mov", ".m4v", ".mp4", ".m4p", ".qt", ".mkv", ".webm", ".vmdk"]
*/
readonly preCompressedFileExtensions?: Array<string> | string | null

/**
* Disable building an universal installer of the archs specified in the target configuration
* *Not supported for nsis-web*
* @default true
*/
readonly buildUniversalInstaller?: boolean
}

/**
Expand Down
19 changes: 18 additions & 1 deletion test/snapshots/windows/webInstallerTest.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,13 @@ exports[`web installer 1`] = `
},
],
"packages": {
"arm64": {
"blockMapSize": "@blockMapSize",
"file": "TestApp-1.1.0-arm64.nsis.7z",
"path": "TestApp-1.1.0-arm64.nsis.7z",
"sha512": "@sha512",
"size": "@size",
},
"x64": {
"blockMapSize": "@blockMapSize",
"file": "TestApp-1.1.0-x64.nsis.7z",
Expand All @@ -114,11 +121,17 @@ exports[`web installer 1`] = `
},
},
{
"arch": "x64",
"file": "Test App ßW Web Setup 1.1.0.exe",
"safeArtifactName": "TestApp-WebSetup-1.1.0.exe",
"updateInfo": {
"packages": {
"arm64": {
"blockMapSize": "@blockMapSize",
"file": "TestApp-1.1.0-arm64.nsis.7z",
"path": "TestApp-1.1.0-arm64.nsis.7z",
"sha512": "@sha512",
"size": "@size",
},
"x64": {
"blockMapSize": "@blockMapSize",
"file": "TestApp-1.1.0-x64.nsis.7z",
Expand All @@ -129,6 +142,10 @@ exports[`web installer 1`] = `
},
},
},
{
"arch": "arm64",
"file": "TestApp-1.1.0-arm64.nsis.7z",
},
{
"arch": "x64",
"file": "TestApp-1.1.0-x64.nsis.7z",
Expand Down
22 changes: 22 additions & 0 deletions test/snapshots/windows/winPackagerTest.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,23 @@
exports[`beta version 1`] = `
{
"win": [
{
"arch": "arm64",
"file": "Test App ßW Setup 3.0.0-beta.2-arm64.exe",
"safeArtifactName": "TestApp-Setup-3.0.0-beta.2-arm64.exe",
"updateInfo": {
"sha512": "@sha512",
"size": "@size",
},
},
{
"file": "Test App ßW Setup 3.0.0-beta.2-arm64.exe.blockmap",
"safeArtifactName": "TestApp-Setup-3.0.0-beta.2-arm64.exe.blockmap",
"updateInfo": {
"sha512": "@sha512",
"size": "@size",
},
},
{
"arch": "x64",
"file": "Test App ßW Setup 3.0.0-beta.2.exe",
Expand Down Expand Up @@ -31,6 +48,11 @@ exports[`icon not an image 1`] = `"ERR_ICON_UNKNOWN_FORMAT"`;
exports[`win zip 1`] = `
{
"win": [
{
"arch": "arm64",
"file": "Test App ßW-1.1.0-arm64-win.zip",
"safeArtifactName": "TestApp-1.1.0-arm64-win.zip",
},
{
"arch": "x64",
"file": "Test App ßW-1.1.0-win.zip",
Expand Down
5 changes: 4 additions & 1 deletion test/src/windows/webInstallerTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { app } from "../helpers/packTester"
test.ifNotCiMac(
"web installer",
app({
targets: Platform.WINDOWS.createTarget(["nsis-web"], Arch.x64),
targets: Platform.WINDOWS.createTarget(["nsis-web"], Arch.x64, Arch.arm64),
config: {
publish: {
provider: "s3",
Expand All @@ -23,6 +23,9 @@ test.ifNotCiMac(
loadBrowserProcessSpecificV8Snapshot: true,
grantFileProtocolExtraPrivileges: undefined, // unsupported on current electron version in our tests
},
nsisWeb: {
buildUniversalInstaller: false,
},
},
})
)
Expand Down
9 changes: 6 additions & 3 deletions test/src/windows/winPackagerTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,25 @@ import { CheckingWinPackager } from "../helpers/CheckingPackager"
import { app, appThrows, assertPack, platform } from "../helpers/packTester"
import * as fs from "fs/promises"

test.ifWinCi(
test.ifNotCiMac(
"beta version",
app({
targets: Platform.WINDOWS.createTarget(["nsis"], Arch.x64),
targets: Platform.WINDOWS.createTarget(["nsis"], Arch.x64, Arch.arm64),
config: {
extraMetadata: {
version: "3.0.0-beta.2",
},
nsis: {
buildUniversalInstaller: false,
},
},
})
)

test.ifNotCiMac(
"win zip",
app({
targets: Platform.WINDOWS.createTarget(["zip"], Arch.x64),
targets: Platform.WINDOWS.createTarget(["zip"], Arch.x64, Arch.arm64),
config: {
downloadAlternateFFmpeg: true,
electronFuses: {
Expand Down
Loading