Page MenuHomePhabricator

BlueSpice related tests fails on gate-and-submit-1.31 in quibble-composer tests
Closed, DeclinedPublic

Description

gate-and-submit-1.31 fails for BlueSpice related repositories because of PHP incorrect version. Output from https://integration.wikimedia.org/ci/job/quibble-composer-mysql-php70-docker/7887/console:

23:54:13 [266.5MB/6.54s] Dependency resolution completed in 0.198 seconds
23:54:13 [266.5MB/6.54s] Your requirements could not be resolved to an installable set of packages.
23:54:13 [266.5MB/6.54s] 
23:54:13   Problem 1
23:54:13     - This package requires php >=7.2.9 but your PHP version (7.0.33) does not satisfy that requirement.
23:54:13   Problem 2
23:54:13     - Installation request for mediawiki/mediawiki-codesniffer 28.0.0 -> satisfiable by mediawiki/mediawiki-codesniffer[v28.0.0].
23:54:13     - mediawiki/mediawiki-codesniffer v28.0.0 requires php >= 7.2.0 -> your PHP version (7.0.33) does not satisfy that requirement.
23:54:13   Problem 3
23:54:13     - Installation request for symfony/yaml 4.3.4 -> satisfiable by symfony/yaml[v4.3.4].
23:54:13     - symfony/yaml v4.3.4 requires php ^7.1.3 -> your PHP version (7.0.33) does not satisfy that requirement.
23:54:13 
23:54:14 [218.2MB/6.58s] Memory usage: 218.24MB (peak: 266.53MB), time: 6.58s
23:54:14 INFO:quibble.cmd:Run composer update for mediawiki/core finished in 6.863 s
23:54:14 Traceback (most recent call last):
23:54:14   File "/usr/local/bin/quibble", line 11, in <module>
23:54:14     load_entry_point('quibble==0.0.38', 'console_scripts', 'quibble')()
23:54:14   File "/usr/local/lib/python3.5/dist-packages/quibble/cmd.py", line 478, in main
23:54:14     cmd.execute(plan)
23:54:14   File "/usr/local/lib/python3.5/dist-packages/quibble/cmd.py", line 451, in execute
23:54:14     command.execute()
23:54:14   File "/usr/local/lib/python3.5/dist-packages/quibble/commands.py", line 347, in execute
23:54:14     subprocess.check_call(cmd, cwd=self.mw_install_path)
23:54:14   File "/usr/lib/python3.5/subprocess.py", line 271, in check_call
23:54:14     raise CalledProcessError(retcode, cmd)
23:54:14 subprocess.CalledProcessError: Command '['composer', 'update', '--ansi', '--no-progress', '--prefer-dist', '--profile', '-v']' returned non-zero exit status 2

Event Timeline

Kizule renamed this task from BlueSpice related tests fails on gate-and-submit-1.31 to BlueSpice related tests fails on gate-and-submit-1.31 in quibble-composer tests.Oct 17 2019, 9:58 PM
Kizule triaged this task as Medium priority.
Kizule changed the subtype of this task from "Bug Report" to "Task".

Hmm, in https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/542220/ @Reedy set minimum version of PHP to 7.29, but not in REL1_31 (I think). Why REL1_31_dev use configuration for REL1_34?

As far as I understand from @hashar's explanation in another place (T224285#5216226) this is because in mediawiki/core (and other extensions listed in the dependencies) there is no REL1_31_dev branch, therefore quibble falls back to master branch [1]. But in master branch there are already libraries/dependencies used that do not support older PHP versions anymore.

I have seen from the cloner.py code that one can set an "indicated branch" by using project_branches. I believe these are coming from the zuul/layout.yaml configuration. @hashar, would it be possible to configure our BlueSpice* repos to indicate REL1_31 when REL1_31_dev is tested?

If not there could be other solutions:

(A) For BlueSpice we already require PHP 7.2+, so we could just disable tests with older versions in general
(B) Modify quibble's cloner.py to

  • either support a naming-convention-fallback-pattern: "If the indicated branch (REL1_31_dev) does not exists, strip a potential _dev suffix (--> REL1_31) and try again" (simple)
  • or allow allow a "branch-map" in the CI configuration that we can set for BlueSpice* repos to "REL1_31_dev" => "REL1_31" (difficult)

We could provide a patch for this.

@hashar, what do you recommend?

[1] https://github.com/wikimedia/quibble/blob/5909b343ec18c4e6147018359826237065336a93/zuul/lib/cloner.py#L175-L193

As far as I understand from @hashar's explanation in another place (T224285#5216226) this is because in mediawiki/core (and other extensions listed in the dependencies) there is no REL1_31_dev branch, therefore quibble falls back to master branch [1]. But in master branch there are already libraries/dependencies used that do not support older PHP versions anymore.

I have seen from the cloner.py code that one can set an "indicated branch" by using project_branches. I believe these are coming from the zuul/layout.yaml configuration. @hashar, would it be possible to configure our BlueSpice* repos to indicate REL1_31 when REL1_31_dev is tested?

If not there could be other solutions:

(A) For BlueSpice we already require PHP 7.2+, so we could just disable tests with older versions in general
(B) Modify quibble's cloner.py to

  • either support a naming-convention-fallback-pattern: "If the indicated branch (REL1_31_dev) does not exists, strip a potential _dev suffix (--> REL1_31) and try again" (simple)
  • or allow allow a "branch-map" in the CI configuration that we can set for BlueSpice* repos to "REL1_31_dev" => "REL1_31" (difficult)

We could provide a patch for this.

@hashar, what do you recommend?

[1] https://github.com/wikimedia/quibble/blob/5909b343ec18c4e6147018359826237065336a93/zuul/lib/cloner.py#L175-L193

Maybe it should be better to disable tests for PHP <7.2 in BlueSpice. But let it to @hashar

There is another issue... https://integration.wikimedia.org/ci/job/quibble-composer-mysql-php72-docker/6248/console

12:02:07 Creating main page with default content
12:02:07 done
12:02:07 Creating tables for enabled extensions
12:02:07 PHP Fatal error:  Declaration of BlueSpice\Hook::msg($key) must be compatible with MessageLocalizer::msg($key, ...$params) in /workspace/src/extensions/BlueSpiceFoundation/src/Hook.php on line 32
12:02:07 INFO:quibble.cmd:Install MediaWiki, db=mysql db_dir=/workspace/db vendor=False finished in 10.553 s

This is because we implement MessageLocalizer from REL1_31 which has a different signature from the one in master. So even if we'd just use the PHP 7.2 environment this will still happen. So I believe there is no way around my solution (B). @hashar Would it be okay if we'd provide a patch for quibble that allows pattern-based-fallback or a branch-map? Which would you prefer? Which requirements do you have for such a patch to get merged?

@hashar I have uploaded a patchset for the cloner.py: https://gerrit.wikimedia.org/r/#/c/integration/quibble/+/550087/
ATTENTION: I am new to Python and did not test this code very well. Could you please have a look and give me some advice? Thanks in advance!

There are some technical details at T224285#5216226

+ @awight since he had a similar use case for the DonationInterface extension

I am at a conference this week and out of town next week as well so I can't really do any review unfortunately :-\

The REL1_31-dev branching model for BlueSpice extension is definitely not supported by the CI tooling. I went describing the issue that has been encountered back in May when those branches have been introduced (T224285#5216226). And indeed CI fallback to master when the -dev branch does not exist.

Zuul / CI has support to integrate against a specific branch, we use that for the DonationInterface which is developed in the master branch but targets fundraising/REL1_31 branch. IIRC the CI job does something like:

quibble --branch=fundraising/REL1_31 mediawiki/extensions/DonationInterface <rest of dependencies there>

But that requires all participating repositories to have the branch created. Thus we have mediawiki/core, mediawiki/vendor and other dependencies to ALL have a fundraising/REL1_31, else indeed we fall back to the master branch.

Anyway, instead of adding some heuristic in the cloner.py, I would rather have a fallback branch parameter that would let one override the currently hardcoded "fallback_branch = 'master'". Then for BlueSpice extension we could have a job that does:

quibble --fallback-branch=REL1_31

Then if CI runs a change for BlueSpiceFoobar REL1_31-dev branch, for other repositories it would still tries to checkout a REL1_31-dev branch in other repositories and end up falling back to REL1_31 instead of master. There are potential race condition involved though such as a change being process for REL1_31 in parallel which might potentially conflict, but I guess that is a corner case issue.

I guess we would need an additional set of CI Jenkins jobs to be added that would use that specific parameter. That would add some extra complexity to the CI workflow though and I am not sure I am willing to go on that path.

My point from T224285#5216226 still stand, I would rather see the development to happen in the REL1_31 branch and then have them released by mean of tagging the stable commit and releasing that. That sounds easier overall compared to having to maintain forked development branches across several repositories :)

Thanks for your feedback, I see your concerns. To be honest, I am also not very happy with the REL1_31_dev-branch-approach. But unfortunately I do not see any other way to do it. This is because of the fact that we are developing various versions in parallel. E.g. REL1_31 will be version "3.1.x" (next patch level release), while REL1_31_dev will already be a new minor version "3.2" (next minor-level-release), which may have new features. If we developed "3.2" functionality in REL1_31 and only release by tagging, we could not have patch-level releases once a single minor-level-feature was merged in one of the BlueSpice* repos.

I will happily provide patches to support --fallback-branch parameter, if that enables us to test different branches with different fallbacks at the same time. E.g. REL1_31_dev -> REL1_31, REL1_35_dev => REL1_35. Just setting one certain fallback-branch to all BlueSpice* repos, regardless which branch is under test will not work well for us.

I still believe the "branch-name-pattern-heuristic" would be an easy to implement and very much satisfying solution ("Convention over configuration").

What we are trying to achieve here is to enable a development workflow against the LTS version of MediaWiki. So we use the current MediaWiki LTS version (currently 1.31) as a basis for our extensions. This is mainly because we simply do not have the dev resources to keep updated with the fast moving master branch of MediaWiki. Within the ~2 years of LTS support, we provide bug and security fixes (patch releases) for the current stable extension as well as feature improvements (minor releases). This is why we need to maintain at least two lines of development. I would assume a development workflow against the LTS version is something that is not only specific to BlueSpice, but would strengthen the concept of MediaWiki LTS in general.

I like the idea of a base branch (REL1_31/dev) instead of the more random pattern we are currently using. Please note that any naming pattern would create a generic mechanism. So if any extension developer wants to create a bigger change (and we see several extensions that do their own versioning), they could open a feature branch and still get the benefit of testing within the settings of one of the REL1_x branches.

A --fallback-branch parameter would also work well for us. The backdraw here is imho, that it is not generic. So people who want to use it would need to have some in-depth knowledge of how to configure the test system, and so the mechanism will only be used by a few.

How about changing the pattern from *<REL-branchname>* to */<REL-branchname> (or even */<arbitrary-branchname>)? This would follow the "fundraising" example. We can rename our branches accordingly.

How about changing the pattern from *<REL-branchname>* to */<REL-branchname> (or even */<arbitrary-branchname>)? This would follow the "fundraising" example. We can rename our branches accordingly.

Everything about "the fundrasiing example" is explicitly something we don't encourage and wouldn't support for aonyone.

A --fallback-branch approach to the cloner is moderately "simple" but would explode the number of configs. Right now we only have variation within branches based on what PHP versions we support, so we can do it inline and "relatively" simply with "only" 200 lines of configuration. Including also variance on target branch names would add special rules on a per-repo basis, creating a matrix of config options (e.g. REL1_35 is going to support PHP74 but REL1_31 doesn't even support PHP73).

I don't think it'd be managable.

Magic auto-fallbacks in the cloner make me worried that something will blow up at some point, however.

I am sorry, but I think we might have mixed things up in this discussion.

Initially this was about a PHP version issue. As far as I understand our extensions all use the extension/skin-quibble-composer [2] (or extension-broken) template. This uses quibble-composer-mysql-php7[0-3]-docker "jobs". So we could easily configure all BlueSpice* repos to be tested with the proper PHP versions. Actually the configuration looks like the CI environment is already dispatching on the "branch-under-test". I believe we can find a proper configuration here with what is already possible.

Now there is another issue. We build BlueSpice extensions only on MediaWiki LTS versions. Within this frame we publish patchlevel-, minor- and maybe even major-releases. Our pachtlevel-code always lives in the REL1_31 branch. For minor/major releases we have introduced the branch REL1_31_dev. This indicates, that we are still building against MW 1.31, but the code lives seperately from the patchlevel (which we publish more frequently). Unfortunately all tests on this non-standard-branch REL1_31_dev break, as Quibble does not understand that it must use the REL1_31 version of MediaWiki (and dependency-extensions), but clones master. Now there is actually a way to configure the "fallback-branch" that is used if Quibble does not find the "branch-under-test" in the MediaWiki core repo.
As far as I understand we clould already configure the the CI to use REL1_31 as a fallback branch for REL1_31_dev. But this would mean a lot of maintenance work, as it might need to be configured for each repo. Also if we move on to the next LTS (REL1_35) we will have to configure it again.
So my proposal in this case (not talking about the PHP version issue from above) is to introduce a simple pattern based fallback branch mechanism. As far as I can see this is easy to implement and will not affect any of the WMF processes. It will just allow Quibble to understand that an extension with a whatever/REL1_31 branch wants to be tested in an environment where MW core has REL1_31 instead of master.

Maybe I have just some misconception of how the CI works. Please help me to understand.

[1] https://github.com/wikimedia/integration-config/blob/4c2ee753548898a90c06a1501ab5eb8efe03c83e/zuul/layout.yaml#L4569-L4571
[2] https://github.com/wikimedia/integration-config/blob/4c2ee753548898a90c06a1501ab5eb8efe03c83e/zuul/layout.yaml#L1141-L1194

Sorry that took time. I talked about this with James. +1 on the --fallback-branch in Quibble

We will need to generate more specific jobs in JJB that would pass the parameter.

James and I had a conversation about this patch, we agreed that using a convention as introduced here is a good thing. Better than adding yet another option to Quibble and having to generate a bunch of more specific jobs.

will look at reviewing/merging/deploying this next week. Though I am also running the mediawiki deployment train.

Hi @hashar! Can we revive this task please? I didn't find any information about a new feature like --fallback-branch in quibble or how to configure it in the CI. Therefore I assume this may just have been forgotten. Is there a chance to have a pattern based fallback in quibble in the next time? I am completely fine with any pattern, as long as it allows me to have a custom branch name that makes quibble clone the proper versions of MediaWiki core and dependency extensions.

@Jdforrester-WMF apparently you and @hashar agreed on merging this patch in T235807#5987659. Could could we please make this happen?

@Reedy Why has this been declined? There was a discussion, there was a patch, there were no real objections [1].

[1] https://phabricator.wikimedia.org/T235807#5987659

Probably because 1.31 saw its end of life months ago. If you still run 1.31.x you have security issues by now.

This is not about REL1_31 at all. It applies to REL1_35 and every other branch as well. Yes, we do still publish updates for the unsupported version of MediaWiki. But no CI tests run anymore for those, which is okay. We even add backports of security fixes from higher branches to our final packages.

Change 550087 had a related patch set uploaded (by Kosta Harlan; author: Robert Vogel):

[integration/quibble@master] Add pattern based fallback for branches

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

Change 550087 abandoned by Hashar:

[integration/quibble@master] Add pattern based fallback for branches

Reason:

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

Change 922113 had a related patch set uploaded (by Hslater; author: Hslater):

[integration/quibble@master] Implement pattern-based branch fallback

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

I noticed that this task was previously declined, but I wanted to bring it to your attention again. The issue that prompted this task still persists.
Given the previous discussion and the consensus that the convention introduced here is beneficial, I believe it's worth reevaluating this task.
I've made a relevant commit (922113) that addresses the concerns raised. It provides a potential solution to the problem.
I kindly request you to take another look at this task considering the recent changes and the ongoing need for a resolution.

Change 922113 abandoned by Hashar:

[integration/quibble@master] Implement pattern-based branch fallback

Reason:

Dupe of abandoned patch https://gerrit.wikimedia.org/r/c/integration/quibble/+/550087

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

I have abandoned the new change which was the same as the previous one.

mediawiki/core, extensions and skins have release branches which are considered the stable ones. The CI configuration (Quibble, Zuul, Jenkins jobs) assumes the branch exists and that the code of a given repository/branch is only intended to work with the same branch.

Theoretically, we could add an extra layer of branching (REL1_XX-dev) with the extra fallback proposed above, however it has a few issues:

  • When a change is made to a REL1_XX-dev branch, the other repositories should checkout REL1_XX-dev and thus we should cut that branch on all repositories participating. Else you can have a change made to a dev branch that fails cause other repositories uses the REL stable branches.
  • The Zuul configuration would need more test and gate-and-submit pipelines for each of those -dev branches. Our setup already has troubles catching up with all those pipelines.

I believe the easiest would be to adopt the same pattern used for other MediaWiki extensions and skins: develop on the master branch and backport fixes to the REL branch. I don't see the benefit of having an extra layer of branching (-dev branches) which adds an extra level of complexity.

More or less related is T196467 which is about having the master branch of an extension to be tested against multiple REL branches. That one can't be easily supported with the current CI.