Skip to content

Conversation

@dcorson-ticino-com
Copy link
Contributor

@dcorson-ticino-com dcorson-ticino-com commented Sep 7, 2021

Compiles using Qt6 and Qt5

  • changes in string constants
  • certain functions have other parameters under Qt6
  • the biggest change is that Qt6 uses other locale codes than Qt5, functions added to convert between Qt5 and Qt6 locale
  • some Qt functions changed names

Compiles using Qt6 and Qt5

Maybe Qt6 is friendlier with MacOS

Status of this Pull Request
This compiles and works under Win10

What is missing until this pull request can be merged?
Testing under the other OSes

Checklist

  • I've verified that this Pull Request follows the general code principles
  • I tested my code and it does what I want
  • My code follows the style guide at least my local clang-format, we will see with the autobuild
  • I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
  • I've filled all the content above

dcorson-ticino-com and others added 21 commits June 3, 2021 10:31
This compiles and runs under Qt6.
There is a problem with the country codes, but it appears otherwise to
run well (client).

Change list:
-All cases of endl changed to Qt::endl
-All case of QString::null changed to QString()
-enum ESvrRegStatus -> enum class ESvrRegStatus
-QVBoxLayout setMargins(0) -> setContentsMargins(0,0,0,0)
-QRegExp -> QRegularExpression
-QtConcurrent, inverted arguements
-in the recorder files some includes added
-unneeded include QtConcurrent commented out

We need to test everything to be sure it works as expected, but the
basics, protocol and audio, appear to be running well.
QtConcurrent parameter order changes between 5 and 6, in connectdlg,
nothing else.

Note there is still a problem in the locale mgmt in 6.  But it compiles
w/o error and runs protokol and audio error free as far as I can see.
for Qt version >= 6.0.0
with NSApp.dockTile.badgeLabel
@jujudusud
Copy link
Member

Do you want me to test under Arch Linux ?

@ghost
Copy link

ghost commented Sep 7, 2021

I compile Jamulus on CentOS 6 with Qt version 5.6.1.
That means that Qt6 should remain compatiible with
older Qt. I will let you know when Jamulus git master
has problems (unless I get a heads-up before).

@dcorson-ticino-com
Copy link
Contributor Author

dcorson-ticino-com commented Sep 7, 2021

OK this is not going to work as the IPv6 code uses a class that no longer exists QRegExp in Qt6.
It needs to be rewritten with QRegularExpression :(

@softins
Copy link
Member

softins commented Sep 7, 2021

OK this is not going to work as the IPv6 code uses a class that no longer exists QRegExp.
It needs to be rewritten with QRegularExpression :(

Ok, well we can fix that after 3.8.1 - we aren't trying to squeeze Qt6 work into this upcoming release.

@ann0see ann0see added this to the Release 3.9.0 milestone Sep 8, 2021
@ngocdh
Copy link
Contributor

ngocdh commented Sep 23, 2021

OK this is not going to work as the IPv6 code uses a class that no longer exists QRegExp in Qt6.
It needs to be rewritten with QRegularExpression :(

Thanks to your instructions @dcorson-ticino-com I've rewritten the QRegExp code as follows:

    QRegularExpression rx1 ( "^\\[([^]]*)\\](?::(\\d+))?$" ); // [addr4or6] or [addr4or6]:port
    QRegularExpression rx2 ( "^([^:]*)(?::(\\d+))?$" );       // addr4 or addr4:port or host or host:port

    QString strPort;
    
    QRegularExpressionMatch rx1match = rx1.match ( strAddress );
    QRegularExpressionMatch rx2match = rx2.match ( strAddress );

    // parse input address with rx1 and rx2 in turn, capturing address/host and port
    if ( rx1match.capturedStart () == 0 )
    {
        // literal address within []
        strAddress   = rx1match.captured ( 1 );
        strPort      = rx1match.captured ( 2 );
        bLiteralAddr = true; // don't allow hostname within []
    }
    else if ( rx2match.capturedStart () == 0 )
    {
        // hostname or IPv4 address
        strAddress = rx2match.captured ( 1 );
        strPort    = rx2match.captured ( 2 );
    }

Would you be able to verify if this works for you using the new Qt6.2.0? I'm actually curious to see how Qt6 affects mobile devices.

@dcorson-ticino-com
Copy link
Contributor Author

dcorson-ticino-com commented Sep 23, 2021

Would you be able to verify if this works for you using the new Qt6.2.0? I'm actually curious to see how Qt6 affects mobile devices.

On vacation without my dev environment this week. I will look at this around Wed next week.
D

@ghost
Copy link

ghost commented Sep 23, 2021

I hope that Qt5 will remain an option for a good while by use of a compile option (or Qt6 option).

@dcorson-ticino-com
Copy link
Contributor Author

@ngocdh The code you provided compiles OK under 6.1.

Unfortunately there are other problems now in socket.cpp
socket.cpp:287: error: 'inet_pton' was not declared in this scope
I have the impression that this is a Win10 problem, but have not found a solution.

Where did you get Qt 6.2 from? The Qt utility does not offer me more than 6.1

@softins
Copy link
Member

softins commented Sep 28, 2021

Unfortunately there are other problems now in socket.cpp socket.cpp:287: error: 'inet_pton' was not declared in this scope I have the impression that this is a Win10 problem, but have not found a solution.

This line compiles fine in the Windows build environment on Github, so I guess there might be something missing in your local build environment?

@ngocdh
Copy link
Contributor

ngocdh commented Oct 3, 2021

Where did you get Qt 6.2 from? The Qt utility does not offer me more than 6.1

Maybe update the utility first? I'm not sure though, just saying

I merged my iOS code and yours, compiled ok, but the app couldn't launch. I got the following output:

2021-10-03 23:08:43.928993+0200 Jamulus[15122:6998706] - allocated port number: 22134
2021-10-03 23:08:43.941280+0200 Jamulus[15122:6998706] Error: You are creating QApplication before calling UIApplicationMain.
If you are writing a native iOS application, and only want to use Qt for
parts of the application, a good place to create QApplication is from within
'applicationDidFinishLaunching' inside your UIApplication delegate.
dyld4 config: DYLD_LIBRARY_PATH=/usr/lib/system/introspection DYLD_IMAGE_SUFFIX=_debug DYLD_INSERT_LIBRARIES=/Developer/usr/lib/libBacktraceRecording.dylib:/Developer/Library/PrivateFrameworks/DTDDISupport.framework/libViewDebuggerSupport.dylib
(lldb) 

Someone knows how to fix this please? @emlynmac maybe?
Here's my branch: https://github.com/ngocdh/jamulus/tree/Qt6

@emlynmac
Copy link
Contributor

emlynmac commented Oct 4, 2021

I can have a look; seems like startup is not happening in the right order.
Ok, what are the steps here to build for iOS and how do I get QT6 installed in the right place?
I have QT5 installed with home-brew.

@ngocdh
Copy link
Contributor

ngocdh commented Oct 4, 2021

Compiling instructions can be found here: https://github.com/jamulussoftware/jamulus/blob/master/COMPILING.md

To install Qt6.2.0 LT, I simply used the maintenance tool.

@ann0see
Copy link
Member

ann0see commented Nov 3, 2021

Any updates here? Having Qt6 support would be great.

@dcorson-ticino-com
Copy link
Contributor Author

dcorson-ticino-com commented Nov 3, 2021 via email

@npostavs
Copy link
Contributor

npostavs commented Nov 5, 2021

I could help out with git wrangling if you need it. But it looks like github doesn't think there are any conflicts to be resolved?

@ann0see
Copy link
Member

ann0see commented Nov 11, 2021

@npostavs you can ofc. take over the PR and get the it to work by forking it.

@dcorson-ticino-com
Copy link
Contributor Author

But it looks like github doesn't think there are any conflicts to be resolved?

That's because the code is still on my PC. Git claims merging problems that I am unable to resolve with any tool I have tried.

@npostavs
Copy link
Contributor

That's because the code is still on my PC. Git claims merging problems that I am unable to resolve with any tool I have tried.

Could you perhaps just push your changes to a new branch without resolving anything so we can see them?

@dcorson-ticino-com
Copy link
Contributor Author

dcorson-ticino-com commented Nov 12, 2021

Could you perhaps just push your changes to a new branch without resolving anything so we can see them?

Will do next week, leaving now for a long weekend :)

@ann0see
Copy link
Member

ann0see commented Dec 16, 2021

@dcorson-ticino-com can you maybe just upload the code as zip file if everything fails with git?

@dcorson-ticino-com
Copy link
Contributor Author

To where can I upload it ?

@softins
Copy link
Member

softins commented Dec 17, 2021

I did some work on this branch a few weeks ago to get it up to date with master. I resolved the conflicts and successfully built with Qt5, but didn't yet get it to build with Qt6 for Windows on the github CI. See https://github.com/softins/jamulus/actions/runs/1491465763 and my branch Qt6-merged

@dcorson-ticino-com
Copy link
Contributor Author

Thanks Tony.
I hope that with qt6 the MAC problems can be resolved.
I must say that the never-ending problems with git have severely braked my interest in working on the Jamulus SW, unfortunately.

@dcorson-ticino-com
Copy link
Contributor Author

It heads into the bushes at about line 464 of the list not finding files.
I remember I had to change some of the Windows Autobuild files .ps1 to get it to compile, locally at least, but unfortunately couldn't tell you now what that was.

@ann0see
Copy link
Member

ann0see commented Dec 30, 2021

@softins I think the problem is that the initialise build environment function needs the QT variables. Probably we need to modify this function?

@ann0see
Copy link
Member

ann0see commented Jan 25, 2022

@dcorson-ticino-com going to close this in favor of #1795 (comment)

@ann0see ann0see closed this Jan 25, 2022
@hoffie hoffie mentioned this pull request Jan 31, 2022
11 tasks
@ann0see ann0see removed this from the Release 3.9.0 milestone Feb 16, 2022
@dcorson-ticino-com dcorson-ticino-com deleted the Qt6 branch March 23, 2022 18:02
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.

7 participants