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
Add support for prebuilt asars #823
Changes from all commits
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 |
---|---|---|
|
@@ -14,6 +14,7 @@ class App { | |
constructor (opts, templatePath) { | ||
this.opts = opts | ||
this.templatePath = templatePath | ||
this.asarOptions = common.createAsarOpts(opts) | ||
|
||
if (this.opts.prune === undefined) { | ||
this.opts.prune = true | ||
|
@@ -68,6 +69,10 @@ class App { | |
} | ||
} | ||
|
||
get appAsarPath () { | ||
return path.join(this.originalResourcesDir, 'app.asar') | ||
} | ||
|
||
relativeRename (basePath, oldName, newName) { | ||
debug(`Renaming ${oldName} to ${newName} in ${basePath}`) | ||
return fs.rename(path.join(basePath, oldName), path.join(basePath, newName)) | ||
|
@@ -80,11 +85,14 @@ class App { | |
/** | ||
* Performs the following initial operations for an app: | ||
* * Creates temporary directory | ||
* * Copies template into temporary directory | ||
* * Copies user's app into temporary directory | ||
* * Prunes non-production node_modules (if opts.prune is either truthy or undefined) | ||
* * Remove default_app (which is either a folder or an asar file) | ||
* * Creates an asar (if opts.asar is set) | ||
* * If a prebuilt asar is specified: | ||
* * Copies asar into temporary directory as app.asar | ||
* * Otherwise: | ||
* * Copies template into temporary directory | ||
* * Copies user's app into temporary directory | ||
* * Prunes non-production node_modules (if opts.prune is either truthy or undefined) | ||
* * Creates an asar (if opts.asar is set) | ||
* | ||
* Prune and asar are performed before platform-specific logic, primarily so that | ||
* this.originalResourcesAppDir is predictable (e.g. before .app is renamed for Mac) | ||
|
@@ -93,32 +101,40 @@ class App { | |
debug(`Initializing app in ${this.stagingPath} from ${this.templatePath} template`) | ||
|
||
return fs.move(this.templatePath, this.stagingPath, { clobber: true }) | ||
.then(() => this.copyTemplate()) | ||
.then(() => this.removeDefaultApp()) | ||
.then(() => { | ||
if (this.opts.prebuiltAsar) { | ||
return this.copyPrebuiltAsar() | ||
} else { | ||
return this.buildApp() | ||
} | ||
}) | ||
} | ||
|
||
buildApp () { | ||
return this.copyTemplate() | ||
.then(() => this.asarApp()) | ||
} | ||
|
||
copyTemplate () { | ||
return fs.copy(this.opts.dir, this.originalResourcesAppDir, { | ||
filter: ignore.userIgnoreFilter(this.opts), | ||
dereference: this.opts.derefSymlinks | ||
}).then(() => hooks.promisifyHooks(this.opts.afterCopy, [ | ||
const hookArgs = [ | ||
this.originalResourcesAppDir, | ||
this.opts.electronVersion, | ||
this.opts.platform, | ||
this.opts.arch | ||
])).then(() => { | ||
if (this.opts.prune) { | ||
return hooks.promisifyHooks(this.opts.afterPrune, [ | ||
this.originalResourcesAppDir, | ||
this.opts.electronVersion, | ||
this.opts.platform, | ||
this.opts.arch | ||
]) | ||
} else { | ||
return true | ||
} | ||
] | ||
|
||
return fs.copy(this.opts.dir, this.originalResourcesAppDir, { | ||
filter: ignore.userIgnoreFilter(this.opts), | ||
dereference: this.opts.derefSymlinks | ||
}) | ||
.then(() => hooks.promisifyHooks(this.opts.afterCopy, hookArgs)) | ||
.then(() => { | ||
if (this.opts.prune) { | ||
return hooks.promisifyHooks(this.opts.afterPrune, hookArgs) | ||
} | ||
return true | ||
}) | ||
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. I don't know whether afterPrune and afterCopy hooks should be run, since neither of those operations actually took place, and 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. Yeah, I didn't know what those hooks were for... I've changed it to just return early. 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. I don't think returning early is the right solution... we may have to adjust the hook signature so that it's either the path to the resources app directory, or the path to the @MarshallOfSound since you have dealt with a lot of code relating to the hooks, you might have some insight here. |
||
} | ||
|
||
removeDefaultApp () { | ||
|
@@ -146,15 +162,47 @@ class App { | |
.catch(/* istanbul ignore next */ () => null) | ||
} | ||
|
||
prebuiltAsarWarning (option, triggerWarning) { | ||
if (triggerWarning) { | ||
common.warning(`prebuiltAsar and ${option} are incompatible, ignoring the ${option} option`) | ||
} | ||
} | ||
|
||
copyPrebuiltAsar () { | ||
if (this.asarOptions) { | ||
common.warning('prebuiltAsar has been specified, all asar options will be ignored') | ||
} | ||
|
||
for (const hookName of ['afterCopy', 'afterPrune']) { | ||
if (this.opts[hookName]) { | ||
throw new Error(`${hookName} is incompatible with prebuiltAsar`) | ||
} | ||
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. This should have a test. 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. what do you mean? Test that it warns? or test that the other options are ignored? If the later, how should that be done? 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. Test that it warns. This is an example of doing that (although it should probably be genericized into a function): 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. done. |
||
} | ||
|
||
this.prebuiltAsarWarning('ignore', this.opts.originalIgnore) | ||
this.prebuiltAsarWarning('prune', !this.opts.prune) | ||
this.prebuiltAsarWarning('derefSymlinks', this.opts.derefSymlinks !== undefined) | ||
|
||
const src = path.resolve(this.opts.prebuiltAsar) | ||
|
||
return fs.stat(src) | ||
.then(stat => { | ||
if (!stat.isFile()) { | ||
throw new Error(`${src} specified in prebuiltAsar must be an asar file.`) | ||
} | ||
|
||
debug(`Copying asar: ${src} to ${this.appAsarPath}`) | ||
return fs.copy(src, this.appAsarPath, {overwrite: false, errorOnExist: true}) | ||
}) | ||
} | ||
|
||
asarApp () { | ||
const asarOptions = common.createAsarOpts(this.opts) | ||
if (!asarOptions) { | ||
if (!this.asarOptions) { | ||
return Promise.resolve() | ||
} | ||
|
||
const dest = path.join(this.originalResourcesDir, 'app.asar') | ||
debug(`Running asar with the options ${JSON.stringify(asarOptions)}`) | ||
return pify(asar.createPackageWithOptions)(this.originalResourcesAppDir, dest, asarOptions) | ||
debug(`Running asar with the options ${JSON.stringify(this.asarOptions)}`) | ||
return pify(asar.createPackageWithOptions)(this.originalResourcesAppDir, this.appAsarPath, this.asarOptions) | ||
.then(() => fs.remove(this.originalResourcesAppDir)) | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
{ | ||
"name": "PrebuiltAsar", | ||
"version": "0.0.1", | ||
"description": "A prebuilt asar app", | ||
"main": "main.js", | ||
"devDependencies": { | ||
"electron": "1.3.1" | ||
} | ||
} |
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.
I would rather have a real
if
statement than a multi-line ternary statement.