New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Some Pyside2 fix #3875
Some Pyside2 fix #3875
Conversation
Fix pyinstaller#3689 Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
os.path.join result was bin/qmake and not /bin/qmake Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
8404a39
to
4a0b7a3
Compare
if qmldir: | ||
qmldir = exec_command_stdout(qmake, "-query", "QT_INSTALL_QML").strip() | ||
|
||
if qmldir is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why test for None
here instead of if not qmldir
? If qmldir
was ''
, I would think the same error message would apply.
Also, please add a changelog entry.
Finally, if you have any tests to demonstrate that this works for fixes something that would be helpful. Thanks for working on this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why test for None here instead of if not qmldir? If qmldir was '', I would think the same error message would apply.
Yes, I'll move to not qmldir
. The problem with the original test wast that if qmldir is a valid path like: /usr/lib/qt/qml
an error message is applied, the check itself was done wrong. You can test it by running qmake -query QT_INSTALL_QML
, this path should be used to set the correct qmldir path.
Besides that, if qmldir:
will return an error message always, this shows that only None
is valid. It appears that the original author did something wrong.
If you want to test this PR, I can do a small repository for you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. In terms of tests, here's what begin run currently. Any ideas on adding to this, or fixing/changing it? If there's a test that demonstrates how your fixes make something work, that would be ideal.
@xfail(is_linux and is_py35, reason="Fails on linux >3.5")
@xfail(is_darwin, reason="Fails on OSX")
@xfail(is_win and is_py35 and not is_py36, reason="Fails on win == 3.6")
@importorskip('PySide2')
def test_PySide2_QWebEngine(pyi_builder):
pyi_builder.test_source(
"""
from PySide2.QtWidgets import QApplication
from PySide2.QtWebEngineWidgets import QWebEngineView
from PySide2.QtCore import QUrl
app = QApplication( [] )
view = QWebEngineView()
view.load( QUrl( "http://www.pyinstaller.org" ) )
view.show()
view.page().loadFinished.connect(lambda ok: app.quit())
app.exec_()
""")
@importorskip('PySide2')
def test_PySide2_QtQuick(pyi_builder):
pyi_builder.test_source(
"""
import sys
# Not used. Only here to trigger the hook
import PySide2.QtQuick
from PySide2.QtGui import QGuiApplication
from PySide2.QtQml import QQmlApplicationEngine
from PySide2.QtCore import QTimer, QUrl
app = QGuiApplication([])
engine = QQmlApplicationEngine()
engine.loadData(b'''
import QtQuick 2.0
import QtQuick.Controls 2.0
ApplicationWindow {
visible: true
color: "green"
}
''', QUrl())
if not engine.rootObjects():
sys.exit(-1)
# Exit Qt when the main loop becomes idle.
QTimer.singleShot(0, app.exit)
sys.exit(app.exec_())
""")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll take a look asap. I'm using python 3.7, maybe this PR fix some problems related to it ? I'm not a heavy pyinstaller user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bjones1 Is it possible to run only PySide tests ?
I tried with py.test tests/functional -k test_PySide2_QtQuick
, is that how I'm supposed to run it ? I took a look in: https://pyinstaller.readthedocs.io/en/latest/development/testing.html
But was not clear for individual tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's (mostly) correct -- see the pytest manual. The -k
parameter runs all tests that contain the provided string in the test name. I'd use pytest -k PySide2
.
@patrickelectric, closing for now. When you have time to work on this, we can re-open it. |
Fix #3689 and search path for qmake.
Helps mherrmann/fbs-tutorial#18