inpsyde / wp-translation-downloader

Composer plugin to download WordPress translations
MIT License
45 stars 3 forks source link

[Feature Request]: Support Containerized WordPress Environments #24

Closed gioamato closed 2 years ago

gioamato commented 2 years ago

Is your feature request related to a problem?

As for actual docs (if i'm not missing something) this plugin downloads WordPress core translations when a package with type wordpress-core is added in composer.json.

This is not the case when using a containerized approach to WordPress, like the official Docker image: https://hub.docker.com/_/wordpress

Of course WordPress can download it's translations in a classical/stateful environment, but in a cloud-native enviroment (stateless) the required translations should be added via build step, e.g.: https://github.com/gioamato/stateless-wordpress/tree/master/wordpress

Describe the desired solution

It could be useful if this plugin could support also these type of environments, letting the user instruct the package to download required translations.

Describe the alternatives that you have considered

It could work adding a configuration property that instructs the plugin to download the WordPress core translations.

Additional context

No response

Code of Conduct

Chrico commented 2 years ago

Good morning. ☕

Thanks for your issue. Normally the wp-translation-downloader has no restrictions to anything aside from what is defined in require in composre.json. We run it inside Containers, WSL, on different OS, even on Github Actions (which is using a Container 🙈 ).

Is there any output shown what is happening? Can you please describe your workflow? Is composer executed inside of the container our outside and everything mapped into it? :)

gioamato commented 2 years ago

If you use the WordPress container with a previous build step there is no need to require wordpress inside composer.json as the WordPress system files are already inside the container. In this case wp-translation-downloader will not download translations for it. E.g.:

Dockerfile:

FROM composer:2.2.9 as build
WORKDIR /usr/src/app
COPY composer.* ./
RUN composer install --no-dev

FROM wordpress:5.9.2-fpm-alpine
COPY --from=build /usr/src/app/wp-content /usr/src/wordpress/wp-content
RUN chown -R www-data:www-data /usr/src/wordpress

composer.json:

{
  "name": "wordpress/test",
  "repositories": [
    {
      "type":"composer",
      "url":"https://wpackagist.org",
      "only": ["wpackagist-plugin/*", "wpackagist-theme/*"]
    }
  ],
  "config": {
    "sort-packages": true,
    "allow-plugins": {
      "composer/installers": true,
      "inpsyde/wp-translation-downloader": true
    }
  },
  "require": {
    "inpsyde/wp-translation-downloader": "^2.0",
    "wpackagist-plugin/autodescription": "4.2.3"
  },
  "extra": {
    "wp-translation-downloader": {
      "languages": [
          "it_IT"
      ],
      "directory": "wp-content/languages"
    }
  }
}

In this case the it_IT translations wil be downloaded for the wpackagist-plugin/autodescription plugin but not for wordpress.

Chrico commented 2 years ago

Ah now i understand it...it is not downloaded, because it only downloads what in require is defined. wp-translation-downloader is a Composer plugin and works on the packages Composer is installing/downloading. It does not know about the enviornment or where it is installed, so i cannot install WP without having it defined in require :)

gioamato commented 2 years ago

Exactly. That's why i started talking about supporting containerized WordPress environments based on the official image.

I could add a property inside config section to instruct the package to download core translations, if it's ok for you.

gmazzap commented 2 years ago

Hi @gioamato

One reason why this plugin expects the "require" is that it needs to know the version of translations it needs to download. So an eventual configuration can't just be a boolean.

However, I think that a configuration like "force-core-translations": "5.8" 1 could make sense.

Then here: https://github.com/inpsyde/wp-translation-downloader/blob/master/src/Plugin.php#L201, the plugin could check if that configuration is set, and if so, could push to the $packages array a "virtual" WP Composer package, simulating it was required in composer.json.

     /**
     * @param PackageInterface[] $packages
     */
    public function doUpdatePackages(array $packages)
    {
        /* omissis... */

        $forcedCoreVersion = $this->pluginConfig->forcedCoreTranslations();
        if ($forcedCoreVersion) {
            $wpPackage = new \Composer\Package\Package(
                'wordpress/wordpress-core',
                $forcedCoreVersion,
                $forcedCoreVersion,
            );
            $wpPackage->setType('wordpress-core');

            $packages[] = $wpPackage;
        }

        /* omissis... */
    }

at that point, the plugin will act as usual and download core translations.

If @Chrico agrees, and @gioamato has the time to prepare a PR in that direction, I think it would be a good addition. I've found myself in the position of having to require a WP core package only to ensure translations are downloaded, even I don't need the WP files pulled by Composer.


1 Please note that version in the force-core-translations would be required to be an exact version, like 5.8 or 5.9.2 and not a requirement constraint like, 5.8.*, >=5.8, etc. And that because when a package is required via Composer, it is Composer that resolves the requirent into a version number, but in this case we would need to create a package object on the fly, and while in theory would be possible to use WP API to pull all core versions and resolve the constraint, that would too much complexity for the use case.

gioamato commented 2 years ago

Hey @gmazzap, I agree with you and your proposed solution. I will manage to reserve some time to craft this PR as soon as everyone agree.

Could you detail more the use cases where you had to require a WP core package without needing it? In my use case dependabot keeps WordPress up to date, so a manual step is required to update and match the version in force-core-translations.

gmazzap commented 2 years ago

@gioamato In some situations we have WordPress installed by "managed WP hostings" (like https://wpvip.com), and for those we ave custom installers to "marry" their approach with our Composer-based approach.

In those cases, I don't need WP to be installed by Composer, because WordPress files are given by the hosting.

What I do is to require johnpbloch/wordpress-core in project's composer.json. That package is a bare WordPress wrapper (without any installer) and so at the end WordPress files ends up in a vendor/johnpbloch/wordpress-core that will sit there unused by anyone, but makes the translation downloader happy and I get my translations installed. Considering disk space is not an issue and using cache the deployment time for a single package is below a second, I don't bother too much.

Please note that there's also a johnpbloch/wordpress package that has an installer, make sure you use the one with the -core prefix f you don't want to mess with your installation.

Chrico commented 2 years ago

Approach with force-core-translations sounds good to me. Had something similar in mind by reading this (we had this discussion also a few times new @gmazzap 😬 ).

If it's urgent someone needs to take over since i'm mostly full this week. Please ensure that tests are added as well to the behat test suite for this. :)

Chrico commented 2 years ago

After doing the 2.1 release i may have time in the upcoming weeks to add this feature as well. But i would not implement force-core-translations as configuration. 🤔 Currently i have following in my mind:

{
    "additionalPackages": [
        {
            "name": "johnpbloch/wordpress-core",
            "type": "wordpress-core"
            "version": "5.8"
        }
    ]
}

I guess this could become handy, when for example a hoster already has some pre-installed plugins/themes/features, which also need translations to allow pulling them.

gmazzap commented 2 years ago

@Chrico That could work as well, and indeed it could be useful in other situations.

I would say it sounds a bit too much to just say "I need translation for core" considering that core is always there, especially when you have to adjust the "version" config every time. So please if you follow this approach, make sure to support the * requirement for version that, by the way, is a valid Composer contraint.

Chrico commented 2 years ago

So please if you follow this approach, make sure to support the * requirement for version that, by the way, is a valid Composer contraint.

@gmazzap Wondering how we should resolve that, since we don't know the latest version of something. We could use the Composer\Semver\VersionParser() from https://github.com/composer/semver , which is part of Composer, but it will not magically find out to what ^5.9 actually resolves to 5.9.3 without calling Packagist API via Composer\Package\Version\VersionSelector::findBestCandidate(). 🤔

By looking into the WordPress API , having no version added would result into "latest":

So we could either allow a specific version like 5.9, 5.9.3 or none set, which say "use no version and return latest". 🤔

Chrico commented 2 years ago

Created a new branch for this feature: https://github.com/inpsyde/wp-translation-downloader/tree/feature-virtual-packages

The new node will be virtual-packages in configuration file which allows to either set a "fixed" version or just use the "latest" by not adding version to the package.

Read more in docs here: https://github.com/inpsyde/wp-translation-downloader/blob/feature-virtual-packages/docs/Configuraton.md#virtual-packages


CC @gmazzap ;-)

Chrico commented 2 years ago

This was released with version 2.2 https://github.com/inpsyde/wp-translation-downloader/releases/tag/2.2

CC @gioamato

gioamato commented 2 years ago

Thank you @Chrico, this approach also solves the issue of updating the translations in automated environments (like using dependabot) omitting version specification so the latest version is downloaded at build time.

Just a note: committing the wp-translation-downloader.lock leads to translations missing from production because during the github action run the translations will be locked and not downloaded.

So, just add wp-translation-downloader.lock to .gitignore in automated envs (dependabot + automerge + actions).