-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
qt: use upstream package folder architecture #24193
qt: use upstream package folder architecture #24193
Conversation
This comment has been minimized.
This comment has been minimized.
Hooks produced the following warnings for commit 223fbdbqt/5.15.14@#25cd2ceb899ad6d465b68999538ebee8
qt/5.15.11@#4688cdee15c40741cb25f9328d3b6ccc
qt/6.7.1@#615c3712f5d5e99bd3481aa01f451abe
qt/5.15.12@#f8f44d7bf79241afa050b4e94925ef14
qt/6.6.1@#56c6d0ff43e530ee459c1c7238f39e15
qt/5.15.13@#ef4606a51f6eba0636beca056b50c04d
qt/6.3.2@#cbfe7b2d962017e80873dda970ec3dcd
qt/6.5.3@#0122819fc708caa5fdb8d6122aba7eac
qt/6.7.0@#584115e99c27d1fa159468e09438448b
qt/6.6.3@#0d6700a3aa39660e870a126e7d3bb40f
qt/6.6.0@#e28a0d82e13575469529f182cca85b02
qt/6.4.2@#02b2dc1b97e0ccd0d2813be55b00ebcf
qt/5.15.10@#13da246ded307a5b2ed7dcf5a59ba6a0
qt/5.15.9@#07312ce1f2ae555243453553b42561bc
qt/6.6.2@#0f9539b69936f5fcdc49cd53c8947f40
|
Thanks for rescuing this (and to Eric for allowing to use the commits), we'll check this tomorrow, try to move this forward asap :) |
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.
Got some local compilation issues for the recipe, would appreciate your insight!
if os.path.isfile(os.path.join(self.package_folder, path_)): | ||
exe_path = path_ | ||
break | ||
else: | ||
assert False, f"Could not find executable {target}{extension} in {self.package_folder}" |
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.
When trying to build locally to test this out, I'm finding that I'm hitting this assert. Any idea how to tackle it?
I'm building in
[settings]
arch=armv8
build_type=Release
compiler=apple-clang
compiler.cppstd=gnu17
compiler.libcxx=libc++
compiler.version=15
os=Macos
if that helps :)
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.
wanted to test on my side but Qt 6.7.1 build failed, will try with a different version later. Testing with v1 and
[settings]
arch=x86_64
arch_build=x86_64
build_type=Release
compiler=apple-clang
compiler.libcxx=libc++
compiler.version=15
os=Macos
os_build=Macos
[options]
[build_requires]
[env]
[conf]
tools.cmake.cmaketoolchain:generator=Ninja
(Xcode 15.4)
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.
Hi! If using Conan 2.4, you might want to use this fix #24236 for it to compile (Or wait until it gets merged in a few hours/tomorrow and update this PR with the changes)
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.
in my case build error was from Qt, and that PR is about Windows :)
/Applications/Xcode-15.4.0.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.5.sdk/usr/include/c++/v1/__memory/unique_ptr.h:66:19: error: invalid application of 'sizeof' to an incomplete type 'QQmlJS::Dom::BindingValue'
...
/Users/kambala/.conan/data/qt/6.6.3/_/_/source/src/qtdeclarative/src/qmldom/qqmldomelements_p.h:513:7: note: forward declaration of 'QQmlJS::Dom::BindingValue'
class BindingValue;
just built with -o 'qt/*:essential_modules=False'
and all went fine, didn't face that error. Could you post exact conan command you issued and the exact assert message (i.e. which executable wasn't found)?
Also pipeline was green...
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.
Also pipeline was green...
At the moment this is unfortunately misleading, because the recipe is currently set to build on CI with different options than a user would get, see: #23911
This is wrong and needs to be fixed - however because of the large amount of versions we are maintaining, this would result in very long CI builds.
Will try to troubleshoot this locally, the expectation is that with -o 'qt/*:essential_modules=True'
the changes in this PR (which are otherwise great) should not result in failures
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 get this too, with a simple
conan create . --version=6.7.1
keeping in mind that this causes essential_modules=True
(by default)
ERROR: qt/6.7.1: Error in package() method, line 871
assert False, f"Could not find executable {target}{extension} in {self.package_folder}"
AssertionError: Could not find executable qmltestrunner in /xxx/p/b/qt6f56e4472f231/p
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 is this being changed from a warning to an assert in this PR?
On master it doesn't find this tool either qmltestrunner
, but it is a warning.
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.
qmltestrunner
is not a documented qt app as far as I can find, and it seems like its only built when certain conditions are met (including, potentially, the building of tests) - so I've added a commit to remove it from the list of checked executables, until we can work out if this is needed and how to fix this
as a side note, and I appreciate the changes on this PR existed previously, it is very difficult to review pull requests when there are changes that have nothing to do with the PR description :(
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.
Tried building with v2 and faced that error as well.
Also input from Eric:
it is meant to avoid regressions, because this PR changed the folder in which executables are packaged
If this assert is triggered for executable qmltestrunner ,then someone should go take a look in the package folder to find where is is stored, and then add the path in this list https://github.com/conan-io/conan-center-index/pull/24193/files#diff-c548829da9d38ae15a8cb4447140bc50ac3f38250d1f348decd9569dc3298b24R862
qmltestrunner may only be built when-o qt/*:qtshadertools=True
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.
qmltestrunner
is not packaged - either before or after the changes in this PR, because it is not being built.
The problem exists outside of the changes in this PR and it unrelated - so let's address the package structure first.
223fbdb
to
eba192a
Compare
This comment has been minimized.
This comment has been minimized.
This comment was marked as outdated.
This comment was marked as outdated.
Hooks produced the following warnings for commit eba192aqt/6.7.1@#c2ce7245018eacbfc724fb06848d45da
qt/6.6.3@#9ea4d1a67f7e509d784db41fd424683b
qt/6.6.0@#c28f4d6bd5e25a3a537c90ce197b12dd
qt/6.6.2@#34292f7455adf5f5b2151a5a751a0723
qt/6.7.0@#8b5975af97545c6b30cd2e59d38b756f
qt/6.6.1@#a42d60d2a996fedb63f3069b92ca14d3
qt/6.5.3@#75322aff90776d40e7d7aaefd613ad5b
qt/6.4.2@#f1a8540f259e1514e008b357ddd55836
qt/6.3.2@#b2712c05ed5356383acb8b5ba8bb29d3
|
This change also fixes our problem with not finding the QtQuick-modules on application-startup. These modules get deployed in
With this change the modules are found below |
accoring to conan-io#24193 (comment) they have the similar issue as conan-io#23660
accoring to conan-io#24193 (comment) they have the similar issue as conan-io#23660
Specify library name and version: qt/*
copy of #23944 by Eric's permission
fixes #23660