Tuesday, 2022-03-15

dcalisteGood morning pvuorela, sorry for being late.08:28
pvuoreladcaliste: good morning. np.08:28
dcalisteThank you for the quick reviews on mKCal PRs.08:31
pvuoreladcaliste: sure. good stuff there. have been checking them closely now.08:31
pvuorelatried your wip commit on nemo-calendar but unfortunately it's now failing even more on tst_calendarevent.08:32
dcalisteReally ? It was fine for me yesterday evening. I had some issues with agenda model test because of the Manager::storageModified signal.08:32
dcalisteStrange also, because the only change in the WIP commit regarding calendar event test is the getEventAttendee one.08:35
pvuorelaeventSpy.wait() failing there quite many times.08:36
dcalisteThis one is not necessary a good fix by the way. The issue at hand is this one :08:36
dcaliste(eventSpy failing but not always looks like a race condition, I'll investigate keeping this in mind)08:36
dcaliste- in calendarworker.cpp#377, an organizer is created and his email is set, but not its name.08:37
dcaliste- so the event organizer.email() is not null, but organizer.name() is.08:38
dcaliste- in mkcal, with a recent fix, the organizer is stored with the corresponding attendee details, including the name.08:38
dcaliste- so after reread of the database, the organizer has a name.08:39
dcaliste- but now, with the updated signal, there is no round trip through the database, and the organizer is left without name.08:39
dcaliste- then in getEventAttendee, an attendee is deduplicated from the organizer by testing name and email, see around calendarutils.cpp#36508:40
dcaliste- so if the organizer has no name, like in the test, it is not properly deduplicated from the corresponding attendee and this result in the attendee list containing both the organozer as attendee and the organizer with no name so 4 attendees instead of 3 in the test().08:42
dcalisteIn the WIP commit, I'm mimiccing the mKCal behaviour by restoring the organizer with its attendee description. Not sure it's the right way to do it...08:43
pvuorelauhm, ok.08:45
pvuorelaperhaps in longer term we should some day separate organizer from the chair role.08:45
dcalisteYes, at the moment, I try not to induce regressions !08:47
dcalisteAbout the eventSpy failing, can you tell me where they are ?08:47
dcalisteI've tried again on JollaC and it's not failing. I guess it's because the proc is slower than on more recent device.08:48
dcalisteThe immediate possibility I see is that the signal is emitted before we enter the wait() loop.08:48
pvuorelatst_calendarevent.cpp(228) tst_calendarevent.cpp(303) tst_calendarevent.cpp(360) tst_calendarevent.cpp(612) tst_calendarevent.cpp(651) tst_calendarevent.cpp(780)08:48
dcalisteOk, not the case for 228. There is a timer of 5ms in the manager if I remember correctly that should ensure that we have time to enter the waiting loop before doing anything else.08:50
dcalisteHow are you running the test ? I'm doing myself "rm -f db3; SQLITESTORAGEDB=db3 ./tst_calendarevent testSave"08:51
dcalistethe changed branch of mKCal being compiled and installed on device before running this.08:52
pvuorelatestrunner-lite -f /opt/tests/nemo-qml-plugins-qt5/calendar/tests.xml -o nemocal08:52
dcalisteOk, let me try this one...08:53
pvuorelahm, not sure what on the wip commit could break it so much.08:54
pvuorelai'll try rebuilding to rule out any funny stuff.08:55
dcalisteDo you know where is blts-tool ? It seems to be required to run the tests via testrunner-lite08:58
pvuoreladcaliste: it's qa extra repository. need to be added manually.09:00
pvuorelanot entirely sure what the repository should be for you.09:00
pvuorelahey but anyhow, tried now rebuilding and reinstalling and the tests passed. sorry for the hassle. think i might have had bad mkcal version.09:01
dcalisteAh, that's also a good reason !09:01
pvuorelathe sources here if it's any help. https://github.com/mer-qa/blts-tools/09:02
dcalisteI need to add the right version dependency in spec file to avoid this to happen.09:02
pvuorelai was thinking of bumping mkcal version to 0.6.0 with these update signal adjustments.09:03
dcalisteThanks for blts-tool. I'll compile it and install it. It may provide some slightly different environment for tests.09:03
dcalisteOk, so I'll put that version requirement in QML binding spec file.09:04
pvuorelaon that organizer adjustment. i'm guessing it could be fine to prioritize the attendee event details for now but that could perhaps have some rationale mentioned.09:04
pvuorelathe calendarworker error output has a small typo09:04
dcalisteRight, I'll drop a comment there.09:04
dcalisteIndeed, I've just noticed it myself too. I'll fix it also. Thanks.09:05
pvuorelaand the final part on tst_calendarmanager seems sensical so with those small adjustments it could be all ok.09:05
dcalisteThere are the issues with agenda model.09:06
dcalisteWaiting on CalendarManager::storageModified to ensure that events are saved is not a possibility anymore in the initTestCase.09:07
dcalisteI need to check if that's the root problem of the issues reported later not finding CalendarStoredEvent.09:07
pvuorelahm, ok.09:08
pvuorelaso did you get some failure yourself?09:08
dcalisteI'll try to sort this out this morning after the meeting.09:08
dcalisteYes it's failing with a lot of warnings like :09:09
dcalisteCalendarStoredEvent* CalendarManager::eventObject(const QString&, const QDateTime&) No event with uid "7D995F1F-FE55-409B-9EC6-0D563A00726F"09:09
dcalisteAfter this is fixed and approved, I'll rebase https://github.com/sailfishos/nemo-qml-plugin-calendar/pull/30 on top.09:12
pvuorelasounds good.09:13
dcalisteThe changes are extensive of course, so take your time for the review. As I mentioned in the PR message, I've separated the work into several smaller commits, where tests should be ok at each step. Maybe it could be easier like that.09:14
dcalisteBut sometimes it imposed to be a bit silly by switching several time back and forth between CalendarData::Event and KCalendarCore::Incidence::Ptr.09:15
dcalisteBecause this or that part was not yet ported.09:15
dcalisteOk, pvuorela, I've fixed the agenda model test and squashed the commits (update the mkcal spec dependency, added a comment in getEventAttendee and correct the mispell).09:37
dcalisteTests are passing for me now.09:37
pvuorelastill a typo in the warning :)09:39
dcalisteThe problem with agenda model (and maybe some others) was related to the fact that the calendarworker sent list was reset on storage modification, while the corresponding list in manager was reset after data load.09:39
dcalisteFixed !09:41
pvuorelatests still passed. i'll check the thing still with thought and hopefully merge it later.09:43
dcalisteSure thank you.09:45
pvuoreladcaliste: suppose we're done. could go get some lunch. after that think i could start merging things if nothing new still pops up.10:04
dcalisteOk, that's great. Thank you. Enjoy your meal.10:04
pvuorelathanks. and for the PRs!10:05
poetasterrinigus: how to I specify noarch on obs correctly. Had BuildArch: noarch ina spec.13:30
nephros-nc-bridge[nctalk] <nephros> poetaster: that should be correct.14:37
poetasternephros-nc-bridge, I was a bit thrown that obs built it for all targets and arches14:57
poetasternephros-nc-bridge, trying to reduce cruft since I have a number of 'noarch' needlessly wasting space14:58
poetasternephros-nc-bridge, perhaps a     <repository><arch>noarch</arch> clause?14:59
riniguspoetaster: in this respect, you would get that noarch app built for all target-arch combinations. don't worry, that's the way OBS is16:08
nephros-nc-bridge[nctalk] <nephros> poetaster, rinigus: in the case the whole app is noarch (not just a sub package), I usually just disable all repos in that package and build against latest.16:19
nephros-nc-bridge[nctalk] <nephros> Chum obviously needs all enabled though16:19
nephros-nc-bridge[nctalk] <nephros> rinigus: about that obs policy: "no binary source packages": I dabble in Kodi building, and that needs java/jre to build. Is it a terrible crime to put a IcedTea JRE tarball in the sources?16:21
rinigusnephros: this is a good question! that binary is used just for building, right? what would be its arch? is it possible to make it useful on devices?16:48
rinigusI am on gentoo myself and we have few binary packages on it, not all is from source. maybe we should approach it in a similar manner... In principle, we should be able to have exceptions when we all know why these exceptions are needed16:50
riniguscc piggz ^16:50
nephros-nc-bridge[nctalk] <nephros> rinigus: I doubt people need a JRE packaged for use but I'm NOT going to try building it from source ;)16:55
nephros-nc-bridge[nctalk] <nephros> In the case of kodi its needed to generate some files, none of java or the jre ends up in the built package.16:55
nephros-nc-bridge[nctalk] <nephros> ATM I'm using the Gentoo binary tarball.16:56
rinigusnephros: but that jre is which arch?16:56
nephros-nc-bridge[nctalk] <nephros> arm at the moment. I think it works on aarch64 too.16:56
nephros-nc-bridge[nctalk] <nephros> https://build.sailfishos.org/package/show/home:nephros:devel:kodi/kodi16:58
nephros-nc-bridge[nctalk] <nephros> But the aarch64 build is stuck in DoD at the moment.16:59
rinigusKeto and lbt_: see above, DoD packages reported above17:44
rinigusnephros: it looks fine by me. let's see if it works for aarch64. in principle, you can add then jre for every arch and then use accordingly17:46
KetoI don't know whats wrong with the DoD17:59
*** marja is now known as Guest224718:26
raspOk i'm back18:42
raspGot an xperia on the way cause i sold out18:42
nephros-nc-bridge[nctalk] <nephros> rinigus: thanks for checking and confirming. Don't want to break too many rules :)18:47
attahpoetaster: so, what am i installing?18:53
rinigusnephros: I think it makes sense. We would just have to verify somehow the binaries that are included in this manner.19:11

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