A few things to note:

• The CLDR extension is really called "CLDR" within MediaWiki, but the git repo (and extension directory) is "cldr" (lowercase)
• For CI's dependencies.yaml, it seems like lowercase cldr is correct given the usages for other extensions
• For extension.json, the correct version would be uppercase according to local tests, but in CI it fails (as can be seen in other patch sets for the same patch, example) with A dependency error was encountered while installing the extension "CampaignEvents": Could not find the registration file for the extension "CLDR"
• The "infinite wait" failure mode is weird and needs to be looked into and made immediate and more explicit
• I'm not sure what's causing the infinite wait; trying a made up extension name fails immediately with the "Could not find the registration file" error.
• This is blocking my team (Connection-Team) because we need this dependency for our current work (T393967)

Reproduced the quibble run locally using parameters from https://integration.wikimedia.org/ci/job/quibble-vendor-mysql-php81/14422/console and running

docker run -it --rm --entrypoint=quibble-with-supervisord \
  --env-file ./.env \
  -v "$(pwd)"/cache:/cache \
  -v "$(pwd)"/log:/log \
  -v "$(pwd)"/ref:/srv/git:ro \
  -v "$(pwd)"/src:/workspace/src \
  docker-registry.wikimedia.org/releng/quibble-bullseye-php81:1.14.1-s2 \
  --packages-source vendor --db mysql --db-dir /workspace/db \
  --git-parallel=8 \
  --skip selenium,npm-test,phpunit-standalone,api-testing

This worked, so I then added --skip-zuul --skip-deps as suggested in the docs to skip the slow parts.

First, I modified extension.json to use lowercase "cldr" and retried. It got here:

INFO:mw.maintenance.install:php maintenance/install.php --scriptpath= --server=http://127.0.0.1:9413 --dbtype=mysql --dbname=wikidb --dbuser=wikiuser --dbpass=secret --dbserver=localhost:/workspace/db/quibble-mysql-tly91o8n/socket --with-extensions --pass=testwikijenkinspass TestWiki WikiAdmin

*******************************************************************************
NOTE: Do not run maintenance scripts directly, use maintenance/run.php instead!
      Running scripts directly has been deprecated in MediaWiki 1.40.
      It may not work for some (or any) scripts in the future.
*******************************************************************************

then it hung and eventually ate all the memory on my laptop. Great! So I changed the entrypoint to just bash, and once inside the container, ran

$ php maintenance/install.php --scriptpath= --server=http://127.0.0.1:9413 --dbtype=mysql --dbname=wikidb --dbuser=wikiuser --dbpass=secret --dbserver=localhost:/workspace/db/quibble-mysql-tly91o8n/socket --with-extensions --pass=testwikijenkinspass TestWiki WikiAdmin

which also reproduced the crash. So, this looks like a MW bug rather than quibble. To verify, I went to a separate directory, made a fresh clone of MW with CLDR and tried there:

$  git clone https://gerrit.wikimedia.org/r/mediawiki/core.git mediawiki
$ git clone https://gerrit.wikimedia.org/r/mediawiki/skins/Vector.git mediawiki/skins/Vector
$ cd mediawiki/extensions
$ git clone https://gerrit.wikimedia.org/r/mediawiki/extensions/CampaignEvents
$ git clone https://gerrit.wikimedia.org/r/mediawiki/extensions/cldr
$ vim CampaignEvents/extension.json # Added lowercase cldr dependency
$ cd ..
$ composer update
$ php maintenance/install.php --scriptpath= --server=http://127.0.0.1:9413 --dbtype=mysql --dbname=wikidb --dbuser=wikiuser --dbpass=secret --dbserver=localhost:/workspace/db/quibble-mysql-tly91o8n/socket --with-extensions --pass=testwikijenkinspass TestWiki WikiAdmin

Reproduced it, so changing the tags here accordingly.

I'll continue investigating this here, and I filed T398256 for the dependency not working when uppercase (as it should be)

First, I want to confirm that this is caused by the non-canonical case, so I tried adding doesnotexist as dependency, and as expected, it failed immediately with

Error: A dependency error was encountered while installing the extension "CampaignEvents": Could not find the registration file for the extension "doesnotexist"

Then I tried clDR but that also failed immediately! So, it might be that the lowercase version matches a directory name. No point making more attempts, so I just enabled xdebug to debug the installer. I did not take notes as I was doing this because I needed full focus to understand what I was reading, but there's some confusion between extension name and extension directory.

Relevant code is in Installer::readExtension(). First it tries installing CampaignEvents alone; but that fails because we declared a dependency, and ExtensionRegistry::readFromQueue() will throw ExtensionDependencyError. So far so good: Installer learns about the missing extension, so it adds it to the $extDeps array and calls itself recursively. This works, and CLDR gets loaded. But then, when it checks for incompatibilities, we have:

• $data['credits'] refers to the CLDR extension by name, so using the uppercase form
• $extDependencies contains the lowercase version, because that's what we put in extension.json

And therefore we have a mismatch/incompatibility. This is once again reported to Installer, who then tries to add the same cldr string to the array of missing extensions and calls itself recursively, over and over again until it goes OOM.

Also, Installer does a file_exist check and that's what prevents this from failing earlier: it is assumed that if the file exists, then everything will be fine. But it's not, because dependencies will check the extension name, not the path. And likewise, if I specify the dependency using uppercase, the same file_exists check will fail early. So, there's currently no good way to specify this dependency.

TLDR: Installer::readExtension() takes an $extDeps (same for skins, but I'm only considering extensions for simplicity) parameter. The method itself treats it as a list of extension directory names, running an early file_exists to weed out invalid extensions. However, recursive calls (which are the only calls passing a non-empty list) pass a list of extension names obtained from ExtensionDependencyError::missingExtensions. So, if you have an extension whose name and directory name are different:

• Specifying the dependency using the extension name (correct way) fails immediately because Installer treats it as a path, but there's no extension.json there
• Specifying the dependency using the directory name (wrong) leads to an infinite loop because readExtension thinks the dependency is valid, but ExtensionRegistry rejects it

The CheckDependencies maintenance script, which seems mostly copied from Installer, has the same issue.

I can confirm Gerrit repository uses lower case (mediawiki/extensions/cldr).

When it was added as a dependency by https://gerrit.wikimedia.org/r/c/integration/config/+/1163347 , the original patch used the upper case version (CLDR) which would not have worked to git clone the repository since the value is passed as-is to craft the URL.

There are other extensions depending on cldr in CI then none have the dependency in extension.json which I guess explains why it was never encountered.

As a quick workaround, it looks like you can drop the requires in extension.json.

So let me see if I understand this correctly, the file_exists() check should actually be checking in the directory for a copy of extension.json and reading the name from there? which would be obviously less performant, but more accurate.

I have just realised the error in my thinking, disregard.

Thinking about this a bit more: if we have an extension name (e.g., from an extension.json requires) but not the extension path, there's no way to know where to look for that extension; the opposite is obviously possible (if we know the path, we can figure out the name). So, this implies that if any piece of code takes extension names as inputs, it cannot be changed to work with extension paths.

Dependency checks (VersionChecker::checkArray) are done on extension names, as that's what the requires key in extension.json uses; when running these checks, we don't know the paths of those dependencies. So, the ExtensionDependencyError that is thrown only knows about missing extension names, not paths. And because data from ExtensionDependencyError is used as input in recursive calls to Installer::readExtension(), that means readExtension (in its current form) can only take names as input and not paths.

On the other hand, readExtension calls ExtensionRegistry::readFromQueue, but that works with paths, not names (like wfLoadExtension, for example). So, readExtension needs to take paths as input so they can be passed to ExtensionRegistry. And that gives us a contradiction.

I imagine we want to avoid breaking changes to the format used by either the requires key or ExtensionRegistry. Therefore, readExtension cannot exist in its current form.

My first idea for a fix would be to stop loading one extension at a time, and just load all extensions at once. If that fails due to missing dependencies, we just exit with an error without retrying anything. I'm not sure how easy it would be to implement this, though. The fact that we still support non-JSON extension makes this harder than it needs to be. I'm wondering if we ever plan to stop supporting PHP registration files, since we've had JSON registration for 10 years now. That'd be T258845.

For a quicker fix, maybe we could:

• Check that the dependency we're trying to add isn't already in the list of things we're trying to load recursively, and fail immediately if that is the case (prevent infinite loop with wrong dependency format)
• Drop the special-case file_exists checks and just rely on ExtensionRegistration and VersionChecker to do their job (do not fail for correct format)

In T398224#10963463, @Daimona wrote:

For a quicker fix, maybe we could:

• Check that the dependency we're trying to add isn't already in the list of things we're trying to load recursively, and fail immediately if that is the case (prevent infinite loop with wrong dependency format)
• Drop the special-case file_exists checks and just rely on ExtensionRegistration and VersionChecker to do their job (do not fail for correct format)

On second thought, that wouldn't work because we'd still be passing extension names (not paths) to ExtensionRegistration.

Change #1165574 had a related patch set uploaded (by Cmelo; author: Cmelo):

[mediawiki/core@master] Resolve mismatch between extension folder and declared name during install

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

Thanks for all this investigation @Daimona

I think a quick fix for this issue would be to enhance the logic inside the catch block of the readExtension method.

The current implementation expects that the extension name used in extension.json matches the folder name of the extension. However, this is not always true. For this example:

• The folder is named: cldr
• But the "name" in extension.json is: CLDR

So, if another extension (e.g., CampaignEvents) declares CLDR as a dependency, the installer looks for a folder named CLDR, doesn't find it, and fails.

A Solution could be:

If readExtension() fails and enters the catch ( ExtensionDependencyError $e ), before retrying the call to readExtension(), we could:

1. Loop through all subfolders inside the extensions/ directory.
2. For each one, read its extension.json.
3. Compare the "name" property from the JSON with the missing extension name.
4. If there's a match, we know which folder it’s in.
5. Use the actual folder name as the extension dependency (instead of the "name" from the JSON).

This way, readExtension() will now receive a valid folder path and proceed successfully.

Example Fix in Action:

If CampaignEvents lists "CLDR" as a dependency:

• The first call to readExtension() fails, since there's no folder named CLDR.
• In the catch block, we scan all folders under extensions/.
• We find a folder cldr with extension.json having "name": "CLDR".
• We map CLDR → cldr and retry using the correct folder name.
• Problem solved.

If no match is found, that means the dependency really is missing. In this case, we throw:

throw new \RuntimeException( "Extension '$missing' not found in 'extensions/' directory." );

This also avoids the risk of an infinite loop when the dependency cannot be resolved.

Code example:

} elseif ( $e->missingExtensions || $e->missingSkins ) {
    // There's an extension missing in the dependency tree,
    // so add those to the dependency list and try again
    $resolvedExtDeps = $this->resolveExtensionPaths( $e->missingExtensions );
    $status = $this->readExtension(
        $fullJsonFile,
        array_merge( $extDeps, $resolvedExtDeps ),
        array_merge( $skinDeps, $e->missingSkins )
    );
    if ( !$status->isOK() && !$status->hasMessage( 'config-extension-dependency' ) ) {
        $status = Status::newFatal( 'config-extension-dependency',
            basename( dirname( $fullJsonFile ) ), $status->getMessage() );
    }
    return $status;
}

private function resolveExtensionPaths( array $missingExtensions ): array {
    $resolved = [];
    $extDir = $this->getVar( 'IP' ) . '/extensions';

    foreach ( $missingExtensions as $missing ) {
        $found = false;
        foreach ( scandir( $extDir ) as $dir ) {
            if ( !is_dir( "$extDir/$dir" ) || $dir === '.' || $dir === '..' ) {
                continue;
            }

            $extJson = "$extDir/$dir/extension.json";
            if ( !file_exists( $extJson ) ) {
                continue;
            }

            $json = json_decode( file_get_contents( $extJson ), true );
            if ( isset( $json['name'] ) && $json['name'] === $missing ) {
                $resolved[] = $dir;
                $found = true;
                break;
            }
        }

        if ( !$found ) {
            throw new \RuntimeException( "Extension '$missing' not found in 'extensions/' directory." );
        }
    }

    return $resolved;
}

And here is a patch applying it, I will also ask about this on #engineering-all

We briefly discussed that approach. I think it would work but it makes the code more complex rather than simpler, and it adds one layer of iteration rather than removing one. Also, it would need memoization to avoid the ~quadratic complexity it introduces, and error handling could be messy (the POC above just bails with a RuntimeException instead of a pretty error). So, I would only do this if batching extensions isn't possible.

I'll try making a POC to implement the approach from T398224#10963463 and see how it goes.

Change #1165627 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@master] [POC] installer: load info for all extensions at once

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

In T398224#10964915, @Daimona wrote:

I'll try making a POC to implement the approach from T398224#10963463 and see how it goes.

I did something, but it is a bit messy. There are some notes in the commit message but I have last updated them for patch set 4. One of the problems I found is that ExtensionRegistry doesn't give us enough information, like path to name mappings, a list of extensions with unmet dependencies, etc. I suppose that's why Installer is basically reinventing the wheel by trying to load one thing at a time.

The other problem is that even with the current version of my change, we still load extensions and skins separately. But then, that means we wouldn't be able to handle an extension depending on a skin, or vice versa.

I could keep making changes, like loading extensions and skins together (and then splitting the information), and processing transitive dependencies, etc. But I would first like someone (ideally a maintainer / code steward for Installer) to weigh in, because the change wouldn't be small. If the change is deemed to large or otherwise problematic, we can do the name-to-path approach that @cmelo proposed above.

Thanks @Daimona, I have updated the patch to create the map, so it scans the extensions folder only once, and then checks the mapping to get the extension path.

Change #1188357 had a related patch set uploaded (by Hashar; author: Hashar):

[mediawiki/extensions/cldr@master] Use lower case extension name to match repo name

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

Change #1188360 had a related patch set uploaded (by Hashar; author: Hashar):

[mediawiki/extensions/CampaignEvents@master] Use extension registry to require 'cldr'

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

Change #1188421 had a related patch set uploaded (by Hashar; author: Hashar):

[mediawiki/extensions/WikimediaMessages@master] Use lower case 'cldr' to check if it is loaded

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

Change #1188426 had a related patch set uploaded (by Hashar; author: Hashar):

[mediawiki/extensions/CentralNotice@master] Use lower case 'cldr' to check if it is loaded

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

Change #1188427 had a related patch set uploaded (by Hashar; author: Hashar):

[mediawiki/extensions/Cite@master] Use lower case 'cldr' to check if it is loaded

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

Change #1188430 had a related patch set uploaded (by Hashar; author: Hashar):

[mediawiki/extensions/WikimediaEvents@master] Use lower case 'cldr' to check if it is loaded

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

Change #1188431 had a related patch set uploaded (by Hashar; author: Hashar):

[mediawiki/extensions/Wikibase@master] Use lower case 'cldr' to check if it is loaded

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

Change #1188435 had a related patch set uploaded (by Hashar; author: Hashar):

[mediawiki/extensions/CampaignEvents@master] Use lower case 'cldr' to check if it is loaded

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

Change #1188448 had a related patch set uploaded (by Hashar; author: Hashar):

[mediawiki/extensions/UniversalLanguageSelector@master] Use lower case 'cldr' to check if it is loaded

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

I think this can be addressed by simply changing the name to the all lower case cldr in the mediawiki/extensions/cldr extension.json.

The chain of patches would be:

A) In CampaignEvents 1188435 - Use lower case 'cldr' to check if it is loaded. This builds upon @cmelo patch 7d8f751251f8d6dd2a2b6c2fee9af452ad34dca6 but make it recognizes cldr on top of CLDR.

B) Adjust all extensions to recognize both cldr or CLDR. I have sent patches that are required by the next step.

C) Make the cldr name in extension registration consistent with the repository basename. https://gerrit.wikimedia.org/r/c/mediawiki/extensions/cldr/+/1188357 This change can potentially breaks something somewhere I have marked it Code-Review -2. It depends on patches from the previous step.

D) Change CampaignEvents to requires cldr in extension.json and remove the workaround made by @cmelo. https://gerrit.wikimedia.org/r/c/mediawiki/extensions/CampaignEvents/+/1188360

E) Eventually later get rid of traces of CLDR in the code.

In T398224#11182581, @hashar wrote:

I think this can be addressed by simply changing the name to the all lower case cldr in the mediawiki/extensions/cldr extension.json.

For CLDR, yeah. But there are more extensions that would suffer from the same bug, unfortunately. At the very minimum, everything whose name includes spaces, and there's a lot of them. In the installer, and perhaps other parts of the code, there's confusion between directory name and extension name, so we should do something about it. I'm just not sure what...

Change #1188803 had a related patch set uploaded (by Hashar; author: Hashar):

[mediawiki/extensions/WikibaseLexeme@master] Use lower case 'cldr' to check if it is loaded

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

In T398224#11182958, @Daimona wrote:

For CLDR, yeah. But there are more extensions that would suffer from the same bug, unfortunately. At the very minimum, everything whose name includes spaces, and there's a lot of them. In the installer, and perhaps other parts of the code, there's confusion between directory name and extension name, so we should do something about it. I'm just not sure what...

I have discovered that with AbuseFilter which has the name Abuse Filter and that is what is used. Maybe we should use the directory name everywhere, deprecate name in favor of displayName and use that solely for display purpose such as Special:Version and even there I imagine it could be an i18n message.

My limited scope is to fix it for the cldr extension which is used at several places.

In T398224#11186621, @hashar wrote:

I have discovered that with AbuseFilter which has the name Abuse Filter and that is what is used.

Yeah, the AbuseFilter one hit me a couple times long ago.

Maybe we should use the directory name everywhere, deprecate name in favor of displayName and use that solely for display purpose such as Special:Version

This makes sense. And then the extension's "internal name" would just be the directory name, with no potential for confusion. I'd probably support it unless there are good reasons for not doing it.

and even there I imagine it could be an i18n message.

That's the namemsg key that already exist. Maybe we could leave just that (and drop name).

Change #1188430 merged by jenkins-bot:

[mediawiki/extensions/WikimediaEvents@master] Use lower case 'cldr' to check if it is loaded

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

Change #1188421 merged by jenkins-bot:

[mediawiki/extensions/WikimediaMessages@master] Use lower case 'cldr' to check if it is loaded

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

Change #1188448 merged by jenkins-bot:

[mediawiki/extensions/UniversalLanguageSelector@master] Use lower case 'cldr' to check if it is loaded

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

Change #1188431 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] Use lower case 'cldr' to check if it is loaded

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

Change #1188427 merged by jenkins-bot:

[mediawiki/extensions/Cite@master] Use lower case 'cldr' to check if it is loaded

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

Change #1188426 merged by jenkins-bot:

[mediawiki/extensions/CentralNotice@master] Use lower case 'cldr' to check if it is loaded

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

Change #1188803 merged by jenkins-bot:

[mediawiki/extensions/WikibaseLexeme@master] Use lower case 'cldr' to check if it is loaded

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

Change #1188435 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] Use lower case 'cldr' to check if it is loaded

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

I have send patches to all extension to have them check either cldr or CLDR is loaded using:

$registry->isLoaded( 'CLDR' ) || $registry->isLoaded( 'cldr' )

https://gerrit.wikimedia.org/r/c/mediawiki/extensions/cldr/+/1188357 change the names in extension.json from CLDR to cldr. It depends on all all the other patches I have made and which are all merged by now.

Finally if all went well, we can change CampaigEvents to require cldr from extension.json and remove the workaround that was introduced earlier. The patch is https://gerrit.wikimedia.org/r/c/mediawiki/extensions/CampaignEvents/+/1188360

Note: my patches do not fix the underlying issue this task refers to. It just workaround the defect by having the extension directory and name to be the same :-]

Change #1188357 merged by jenkins-bot:

[mediawiki/extensions/cldr@master] Use lower case extension name to match repo name

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

Change #1188360 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] Use extension registry to require 'cldr'

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

The series of patches I have made were to have the cldr extension name to have a lower case cldr name in extension.json. Most of the patches were to add have calls to ExtensionRegistry()->isLoaded() to be made against each of cldr and CLDR. That is a way to "fix" it when it is problematic for an extension.

My use case was for CI, I haven't looked at the installer use case which remains to be solved (though is is now addressed for cldr).