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

Jump to bottom

Add support for Qt6 #66

Closed
wants to merge 2 commits into from
Closed

Add support for Qt6 #66

wants to merge 2 commits into from

Conversation

cristeab
Copy link

@cristeab cristeab commented Feb 1, 2021

Tested on macOS 11.2. Running tests/qtcsv_tests gives a malloc corruption:

qtcsv_tests(29792,0x1076c0e00) malloc: Region cookie corrupted for region 0x7ff341c00000 (value is 0)[0x7ff341c0407c]
qtcsv_tests(29792,0x1076c0e00) malloc: *** set a breakpoint in malloc_error_break to debug

@iamantony
Copy link
Owner

Wow, @cristeab thanks for the fix! Looks very good. The only thing that is bothers me is why tests are failing...

It seems that support of Qt6 brings a lot of ifdef-s in the code. Plus users will need to download special Qt5 Compatibility Module. I think this is not a user-friendly approach. Ideally qtcsv should need only core Qt lib that is available in every possible Qt "package".

Maybe a better approach would be to create a separate branch of qtcsv (based on current master) with Qt4/Qt5 support. And in master branch switch to Qt6.

@cristeab
Copy link
Author

cristeab commented Feb 2, 2021

I will investigate to see why this crash happens. Qt6 has still many bugs and we should see this year Qt6.1 and Qt6.2 with many improvements

@iamantony
Copy link
Owner

Thanks!

@markusdd
Copy link

markusdd commented Mar 5, 2021 •
edited
Loading

Before seeing this pull request I also tried building against QT6 and I second the idea of branching.

With the compat module stuff gets ugly real fast. QT6.0.2 is out, soc hecking against that seems reasonable.

markusdd added a commit to markusdd/qtcsv that referenced this pull request Mar 5, 2021
@markusdd
as per PR iamantony#66 discussion, clean up QT6 version by dropping l…
6fa39ba 
…egacy
This was referenced Mar 5, 2021
Merged
Closed
iamantony pushed a commit that referenced this pull request Apr 22, 2021
@markusdd
"Cleaned up" Qt6 version without legacy (based on #66) (#67)
6a6292a 
Switching to Qt6

* removing support of Qt4 and Qt5. Now only Qt6
* use QList<QString> and QStringConverter::Encoding in library interface instead of deprecated types
* minor style changes
* move all CI jobs to appveyor

Special thanks to Markus Krause (https://github.com/markusdd)
@iamantony
Copy link
Owner

@cristeab thanks or starting an initiative of adding Qt6 support!
Closing this PR in favor of PR#67.

@iamantony iamantony closed this Apr 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cristeab @iamantony @markusdd