Page MenuHomePhabricator

Git repos with direct pushes bypassing gerrit/CI could be a maintenance burden for libup patches
Closed, ResolvedPublic

Description

The libup patches

were fail on phpcs errors in the current CI jobs.
This should normally not happen, because CI is testing with phpcs and avoid merging.

But this git repos receive direct pushed, which bypass gerrit and CI

For DynamicPageListEngine it is https://gerrit.wikimedia.org/g/mediawiki/extensions/DynamicPageListEngine/+/913bd0c07ad0513008f449eb1192b1c9554f951f
For DataTable2 it is https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/DataTable2/+/c152d0cf42d12e4f62e22377c3be343a940153f5

Bypass the tests of the CI pipeline brings in some problems for the auto-merge mode of libup. Libup auto-merge trival patch sets to reduce human interaction with the patch set.
A failing CI brings in some maintenance burden for it.

How to process with this situation?

Possible solution:

  • Disable direct git pushes
  • Exclude the repos from libup
  • Keep status quo and hope someone fix the repo for the libup patch set
  • Simplify CI jobs to merge without tests
  • ...

It seems the setup with direct pushes is official support for extensions on gerrit. I have no idea how much extensions are affected.

This is only the aspect about direct pushes. There are other situation which can break CI and make it harder for libup to merge (force merges of patch set through gerrit with broken tests, hard deprecation in core/extensions of still used functions, remove of functions/table columns still used, version updates of unpinned dependency, changes in CI infratstructure, ...)

Event Timeline

My understanding was that we did not allow direct "open push" for master of extensions because of problems like this and that there's no way to review/comment on direct pushes, which IIRC was also a problem for TWN. Is that correct @QChris?

I would recommend just disabling direct push on these.

I did not intentionally configure anything different from the way it is supposed to work. The commits cited by MarcoAurelio have been done almost four years ago, I do not remember what was behind them.

Feel free to modify the git repo settings the way they should be.

Change 760983 had a related patch set uploaded (by MarcoAurelio; author: MarcoAurelio):

[mediawiki/extensions/DynamicPageListEngine@refs/meta/config] Remove direct push access from the repository

https://gerrit.wikimedia.org/r/760983

Change 760984 had a related patch set uploaded (by MarcoAurelio; author: MarcoAurelio):

[mediawiki/extensions/DataTable2@refs/meta/config] Modify repository access settings

https://gerrit.wikimedia.org/r/760984

Change 760983 merged by Zabe:

[mediawiki/extensions/DynamicPageListEngine@refs/meta/config] Remove direct push access from the repository

https://gerrit.wikimedia.org/r/760983

Change 760984 merged by Zabe:

[mediawiki/extensions/DataTable2@refs/meta/config] Modify repository access settings

https://gerrit.wikimedia.org/r/760984

Does that mean all extensions with push and/or pushMerge in the access rules should be cleaned up?

For example
https://gerrit.wikimedia.org/r/admin/repos/mediawiki/extensions/AutoCreateCategoryPages,access
https://gerrit.wikimedia.org/r/admin/repos/mediawiki/extensions/AccessControl,access
Around 150 repos

Also mediawiki has push - https://gerrit.wikimedia.org/r/admin/repos/mediawiki,access - but not for the global refs/*
Some specials are also in DonationInterface, GuidedTour, VisualEditor

After clean up: It is possible to change the ACL for the project owners and/or mediawiki group to apply no new push/pushMerge to a repo? Just limit the right to platform-engineering?

Umherirrender claimed this task.

ACL for skins and extensions looking clean now

hashar added a parent task: Restricted Task.Feb 13 2024, 6:38 AM