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.

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

Qt6: generator/typesystem.cpp: XML workround #123

Merged
merged 1 commit into from
Oct 9, 2023
Merged

Qt6: generator/typesystem.cpp: XML workround #123

merged 1 commit into from
Oct 9, 2023

Conversation

jbowler
Copy link
Contributor

@jbowler jbowler commented Oct 5, 2023

This works around the need to rewrite the XML parser in generator/typesystem.cpp by invoking the Qt5 compatibility library; this is the only thing in the generator which requires it but the alternative is a complete rewrite of the XML reading code.

This works around the need to rewrite the XML parser in
generator/typesystem.cpp by invoking the Qt5 compatibility library;
this is the only thing in the generator which requires it but the
alternative is a complete rewrite of the XML reading code.

Signed-off-by: John Bowler <jbowler@acm.org>
@jbowler
Copy link
Contributor Author

jbowler commented Oct 5, 2023

with this change generator/ can build against Qt6. Next step is to get the generated files.

Copy link
Contributor

@mrbean-bremen mrbean-bremen left a comment

Choose a reason for hiding this comment

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

Are you also planning to add CI tests for Qt6? Would be helpful...

generator/typesystem.cpp Show resolved Hide resolved
@mrbean-bremen mrbean-bremen merged commit db96e3c into MeVisLab:qt6 Oct 9, 2023
11 checks passed
@jbowler
Copy link
Contributor Author

jbowler commented Oct 9, 2023

Are you also planning to add CI tests for Qt6? Would be helpful...

Unless it's possible to obtain working generated cpp files the src build doesn't complete. The generator needs changes to recognize constexpr at least; I have those changes. However the generator/typesystem_*.xml needs fixing for both 5.15LTS and 6.5LTS and debugging what is failing is hard because of the error reporting (errors are reported in the output of pythonqt_generator but there is no clue as to which Qt header is causing them, then in the src/ build the misgenerated files produce obvious errors.)

@jbowler jbowler deleted the pull-request-123 branch October 9, 2023 12:51
@mrbean-bremen
Copy link
Contributor

True. I'm a bit unsure how to handle compatibility in this case. The idea for Qt6 is to use a separate generator based on shiboken, including separate typesystem files, but I don't know to handle the changes in Qt 5.15 in a backwards compatible way.

I don't think there is a possibility to add version-dependent entries in the typesystem files, so maybe we need to have separate typesystem files for separate versions, at least for pre- and post 5.15 versions. The generator would have be to adapted to use the correct typesystem files depending on the Qt version. Adapting the generator to also correctly parse Qt6 would probably be too much - @florianlink mentioned that it should be easier to use a shiboken-based generator instead of manually having to adapt the current one, and he knows the code best.

@jbowler
Copy link
Contributor Author

jbowler commented Oct 9, 2023

I don't think there is a possibility to add version-dependent entries in the typesystem files.

Some previous contributions seemed to be trying to do that:

58ccd53

But XML does not define <!-- foo --> as anything other than a comment so what that commit does is raise the question of which one of the lines is being used. It's what I'm working on at present; the error/warning reporting from pythonqt_generator is not good enough to fix the typesystem files in finite time.

So maybe we need to have separate typesystem files for separate versions, at least for pre- and post 5.15 versions.

I think so. I started changing them in-place then stopped (because of the error message problem). I've got Qt5.15 and Qt6 changes in new subdirectories generator/typesystem_515 and generator/typesystem_6. I'm not sure yet what I need to change to get a particular build to use those if they exist and/or whether I can build a search path.

The generator would have be to adapted to use the correct typesystem files depending on the Qt version. Adapting the >generator to also correctly parse Qt6 would probably be too much

So far as the parsing is concerned it just looks like the missing handling for 'constexpr' on functions.

It just depends on what the timescale is for the shiboken version; there's not a lot of point during 5.15LTS at present as 5.11 sort-of-works.

@mrbean-bremen
Copy link
Contributor

mrbean-bremen commented Oct 9, 2023

Some previous contributions seemed to be trying to do that:

I think these are really just comments that are not used for parsing.

I'm not sure yet what I need to change to get a particular build to use those if they exist and/or whether I can build a search path.

I haven't looked at this either, but it shouldn't be too complicated, given that the xml files have to be read in from a specific path at some time in the generator code. I may have a closer look later.

So far as the parsing is concerned it just looks like the missing handling for 'constexpr' on functions.

If this is indeed the case, we may just forgo the shiboken path and use the current parser - that would be be way easier. I had a closer look at shiboken, and at a first glance it did seem quite a bit of work to adapt it, at least to me.

@jbowler
Copy link
Contributor Author

jbowler commented Oct 9, 2023

I haven't looked at this either, but it shouldn't be too complicated, given that the xml files have to be read in from a specific path at some time in the generator code. I may have a closer look later.

They aren't read from files by the generator; they are resources. See my next pull request; it took me forever (well, a few days) to work out why changing the .xml files didn't change the output of the generator. The changes only take effect after a "make" within generator/

I was intending to supply a fix. I don't think I can do a search path within qmake but I'm pretty sure I can find the subdirectories I specified. My proposal if I can do it is to move all the required files into subdirectories (all the .xml files, build-all.txt and the master cpp file) and choose the right one within the build.

@mrbean-bremen
Copy link
Contributor

Ok - I was clearly too optimistic. Thanks for your time and effort!

@usiems
Copy link
Contributor

usiems commented Oct 10, 2023

I don't think there is a possibility to add version-dependent entries in the typesystem files, so maybe we need to have separate typesystem files for separate versions, at least for pre- and post 5.15 versions. The generator would have be to adapted to use the correct typesystem files depending on the Qt version.

It also might be possible to introduce reading version specifiers from the XML tags, something like qtVersionGreaterEqualThan="5.15" or qtVersionLessThan="6" (if we want to be really verbose), and every tag where the version doesn't fit is simply dropped. If this is a workable strategy depends on how many tags would need to be version specific. I imagine that there is still great overlap between Qt 6 and Qt 5, and classes and methods that are not in the header files (anymore) are ignored anyway. At least this would spare us to copy the typesystem files - though starting from scratch would also have advantages.

mrbean-bremen pushed a commit to mrbean-bremen/pythonqt that referenced this pull request Oct 29, 2023
- works around the need to rewrite the XML parser in
generator/typesystem.cpp by invoking the Qt5 compatibility library;
this is the only thing in the generator which requires it but the
alternative is a complete rewrite of the XML reading code
mrbean-bremen pushed a commit that referenced this pull request Oct 29, 2023
- works around the need to rewrite the XML parser in
generator/typesystem.cpp by invoking the Qt5 compatibility library;
this is the only thing in the generator which requires it but the
alternative is a complete rewrite of the XML reading code
mrbean-bremen pushed a commit that referenced this pull request Nov 1, 2023
- works around the need to rewrite the XML parser in
generator/typesystem.cpp by invoking the Qt5 compatibility library;
this is the only thing in the generator which requires it but the
alternative is a complete rewrite of the XML reading code
mrbean-bremen pushed a commit that referenced this pull request Nov 2, 2023
- works around the need to rewrite the XML parser in
generator/typesystem.cpp by invoking the Qt5 compatibility library;
this is the only thing in the generator which requires it but the
alternative is a complete rewrite of the XML reading code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants