Re: Reminder: action required when updating dependencies or build options
On Tue, 2021-01-12 at 16:50 +, Philip Withnall wrote: > On Tue, 2021-01-12 at 09:31 -0600, Michael Catanzaro wrote: > > Hi developers, > > > > Please remember that action is required when updating your > > dependencies > > or build options. You need to either make sure gnome-build-meta is > > OK > > with your changes, or ask release team to investigate on your > > behalf. > > > > We've had at least four separate breakages from four separate > > projects > > in the past few days. This is too much. Please be careful. Thanks! > > Sorry to have caused one of those breakages. I think this is going to > keep happening, though, since remembering to ping an external party > when dependencies change is hard. It’s something that happens > infrequently, and is not part of people’s normal > code/compile/review/merge cycle, so it’s not surprising they forget. > > Perhaps there’s scope for a CI bot which reminds developers to look > at > updating gnome-build-meta whenever a meson.build is changed. That bot > could be set up for every project which is listed in gnome-build- > meta. > > Or, if it’s appropriate, the bot could file an issue against gnome- > build-meta and assign the developer who’s touching meson.build to > that > issue. Or something. The same problems that existed in 2019 still exist: https://mail.gnome.org/archives/desktop-devel-list/2019-September/msg00014.html Developers still have no visibility over the gnome-build-meta builds. ___ desktop-devel-list mailing list desktop-devel-list@gnome.org https://mail.gnome.org/mailman/listinfo/desktop-devel-list
Re: Reminder: action required when updating dependencies or build options
(I tried to send this to the release-team@ alias/mailing-list, but that was shuttered last August, so seeing as it would be public on discourse anyway) On Tue, 2021-01-12 at 09:31 -0600, Michael Catanzaro wrote: > Hi developers, > > Please remember that action is required when updating your > dependencies > or build options. You need to either make sure gnome-build-meta is OK > with your changes, or ask release team to investigate on your behalf. > > We've had at least four separate breakages from four separate > projects > in the past few days. This is too much. Please be careful. Thanks! FYI, one of those breakages was a number of smaller API and ABI changes in libgweather that I tried to bundle up together to minimise breakage. I tried to my best to document all the API changes in the NEWS file when doing a release. And I filed issues against each one of the libgweather users in GNOME: https://gitlab.gnome.org/GNOME/evolution-data-server/-/issues/288 https://gitlab.gnome.org/GNOME/evolution/-/issues/1320 https://gitlab.gnome.org/GNOME/gnome-settings-daemon/-/issues/584 https://gitlab.gnome.org/GNOME/gnome-weather/-/issues/141 https://gitlab.gnome.org/GNOME/gnome-calendar/-/issues/682 https://gitlab.gnome.org/GNOME/gnome-clocks/-/issues/170 https://gitlab.gnome.org/GNOME/gnome-initial-setup/-/issues/119 https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/3577 https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/1565 There were bugs, there usually are, and it broke gnome-build-meta. This shouldn't have been a problem because: 1) libgweather doesn't have any ABI or API guarantees, even if it tries to avoid them (GWEATHER_I_KNOW_THIS_IS_UNSTABLE) 2) it was a one-line fix in gnome-build-meta to pin libgweather to an older version until all the above bugs (or the applicable ones) were fixed. Instead we have commits bypassing the CI pipelines _and_ reviews, which lack any sort of details about the bugs and point to build logs which will vanish when storage is needed, and don't follow the commit message style of the individual applications. Cancelled pipeline runs because you realise too late that it will fail anyway: https://gitlab.gnome.org/GNOME/libgweather/-/commits/master/ Commits that weren't even compile-tested locally: https://gitlab.gnome.org/GNOME/gnome-calendar/-/merge_requests/156 I get that there might be a thrill to being a build sheriff, but if you're going to commit directly to master without reviews or CI checks, you need to be absolutely 100% spotless. This is an example of a perfect commit, which passed the CI, followed the existing commit style, and was reviewed and merged within 20 minutes: https://gitlab.gnome.org/GNOME/libgweather/-/merge_requests/93 All that to say that I'm not going to work on libgweather anymore. Make sure to get those fixed urgently if you want weather to carry on working in GNOME 40: https://gitlab.gnome.org/GNOME/libgweather/-/issues/67 https://gitlab.gnome.org/GNOME/libgweather/-/issues/70 Bye ___ desktop-devel-list mailing list desktop-devel-list@gnome.org https://mail.gnome.org/mailman/listinfo/desktop-devel-list
Re: Reminder: action required when updating dependencies or build options
Hi, Yesterday, after I committed the typo to Calendar, I promised to use merge requests from now on when committing build fixes. Previously, I had promised to do this only for your projects, but yesterday I forgot, and not for the first time. I understand that's frustrating. From now on, I'll use MRs for all projects that use CI. I had already promised this in [1] in hopes of smoothing things over with you, but I see that didn't work. I had also attempted to apologize via IRC. You are right about the importance of MRs. I actually did at first temporarily switch libgweather.bst to use a 3.36 release tarball when I didn't see any easy way to fix your vapi file [2]. This turned out to not be needed because, with some help from Rico, we found a simple and obviously-correct solution to the problem [3]. I followed that with [4] to keep your CI working; it would have been better had I pushed both at the same time to avoid the canceled pipeline, but I hope that's not a big deal. Next time, I will use a MR and wait for your CI before merging. That said, keeping things building from git master is far preferable to pinning old tarball versions that we will forget to undo later, so I'd prefer to continue to reserve pinning to tarball versions as an absolute last resort, not as normal practice. [1] https://gitlab.gnome.org/GNOME/gnome-calendar/-/merge_requests/156 [2] https://gitlab.gnome.org/GNOME/gnome-build-meta/-/commit/e3639a2e1238a1d769a7ba2e8b50164d9bbd6bcc [3] https://gitlab.gnome.org/GNOME/libgweather/-/commit/f1f0bdd9ab47ff8ddba17e2125802c928226da64 [4] https://gitlab.gnome.org/GNOME/libgweather/-/commit/fc714b10c4cd78cf94d1d9f01f4b72d78a568047 On Wed, Jan 13, 2021 at 12:18 pm, Bastien Nocera wrote: I get that there might be a thrill to being a build sheriff, but if you're going to commit directly to master without reviews or CI checks, you need to be absolutely 100% spotless. It's not a thrill, it's purely frustrating. I accomplished almost nothing yesterday afternoon other than chasing build failures. But if we want to have flatpak runtime and GNOME OS updates, we have to do it. I hope you'll reconsider your decision to drop libgweather over this. Regardless, thanks for maintaining libgweather for so long. Michael ___ desktop-devel-list mailing list desktop-devel-list@gnome.org https://mail.gnome.org/mailman/listinfo/desktop-devel-list
Re: Reminder: action required when updating dependencies or build options
On Wed, 2021-01-13 at 10:06 -0600, Michael Catanzaro wrote: > Hi, > > Yesterday, after I committed the typo to Calendar, I promised to use > merge requests from now on when committing build fixes. Previously, I > had promised to do this only for your projects, but yesterday I > forgot, > and not for the first time. I understand that's frustrating. From now > on, I'll use MRs for all projects that use CI. I had already promised > this in [1] in hopes of smoothing things over with you, but I see > that > didn't work. I had also attempted to apologize via IRC. "I need to remember not to [push commits directly to the main branch] for your modules, sorry" > You are right > about the importance of MRs. > > I actually did at first temporarily switch libgweather.bst to use a > 3.36 release tarball when I didn't see any easy way to fix your vapi > file [2]. This turned out to not be needed because, with some help It was still needed. There were 8 modules that needed to have their compatibility checked, for which I filed individual issues. The vapi fix wasn't going to be enough to fix the other 7 modules, and waiting for those fixes to roll in could have happened in a branch for gnome-build-meta, with that module's CI making sure that all the ducks were in a row, and each fix actually peer-reviewed (or at least CI checked). > > I hope you'll reconsider your decision to drop libgweather over this. This isn't what made me drop libgweather. This was the last straw. What started it was this commit which you pushed directly to the main branch, and claimed that building was broken for weeks. It was broken for a couple of hours: https://gitlab.gnome.org/GNOME/grilo-plugins/-/commit/4329dda2c53f9a7bd2a3f6dd9334d4eb5207e9fd And similar behaviour over the years where you seemed to be under the impression that I was the only person in GNOME that felt that peer- reviewed patches and CI-gated merge requests were a requirement. Until the main development branch is protected and it's not possible to push directly there, I don't see myself being interested in contributing to modules where I filled the maintainer gap. ___ desktop-devel-list mailing list desktop-devel-list@gnome.org https://mail.gnome.org/mailman/listinfo/desktop-devel-list
Re: Reminder: action required when updating dependencies or build options
On Wed, Jan 13, 2021 at 7:53 pm, Bastien Nocera wrote: > "I need to remember not to [push commits directly to the main branch] > for your modules, sorry" I was trying to be sincere, not dismissive: mcatanzaro: "I need to remember not to for your modules, sorry" "Can hardly complain about people not following dependency rules when I can't remember simple things like this myself" After that discussion, I then proceeded to land a broken commit in gnome-calendar, at which point I no longer had a leg to stand on. That's when I said I would use MRs for all modules, not only yours: https://gitlab.gnome.org/GNOME/gnome-calendar/-/merge_requests/156 It's a little more work, but you're right that it's a good idea and best-practice. I had hoped that concession would deescalate this situation. Clearly not. > It was still needed. There were 8 modules that needed to have their > compatibility checked, for which I filed individual issues. > > The vapi fix wasn't going to be enough to fix the other 7 modules, and > waiting for those fixes to roll in could have happened in a branch for > gnome-build-meta, with that module's CI making sure that all the ducks > were in a row, and each fix actually peer-reviewed (or at least CI > checked). Bastien, how could I know about any of this? If you wanted to do a planned transition, you could have told us in advance so we could coordinate on how to do it. We might have created a libgweather compatibility element with the old API version, for example, and switched dependencies to use that ahead of time. Instead, you got an uncoordinated emergency response where you had no control over the resolution. Fortunately, Calendar was the only core app that required changes. Other developers' work is blocked until GNOME is building again, so we can't just wait for you to come online to sort things out. This delayed a WebKit update, for example. Nowadays GNOME consists of hundreds of interconnected components, and we can't afford to have fiefdoms when the build is broken. > This isn't what made me drop libgweather. This was the last straw. What > started it was this commit which you pushed directly to the main > branch, and claimed that building was broken for weeks. It was broken > for a couple of hours: > https://gitlab.gnome.org/GNOME/grilo-plugins/-/commit/4329dda2c53f9a7bd2a3f6dd9334d4eb5207e9fd I don't remember what happened four years ago, sorry. I assume that jhbuild was broken, otherwise that would probably not have drawn my attention. > And similar behaviour over the years where you seemed to be under the > impression that I was the only person in GNOME that felt that peer- > reviewed patches and CI-gated merge requests were a requirement. > > Until the main development branch is protected and it's not possible to > push directly there, I don't see myself being interested in > contributing to modules where I filled the maintainer gap. This is disappointing. If you restrict commits, please let release team know, because we have a rule that we only build modules from git master if we can commit to them to fix build failures. We would just need to switch your modules to build from tarball instead. Building core software from tarball instead of git would reduce the value of GNOME OS and the master runtime for everyone, but it is an option. Michael ___ desktop-devel-list mailing list desktop-devel-list@gnome.org https://mail.gnome.org/mailman/listinfo/desktop-devel-list
Re: Reminder: action required when updating dependencies or build options
On Wed, 2021-01-13 at 15:15 -0600, Michael Catanzaro wrote: > > And similar behaviour over the years where you seemed to be under > the > > impression that I was the only person in GNOME that felt that > peer- > > reviewed patches and CI-gated merge requests were a requirement. > > > > Until the main development branch is protected and it's not > possible > to > > push directly there, I don't see myself being interested in > > contributing to modules where I filled the maintainer gap. > > This is disappointing. If you restrict commits, please let release > team > know, because we have a rule that we only build modules from git > master > if we can commit to them to fix build failures. We would just need to > switch your modules to build from tarball instead. Building core > software from tarball instead of git would reduce the value of GNOME > OS > and the master runtime for everyone, but it is an option. Given that you’ve just committed to submitting MRs and waiting for CI to pass, rather than pushing directly to master, perhaps this rule should be rethought? Philip ___ desktop-devel-list mailing list desktop-devel-list@gnome.org https://mail.gnome.org/mailman/listinfo/desktop-devel-list
Re: Reminder: action required when updating dependencies or build options
On Wed, Jan 13, 2021 at 9:28 pm, Philip Withnall wrote: Given that you’ve just committed to submitting MRs and waiting for CI to pass, rather than pushing directly to master, perhaps this rule should be rethought? Hm... as long as we have permission to merge the MR after CI has passed, or to bypass the CI if it's broken due to a preexisting issue, then we should be good. However, I'm not sure GitLab allows this? E.g. we discovered yesterday that I didn't have permission to commit to gnome-remote-desktop until Jonas played with the settings; there, I had created a MR, but only project maintainers were able to merge it. The worst-case scenario would be: need to revert a commit, CI is already broken due to some preexisting unrelated issue that we don't know how to fix, can't land revert via MR because it requires CI to pass, can't change the setting to allow bypassing CI because the only way to get permission to change the setting is to update the doap, can't update the doap without first passing CI ___ desktop-devel-list mailing list desktop-devel-list@gnome.org https://mail.gnome.org/mailman/listinfo/desktop-devel-list
