Skip to content

Commit bfca0a7

Browse files
develardevelar
committed
fix: zip, dmg and exe filenames do not use productName as intended
1 parent 5eea750 commit bfca0a7

File tree

8 files changed

+97
-77
lines changed

8 files changed

+97
-77
lines changed

‎src/builder.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,13 +76,13 @@ export async function build(options: BuildOptions = {}): Promise<void> {
7676
const packager = new Packager(options, repositoryInfo)
7777
if (options.publish != null && options.publish !== "never") {
7878
let publisher: BluebirdPromise<Publisher> = null
79-
packager.artifactCreated((file, platform) => {
79+
packager.artifactCreated(event => {
8080
if (publisher == null) {
8181
publisher = <BluebirdPromise<Publisher>>createPublisher(packager, options, repositoryInfo, isPublishOptionGuessed)
8282
}
8383

8484
if (publisher != null) {
85-
publisher.then(it => publishTasks.push(<BluebirdPromise<any>>it.upload(file)))
85+
publisher.then(it => publishTasks.push(<BluebirdPromise<any>>it.upload(event.file, event.artifactName)))
8686
}
8787
})
8888
}

‎src/gitHubPublisher.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import ProgressBar = require("progress")
1515
const __awaiter = require("./awaiter")
1616

1717
export interface Publisher {
18-
upload(path: string): Promise<any>
18+
upload(file: string, artifactName?: string): Promise<any>
1919
}
2020

2121
export interface PublishOptions {
@@ -66,15 +66,15 @@ export class GitHubPublisher implements Publisher {
6666
}
6767
}
6868

69-
async upload(path: string): Promise<void> {
70-
const fileName = basename(path)
69+
async upload(file: string, artifactName?: string): Promise<void> {
70+
const fileName = artifactName || basename(file)
7171
const release = await this.releasePromise
7272
if (release == null) {
7373
return null
7474
}
7575

7676
const parsedUrl = parseUrl(release.upload_url.substring(0, release.upload_url.indexOf("{")) + "?name=" + fileName)
77-
const fileStat = await stat(path)
77+
const fileStat = await stat(file)
7878
let badGatewayCount = 0
7979
uploadAttempt: for (let i = 0; i < 3; i++) {
8080
const progressBar = (<ReadStream>process.stdin).isTTY ? new ProgressBar(`Uploading ${fileName} [:bar] :percent :etas`, {
@@ -96,7 +96,7 @@ export class GitHubPublisher implements Publisher {
9696
"Content-Length": fileStat.size
9797
}
9898
}, this.token, (request, reject) => {
99-
const fileInputStream = createReadStream(path)
99+
const fileInputStream = createReadStream(file)
100100
fileInputStream.on("error", reject)
101101
fileInputStream
102102
.pipe(progressStream({

‎src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
export { Packager } from "./packager"
2-
export { PackagerOptions } from "./platformPackager"
2+
export { PackagerOptions, ArtifactCreated } from "./platformPackager"
33
export { AppMetadata, DevMetadata, Platform, getProductName } from "./metadata"

‎src/macPackager.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ export default class MacPackager extends PlatformPackager<OsXBuildOptions> {
5353
}
5454

5555
packageInDistributableFormat(outDir: string, appOutDir: string): Promise<any> {
56-
const artifactPath = path.join(appOutDir, this.metadata.name + "-" + this.metadata.version + ".dmg")
56+
const artifactPath = path.join(appOutDir, `${this.appName}-${this.metadata.version}.dmg`)
5757
return BluebirdPromise.all([
5858
new BluebirdPromise<any>((resolve, reject) => {
5959
log("Creating DMG")
@@ -83,17 +83,18 @@ export default class MacPackager extends PlatformPackager<OsXBuildOptions> {
8383
emitter.on("error", reject)
8484
emitter.on("finish", () => resolve())
8585
})
86-
.then(() => this.dispatchArtifactCreated(artifactPath)),
86+
.then(() => this.dispatchArtifactCreated(artifactPath, `${this.metadata.name}-${this.metadata.version}.dmg`)),
8787

8888
this.zipMacApp(appOutDir)
89-
.then(it => this.dispatchArtifactCreated(it))
89+
.then(it => this.dispatchArtifactCreated(it, `${this.metadata.name}-${this.metadata.version}-mac.zip`))
9090
])
9191
}
9292

9393
private zipMacApp(outDir: string): Promise<string> {
9494
log("Creating ZIP for Squirrel.Mac")
95+
// we use app name here - see https://github.com/electron-userland/electron-builder/pull/204
96+
const resultPath = `${this.appName}-${this.metadata.version}-mac.zip`
9597
// -y param is important - "store symbolic links as the link instead of the referenced file"
96-
const resultPath = `${this.metadata.name}-${this.metadata.version}-mac.zip`
9798
const args = ["-ryXq", resultPath, this.appName + ".app"]
9899

99100
// todo move to options

‎src/packager.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ import { all, executeFinally } from "./promise"
55
import { EventEmitter } from "events"
66
import { Promise as BluebirdPromise } from "bluebird"
77
import { InfoRetriever } from "./repositoryInfo"
8-
import { AppMetadata, Platform, DevMetadata } from "./metadata"
9-
import { PackagerOptions, PlatformPackager, BuildInfo } from "./platformPackager"
8+
import { AppMetadata, DevMetadata } from "./metadata"
9+
import { PackagerOptions, PlatformPackager, BuildInfo, ArtifactCreated } from "./platformPackager"
1010
import MacPackager from "./macPackager"
1111
import WinPackager from "./winPackager"
1212
import * as errorMessages from "./errorMessages"
@@ -38,7 +38,7 @@ export class Packager implements BuildInfo {
3838
this.appDir = this.computeAppDirectory()
3939
}
4040

41-
artifactCreated(handler: (file: string, platform: Platform) => void): Packager {
41+
artifactCreated(handler: (event: ArtifactCreated) => void): Packager {
4242
addHandler(this.eventEmitter, "artifactCreated", handler)
4343
return this
4444
}

‎src/platformPackager.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,12 @@ export abstract class PlatformPackager<DC extends PlatformSpecificBuildOptions>
8080
return (directories == null ? null : directories.buildResources) || "build"
8181
}
8282

83-
protected dispatchArtifactCreated(file: string) {
84-
this.info.eventEmitter.emit("artifactCreated", file, this.platform)
83+
protected dispatchArtifactCreated(file: string, artifactName?: string) {
84+
this.info.eventEmitter.emit("artifactCreated", {
85+
file: file,
86+
artifactName: artifactName,
87+
platform: this.platform,
88+
})
8589
}
8690

8791
async pack(platform: string, outDir: string, appOutDir: string, arch: string): Promise<any> {
@@ -155,4 +159,11 @@ function checkConflictingOptions(options: any): void {
155159
throw new Error(`Option ${name} is ignored, do not specify it.`)
156160
}
157161
}
162+
}
163+
164+
export interface ArtifactCreated {
165+
readonly file: string
166+
readonly artifactName?: string
167+
168+
readonly platform: Platform
158169
}

‎src/winPackager.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,6 @@ export default class WinPackager extends PlatformPackager<WinBuildOptions> {
107107
noMsi: true,
108108
}, this.customBuildOptions)
109109

110-
// we use metadata.name instead of appName because appName can contains unsafe chars
111-
const installerExePath = path.join(installerOutDir, this.metadata.name + "Setup-" + version + archSuffix + ".exe")
112110
try {
113111
await require("electron-winstaller-fixed").createWindowsInstaller(options)
114112
}
@@ -141,8 +139,8 @@ export default class WinPackager extends PlatformPackager<WinBuildOptions> {
141139
}
142140

143141
const promises: Array<Promise<any>> = [
144-
rename(path.join(installerOutDir, "Setup.exe"), installerExePath)
145-
.then(it => this.dispatchArtifactCreated(it)),
142+
rename(path.join(installerOutDir, "Setup.exe"), path.join(installerOutDir, `${this.appName}Setup-${version}${archSuffix}.exe`))
143+
.then(it => this.dispatchArtifactCreated(it, `${this.metadata.name}Setup-${version}${archSuffix}.exe`)),
146144
]
147145

148146
if (archSuffix === "") {

‎test/src/helpers/packTester.ts

Lines changed: 66 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { parse as parsePlist } from "plist"
55
import { CSC_LINK, CSC_KEY_PASSWORD } from "./codeSignData"
66
import { expectedLinuxContents } from "./expectedContents"
77
import { readText } from "out/promisifed-fs"
8-
import { Packager, PackagerOptions, Platform, getProductName } from "out"
8+
import { Packager, PackagerOptions, Platform, getProductName, ArtifactCreated } from "out"
99
import { normalizePlatforms } from "out/packager"
1010
import { exec } from "out/util"
1111
import pathSorter = require("path-sort")
@@ -73,15 +73,15 @@ export async function assertPack(fixtureName: string,
7373
async function packAndCheck(projectDir: string, packagerOptions: PackagerOptions): Promise<void> {
7474
const packager = new Packager(packagerOptions)
7575

76-
const artifacts: Map<Platform, Array<string>> = new Map()
77-
packager.artifactCreated((file, platform) => {
78-
assertThat(path.isAbsolute(file)).true()
79-
let list = artifacts.get(platform)
76+
const artifacts: Map<Platform, Array<ArtifactCreated>> = new Map()
77+
packager.artifactCreated(event => {
78+
assertThat(path.isAbsolute(event.file)).true()
79+
let list = artifacts.get(event.platform)
8080
if (list == null) {
8181
list = []
82-
artifacts.set(platform, list)
82+
artifacts.set(event.platform, list)
8383
}
84-
list.push(file)
84+
list.push(event)
8585
})
8686

8787
await packager.build()
@@ -90,44 +90,41 @@ async function packAndCheck(projectDir: string, packagerOptions: PackagerOptions
9090
return
9191
}
9292

93-
for (let key of artifacts.keys()) {
94-
artifacts.set(key, pathSorter(artifacts.get(key)))
95-
}
96-
97-
const expandedPlatforms = normalizePlatforms(packagerOptions.platform)
98-
if (expandedPlatforms.includes("darwin")) {
99-
await checkOsXResult(packager, artifacts.get(Platform.OSX))
100-
}
101-
else if (expandedPlatforms.includes("linux")) {
102-
const productName = getProductName(packager.metadata, packager.devMetadata)
103-
const expectedContents = expectedLinuxContents.map(it => {
104-
if (it === "/opt/TestApp/TestApp") {
105-
return "/opt/" + productName + "/" + productName
106-
}
107-
else if (it === "/usr/share/applications/TestApp.desktop") {
108-
return `/usr/share/applications/${productName}.desktop`
109-
}
110-
else {
111-
return it.replace(new RegExp("/opt/TestApp/", "g"), `/opt/${productName}/`)
93+
for (let platform of normalizePlatforms(packagerOptions.platform)) {
94+
if (platform === "darwin") {
95+
await checkOsXResult(packager, artifacts.get(Platform.OSX))
96+
}
97+
else if (platform === "linux") {
98+
const productName = getProductName(packager.metadata, packager.devMetadata)
99+
const expectedContents = expectedLinuxContents.map(it => {
100+
if (it === "/opt/TestApp/TestApp") {
101+
return "/opt/" + productName + "/" + productName
102+
}
103+
else if (it === "/usr/share/applications/TestApp.desktop") {
104+
return `/usr/share/applications/${productName}.desktop`
105+
}
106+
else {
107+
return it.replace(new RegExp("/opt/TestApp/", "g"), `/opt/${productName}/`)
108+
}
109+
})
110+
111+
// console.log(JSON.stringify(await getContents(projectDir + "/dist/TestApp-1.0.0-amd64.deb", productName), null, 2))
112+
// console.log(JSON.stringify(await getContents(projectDir + "/dist/TestApp-1.0.0-i386.deb", productName), null, 2))
113+
114+
assertThat(await getContents(projectDir + "/dist/TestApp-1.0.0-amd64.deb", productName)).deepEqual(expectedContents)
115+
if (packagerOptions == null || packagerOptions.arch === null || packagerOptions.arch === "ia32") {
116+
assertThat(await getContents(projectDir + "/dist/TestApp-1.0.0-i386.deb", productName)).deepEqual(expectedContents)
112117
}
113-
})
114-
115-
// console.log(JSON.stringify(await getContents(projectDir + "/dist/TestApp-1.0.0-amd64.deb", productName), null, 2))
116-
// console.log(JSON.stringify(await getContents(projectDir + "/dist/TestApp-1.0.0-i386.deb", productName), null, 2))
117-
118-
assertThat(await getContents(projectDir + "/dist/TestApp-1.0.0-amd64.deb", productName)).deepEqual(expectedContents)
119-
if (packagerOptions == null || packagerOptions.arch === null || packagerOptions.arch === "ia32") {
120-
assertThat(await getContents(projectDir + "/dist/TestApp-1.0.0-i386.deb", productName)).deepEqual(expectedContents)
121118
}
122-
}
123-
else if (expandedPlatforms.includes("win32") && (packagerOptions == null || packagerOptions.target == null)) {
124-
await checkWindowsResult(packagerOptions, artifacts.get(Platform.WINDOWS))
119+
else if (platform === "win32" && (packagerOptions == null || packagerOptions.target == null)) {
120+
await checkWindowsResult(packager, packagerOptions, artifacts.get(Platform.WINDOWS))
121+
}
125122
}
126123
}
127124

128-
async function checkOsXResult(packager: Packager, artifacts: Array<string>) {
125+
async function checkOsXResult(packager: Packager, artifacts: Array<ArtifactCreated>) {
129126
const productName = getProductName(packager.metadata, packager.devMetadata)
130-
const packedAppDir = path.join(path.dirname(artifacts[0]), (productName || packager.metadata.name) + ".app")
127+
const packedAppDir = path.join(path.dirname(artifacts[0].file), (productName || packager.metadata.name) + ".app")
131128
const info = parsePlist(await readText(path.join(packedAppDir, "Contents", "Info.plist")))
132129
assertThat(info).has.properties({
133130
CFBundleDisplayName: productName,
@@ -139,30 +136,43 @@ async function checkOsXResult(packager: Packager, artifacts: Array<string>) {
139136
const result = await exec("codesign", ["--verify", packedAppDir])
140137
assertThat(result[0].toString()).not.match(/is not signed at all/)
141138

142-
assertThat(artifacts.map(it => path.basename((it))).sort()).deepEqual([
139+
assertThat(artifacts.map(it => path.basename(it.file)).sort()).deepEqual([
140+
`${productName}-1.0.0-mac.zip`,
141+
`${productName}-1.0.0.dmg`,
142+
].sort())
143+
144+
assertThat(artifacts.map(it => it.artifactName).sort()).deepEqual([
143145
"TestApp-1.0.0-mac.zip",
144-
"TestApp-1.0.0.dmg"
146+
"TestApp-1.0.0.dmg",
145147
].sort())
146148
}
147149

148-
async function checkWindowsResult(packagerOptions: PackagerOptions, artifacts: Array<string>) {
149-
const expected32 = [
150-
"RELEASES-ia32",
151-
"TestAppSetup-1.0.0-ia32.exe",
152-
"TestApp-1.0.0-ia32-full.nupkg",
153-
]
154-
const expected64 = [
155-
"RELEASES",
156-
"TestAppSetup-1.0.0.exe",
157-
"TestApp-1.0.0-full.nupkg",
158-
]
159-
const expected = packagerOptions != null && packagerOptions.arch === "x64" ? expected64 : expected32.concat(expected64)
160-
const filenames = artifacts.map(it => path.basename((it)))
161-
assertThat(filenames.slice().sort()).deepEqual(expected.sort())
150+
async function checkWindowsResult(packager: Packager, packagerOptions: PackagerOptions, artifacts: Array<ArtifactCreated>) {
151+
const productName = getProductName(packager.metadata, packager.devMetadata)
152+
153+
function getWinExpected(archSuffix: string) {
154+
return [
155+
`RELEASES${archSuffix}`,
156+
`${productName}Setup-1.0.0${archSuffix}.exe`,
157+
`TestApp-1.0.0${archSuffix}-full.nupkg`,
158+
]
159+
}
160+
161+
const archSuffix = packagerOptions != null && packagerOptions.arch === "x64" ? "" : "-ia32"
162+
const expected = archSuffix == "" ? getWinExpected(archSuffix) : getWinExpected(archSuffix).concat(getWinExpected(""))
163+
164+
const filenames = artifacts.map(it => path.basename(it.file))
165+
assertThat(filenames.slice().sort()).deepEqual(expected.slice().sort())
162166

163167
let i = filenames.indexOf("RELEASES-ia32")
164168
if (i !== -1) {
165-
assertThat((await readText(artifacts[i])).indexOf("ia32")).not.equal(-1)
169+
assertThat((await readText(artifacts[i].file)).indexOf("ia32")).not.equal(-1)
170+
}
171+
172+
if (archSuffix == "") {
173+
const expectedArtifactNames = expected.slice()
174+
expectedArtifactNames[1] = `TestAppSetup-1.0.0${archSuffix}.exe`
175+
assertThat(artifacts.map(it => it.artifactName).filter(it => it != null)).deepEqual([`TestAppSetup-1.0.0${archSuffix}.exe`])
166176
}
167177
}
168178

0 commit comments

Comments
 (0)