Unity 8

Merge lp:~aacid/unity8/enable_debugging_flag into lp:unity8

enable_debugging_flag
Merge into trunk

Proposed by Albert Astals Cid on 2016-04-05

Status:	Merged
Approved by:	Michael Zanetti on 2016-04-08
Approved revision:	2329
Merged at revision:	2358
Proposed branch:	lp:~aacid/unity8/enable_debugging_flag
Merge into:	lp:unity8
Diff against target:	83 lines (+43/-0) 3 files modified src/Dash/main.cpp (+5/-0) src/main.cpp (+5/-0) src/qmldebuggerutils.h (+33/-0)
To merge this branch:	bzr merge lp:~aacid/unity8/enable_debugging_flag
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Michael Zanetti (community)		2016-04-05	Approve on 2016-04-08
Unity8 CI Bot	continuous-integration		Needs Fixing on 2016-04-08
Review via email: mp+290969@code.launchpad.net

Commit Message

Add env var to enable js debugging/profiling even if not built on debug mode

Description of the Change

How to test:
* Compile with -DCMAKE_BUILD_TYPE=release
* run builddir/src/Dash/unity8.dash
* See nothing about js debugging is printed
* run ENABLE_QML_DEBUGGER=soemthing builddir/src/Dash/unity8-dash
* See how "QML debugging is enabled. Only use this in a safe environment." is printed

* Are there any related MPs required for this MP to build/function as expected?
No

* Did you perform an exploratory manual test run of your code change and any related functionality?
Yes

* Did you make sure that your branch does not contain spurious tags?
Yes

* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A

* If you changed the UI, has there been a design review?
N/A

Gerry Boland (gerboland) wrote on 2016-04-05:

/me wonders if we should enable debugging if the -qmljsdebugger switch is set. One less hoop to jump through. Just a thought.

Albert Astals Cid (aacid) wrote on 2016-04-05:

> /me wonders if we should enable debugging if the -qmljsdebugger switch is set.
> One less hoop to jump through. Just a thought.

went for a new flag to make it clearer it is a different thing, i.e. otherwise you may run -qmljsdebugger on a different binary and wonder why it's not working.

This way you could still use ENABLE_QML_DEBUGGER on a different binary and wonder why it doesn't work but a grep over unity8 codebase will show why.

lp:~aacid/unity8/enable_debugging_flag updated on 2016-04-05

2327. By Albert Astals Cid on 2016-04-05: Add to unity8 too

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-04-05:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/928/rebuild

review: Needs Fixing (continuous-integration)

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-04-05:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/929/rebuild

review: Needs Fixing (continuous-integration)

Michael Zanetti (mzanetti) wrote on 2016-04-08:

if the -qmljsdebugger argument is always required (as it turned out to be in my tests) then not sure we need the env variable indeed, as Gerry said. I'd also be ok with the other way round, somehow configure it to some port (preferably 3768 as that's the preselected in our IDE) so exporting the env var is good enough and -qmljsdebugger arg is not needed any more.

review: Needs Fixing

lp:~aacid/unity8/enable_debugging_flag updated on 2016-04-08

2328. By Albert Astals Cid on 2016-04-08: Make enabling the qml debugger depend on the -qmljsdebugger command line option being there or not
2329. By Albert Astals Cid on 2016-04-08: eof

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-04-08:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/960/rebuild

review: Needs Fixing (continuous-integration)

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-04-08:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/961/rebuild

review: Needs Fixing (continuous-integration)

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-04-08:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/963/rebuild

review: Needs Fixing (continuous-integration)

Michael Zanetti (mzanetti) wrote on 2016-04-08:

A small nitpick perhaps... Your code accepts "--qmljsdebugger", however the qmljsdebugger only accepts it with one "-". This ends up with this message:

QML debugging is enabled. Only use this in a safe environment.
unity8-dash: unrecognized option '--qmljsdebugger=port:3768'

review: Needs Fixing

Michael Zanetti (mzanetti) wrote on 2016-04-08:

ok, turns out this is just a bogus debug print from Qt, it actually still works with -- too.

* Did you perform an exploratory manual test run of the code change and any related functionality?

yes

* Did CI run pass? If not, please explain why.

yes

* Did you make sure that the branch does not contain spurious tags?

yes

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Albert Astals Cid

Mohammad Rakibul Hasan

Unity Team

 === modified file 'src/Dash/main.cpp'
 --- src/Dash/main.cpp	2015-12-16 13:58:39 +0000
 +++ src/Dash/main.cpp	2016-04-08 09:38:49 +0000
@@ -27,6 +27,7 @@
  #include <libintl.h>
  #include <paths.h>
++#include "../qmldebuggerutils.h"
  #ifdef UNITY8_ENABLE_TOUCH_EMULATION
      #include "../MouseTouchAdaptor.h"
  #endif
@@ -34,6 +35,10 @@
  int main(int argc, const char *argv[])
+ {
++    if (enableQmlDebugger(argc, argv)) {
++        QQmlDebuggingEnabler qQmlEnableDebuggingHelper(true);
++    }
++
      QGuiApplication *application = new QGuiApplication(argc, (char**)argv);
      QCommandLineParser parser;
 === modified file 'src/main.cpp'
 --- src/main.cpp	2015-10-20 16:43:53 +0000
 +++ src/main.cpp	2016-04-08 09:38:49 +0000
@@ -16,6 +16,7 @@
  // local
  #include "ShellApplication.h"
++#include "qmldebuggerutils.h"
  int main(int argc, const char *argv[])
+ {
@@ -25,6 +26,10 @@
          isMirServer = true;
+     }
++    if (enableQmlDebugger(argc, argv)) {
++        QQmlDebuggingEnabler qQmlEnableDebuggingHelper(true);
++    }
++
      ShellApplication *application = new ShellApplication(argc, (char**)argv, isMirServer);
      int result = application->exec();
 === added file 'src/qmldebuggerutils.h'
 --- src/qmldebuggerutils.h	1970-01-01 00:00:00 +0000
 +++ src/qmldebuggerutils.h	2016-04-08 09:38:49 +0000
@@ -0,0 +1,33 @@
++/*
++ * Copyright (C) 2016 Canonical, Ltd.
++ *
++ * This program is free software; you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License as published by
++ * the Free Software Foundation; version 3.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ */
++
++#ifndef QMLDEBUGGERUTILS_H
++#define QMLDEBUGGERUTILS_H
++
++bool enableQmlDebugger(int argc, const char *argv[])
++{
++    for (int i = 1; i < argc; ++i) {
++        QByteArray arg = argv[i];
++        if (arg.startsWith("--"))
++            arg.remove(0, 1);
++        if (arg.startsWith("-qmljsdebugger=") || (arg == "-qmljsdebugger" && i < argc - 1)) {
++            return true;
++        }
++    }
++    return false;
++}
++
++#endif