Allow running python plugins from build dir (output dir)

[strk]

Takes a step forward toward closing http://hub.qgis.org/issues/5879
More work is needed to ensure all python plugins install themselves
under the output/python/plugins dir.

[brushtyler]

This change seems to break something.

Have a look at https://github.com/qgis/Quantum-GIS/blob/master/src/python/qgspythonutilsimpl.cpp#L459:
both the if and else statements return a string containing python code,
in your change the else statement returns a path as string while the if is unchanged.

[strk]

On Thu, Oct 11, 2012 at 06:00:43AM -0700, Giuseppe Sucameli wrote:

@@ -460,13 +457,13 @@ QString QgsPythonUtilsImpl::homePythonPath()
}
else
{

• return '"' + settingsDir + "python"";
• return settingsDir + "python";

This change seems to break something.

Have a look at https://github.com/qgis/Quantum-GIS/blob/master/src/python/qgspythonutilsimpl.cpp#L459:
both the if and else statements return a string containing python code,
in your code the else statement returns a path as string instead.

Yes, but the callers add the quotes when needed, doesn't them ?
But then I guess the xpanduser part wouldn't want to be quoted.
The problem was with https://github.com/qgis/Quantum-GIS/blob/master/src/python/qgspythonutilsimpl.cpp#L86
resulting in a weird sys.path (containing + in it).

[brushtyler]

AFAICS after your change (and maybe also before it) the if/else is not needed at all, you can just replace it with return settingsDir + "python";

Here's the code before the change:

  QString settingsDir = QgsApplication::qgisSettingsDirPath();
  if ( settingsDir == QDir::homePath() + "/.qgis/" )
  {
    return "os.path.expanduser(\"~/.qgis/python\")";
  }
  else
  {
    return '"' + settingsDir + "python\"";
  }

that always returns settingsDir + "/python".

[strk]

On Thu, Oct 11, 2012 at 09:03:31AM -0700, Giuseppe Sucameli wrote:

AFAICS after your change (and maybe also before it) the if/else is not needed at all, you can just replace it with return settingsDir + "python";

Here's the code before the change:

  QString settingsDir = QgsApplication::qgisSettingsDirPath();
  if ( settingsDir == QDir::homePath() + "/.qgis/" )
  {
    return "os.path.expanduser(\"~/.qgis/python\")";
  }
  else
  {
    return '"' + settingsDir + "python\"";
  }

that always returns settingsDir + "/python", i.e. ~/.qgis/python

It depends on the value returned by qgisSettingsDirPath(), doesn't it ?
I've no idea what purpose that setting is supposed to serve
nor the rationale for having python expand ~ differently than QT did.

Anyway, if nothing complains I'm happy to see the conditional dropped...

--strk;

http://www.cartodb.com - Map, analyze and build applications with your data

                                   ~~ http://strk.keybit.net

[etiennesky]

does "~" work on windows?

[strk]

On Thu, Oct 11, 2012 at 09:22:51AM -0700, Etienne Tourigny wrote:

does "~" work on windows?

Do you mean to say that the conditional is there to avoid using "~" ?
Because it doesn't look like that code would avoid that...

[brushtyler]

Let's look:
QDir::homePath() returns "/home/brushtyler" on my Ubuntu.
Then if ( settingsDir == QDir::homePath() + "/.qgis/" ) means if settingsDir == "/home/brushtyler/.qgis/"
and the function returns /home/brushtyler/.qgis = settingsDir + "/.qgis/"
otherwise the function returns settingsDir + "/.qgis/.

[brushtyler]

The os.path.expanduser() was added in b60a63b, then changed in ed0cb5d

[etiennesky]

The question was - does "~" work on windows?

also, why the conditional? Shouldn't QgsApplication::qgisSettingsDirPath() + "python" be enough???

sorry for butting in, I haven't looked into this much.

[brushtyler]

The question was - does "~" work on windows?

Are you talking about os.path.expanduser("~/.qgis/python")? It works also on win (see http://docs.python.org/library/os.path.html#os.path.expanduser)

why the conditional? Shouldn't QgsApplication::qgisSettingsDirPath() + "python" be enough???

Exactly what I wrote, removing the if/else at all. That block was added in ed0cb5d, maybe there's something we don't know, I'm looking at the related tickets.

[brushtyler]

Found, have a look at http://hub.qgis.org/issues/2512.
The problem occurs with non-ascii chars, escaping the string in runString( "sys.path = [" + newpaths.join( "," ) + "] + sys.path" ); is not enough.

I guess that code need a bigger refactoring than just changing few lines of code.

[strk]

On Thu, Oct 11, 2012 at 10:14:29AM -0700, Giuseppe Sucameli wrote:

Found, have a look at http://hub.qgis.org/issues/2512.
The problem occurs with non-ascii chars, escaping the string in runString( "sys.path = [" + newpaths.join( "," ) + "] + sys.path" ); is not enough.

I guess that code need a bigger refactoring than just changing few lines of code.

Do you think we can get to produce a testcase for that issue ?
HINT: tomorrow is Test Friday !

I have to admit I still fail to understand the issue.
Is it python failing when a "string" contains a non-ascii char ?

--strk;

http://www.cartodb.com - Map, analyze and build applications with your data

                                   ~~ http://strk.keybit.net

[wonder-sk]

It is getting too easy to get lost in the quoting of the paths - IMHO the right solution would be to set the paths using Python C API to ensure that the paths with international characters are still properly passed to python side.

[strk]

On Thu, Oct 11, 2012 at 12:07:58PM -0700, Martin Dobias wrote:

It is getting too easy to get lost in the quoting of the paths - IMHO the right solution would be to set the paths using Python C API to ensure that the paths with international characters are still properly passed to python side.

I still don't understand what's the problem with international chars.
Can it be just a matter of telling runString that the string will going
to be utf8 ?

[strk]

I had a chance to try a simpler change, since I think last time I tested I was being confused by a lot more parameters.
This one would be much simpler:

diff --git a/src/python/qgspythonutilsimpl.cpp b/src/python/qgspythonutilsimpl.cpp
index d0be929..cea90c0 100644
--- a/src/python/qgspythonutilsimpl.cpp
+++ b/src/python/qgspythonutilsimpl.cpp
@@ -445,9 +445,6 @@ QString QgsPythonUtilsImpl::pythonPath()

 QString QgsPythonUtilsImpl::pluginsPath()
 {
-  if ( QgsApplication::isRunningFromBuildDir() )
-    return QString(); // plugins not used
-  else
     return pythonPath() + "/plugins";
 }

It worked fine in loading plugins from output/python/plugin dir.

[brushtyler]

No more plus signs (+) in path???

[strk]

Nope, just tried again with master, it's all fine. Only needs plugin in output/python/plugin then.
Maybe the plus signs I only saw when printing the string to standard output, but I guess they get interpreted by python which takes them as string concatenation before they're ever used.

Opening a new pull request with this small patch.