Tuesday, 2020-09-15

*** zbenjamin is now known as Guest8671801:09
*** zbenjamin_ is now known as zbenjamin01:09
*** Ischwitch is now known as Ingvix04:05
dcalisteHello chriadam, how are you ?07:00
chriadamhi dcaliste, I'm well thanks - how are you?07:01
dcalisteGreat, great. I've worked recently on the Qt6 transition of QMF.07:01
chriadamnice one - how's it going?07:02
dcalisteI can say that it's not smooth :/07:02
dcalisteIt's not just updating some API.07:02
chriadamoh, there are behaviour breaks which need more substantial rewriting?07:03
chriadamI 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
chriadamunfortunately that means that this week my schedule has been basically replaced07:09
dcalisteSorry, I was with some collegues.07:10
dcalisteSo 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
dcalisteThis requires some rewrite in qmailmessage.cpp, I've done them.07:13
chriadamwhoa, 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
dcalisteFor the codec, mails can come in whatever codecs, japanese, French iso8859-15 with the œ…07:15
dcalisteJapanese, sorry.07:15
chriadamright, so what's the alternative to using QTextCodec to handle those inputs?07:15
dcalisteSo this part, I've rewritten so it's using now QDataStream and using the QTextCodec to move to unicode.07:16
dcalisteBut the "regression" is that we cannot create non UTF8 mails.07:16
dcalisteMaybe it's not so bad ;)07:16
Nico[m]Perfect, utf8 forced email!07:17
chriadamstill, 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
dcalisteThe 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
chriadamI 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 somehow07:18
dcalisteI can live with it myself, particularly knowing that we won't have Qt6 soon in SailfishOS…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
chriadamI wonder what the next Qt version target might be for Sailfish OS.  I suspect 5.15 but not sure...07:21
dcalisteFor instance: https://doc.qt.io/qt-5/qstring-obsolete.html#SplitBehavior-enum07:21
chriadamif so, that one might not be so big a deal07:21
dcalisteIf 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
dcalistebut the biggest pain, I keep it for the end:07:23
chriadamwould 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, perhaps07:23
dcalisteQNetworkSession and QNetworkConfiguration have been removed.07:24
dcalisteSo imap and smtp are not compiling anymore and this is _huge_ changes.07:24
chriadamyes... what the heck07:25
dcalisteSomething 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
dcalisteFor the moment, I remove imap and smtp from the pro file (well put them under lessThan(Qt, 6) conditional.07:26
dcalisteThese plugins will require major changes.07:26
chriadammakes sense07:27
dcalisteI don't know if this code has been moved somewhere or not in a network5compat or something, I didn't find anything.07:27
chriadamapparently Qt don't care about mobile use cases any more.  sigh.07:28
chriadamthere is some network status monitor which is supposed to tell you if you're online or not07:29
chriadambut no true bearer management, apparently07:29
dcalisteI 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
chriadamyes, I agree.07:31
dcalisteWhich brings me to my last question:07:31
dcalisteI created a mess by trying to push all my separated commits of my qt6 branch to gerrit…07:32
dcalisteGerrit created MR for every single commits… I've got dozens.07:32
chriadamyes, gerrit does that07:33
chriadamthere's some way to create a patchset containing multiple commits07:33
dcalisteI don't want to submit and big smashed commit. I will touch tens of files and hundred of lines.07:33
dcalisteWhile I separated everything in logical commits for each API changes.07:34
chriadamseparate commits is definitely preferred07:35
chriadamI (think) that you can stage changes in-order (and they will be applied+tested in a single CI run)07:35
dcalisteWhat do you mean ?07:36
chriadamI mean, yes there will be many different gerrit CRs each one separate (which is fine)07:36
chriadambut then once they are approved, we can stage them all at once07:36
chriadamso even though there are multiple changes, they all get applied and tested in a single CI run07:37
chriadamwe just have to make sure we stage them in the correct order to ensure we don't have any conflicts07:37
dcalisteAh, 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
chriadamsounds good - thank you07:38
dcalisteYou'll have trouble to get the branch though with all commits so you can test locally.07:38
chriadamyou have made really good progress by the sounds of it!07:38
chriadamit's ok, can cherry pick from gerrit etc07:39
dcalisteAll simple tests are passing (hurrah).07:39
dcalisteBut all tests requiring the database are failing. I need to see why.07:39
dcalisteMaybe because it requires to connect to something.07:39
chriadamI 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
dcalisteThe message is always the same with a connection error : QWARN  : tst_StorageManager::initTestCase() void QCopClient::connectToServer() QLocalSocket::ServerNotFoundError "QLocalSocket::connectToServer: Invalid name"07:40
chriadamright.. some rules may have changed for local socket naming, to support all platforms07:41
dcalisteYes, 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
dcalisteMostly affected parts with rewrite is qmailmessage and qmailcodec. And all related tests are fine.07:41
dcalisteSo beside the big issue with imap and smtp, I mostly optimistic.07:42
chriadamyep, it certainly sounds like you've made great progress07:43
chriadamas 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 reviewing07:44
dcalisteAnd 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
dcalisteSure, no problem, duty first ;)07:45
chriadamyup, important part of the social contract, etc ;-)07:45
chriadampvuorela: maybe flypig could also help with reviewing the QMF changes ^07:46
flypigSorry, I've not been following this morning. But I'm happy to: just point me in the right direction.07:48
chriadamdcaliste has been working on porting QMF to build with Qt6 so that it passes upstream CI checks07:49
chriadamhe is still making some changes, but soon they will be ready for review in Qt gerrit07:49
flypigOkay, great.07:49
flypigBut there's no particular review page yet or anything then.07:50
dcalisteThis 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
dcalisteflypig: https://codereview.qt-project.org/q/project:qt-labs/messagingframework+status:open07:51
dcalisteAll of page 1, plus the top one on page 2…07:51
flypigAh, 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
dcalisteIn 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
chriadamflypig: not sure whether gerrit supports multiple-commit changesets.  we need to review them individually, and then stage them altogether.07:52
dcalisteExactly, I separated the MR into logical commits, depending on which part of Qt6 transition they are dealing with.07:52
flypigYeah, I thought it didn't support multiple commits.07:52
flypigOkay, I see. But, won't each have to pass?07:53
flypigThey can all be staged together.07:53
chriadamthe 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
chriadamwe 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
flypigHa, okay :) But they still need reviewing and approving individually.07:55
flypigI'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
dcalisteIndeed, 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
flypigThe small change reviews sounds like good tasks to do while waiting for other stuff.07:58
dcalisteI'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
dcalisteIt would make changes less intrusive and easier to patch back to something 5.6 compatible.07:59
flypigThis one: https://codereview.qt-project.org/c/qt-labs/messagingframework/+/313868 ?08:00
dcalisteExact, this one.08:04
chriadamdcaliste: 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
dcalisteNo, 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
flypigI still didn't merge the volatile property PRs. Sorry dcaliste, I will do.08:05
chriadamsounds good - thanks!08:06
chriadamhave a great week!08:06
dcalisteThank you flypig.08:06
dcalisteHave a good week also both of you.08:06
chriadamthanks :-)08:06
*** Ischwitch is now known as Ingvix09:36
*** frinring_ is now known as frinring10:50
*** Ischwitch is now known as Ingvix12:38
*** panzeroceania_ is now known as panzeroceania14:01
attahcoderus: I figured out what GitHub was messing up in the Docker run... they were inheriting $HOME from the outside env18:23
coderusattah: tell me please what are you talking about?:)18:41
attahcoderus: 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 name18:42
coderusattah: great, any success? :)18:42
attahBut it wouldn't find the build targets the other day18:42
attahYes, now i can build18:42
attahlooking at how to handle parameters now18:42
attahCurrent issue is that it seems i can't refer to inputs in the env section https://github.com/attah/buildfish/blob/master/action.yml18:44
attahi know i can pass them as positional arguments to the docker run, but that feels a bit fragile18:45
attahseems shuffling input to env isn't a thing :(18:52
coderusattah: can you show where it runs?19:28
attahcoderus: It's still flaketown... but here is the working run https://github.com/attah/harbour-seaprint/actions/runs/25614291019:29
attahany 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 nice19:32
coderusattah: try this: https://gist.github.com/CODeRUS/d4cdc9e25d234b7215d1c08a2ff4290019:35
coderusattah: it should be trannslated to named $INPUT_XXX args19:35
coderusor try to not pass any args at all19:36
coderusgithub actions should be responsible for it19:36
attahright, i can get env from the calling job19:37
attahbut if i want to run it twice, both archs for example, env isn't so good i think... i'd rather use inputs19:37
attahah, right... but those inputs are passed through maybe19:38
attahCool, that worked https://github.com/attah/harbour-seaprint/actions/runs/25619177519:45
attahcoderus: 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
attahjust parsing sdk-assistant list?19:49
coderusattah: main idea is for developer to know for which platform yo building app...19:50
attahmaybe you're right... i just basically always go for latest, so i thought it could be nice... although i do that consciously19:51
coderusattah: you create mr to bump sdk version, test if everything builds and merge :)19:54
attahfair point19:54
attahcoderus: So far so good... https://github.com/attah/harbour-seaprint/runs/1119846861?check_suite_focus=true20:42
attahNow uploading etc can be done the same way as you did, although i still need to get away from the latest tag in my Dockerfile20:44
attahbut that is a problem for tomorrow... (:20:45
attahbut it is probably impossible, and i should tag my releases with the sfos version20:46
*** N-Mi_ is now known as N-Mi22:02

Generated by irclog2html.py 2.17.1 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!