-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
base: master
Are you sure you want to change the base?
Changes from all commits
f4f4fe5
01e2a69
f22fb3b
d40e52d
f33abb7
78240ff
07d1c99
1b41b17
90eb7a7
f84cecc
1b2a97a
64c11ce
d5fc3f4
95b4a3d
4e39a84
5f04f59
b5914a3
833d49b
ae6545b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 |
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" | ||
|
@@ -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" | ||
|
@@ -73,8 +86,16 @@ | |
NsisTargetOptions.resolve(this.options) | ||
} | ||
|
||
get shouldBuildUniversalInstaller() { | ||
const buildSeparateInstallers = this.options.buildUniversalInstaller === false | ||
return !buildSeparateInstallers | ||
} | ||
|
||
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() | ||
} | ||
|
||
|
@@ -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 GitHub Actions / test-linux (ArtifactPublisherTest,BuildTest,ExtraBuildTest,RepoSlugTest,binDownloadTest,configura...
|
||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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}";
} There was a problem hiding this comment. Choose a reason for hiding this commentThe 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))) | ||
|
@@ -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) | ||
|
||
|
@@ -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) | ||
|
@@ -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 { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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.There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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