646343 - Allow to debug platform through cfx

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect, P1)

Description

[Alexandre Poirot [:ochameau]]

Attachment: Implement "--no-run" option

Currently using gdb and all associated stuff around is almost impossible or very hard.
We can't pass arguments to firefox to setup debug mode and even if we are able to give some arguments, debug tools won't work as there is problems with stdin/out.
So I propose a really simple option in cfx to simply display a the firefox command to run to execute an addon or a test.

  $ cfx --no-run -b /path/to/your/debug/build/firefox test -f test-leak-
    or-crash.js
  In order to execute this, launch the following command:
  /path/to/your/debug/build/firefox  -profile /path/to/tmp/tmpwgbail.mozrunner -no-remote

This feature will greatly help platform team to use their tooling!

Comment 1

[Myk Melez [:myk] [@mykmelez]]

What are the problems with stdin/out that make the fix for bug 646345 insufficient for satisfying the debugging use case?

Comment 2

[Myk Melez [:myk] [@mykmelez]]

(The reason I ask is that my first inclination is to fix those bugs.  It seems wonky to add a feature just to work around some bugs in our own codebase.)

Comment 3

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 522926 [details] [diff] [review]
Implement "--no-run" option

Per my previous comments, this feels like the wrong fix, since it adds redundant functionality to work around problems with the way cfx handles stdin/stdout.  The right fix is to resolve those stdin/stdout problems.

(This fix also appears to leave profile directories laying around, which is problematic.)

Comment 4

[Alexandre Poirot [:ochameau]]

Blake: Can you tell us if bug 646345, that allow to give extra arguments to firefox, like "-d" are satifying for your debug environnement ?
When we where working together I remember that passing "-d" and removing the automatic kill was not enough, you quickly mentioned something around stdin/out ?

Myk: bug 646345 won't solve the automatic killed done on test timeout:
https://github.com/mozilla/addon-sdk/blob/master/python-lib/cuddlefish/runner.py#L290
This disallow to debug a unit test.

Comment 5

[Alexandre Poirot [:ochameau]]

Blake: Can you tell us if bug 646345, that allow to give extra arguments to firefox, like "-d" are satifying for your debug environnement ?
When we were working together I remember that passing "-d" and removing the automatic kill was not enough, you quickly mentioned something around stdin/out ?

Myk: bug 646345 won't solve the automatic killed done on test timeout:
https://github.com/mozilla/addon-sdk/blob/master/python-lib/cuddlefish/runner.py#L290
This disallow to debug a unit test.

Comment 7

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 522926 [details] [diff] [review]
Implement "--no-run" option

Alex and I talked about this again today.  It still isn't clear if this is the right solution.  It feels like there's something better we can do here.  But we should land this as an experimental feature in the meantime so folks can use it (and give us feedback at their leisure).


>diff --git a/python-lib/cuddlefish/__init__.py b/python-lib/cuddlefish/__init__.py

>+        (("", "--no-run",), dict(dest="no_run",
>+                                     help=("Do not launch firefox, "
>+                                           "just print command to do so."
>+                                           "(usefull for gdb uses)"),
>+                                     action="store_true",
>+                                     default=False,
>+                                     cmds=['run', 'test'])),

This should move to the "Experimental Command-Specific Options" section.

And the help message should read "Instead of launching the application, just show the command for doing so.  Use this to launch the application in a debugger like gdb."


>diff --git a/python-lib/cuddlefish/runner.py b/python-lib/cuddlefish/runner.py

>+    if norun:
>+        print "In order to execute this, launch the following command:"
>+        print " ".join(runner.command)+" "+(" ".join(runner.cmdargs))
>+        return 0

Nit: "To launch the application, enter the following command:"

Nit: add spaces around the plus signs, i.e.:

        print " ".join(runner.command) + " " + (" ".join(runner.cmdargs))


r=myk with these changes.

Comment 8

[Alexandre Poirot [:ochameau]]

Landed:
https://github.com/mozilla/addon-sdk/commit/4497812f665528cf3d7ebb81f8a84c2e7ad15776

Comment 9

[Nickolay_Ponomarev]

the patch breaks 'cfx run --use-server', since you didn't update the
corresponding run_app implementation: 
            from cuddlefish.server import run_app

Comment 10

[Alexandre Poirot [:ochameau]]

Attachment: Fix --use-server regression due to previous patch.

Comment 11

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 528728 [details] [diff] [review]
Fix --use-server regression due to previous patch.

r+a=myk

Comment 12

[Alexandre Poirot [:ochameau]]

Regression fix landed:
https://github.com/mozilla/addon-sdk/commit/00f94413f6655d545b4191713f1fd9872d076f7d

Nickolay: Thanks for the fast report!