Merge pull request #3608 from atom/cp-execfile

Add docs and tests for child_process.execFile

atom/common/api/atom_api_asar.cc

@@ -54,7 +54,6 @@ class Archive : public mate::Wrappable {
     mate::Dictionary dict(isolate, v8::Object::New(isolate));
     dict.Set("size", stats.size);
     dict.Set("offset", stats.offset);
-    dict.Set("executable", stats.executable);
     dict.Set("isFile", stats.is_file);
     dict.Set("isDirectory", stats.is_directory);
     dict.Set("isLink", stats.is_link);

atom/common/asar/archive.cc

@@ -13,6 +13,7 @@
 
 #include "atom/common/asar/scoped_temporary_file.h"
 #include "base/files/file.h"
+#include "base/files/file_util.h"
 #include "base/logging.h"
 #include "base/pickle.h"
 #include "base/json/json_reader.h"
@@ -96,7 +97,6 @@ bool FillFileInfoWithNode(Archive::FileInfo* info,
     return false;
   info->size = static_cast<uint32>(size);
 
-  info->unpacked = false;
   if (node->GetBoolean("unpacked", &info->unpacked) && info->unpacked)
     return true;
 
@@ -107,7 +107,6 @@ bool FillFileInfoWithNode(Archive::FileInfo* info,
     return false;
   info->offset += header_size;
 
-  info->executable = false;
   node->GetBoolean("executable", &info->executable);
 
   return true;
@@ -276,6 +275,13 @@ bool Archive::CopyFileOut(const base::FilePath& path, base::FilePath* out) {
   if (!temp_file->InitFromFile(&file_, info.offset, info.size))
     return false;
 
+#if defined(OS_POSIX)
+  if (info.executable) {
+    // chmod a+x temp_file;
+    base::SetPosixFilePermissions(temp_file->path(), 0755);
+  }
+#endif
+
   *out = temp_file->path();
   external_files_.set(path, temp_file.Pass());
   return true;

atom/common/asar/archive.h

@@ -25,7 +25,7 @@ class ScopedTemporaryFile;
 class Archive {
  public:
   struct FileInfo {
-    FileInfo() : size(0), offset(0) {}
+    FileInfo() : unpacked(false), executable(false), size(0), offset(0) {}
     bool unpacked;
     bool executable;
     uint32 size;

atom/common/lib/asar.coffee

@@ -1,6 +1,5 @@
 asar = process.binding 'atom_common_asar'
 child_process = require 'child_process'
-fs = require 'fs'
 path = require 'path'
 util = require 'util'
 
@@ -84,11 +83,6 @@ overrideAPISync = (module, name, arg = 0) ->
     newPath = archive.copyFileOut filePath
     notFoundError asarPath, filePath unless newPath
 
-    stat = archive.stat filePath
-
-    if stat.executable
-      fs.chmodSync(newPath, 0o755)
-
     arguments[arg] = newPath
     old.apply this, arguments
 
@@ -108,11 +102,6 @@ overrideAPI = (module, name, arg = 0) ->
     newPath = archive.copyFileOut filePath
     return notFoundError asarPath, filePath, callback unless newPath
 
-    stat = archive.stat filePath
-
-    if stat.executable
-      fs.chmodSync(newPath, 0o755)
-
     arguments[arg] = newPath
     old.apply this, arguments
 

docs/tutorial/application-packaging.md

@@ -132,6 +132,7 @@ work. This adds a little overhead for those APIs.
 APIs that requires extra unpacking are:
 
 * `child_process.execFile`
+* `child_process.execFileSync`
 * `fs.open`
 * `fs.openSync`
 * `process.dlopen` - Used by `require` on native modules
@@ -143,6 +144,17 @@ archives is generated by guessing, because those files do not exist on the
 filesystem. So you should not trust the `Stats` object except for getting file
 size and checking file type.
 
+### Executing Binaries Inside `asar` Archive
+
+There are Node APIs that can execute binaries like `child_process.exec`,
+`child_process.spawn` and `child_process.execFile`, but only `execFile` is
+supported to execute binaries inside `asar` archive.
+
+This is because `exec` and `spawn` accept `command` instead of `file` as input,
+and `command`s are executed under shell. There is no reliable way to determine
+whether a command uses a file in asar archive, and even if we do, we can not be
+sure whether we can replace the path in command without side effects.
+
 ## Adding Unpacked Files in `asar` Archive
 
 As stated above, some Node APIs will unpack the file to filesystem when

spec/asar-spec.coffee

@@ -392,6 +392,22 @@ describe 'asar package', ->
           done()
         child.send file
 
+    describe 'child_process.execFile', ->
+      return unless process.platform is 'darwin'
+
+      {execFile, execFileSync} = require 'child_process'
+      echo = path.join fixtures, 'asar', 'echo.asar', 'echo'
+
+      it 'executes binaries', (done) ->
+        child = execFile echo, ['test'], (error, stdout) ->
+          assert.equal error, null
+          assert.equal stdout, 'test\n'
+          done()
+
+      it 'execFileSync executes binaries', ->
+        output = execFileSync echo, ['test']
+        assert.equal String(output), 'test\n'
+
     describe 'internalModuleReadFile', ->
       internalModuleReadFile = process.binding('fs').internalModuleReadFile