| *** zbenjamin is now known as Guest86718 | 01:09 | |
| *** zbenjamin_ is now known as zbenjamin | 01:09 | |
| *** Ischwitch is now known as Ingvix | 04:05 | |
| dcaliste | Hello chriadam, how are you ? | 07:00 |
|---|---|---|
| chriadam | hi dcaliste, I'm well thanks - how are you? | 07:01 |
| dcaliste | Great, great. I've worked recently on the Qt6 transition of QMF. | 07:01 |
| chriadam | nice one - how's it going? | 07:02 |
| dcaliste | I can say that it's not smooth :/ | 07:02 |
| dcaliste | It's not just updating some API. | 07:02 |
| chriadam | oh, there are behaviour breaks which need more substantial rewriting? | 07:03 |
| chriadam | I haven't started the QtPIM qt6 effort - but I believe Bea has started on that work. | 07:08 |
| chriadam | (I have been selected for Jury Duty here, so am only working some days, depending on whether I'm needed or not for some cases.) | 07:08 |
| chriadam | unfortunately that means that this week my schedule has been basically replaced | 07:09 |
| dcaliste | Sorry, I was with some collegues. | 07:10 |
| dcaliste | So switching to Qt6 raises for QMF three or four issues: | 07:11 |
| dcaliste | - minor: QRegExp is deprecated and QRegularExpression API is not a 1:1, so some rewrite with possible errors. But it's in qt5compat. I've started to rewrite the parts in src/libraries. | 07:12 |
| dcaliste | - major: QTextCodec is removed and there is no replacement. To use it, one can link to core5compat, but it means that Core::QTextStream is not using it anymore and QTextStream can only handle UTF8 strings now. | 07:13 |
| dcaliste | This requires some rewrite in qmailmessage.cpp, I've done them. | 07:13 |
| chriadam | whoa, that's an interesting change... which codecs were being used, ooi? | 07:14 |
| dcaliste | - major: toList() and toSet() have been removed and replaced with range constructors. That's a pain, particularly when range constructors where introduced in 5.14… | 07:14 |
| dcaliste | For the codec, mails can come in whatever codecs, japanese, French iso8859-15 with the œ… | 07:15 |
| dcaliste | Japanese, sorry. | 07:15 |
| chriadam | right, so what's the alternative to using QTextCodec to handle those inputs? | 07:15 |
| dcaliste | So this part, I've rewritten so it's using now QDataStream and using the QTextCodec to move to unicode. | 07:16 |
| dcaliste | But the "regression" is that we cannot create non UTF8 mails. | 07:16 |
| dcaliste | Maybe it's not so bad ;) | 07:16 |
| Nico[m] | Perfect, utf8 forced email! | 07:17 |
| chriadam | still, seems pretty major change, that one cannot output text in a different codec, nor read the inputs easily (I guess QDataStream with QTextCodec from compat for reading?) | 07:17 |
| dcaliste | The problem is that the QTextStream is used to read (so transitioned internally to QDataStream) but also to output, and in that case I cannot transition to QDataStream with breaking the API and doing major cahnges. | 07:17 |
| chriadam | yeah. | 07:17 |
| chriadam | I mean, your solution sounds fine to me. I'm just surprised by the upstream Qt decision. I guess I missed the discussion about that one on the development mailing list somehow | 07:18 |
| dcaliste | I can live with it myself, particularly knowing that we won't have Qt6 soon in SailfishOS… | 07:18 |
| chriadam | indeed | 07:18 |
| dcaliste | https://bugreports.qt.io/browse/QTBUG-75665 | 07:18 |
| dcaliste | - minor, various changes in QMetaType API that propgates in QMF in parts that I don't much understand, so applied changes a bit blindly. | 07:19 |
| dcaliste | - minor: various minor API changes here and there that were introduced in Qt5.14 as deprecations are now enforced. So it means a lot of patching later for us to switch back to a version working with Qt5.6. | 07:21 |
| chriadam | I wonder what the next Qt version target might be for Sailfish OS. I suspect 5.15 but not sure... | 07:21 |
| dcaliste | For instance: https://doc.qt.io/qt-5/qstring-obsolete.html#SplitBehavior-enum | 07:21 |
| chriadam | if so, that one might not be so big a deal | 07:21 |
| dcaliste | If indeed we move to 5.15 sooner or later, that would be convenient, but the Qt6 switch is required now upstream to have the CI happy… | 07:22 |
| chriadam | yes | 07:23 |
| dcaliste | but the biggest pain, I keep it for the end: | 07:23 |
| chriadam | would be good if we could keep the qt 5.15-compatible changes in a separate commit than the qt6-only ones, so that we can just avoid cherry picking the qt6 commit, in future, perhaps | 07:23 |
| dcaliste | QNetworkSession and QNetworkConfiguration have been removed. | 07:24 |
| chriadam | ! | 07:24 |
| dcaliste | So imap and smtp are not compiling anymore and this is _huge_ changes. | 07:24 |
| chriadam | yes... what the heck | 07:25 |
| dcaliste | Something that I really don't want to start. Particularly because imap is doing a heavy use of these. I'm sure that I'm going to break something. | 07:25 |
| dcaliste | For the moment, I remove imap and smtp from the pro file (well put them under lessThan(Qt, 6) conditional. | 07:26 |
| dcaliste | These plugins will require major changes. | 07:26 |
| chriadam | makes sense | 07:27 |
| dcaliste | I don't know if this code has been moved somewhere or not in a network5compat or something, I didn't find anything. | 07:27 |
| chriadam | https://bugreports.qt.io/browse/QTBUG-75638 | 07:27 |
| chriadam | https://bugreports.qt.io/browse/QTBUG-80703 | 07:27 |
| chriadam | https://wiki.qt.io/Qt_Contributors_Summit_2019_-_Qt_6_Network_Overview#Removing_bearer_management | 07:28 |
| chriadam | apparently Qt don't care about mobile use cases any more. sigh. | 07:28 |
| chriadam | there is some network status monitor which is supposed to tell you if you're online or not | 07:29 |
| chriadam | but no true bearer management, apparently | 07:29 |
| chriadam | https://doc-snapshots.qt.io/qt6-dev/topics-network-connectivity.html | 07:29 |
| dcaliste | I don't know, and I didn't find any transition guide or how to replace such code. My strategy would be not to compile impa and smtp on CI at the moment and let the tranisiotn continue. Later on, we can migrate the code, because anyway large portion of SailfishOS will have the same issue. | 07:30 |
| chriadam | yes, I agree. | 07:31 |
| dcaliste | Which brings me to my last question: | 07:31 |
| dcaliste | I created a mess by trying to push all my separated commits of my qt6 branch to gerrit… | 07:32 |
| dcaliste | Gerrit created MR for every single commits… I've got dozens. | 07:32 |
| dcaliste | https://codereview.qt-project.org/q/project:qt-labs%252Fmessagingframework+status:open | 07:33 |
| chriadam | yes, gerrit does that | 07:33 |
| chriadam | there's some way to create a patchset containing multiple commits | 07:33 |
| dcaliste | I don't want to submit and big smashed commit. I will touch tens of files and hundred of lines. | 07:33 |
| dcaliste | While I separated everything in logical commits for each API changes. | 07:34 |
| chriadam | separate commits is definitely preferred | 07:35 |
| chriadam | I (think) that you can stage changes in-order (and they will be applied+tested in a single CI run) | 07:35 |
| dcaliste | What do you mean ? | 07:36 |
| chriadam | I mean, yes there will be many different gerrit CRs each one separate (which is fine) | 07:36 |
| chriadam | but then once they are approved, we can stage them all at once | 07:36 |
| chriadam | so even though there are multiple changes, they all get applied and tested in a single CI run | 07:37 |
| chriadam | we just have to make sure we stage them in the correct order to ensure we don't have any conflicts | 07:37 |
| dcaliste | Ah, I see. So I will correct the commit message formatting issues for every commits and then you can start review with pvuorela and mvogt. | 07:38 |
| chriadam | sounds good - thank you | 07:38 |
| dcaliste | You'll have trouble to get the branch though with all commits so you can test locally. | 07:38 |
| chriadam | you have made really good progress by the sounds of it! | 07:38 |
| chriadam | it's ok, can cherry pick from gerrit etc | 07:39 |
| dcaliste | All simple tests are passing (hurrah). | 07:39 |
| dcaliste | But all tests requiring the database are failing. I need to see why. | 07:39 |
| dcaliste | Maybe because it requires to connect to something. | 07:39 |
| chriadam | I hope they didn't break QtSql, but I imagine that everyone would be screaming if that happened, so likely it's something qmf specific. qcop perhaps. | 07:40 |
| dcaliste | The message is always the same with a connection error : QWARN : tst_StorageManager::initTestCase() void QCopClient::connectToServer() QLocalSocket::ServerNotFoundError "QLocalSocket::connectToServer: Invalid name" | 07:40 |
| chriadam | right.. some rules may have changed for local socket naming, to support all platforms | 07:41 |
| dcaliste | Yes, I need to investigate. But I think it may be more related to my setup, because I didn't touch much in that direction. | 07:41 |
| dcaliste | Mostly affected parts with rewrite is qmailmessage and qmailcodec. And all related tests are fine. | 07:41 |
| dcaliste | So beside the big issue with imap and smtp, I mostly optimistic. | 07:42 |
| chriadam | yep, it certainly sounds like you've made great progress | 07:43 |
| chriadam | as mentioned, my availability is probably a bit unpredictable over the next few weeks (depending on jury duty requirements) so not sure how much time I will be able to spend reviewing | 07:44 |
| dcaliste | And I take the occasion to remove all QString related warnings ;) | 07:44 |
| chriadam | (also, if I randomly don't show up to a meeting on Tuesday, it's because I'm stuck in court still, probably) | 07:44 |
| dcaliste | Sure, no problem, duty first ;) | 07:45 |
| chriadam | yup, important part of the social contract, etc ;-) | 07:45 |
| chriadam | pvuorela: maybe flypig could also help with reviewing the QMF changes ^ | 07:46 |
| flypig | Sorry, I've not been following this morning. But I'm happy to: just point me in the right direction. | 07:48 |
| chriadam | dcaliste has been working on porting QMF to build with Qt6 so that it passes upstream CI checks | 07:49 |
| chriadam | he is still making some changes, but soon they will be ready for review in Qt gerrit | 07:49 |
| flypig | Okay, great. | 07:49 |
| flypig | But there's no particular review page yet or anything then. | 07:50 |
| dcaliste | This is work in progress. It is compiling and tests are ok for those not implying Sql. I'm rewording the commit message for the bot to be happy. | 07:50 |
| chriadam | :-) | 07:50 |
| dcaliste | flypig: https://codereview.qt-project.org/q/project:qt-labs/messagingframework+status:open | 07:51 |
| dcaliste | All of page 1, plus the top one on page 2… | 07:51 |
| flypig | Ah, I saw this link in the text above, but didn't really understand it. This is the list of commits that will ultimately combined in to the pull request? | 07:51 |
| dcaliste | In fact, they are all in a branch for me, but I don't know if Gerrit can review a branch, so all commits got their own MR. | 07:51 |
| chriadam | flypig: not sure whether gerrit supports multiple-commit changesets. we need to review them individually, and then stage them altogether. | 07:52 |
| dcaliste | Exactly, I separated the MR into logical commits, depending on which part of Qt6 transition they are dealing with. | 07:52 |
| flypig | Yeah, I thought it didn't support multiple commits. | 07:52 |
| flypig | Okay, I see. But, won't each have to pass? | 07:53 |
| flypig | They can all be staged together. | 07:53 |
| flypig | ? | 07:53 |
| chriadam | the idea is that they would be staged into a single CI run, so they either all pass (together) or they all fail (together) | 07:54 |
| chriadam | we just have to be quick on the mouseclicks and stage them in the correct order to avoid conflicts. | 07:54 |
| chriadam | (unless gerrit has improved in the last few years, I haven't really followed the workflow changes) | 07:55 |
| flypig | Ha, okay :) But they still need reviewing and approving individually. | 07:55 |
| chriadam | yes | 07:56 |
| flypig | I'm just wondering if that's something that can already be started, even though there are some Sql issues you're still looking at dcaliste. | 07:56 |
| lp35 | test | 07:56 |
| dcaliste | Indeed, you can start the review if you have the time. That's very kind, thanks a lot. It's a lot of boring minor changes with some major important changes here and there. | 07:57 |
| flypig | The small change reviews sounds like good tasks to do while waiting for other stuff. | 07:58 |
| dcaliste | I'm going to redo the toList() toSet() removal commit to try to use automatic cast when we provide a QList where a QSet is required and vice-versa. | 07:58 |
| dcaliste | It would make changes less intrusive and easier to patch back to something 5.6 compatible. | 07:59 |
| flypig | This one: https://codereview.qt-project.org/c/qt-labs/messagingframework/+/313868 ? | 08:00 |
| dcaliste | Exact, this one. | 08:04 |
| chriadam | dcaliste: did you have anything else to discuss today? I saw that pvuorela merged some calendar related things, and I think flypig merged some nqpc/kcalcore things, but not sure if there is something else waiting on us currently? | 08:04 |
| dcaliste | No, that's alrigth. Thank you for the merges. We can come back to tha failure-related MRs in coming meetings. I've got another meeting to attend to now in fact also ;) | 08:05 |
| flypig | I still didn't merge the volatile property PRs. Sorry dcaliste, I will do. | 08:05 |
| chriadam | sounds good - thanks! | 08:06 |
| chriadam | have a great week! | 08:06 |
| dcaliste | Thank you flypig. | 08:06 |
| dcaliste | Have a good week also both of you. | 08:06 |
| chriadam | thanks :-) | 08:06 |
| *** Ischwitch is now known as Ingvix | 09:36 | |
| *** frinring_ is now known as frinring | 10:50 | |
| *** Ischwitch is now known as Ingvix | 12:38 | |
| *** panzeroceania_ is now known as panzeroceania | 14:01 | |
| attah | coderus: I figured out what GitHub was messing up in the Docker run... they were inheriting $HOME from the outside env | 18:23 |
| coderus | attah: tell me please what are you talking about?:) | 18:41 |
| attah | coderus: I'm looking at taking your Docker GitHub action example and making it a named GitHub build step, version controlled in its own repo, so people can just reuse it by name | 18:42 |
| coderus | attah: great, any success? :) | 18:42 |
| attah | But it wouldn't find the build targets the other day | 18:42 |
| attah | Yes, now i can build | 18:42 |
| attah | looking at how to handle parameters now | 18:42 |
| attah | Current issue is that it seems i can't refer to inputs in the env section https://github.com/attah/buildfish/blob/master/action.yml | 18:44 |
| attah | i know i can pass them as positional arguments to the docker run, but that feels a bit fragile | 18:45 |
| attah | seems shuffling input to env isn't a thing :( | 18:52 |
| coderus | attah: can you show where it runs? | 19:28 |
| attah | coderus: It's still flaketown... but here is the working run https://github.com/attah/harbour-seaprint/actions/runs/256142910 | 19:29 |
| attah | any ideas how to use "inputs" from https://github.com/attah/buildfish/blob/master/action.yml as something less sketchy than positional "args" to the docker run would be nice | 19:32 |
| coderus | attah: try this: https://gist.github.com/CODeRUS/d4cdc9e25d234b7215d1c08a2ff42900 | 19:35 |
| coderus | attah: it should be trannslated to named $INPUT_XXX args | 19:35 |
| attah | interesting | 19:36 |
| coderus | or try to not pass any args at all | 19:36 |
| coderus | github actions should be responsible for it | 19:36 |
| attah | right, i can get env from the calling job | 19:37 |
| attah | but if i want to run it twice, both archs for example, env isn't so good i think... i'd rather use inputs | 19:37 |
| attah | ah, right... but those inputs are passed through maybe | 19:38 |
| attah | Cool, that worked https://github.com/attah/harbour-seaprint/actions/runs/256191775 | 19:45 |
| attah | coderus: since mb2 doesn't understand "latest" from what i can tell, what do you think is the best idea to look it up? | 19:49 |
| attah | just parsing sdk-assistant list? | 19:49 |
| coderus | attah: main idea is for developer to know for which platform yo building app... | 19:50 |
| attah | maybe you're right... i just basically always go for latest, so i thought it could be nice... although i do that consciously | 19:51 |
| coderus | attah: you create mr to bump sdk version, test if everything builds and merge :) | 19:54 |
| attah | fair point | 19:54 |
| attah | coderus: So far so good... https://github.com/attah/harbour-seaprint/runs/1119846861?check_suite_focus=true | 20:42 |
| attah | Now uploading etc can be done the same way as you did, although i still need to get away from the latest tag in my Dockerfile | 20:44 |
| attah | but that is a problem for tomorrow... (: | 20:45 |
| attah | but it is probably impossible, and i should tag my releases with the sfos version | 20:46 |
| *** N-Mi_ is now known as N-Mi | 22:02 | |
Generated by irclog2html.py 2.17.1 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!