Closed Bug 1031123 Opened 10 years ago Closed 10 years ago

Using sdk/system/child_process for utils.Commander

Categories

(Firefox OS Graveyard :: Gaia::Build, defect)

Product:
Firefox OS Graveyard
Component:
Gaia::Build
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: yurenju, Assigned: gduan)

References

Details

Attachments

(1 file)

PR to master
46 bytes, text/x-github-pull-request
yurenju
: review+
Details | Review 
since child_process module can be used to capture stdout, we should use it in utils.Commander.

George, are you interested in this issue?
Flags: needinfo?(gduan)
Assignee: nobody → gduan
Flags: needinfo?(gduan)
more information from Alex from bug 1029966

(In reply to Alexandre Poirot (:ochameau) from comment #3)
> (In reply to Yuren [:yurenju] from comment #1)
> > Created attachment 8445786 [details] [review]
> > github PR: https://github.com/mozilla-b2g/gaia/pull/20970
> > 
> > Alex, could you review this pull request? and I also found we can't get
> > STDOUT from nsIProccess, do you have any idea for that?
> 
> Yes... nsIProcess is extremelly old and limited.
> We now have some new ways to run processes, like child_process (which is
> supposed to have same API than node):
>  
> http://mxr.mozilla.org/mozilla-central/source/addon-sdk/source/lib/sdk/
> system/child_process.js
> Or subprocess (child_process is based on it):
>  
> http://mxr.mozilla.org/mozilla-central/source/addon-sdk/source/lib/sdk/
> system/child_process/subprocess.js
> These new APIs allow fine control on std IOs!
I will get below error while requiring "sdk/system/child_process"

Exception: Module `@test/options` is not found at file:///Users/tuangeorge/code/gaia/build/@test/options.js
@resource://gre/modules/commonjs/sdk/system.js:15:1
@resource://gre/modules/commonjs/sdk/system/child_process.js:16:1
@file:///Users/tuangeorge/code/gaia/build/test.js:20:1  ==> only one line "require("sdk/system/child_process");"
CommonjsRunner.prototype.run@/Users/tuangeorge/code/gaia/build/xpcshell-commonjs.js:69:5
run@/Users/tuangeorge/code/gaia/build/xpcshell-commonjs.js:84:3

However,
require("sdk/system/child_process/subprocess");
this works just fine.
Hi Alex,
Do you know why I got error like comment 2 when trying to require("sdk/system/child_process") ?

Since that, I use require("sdk/system/child_process/subprocess") in attachment 8447805 [details] [review] .
However, it cannot run in promise pattern and I'm not sure the reason (gecko bug?).
Please see https://github.com/cctuan/gaia/commit/b3c6f5e9fe40ab699cbfed8ce8d5a4015deb16e3 

Could you advise?
Flags: needinfo?(poirot.alex)
My patch run well on local side but somehow failed on travis with below log.

Exception: Module `sdk/system/child_process/subprocess` is not found at resource://gre/modules/commonjs/sdk/system/child_process/subprocess.js
(In reply to George Duan [:gduan] [:喬智] from comment #5)
> My patch run well on local side but somehow failed on travis with below log.

Most likely xulrunner version mismatch, see bug 1032082 comment.

(In reply to George Duan [:gduan] [:喬智] from comment #4)
> Hi Alex,
> Do you know why I got error like comment 2 when trying to
> require("sdk/system/child_process") ?

That's because system.js module is only expected to be loaded in a regular firefox addon.
Firefox addons define a magic module "@loader/options" and "@test/options" with additional runtime options.
We can workaround that by defining these modules with empty objects from here:
https://github.com/mozilla-b2g/gaia/blob/master/build/xpcshell-commonjs.js#L42
  modules: {
    'toolkit/loader': Loader,
+   '@test/options': {},
+   '@loader/options': {}
  }

But ideally, we would patch the Addon-SDK and make it so that system.js doesn't throw if one of these magic modules aren't available!

> Since that, I use require("sdk/system/child_process/subprocess") in
> attachment 8447805 [details] [review] .
> However, it cannot run in promise pattern and I'm not sure the reason (gecko
> bug?).
> Please see
> https://github.com/cctuan/gaia/commit/
> b3c6f5e9fe40ab699cbfed8ce8d5a4015deb16e3 

No clear idea about that one... Could it be that we quit xpcshell process while subprocess is still trying to execute the process? Can you try calling processEvents() after calling subprocess? If that doesn't fix the crash, can you dump() something in done() callback to ensure it is really done before quitting?
Flags: needinfo?(poirot.alex)
(In reply to Alexandre Poirot (:ochameau) from comment #7)
> (In reply to George Duan [:gduan] [:喬智] from comment #5)
> > My patch run well on local side but somehow failed on travis with below log.
> 
> Most likely xulrunner version mismatch, see bug 1032082 comment.
> 
> (In reply to George Duan [:gduan] [:喬智] from comment #4)
> > Hi Alex,
> > Do you know why I got error like comment 2 when trying to
> > require("sdk/system/child_process") ?
> 
> That's because system.js module is only expected to be loaded in a regular
> firefox addon.
> Firefox addons define a magic module "@loader/options" and "@test/options"
> with additional runtime options.
> We can workaround that by defining these modules with empty objects from
> here:
> https://github.com/mozilla-b2g/gaia/blob/master/build/xpcshell-commonjs.
> js#L42
>   modules: {
>     'toolkit/loader': Loader,
> +   '@test/options': {},
> +   '@loader/options': {}
>   }

I tried to modify xpcshell-commonjs.js and only do require('sdk/system/child_process'). It give me new error as below.

Exception: [Exception... "Component returned failure code: 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]"  nsresult: "0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE)"  location: "JS frame :: resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/system.js :: <TOP_LEVEL> :: line 20"  data: no]
@undefined:20:NaN
@resource://gre/modules/commonjs/sdk/system/child_process.js:16:1
@file:///Users/tuangeorge/code/gaia/build/utils-xpc.js:902:1
@file:///Users/tuangeorge/code/gaia/build/utils.js:9:3
@file:///Users/tuangeorge/code/gaia/build/test.js:2:1
CommonjsRunner.prototype.run@/Users/tuangeorge/code/gaia/build/xpcshell-commonjs.js:71:5
run@/Users/tuangeorge/code/gaia/build/xpcshell-commonjs.js:86:3
@-e:1:1
> No clear idea about that one... Could it be that we quit xpcshell process
> while subprocess is still trying to execute the process? Can you try calling

It makes sense. I tried to put a utils.log('AFTER CALLING SUBPROCESS') after executing subprocess.call, However, Segmentation fault: 11 earlier than it.

> processEvents() after calling subprocess? If that doesn't fix the crash, can
> you dump() something in done() callback to ensure it is really done before
> quitting?
Nothing, since it's not executed yet.
someone will update xulrunner in bug 1025731.
Depends on: 1025731
I don't know what is going on for the crash. It may be worth trying with a recent xulrunner to see if that is a crash being already fixed...
Otherwise it could be helpful to have to C++ stacktrace for this crash by running xpcshell manually with gdb:
  ./xulrunner-sdk-30/xulrunner-sdk/bin/run-mozilla.sh -g ./xulrunner-sdk-30/xulrunner-sdk/bin/xpcshell -e "print('foo')"

For the JS exception when using child_process, It is still an issue with system.js. This time, system.js is incompatible with xpcshell environement because the app-info xpcom "@mozilla.org/xre/app-info;1" doesn't have nsIXULAppInfo interface on xpcshell...
http://mxr.mozilla.org/mozilla-central/source/addon-sdk/source/lib/sdk/system.js#18
I think it is just easier to use subprocess...
(In reply to George Duan [:gduan] [:喬智] from comment #9)
> > processEvents() after calling subprocess? If that doesn't fix the crash, can
> > you dump() something in done() callback to ensure it is really done before
> > quitting?
> Nothing, since it's not executed yet.

You should try something like this. Because if you don't use processEvent, the call to subprocess may happen while xpcshell is already trying to quit!

  var q = Q.defer();
  var done = false;
  q.promise.then(function() {
    var p = subprocess.call({
      command: '/bin/ls',
      stdout: function(data) {console.log('success:' + data);},
      stderr: function(data) {console.log('error:' + data);},
      done: function () {
        done = true;
      }
    });
    p.wait();
  });
  q.resolve();
  processEvents(function () {
    return { wait: !done};
  });
Thanks Alex, I will try it later.

However, we still have bug in tbpl.

Exception: Module `sdk/system/child_process/subprocess` is not found at resource://gre/modules/commonjs/sdk/system/child_process/subprocess.js
George, xulrunner for unit test is updated, could you push to try server again?
Flags: needinfo?(gduan)
Comment on attachment 8447805 [details] [review]
PR to master

Hi Yuren,
it's runnable in tbpl and travis now.
could you help me to review it?
Attachment #8447805 - Flags: review?(yurenju.mozilla)
Flags: needinfo?(gduan)
Comment on attachment 8447805 [details] [review]
PR to master

r=yurenju, please address nits and file follow up bugs for build scripts which is using old way to execute command and an addon sdk follow up bug:
1. build.js in settings app (git command)
2. push-to-device.js (adb command)
3. killAppByPid in utils-xpc.sh
4. magically require @test/options module.
Attachment #8447805 - Flags: review?(yurenju.mozilla) → review+
(In reply to Yuren [:yurenju] from comment #16)

> 4. magically require @test/options module.
I've removed @test/options since we required sdk/system/child_process/subprocess, not sdk/system/child_process.
No longer blocks: 1038610, 1038611, 1038613
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: