-
Notifications
You must be signed in to change notification settings - Fork 94
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
TEST ONLY: this adds the PR139 generated files to the master branch #140
Conversation
These files are generated by the a current build of pythonqt_generator from the qt6 branch; source code identical to commit: 38979b7 Note that the actual files were generated before that commit was generated from what appears to be a rebase against master. The source files used are identical to those in the commit. Signed-off-by: John Bowler <[email protected]>
Signed-off-by: John Bowler <[email protected]>
Signed-off-by: John Bowler <[email protected]>
Signed-off-by: John Bowler <[email protected]>
Yes, this mirrors my results; everything looks ok. generated_cpp_514 has a number of instances where cbegin/cend methods which were NOT in the _513 generated files have appeared and return QChar*. I looked at the differences between qpolygon.h version from 5.13 and 5.14; the only change was Q_DECL_NOTHROW declarations being added as an explicit noexcept and no longer protected by Q_COMPILER_RVALUE_REFS. It's not obvious to me why the declarations were getting dropped before. Hand editing the return type in the generated files works to an extent. The relevant change in the log files appears to be from MetaJavaBuilder, in 5.13 it states "skipping function 'QVector::cbegin', unmatched return type 'const_iterator'". In 5.14 the messages aren't there, however there are messages for "reverse_iterator" and so on. It looks like somehow const_iterator is now being recognized leading to generation of the bogus wrappers. UPDATE: the problem is that const_iterator methods are being provided with wrappers in 5.14 (and, I assume, later). typesystem_core.xml has explicit "rejection" clauses for cases where this happened before and the possibly working QRegion iterator (which returns QRect*) is apparently unique; maybe there is a hack somewhere to allow this one to work? With the changes in Qt 5.14 things that had simply been dropped, normally because the const_iterator class could not be found, are no longer dropped. Fixing this requires explicit "rejection" clauses for the class functions that return iterators. |
Needs more work for Qt5.14; PR141 fixes the initial problems but apparently the generator is now trying to make copy constructors for classes that can't be copied (QSignalBlocker for a start.) I can't see a way of fixing that, I don't have enough understanding of the typesystem. |
This PR looks good, CI passed. Are there any problems that prevent it from being merged into master? I have checked PythonQt build steps with qt5.14 and 5.15 versions using generated_cpp_513 from this PR (YuriUfimtsev#5). CI also passed. |
@YuriUfimtsev - first, I wouldn't like to merge code generated from another branch into main, though I'm hoping that that branch can be merged soon. Second, as I wrote elsewhere - I think it would be better to use the generated code to build PythonQt, e.g. first execute the generator and afterwards compile PythonQt, which would find the generated code. The checked in sources are only used as a fallback. I have used this in a branch to check if the code currently generated for Qt6 compiles (it doesn't yet). As this would only use the build-in part of the generated code, it would still be helpful to have an additional build step that builds all the generated code to make sure that it at least compiles. Though that would be the next step... |
I recently noticed that this is not correct - it actually compiles all generated code in the |
@jbowler: I think this can be revived with the current master branch. I'm not sure if the pre-generated directories are needed for each version (we have switched to generate them ourselves on demand, now that the generator is working more reliably). If they are needed, we might also add directories for 5.15 and 6.5, and make a release in the near future that includes them. |
@jbowler - I think now is the time to add these adapted sources (and add 5.15 and 6.5), as I'm about to make a pre-release for 5.15/6 support. Let us know if you are going to finish this, otherwise we can do this ourselves of course. |
Any generated files in the distribution need to have been built in a known way from known Qt sources; the additions have to be made by a core PythonQt maintainer, not me. If nothing else there is going to be an on-going maintenance requirement imposed by upstream (Qt) security patches. The generated_cpp files in this PR should certainly not be added. Any checked in generated_cpp needs to come from an approved Qt source with a known license (and a license that is consistent with that used by PythonQt). I also agree with other comments that under normal circumstances anyone who builds PythonQt should end up running the generator to get generated_cpp, so normally the files should be generated then. This would mean a "private" build would use the header files of the private system, probably a reduced set, and a "system" build of a binary for an operating system distribution would use the set supported by the OSD, a set that may be incomplete as a result of incomplete OSD development. In both cases the license would be appropriate; a commercial user would use the Qt commercial release etc. Otherwise the result may end up accidentally GPLed or accidentally subject to a Qt commercial license. So I feel this PR should be closed as, now, completed and any updates and additions regenerated under carefully controlled conditions. |
Thanks, I see your point. As I wrote, we don't need the generated files ourselves, as I changed our build process to create them from the Qt sources we use by the generator. I actually don't know how other people are using the library - so maybe we just wait until somebody chimes in who actually needs them. As for the license: the checked in sources have always been created from the version of Qt (now) accessible via GitHub, and the license should be compatible with the LGPL we that is used for PythonQt. As the sources are generated from the headers only, I doubt that there would even be a difference if creating them from a closed source version (e.g. > 5.15.11 at the moment). I don't see any problem here, but of course I'm not a lawyer, and I agree that the maintainers of PythonQt are responsible for that. @florianlink may comment on this, as he is the one who added the generated sources in the first place. |
Exactly. We are in complete agreement insofar as that is ever possible. |
This is for checking the suitability of the new generated_cpp_ diretories for use with the "master" branch.