Manage language packs for all dependency types #224

snim2 · 2023-09-22T09:50:32Z

Resolves Management of language files #4
Resolves Repeating ['themes', 'plugins'] everywhere is not DRY #70
Related to Tests should mock the filesystem consistently #226
Related to Refactor WhippetHelpers #227

Summary

This PR adds the ability to manage WordPress language packs with Whippet. Once this PR is merged, it should be possible to run any whippet deps ... command with languages as a dependency (rather than plugins or themes). Commands that do not specify a dependency type (e.g. whippet deps validate) should now take language packs into account.

About WordPress language packs

WordPress language packs are slightly different to other dependency types (core, plugins, themes) and those differences complicate this PR.

Language packs are versioned separately, but they are available for all other dependency types. So, installing the pt_BR language pack might install translations for WordPress Core, and for a number of plugins and themes that are also managed by Whippet.

Whereas with other dependencies we mirror the repositories for each dependency, in this PR we have opted not to do that, due to the complexity of retrieving the "correct" translation packs for each dependency. Instead, we rely on the wordpress.org APIs to describe the location of each translation.

For example, if we have a repo pinned to version 6.3.3 of WordPress Core, which uses the classic-editor plugin and the twentytwenty theme, and we want to install pt_BR translations, we would query the following JSON APIs to find the URI for each language pack:

WordPress Core 6.3.1: https://api.wordpress.org/translations/core/1.0/?version=6.3.3
classic-editor v1.6.3: https://api.wordpress.org/translations/plugins/1.0/?slug=classic-editor&version=1.6.3
twentytwenty v1.0.0: https://api.wordpress.org/translations/themes/1.0/?slug=twentytwenty&version=1.0.0

This is equivalent to the behaviour of wp-cli which provides commands for manually installing and removing translations, e.g.

wp language core install
wp language plugin classic-editor ja
wp language theme install twentytwenty ja

The `whippet.json` and `whippet.lock` files

To add language packs to the Whippet dependencies of a repository, the country codes for the languages should be added to the JSON file, e.g.:

     "languages": [
        {"name": "en_GB"},
        {"name": "pt_BR"},
        {"name": "ja"}
    ]

On running whippet deps update the lockfile will be populated with information from the wordpress.org APIs. Language codes are considered valid if and only if a translation is available for the current pinned version of WordPress Core.

Note that here, the revision for each entry is the revision of the relevant dependency. So, 6.3.1 in the example below refers to the version of WordPress Core, not the version of the translation files, which are versioned separately.

The code in Whippet expects that each lockfile object will contain a name, a src and a revision. For language packs, we may want to install a number of different translation packs for each country code. Rather than complicate the code that manages lockfiles by adding more attributes to the lockfile objects, I have opted to encode dependency types in the names of each translation. So, the pt_BR translations for the twentytwenty theme would be encoded as pt_BR/themes/twentytwenty.

For example:

    "languages": [
        {
            "name": "ja",
            "src": "https://downloads.wordpress.org/translation/core/6.3.1/ja.zip",
            "revision": "6.3.1"
        },
        {
            "name": "ja/plugins/classic-editor",
            "src": "https://downloads.wordpress.org/translation/plugin/classic-editor/1.6.3/ja.zip",
            "revision": "1.6.3"
        },
        {
            "name": "ja/plugins/pdf-image-generator",
            "src": "https://downloads.wordpress.org/translation/plugin/pdf-image-generator/1.5.6/ja.zip",
            "revision": "1.5.6"
        }
    ]

About this PR

Whippet is not as compliant with the SOLID principles as it might be, which means that some parts of the code are not well encapsulated and there are several hidden dependencies.

Because of this, I have made some compromises to ship this code in a reasonable timeframe, without completely refactoring the codebase.

In particular:

As described above, the lockfile uses dependency names to encode more information than just the language code for a translation pack.
There are several places in this PR where we switch on dependency type, e.g.:

foreach (DependencyTypes::getThemeAndPluginTypes() as $type) {
    ...
}
foreach (DependencyTypes::LANGUAGES) as $dep) {
    ...
}

Switching on type is a code smell, and ideally dependency types would be able to update and install "themselves". However, the current code structure in Whippet is that we have classes for commands, not classes for dependencies, and so this would require a large refactoring of code that does no have good test coverage.

There is a dependency between language packs and other dependency types. We need to fetch version information for all other dependencies before we fetch information about language packs, so that we can query the wordpress.org APIs with version information about plugins and themes. This dependency would exist whether we refactor the code or not, and is another reason for switching on dependency type in this branch.
We are not yet using PHP 8.x everywhere, and so support for PHP 7.4 needs to persist. Because of this DependencyTypes is a class with constants, and not an enumeration.

Preparing for manual testing

If you have whippet installed system-wide, you might want to create an alias for testing this branch, i.e.:

alias whippet-test='<path-to-whippet-clone>/whippet/bin/whippet'

Most Whippet commands require a Whippetised repository with some default files and directories. You might want to create some structures in /tmp for testing.

To run whippet deps update you need a minimum set of files and directories:

A valid whippet.json file
.gitignore
config/
wp-content/
wp-content/plugins/

It's a good idea to have at least one plugin with language packs, maybe classic-editor or akismet.

To run whippet deploy you need a slightly different directory structure:

<path>/shared/uploads
<path>/shared/wp-config.php

The wp-config.php file should look like this:

<?php
define('DB_NAME', 'placeholder');
define('DB_PASSWORD', 'placeholder');
define('DB_HOST', 'placeholder');

Then you can create a new whippet app with whippet generate app, move to the whippet-app directory and run whippet deps update to install the dependencies. whippet deploy ../ can be run from within whippet-app.

More information can be found in the documentation.

Testing

Run ./script/test
Test that this branch works as expected without any language packs. Dependency commands are: describe, validate, install, update. You might also want to check deploy and release with and without language packs.
Add some languages to the JSON, for example:

    "languages": [
        {"name": "en_GB"},
        {"name": "ja"}
    ]

Run whippet deps update to populate the lockfile and install all dependencies
Run whippet deps validate to check that the new lockfile is valid
Run whippet deps install (should be idempotent to update)
Check the Git ignore file, the Whippet lockfile and wp-content/languages. There may also be a wp-content/languages/plugins directory.
You may want to remove the wp-content/languages directory, and install the same things via wp-cli to check the install, e.g.:

wp language core install
wp language plugin classic-editor ja
wp language theme install twentytwenty ja

Remove the wp-content/languages directory and run whippet deps update languages/ja or similar to check that a single language pack can be manually updated. Expected behaviour is that the language pack will be installed for core and relevant themes and plugins.
Check that adding an invalid language code to whippet.json returns an error message on whippet deps update.

Includes tests (if new features are introduced)
Commit message includes link to the ticket/issue
Changelog updated
PR open against dxw's Homebrew Tap to point the Whippet formula to the new version number

src/Dependencies/Updater.php

RobjS

This approach feels right to me 👍 I've encountered a few bugs for various configs not currently accounted for.

Separate testing issues comment to follow...

src/Dependencies/Updater.php

src/Dependencies/Installer.php

RobjS · 2023-09-22T11:34:13Z

src/Dependencies/DependencyTypes.php

+
+namespace Dxw\Whippet\Dependencies;
+
+class DependencyTypes


I like this a lot 👍

src/Dependencies/Updater.php

RobjS · 2023-09-22T14:36:09Z

Testing issues:

So it looks like the failure is happening because of the call to WhippetHelpers::whippet_init() added to Updater::construct(), and we haven't previously had any unit tests for code that calls whippet_init(), so this is the first time we've hit the problem.

My ideal solution would probably be for WhippetHelpers not to be a trait, as that seems like an anti-pattern. It means you have to either a) duplicate test code for any classes that inherit that trait and call any of its methods; or b) make use of partial mocks to mock the trait methods the inheriting classes use; neither of which are ideal. It feels like it should either be an instance that gets passed in where needed (so it's possible to mock an instance of WhippetHelpers), or a class full of static methods that can be called as and when needed (and those individual static method calls are mocked). And rather than directly setting properties on the classes that inherit the trait, it should just return values those classes can use as they wish.

But that would involve a reasonably substantial refactor.

It might be possible to stub the call to get_cwd() in WhippetHelpers::find_file() so that it returns the virtual directory instead of the actual one, using something like this: https://github.com/php-mock/php-mock-phpunit. We've used the Peridot equivalent of that in the past.

snim2 · 2023-09-22T14:40:32Z

Testing issues:

So it looks like the failure is happening because of the call to WhippetHelpers::whippet_init() added to Updater::construct(), and we haven't previously had any unit tests for code that calls whippet_init(), so this is the first time we've hit the problem.

My ideal solution would probably be for WhippetHelpers not to be a trait, as that seems like an anti-pattern. It means you have to either a) duplicate test code for any classes that inherit that trait and call any of its methods; or b) make use of partial mocks to mock the trait methods the inheriting classes use; neither of which are ideal. It feels like it should either be an instance that gets passed in where needed (so it's possible to mock an instance of WhippetHelpers), or a class full of static methods that can be called as and when needed (and those individual static method calls are mocked). And rather than directly setting properties on the classes that inherit the trait, it should just return values those classes can use as they wish.

But that would involve a reasonably substantial refactor.

It might be possible to stub the call to get_cwd() in WhippetHelpers::find_file() so that it returns the virtual directory instead of the actual one, using something like this: https://github.com/php-mock/php-mock-phpunit. We've used the Peridot equivalent of that in the past.

OK, I notice that we only really use whippet_init() in a small number of places:

❯ git grep whippet_init
src/Dependencies/Updater.php:26:                $this->whippet_init();
src/Modules/Helpers/WhippetHelpers.php:25:      public function whippet_init()
src/Modules/Plugin.php:60:              $this->whippet_init();
src/Modules/Plugin.php:263:             $this->whippet_init();
src/Modules/Release.php:18:             $this->whippet_init();
tests/Helpers.php:126:          // WhippetHelpers. I think this is because in whippet_init() we ignore
tests/Helpers.php:131:          // repo, and whippet_init() recurses the directory to find a .json file.

So, only Plugin and Release plus the code in this PR, and I do wonder whether those classes should just check whether they have a whippet.lock in the cwd and b0rk otherwise?

Refactoring work as been raised as #227

RobjS · 2023-09-22T15:01:21Z

It looks like Release could also use the application_config property that WhippetHelpers::whippet_init() sets via its call to WhippetHelpers::load_application_config():

https://github.com/dxw/whippet/blob/main/src/Modules/Helpers/WhippetHelpers.php#L42

https://github.com/dxw/whippet/blob/main/src/Modules/Helpers/WhippetHelpers.php#L50

https://github.com/dxw/whippet/blob/main/src/Modules/Release.php#L80

This resolves a deprecation warning.

We have a small number of extant tests with no assertions. These raise a "risky test" warning when running ./script/test. This commit adds some sensible assertions to the tests to avoid this warning.

Previously we hard-coded the list of dependency types wherever we needed to use them. In order to manage language packs within Whippet, we now need to add to the list of dependency types. So, rather than have them scattered over the code, we now have them in one place. Once we have dropped PHP 7.4 support, we should really use an enumeration here. Fixes #70

Note that at this point, it isn't clear how we will need to manage language packs, and it may be that we do not pin languages to versions or hashes in the lockfile. For this reason, the relevant test data is mostly blank, with the expectation that this will change (or more tests will be added) in later commits.

In order to manage language files, we will need a semver version for each plugin and theme in the whippet.json file. These versions are obtainable via the Describer class, but currently the relevant method just prints out version information. This commit splits that code into two parts, one to fetch and return the information, and one to print it, so that we can reuse the code elsewhere. Since both methods are already tested by the existing tests, this commit doesn't add any more.

Language packs in WordPress are available for WP Core, plugins and themes. In a manual install, they can be added with: wp language core install wp language plugin classic-editor ja wp language theme install twentytwenty ja and the wp cli command will work out the required versions of the language packs, based on already installed versions of the core, plugins and themes. The language packs themselves are downloaded a .zip files from wordpress.org. In this commit, we replicate this behaviour, by: * Looking up the versions of language packs that are required for each langauge or theme, based on the APIs documented here: https://codex.wordpress.org/WordPress.org_API#Translations * We add the locations of the relevant .zip files to the 'src' key/value pair in the whippet.lock file with the revision of the dependency (core/plugin/theme) that the language pack relates to. Note that language packs are different to plugins and themes in a number of ways. Relevant to the Updater, is the fact that language packs relate to other dependencies. So, the 'name' value for each one is either: * Just the language code (e.g. 'en_US') if the related dependency is core. * '<language>/<type>/<slug>' otherwise, e.g. 'ja/plugins/classic-editor' or 'ja/themes/twentytwenty'.

This commit adds the ability to install language packs into an existing WordPress application. Languages live in 'wp-content/languages/', they are not unzipped into separate directories. Similarly, lanaguage packs for plugins go in 'wp-content/languages/plugins' and for themes, 'wp-content/languages/themes'. Again, these do not have one directory per theme or plugin, they all go in the same directory. This behaviour can be verified by running a local WordPress app and using wp cli to manually install language packs, e.g.: wp language core install wp language plugin classic-editor ja wp language theme install twentytwenty ja

Language packs are slightly different to other dependencies because a single language pack may have separate sources for WP core, any plugins for which the language is available and any themes. We expect that if a language code is in the .json file, it should have a related language pack for WP core as a minimum, but there may be any number more entries in the lockfile for that language code.

This commit ensures that `whippet deps describe` shows information about language packs. Note that the version numbers shown are the versions of the related dependencies, so for example output like: "languages": { "ja": "6.3.1", "ja/plugins/classic-editor": "1.6.3", } is showing the versions of WordPress Core, and the Classic Editor plugin. Versions of the translations themselves are not when installing language packs, but translations for WP Core, plugins and themes need to be consistent with the the versions of those dependencies that Whippet is installing. Therefore, we don't need to store version informations for language packs themselves.

RobjS · 2024-01-02T14:35:52Z

Initial testing of Whippet behaviour without language packs all seems as expected:

script/test passes
deps describe, deps update, deps install and deps validate all behave as expected.

However, when I run whippet deps update on the following whippet.json:

{
    "src": {
        "plugins": "git@github.com:dxw-wordpress-plugins/"
    },
    "plugins": [
        {"name": "akismet"},
        {"name": "advanced-custom-fields-pro"}
    ],
    "languages": [
        {"name": "en_GB"},
        {"name": "ja"}
    ]
}

With the following config/application.json:

{
    "wordpress": {
        "repository": "git@git.govpress.com:wordpress/snapshot",
        "revision": "v6"
    }
}

The results for the core language packs in whippet.lock doesn't look correct:

{
    "hash": "8239665c75577c36f286f67ef816c7f087fbb639",
    "plugins": [
        {
            "name": "akismet",
            "src": "git@github.com:dxw-wordpress-plugins/akismet",
            "revision": "aec9791a510b77a6b8b5fecc62df5efbd71faa49"
        },
        {
            "name": "advanced-custom-fields-pro",
            "src": "git@github.com:dxw-wordpress-plugins/advanced-custom-fields-pro",
            "revision": "493a3a0b3ea38129bed730776e27cbd72da21bba"
        }
    ],
    "languages": [
        {
            "name": "en_GB",
            "src": "https://downloads.wordpress.org/translation/core/5.9.8/en_GB.zip",
            "revision": "5.9.8"
        },
        {
            "name": "en_GB/plugins/akismet",
            "src": "https://downloads.wordpress.org/translation/plugin/akismet/5.3/en_GB.zip",
            "revision": "5.3"
        },
        {
            "name": "ja",
            "src": "https://downloads.wordpress.org/translation/core/5.9.8/ja.zip",
            "revision": "5.9.8"
        },
        {
            "name": "ja/plugins/akismet",
            "src": "https://downloads.wordpress.org/translation/plugin/akismet/5.3/ja.zip",
            "revision": "5.3"
        }
    ]
}

The WordPress snapshot v6 tag is pointing at the same commit as tag v6.4.2, and according to https://api.wordpress.org/translations/core/1.0/?version=6.4.2, the core language pack zips for that version would be at https://downloads.wordpress.org/translation/core/6.4.2/en_GB.zip and https://downloads.wordpress.org/translation/core/6.4.2/ja.zip.

I think what's happening is that WhippetHelpers::get_wordpress_core_version() is reading the v6 tag from application.json, and then pinging https://api.wordpress.org/translations/core/1.0/?version=6, which returns the URLs listed in whippet.lock (i.e. the 5.9.8 versions). So probably what get_wordpress_core_version() needs to do is check if the version listed in application.json is a full version in x.y.z format, and if not, ping the git repo that's being used for WordPress snapshot and get the full version number that relates to that major version tag?

In repos we have a config file that determines the version of WordPress Core that needs to be deployed to a live site. This can be a full semver version (e.g. 1.2.3) or a "version" that is actually a moveable tag related to a major release (e.g. v6). Previously, we did not check for this edge case, which means that in some cases, we did not return a full semver version and this caused a bug in updating language packs whereby old language packs would be written into repos and lockfiles. Now, we remove any leading 'v' characters and return a fully qualified semver version (e.g. 1.2.3). We make an assumption that the list of available core versions (e.g. https://api.wordpress.org/translations/core/1.0/?version=6) is ordered by most recent release first.

oops

snim2 · 2025-03-04T09:36:40Z

PR #261 merged in the first few repo chores and small fixes/refactorings from this PR. I'm going to close this now because there's a lot of comment churn here which suggests that I haven't tested these changes well enough. Also, this needs a significant refactoring, because I've made an assumption about where the plugin/theme packs can be written to, which doesn't necessarily hold.

If this work comes back, it would be easier to merge a series of smaller PRs than to try and fix this larger one.

snim2 commented Sep 22, 2023

View reviewed changes

src/Dependencies/Updater.php Outdated Show resolved Hide resolved

RobjS reviewed Sep 22, 2023

View reviewed changes

snim2 mentioned this pull request Sep 26, 2023

Refactor WhippetHelpers #227

Open

snim2 added 7 commits November 26, 2023 23:43

Migrate phpunit config to current schema

9ec05bf

This resolves a deprecation warning.

Add assertions to tests that do not have them

60e74aa

We have a small number of extant tests with no assertions. These raise a "risky test" warning when running ./script/test. This commit adds some sensible assertions to the tests to avoid this warning.

FIX off-by-one error when gitignore file is empty

c2137a7

snim2 force-pushed the feature/manage-language-files branch from 2949ad0 to d87eca5 Compare November 26, 2023 23:50

snim2 marked this pull request as ready for review November 26, 2023 23:50

snim2 added 6 commits November 26, 2023 23:53

Document management of language packs

849264b

Codeblocks have lanaguage names

ae02c59

Update Changelog for language types

74f9ec0

snim2 force-pushed the feature/manage-language-files branch from d87eca5 to 74f9ec0 Compare November 26, 2023 23:54

Use PHP7.4 compliant syntax

0e9e619

snim2 force-pushed the feature/manage-language-files branch from 1ef747e to af11347 Compare January 8, 2024 08:01

snim2 force-pushed the feature/manage-language-files branch from af11347 to 9d91b4d Compare January 8, 2024 08:10

snim2 requested a review from RobjS January 8, 2024 09:05

Remove debug code

58e6f6f

oops

snim2 marked this pull request as draft January 10, 2024 14:20

snim2 mentioned this pull request Mar 4, 2025

Chore/small fixes #261

Merged

snim2 closed this Mar 4, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Manage language packs for all dependency types #224

Manage language packs for all dependency types #224

snim2 commented Sep 22, 2023 •

edited

Loading

RobjS left a comment

RobjS Sep 22, 2023

RobjS commented Sep 22, 2023

snim2 commented Sep 22, 2023 •

edited

Loading

RobjS commented Sep 22, 2023 •

edited

Loading

RobjS commented Jan 2, 2024

snim2 commented Mar 4, 2025

Manage language packs for all dependency types #224

Manage language packs for all dependency types #224

Conversation

snim2 commented Sep 22, 2023 • edited Loading

Summary

About WordPress language packs

The whippet.json and whippet.lock files

About this PR

Preparing for manual testing

Testing

RobjS left a comment

Choose a reason for hiding this comment

RobjS Sep 22, 2023

Choose a reason for hiding this comment

RobjS commented Sep 22, 2023

snim2 commented Sep 22, 2023 • edited Loading

RobjS commented Sep 22, 2023 • edited Loading

RobjS commented Jan 2, 2024

snim2 commented Mar 4, 2025

snim2 commented Sep 22, 2023 •

edited

Loading

The `whippet.json` and `whippet.lock` files

snim2 commented Sep 22, 2023 •

edited

Loading

RobjS commented Sep 22, 2023 •

edited

Loading