Skip to content
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.

Sign up for GitHub

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

Backport patch for 6.8.0 and add tests for future failures #249

Merged
merged 7 commits into from
Oct 15, 2024
Merged

Backport patch for 6.8.0 and add tests for future failures #249

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b58fb12
Add a qtpy dependency to our test suite to make sure we don't break
hmaarrfk Oct 13, 2024
f5986bf
Add patch
hmaarrfk Oct 13, 2024
ff132d4
Update patch
hmaarrfk Oct 13, 2024
c856d72
Move patch
hmaarrfk Oct 13, 2024
f9408c8
add one more test even
hmaarrfk Oct 13, 2024
ecc3038
Bump build number
hmaarrfk Oct 14, 2024
6e093ac
avoid test with pyqt
hmaarrfk Oct 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion recipe/meta.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,15 @@ source:
# change tools module location
- patches/tools.patch

# Fix major regression in 6.8.0
# https://github.com/spyder-ide/qtpy/issues/494
# https://github.com/conda-forge/pyside2-feedstock/issues/247
# https://codereview.qt-project.org/c/pyside/pyside-setup/+/597328
# https://bugreports.qt.io/browse/PYSIDE-2888
- patches/try_to_fix_star_import_crash.diff

build:
number: 0
number: 1
entry_points:
- pyside6-rcc = PySide6.scripts.pyside_tool:rcc
- pyside6-uic = PySide6.scripts.pyside_tool:uic
Expand Down Expand Up @@ -81,6 +88,8 @@ requirements:

test:
requires:
# Add some qtpy tests to ensure we don't fully break things
- qtpy # [py==311]
Copy link
Contributor

@jschueller jschueller Oct 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checking with qtpy is overkill imho
checking star import would suffice, but even that is already done in the pyside side so there should be no regressions:
pyside/pyside-setup@2a2d013
but its still +1 even if you want to keep it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok but i might add it back within the year ;)

# these are optional
# test them with one version of python
- qt6-3d # [py==310]
Expand Down Expand Up @@ -148,6 +157,14 @@ test:
# Regression to avoid clobber with qt-main package
- test ! -f $PREFIX/bin/uic # [unix]
- if exist %LIBRARY_BIN%\uic.exe exit 1 # [win]
# Star imports have broken many times on PySide6
# https://github.com/spyder-ide/qtpy/issues/494
# https://github.com/spyder-ide/qtpy/issues/480
# We add it here for safety so we don't release really broken packages
- python -c "import PySide6; from PySide6.QtCore import *"
# Ensure we can still import things for one of the most popular packages
- python -c "from qtpy import API; assert API == 'pyside6', 'PySide6 not in use'" # [py==311]
- python -c "from qtpy.QtCore import Qt" # [py==311]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this test marked for 3.11 only? I think it be better to leave it for any version.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t like adding too many optional dependencies for these nice to have tests.

By having it only for python 3.11 we test it, while still testing minimal dependencies in other pythons


about:
home: https://wiki.qt.io/PySide2
Expand Down
29 changes: 29 additions & 0 deletions recipe/patches/try_to_fix_star_import_crash.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
diff --git a/sources/shiboken6/libshiboken/sbkmodule.cpp b/sources/shiboken6/libshiboken/sbkmodule.cpp
index acadc60..1800f8d 100644
--- a/sources/shiboken6/libshiboken/sbkmodule.cpp
+++ b/sources/shiboken6/libshiboken/sbkmodule.cpp
@@ -93,8 +93,10 @@ static void incarnateHelper(PyObject *module, const std::string_view names,
startPos = dotPos + 1;
dotPos = names.find('.', startPos);
}
- // now we have the type to create.
+ // now we have the type to create. (May be done already)
auto funcIter = nameToFunc.find(std::string(names));
+ if (funcIter == nameToFunc.end())
+ return;
// - call this function that returns a PyTypeObject
auto tcStruct = funcIter->second;
auto initFunc = tcStruct.func;
@@ -178,7 +180,11 @@ void resolveLazyClasses(PyObject *module)
while (!nameToFunc.empty()) {
auto it = nameToFunc.begin();
auto attrNameStr = it->first;
- incarnateType(module, attrNameStr.c_str(), nameToFunc);
+ if (attrNameStr.find('.') == std::string::npos) {
+ incarnateType(module, attrNameStr.c_str(), nameToFunc);
+ } else {
+ nameToFunc.erase(it);
+ }
}
}