I would really be interested if there are other leaf modules that have this issue. If it's just qtquickcontrols2 then perhaps merging into qtdeclarative would be the easiest solution after all.
Taking a quick look through the qtquickcontrols2’s changes, it seems a large number(half?) of them are just Qt Submodule Update Bot and Qt Cherry-pick Bot changes. I would think that merging qqc2 into qtdeclarative could make things simpler, but I would like to hear if anyone knows about any potential downsides this may have. Is it a realistic move to make at this point? On 7 Jun 2021, at 10:22, Mitch Curtis <mitch.cur...@qt.io<mailto:mitch.cur...@qt.io>> wrote: -----Original Message----- From: Volker Hilsheimer <volker.hilshei...@qt.io<mailto:volker.hilshei...@qt.io>> Sent: Friday, 4 June 2021 4:51 PM To: Mitch Curtis <mitch.cur...@qt.io<mailto:mitch.cur...@qt.io>> Cc: development@qt-project.org<mailto:development@qt-project.org> Subject: Re: [Development] Solutions for ensuring that changes in upstream modules are tested with downstream modules before merging On 4 Jun 2021, at 16:10, Mitch Curtis <mitch.cur...@qt.io<mailto:mitch.cur...@qt.io>> wrote: -----Original Message----- From: Volker Hilsheimer <volker.hilshei...@qt.io<mailto:volker.hilshei...@qt.io>> Sent: Friday, 4 June 2021 2:45 PM To: Mitch Curtis <mitch.cur...@qt.io<mailto:mitch.cur...@qt.io>> Cc: development@qt-project.org<mailto:development@qt-project.org> Subject: Re: [Development] Solutions for ensuring that changes in upstream modules are tested with downstream modules before merging On 4 Jun 2021, at 13:56, Mitch Curtis <mitch.cur...@qt.io<mailto:mitch.cur...@qt.io>> wrote: Hi. A common problem I see is that a change in say qtbase or qtdeclarative can cause test failures in modules that depend on them after that change is merged. As a result, dependency updates for the downstream modules can be blocked, requiring the module maintainer to look into the failure, only to discover that it is caused by an upstream module. This causes a lot of time and effort to be diverted away from regular work. Both upstream and downstream developers would benefit from having more immediate CI feedback on these kinds of changes. https://bugreports.qt.io/browse/QTBUG-79454 was created to track this problem. So far the ideas suggested have been: - Run tests of dependent modules when testing a change, in addition to the regular tests for that module - Merge repositories of more tightly coupled modules (e.g. move qtquickcontrols2 into qtdeclarative) It would be beneficial to discuss the advantages and disadvantages of these and other solutions so that we can address this problem. Cheers. Excellent topic for the upcoming Qt Contributors Summit, Mitch, great if you can add it to the list of topics: https://wiki.qt.io/Qt_Contributors_Summit_2021_-_Program Which doesn’t mean that we shouldn't start the discussion on the list, and maybe we can then conclude it at the summit. Cheers, Volker OK, added it to the list. Thanks. As for the topic: I do think that we have cases where too many modules are lumped into a single repo, and cases where we have too much granularity. For the former: I see no immediate reason why QtNetwork needs to be in qtbase (network tests failing is a major cause of unrelated integrations failing), or why the Qt Positioning module needs to be in qtlocation. For the latter: Qt Quick Controls 2 might be better off sharing a commit history with QtQuick However, while we can make adjustments, I also think that there’s no perfect solution. It’s always a tradeoff between speed and autonomy on the one, and integrity across dependencies on the other hand. I agree. Running more tests is a similar tradeoff, and I don’t think blocking an integration into e.g. qtbase because it breaks a leaf module is a good solution to the problem. We should aim for fast feedback, and short meantime to recover such breakages, and making CI runs take even longer helps with neither. In particular if we then can’t make any such changes even though a follow up change in a leaf module is already available and just needs to wait for the dependency update. Then we are in a deadlock, unless we keep the code in qtbase working for both versions of e.g. qtdeclarative, which is not always practical. I could be wrong, but perhaps this is what Ossi's https://bugreports.qt.io/browse/COIN-133 was targeting. In any case, I can't really speak for qtbase as I rarely work on it. The problem is perhaps not that we break things, but that we don’t notice for too long that we broke things? And once we do notice, we often seem to be unwilling to revert the change that did cause the breakage, if things don’t get fixed quickly. This might be the case for qtbase and its leaf modules, but not for qtdeclarative and qtquickcontrols2. The typical situation there is: qtquickcontrols2 dependency update fails. I immediately cannot think of any relevant changes that would cause such a failure, so I quickly look through git log. If I can't find anything there, I need to start bisecting with qtdeclarative (and this assumes that the bisect doesn't result in build errors). It's not a productive use of time. On the other hand, a CI run, while taking extra time if having to run the tests of two or more modules, shouldn't block someone from doing work. There is always another task to work on while waiting for CI. Flakiness will of course make the feedback loop take way longer, but that has always been the case. To know about breakage quicker, a nighly run of all tests against “HEAD of everything” could help. Our dependency updates give us some of that, and we usually respond swiftly when an update fails. It shouldn’t be on the leaf module maintainer alone to follow up on things, and I do have the impression that people do take ownership, at least once they see that their base module change broke stuff. I’m sure we can improve there as well. But now that Qt 6 is out, I’d expect that a “everything at HEAD” toplevel build should work most of the time. My local worktrees are usually at HEAD of everything, and it’s rare that there are build problems at least. For qtdeclarative and qtquickcontrols2 I'm not sure how this would improve anything. It would say that a test failed, but as you said, that's what the dependency update already tells us. It does sound like a nice tradeoff (of running tests for all modules on every change and not running them at all) for modules where the dependency updates don't give any signs of a breakage, though. Lastly, I do think that we could do more in encoding assumptions made across modules in unit tests. If a change in qtdeclarative breaks an assumption made in qqc2, then ideally there’s a unit test in qtdeclarative that would fail. We use private APIs a lot across some modules, but we don’t have unit tests for them in the base module. Agreed, and the qtdeclarative team are great in doing this once they're aware of it, but there are too many use cases for them to have to be aware of in order to fully solve the problem. It's also a slow process (in terms of the rate of breakages across modules) in adding tests upstream to catch issues as they occur downstream. I would really be interested if there are other leaf modules that have this issue. If it's just qtquickcontrols2 then perhaps merging into qtdeclarative would be the easiest solution after all. _______________________________________________ Development mailing list Development@qt-project.org<mailto:Development@qt-project.org> https://lists.qt-project.org/listinfo/development
_______________________________________________ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development