Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Major update v5.0 #25

Merged
merged 49 commits into from
Oct 26, 2020
Merged

Major update v5.0 #25

merged 49 commits into from
Oct 26, 2020

Conversation

ffraenz
Copy link
Owner

@ffraenz ffraenz commented Jun 12, 2020

Checklist:

@ffraenz ffraenz self-assigned this Jun 12, 2020
.travis.yml Outdated Show resolved Hide resolved
composer.json Outdated Show resolved Hide resolved
mcaskill added a commit to mcaskill/composer-wp-pro-plugins that referenced this pull request Sep 7, 2020
@mcaskill
Copy link
Contributor

mcaskill commented Sep 7, 2020

Hi @ffraenz

I was working on upgrading junaidbhura/composer-wp-pro-plugins to support Composer v2 and this pull request was quite helpful to understand the internal changes made to Composer.

I believe I've reached a similar roadblock to you, based on tests of both plugins: in Composer v2, all packages get downloaded first then prepared/installed/updated/uninstalled/cleaned-up. This means changing the package dist URL in PRE_PACKAGE_* and PRE_FILE_DOWNLOAD events is no longer possible.

I bring this up because I'm curious to see what other people have been planning to do about this predicament. I'm under the impression that any Composer plugin that dynamically alters a package's dist URL (and by extension the cache key) will need a custom InstallerInterface or DownloaderInterface to override Composer's; I have not had the chance to test PRE_POOL_CREATE.

I'm available to help out with this plugin.

Thanks,

/cc @junaidbhura

@ffraenz
Copy link
Owner Author

ffraenz commented Sep 7, 2020

Hi @mcaskill, indeed I hit a roadblock when I found out that the remote filesystem could no longer be exchanged in Composer 2 to replace the dist URL. I wrote to one of the Composer maintainers and worked out a set of changes to make an interface available to us. You can now simply call setProcessedUrl on PreFileDownloadEvent to alter the dist URL before downloading a package. Please have a look at the pull request composer/composer#8975.

I still need to incorporate the changes into this repository to finalize v5. I'm always open to suggestions and/or contributions to this plugin, so feel free to join if you're up to it!

@mcaskill
Copy link
Contributor

mcaskill commented Sep 7, 2020

You can now simply call setProcessedUrl on PreFileDownloadEvent to alter the dist URL before downloading a package.

I had not seen the that pull request at the time but I have implemented setProcessedUrl() like you did. The downloading of the ZIP file is, thanks to you, resolved and functional on my end.

The roadblock I'm at is the cache key in FileDownloader::download(). It is hashed before PRE_FILE_DOWNLOAD and PRE_PACKAGE_*, the URI's placeholders on the dist URL are replaced too late.

mcaskill added a commit to mcaskill/composer-wp-pro-plugins that referenced this pull request Sep 9, 2020
@ffraenz ffraenz linked an issue Sep 10, 2020 that may be closed by this pull request
@ffraenz
Copy link
Owner Author

ffraenz commented Sep 10, 2020

As far as I know the caching strategy does not change much moving forward to Composer 2. In this project I make sure a version number is always included in the dist URL (and in composer.lock) to bust caching when the version changes. Re-downloading a package due to changed env vars is a use case I do not plan to support as it kinda works against the package manager core concept. As Composer 2 is not released yet, there is still a chance to integrate a hook, if you think you need one.

mcaskill added a commit to mcaskill/composer-wp-pro-plugins that referenced this pull request Sep 11, 2020
src/PrivateComposerInstaller/Plugin.php Outdated Show resolved Hide resolved
test/PrivateComposerInstaller/PluginTest.php Outdated Show resolved Hide resolved
test/PrivateComposerInstaller/RemoteFilesystemTest.php Outdated Show resolved Hide resolved
@mcaskill
Copy link
Contributor

mcaskill commented Sep 12, 2020

Hi @ffraenz,

I'm sorry, I have been poorly explaining the issue I'm encountering with both your package and the one I'm working on.

As far as I know the caching strategy does not change much moving forward to Composer 2. […]

Yes, the caching strategy hasn't changed. What has changed is the processing strategy.

Your unit tests are only testing what your plugin can do with a package.
They are not testing what Composer is doing with your plugin.

Example:

1. Package
ACME_KEY=foobar
{
    "type": "package",
    "package": {
        "name": "acme/anvil",
        "version": "1.0.0",
        "dist": {
            "type": "zip",
            "url": "https://acme.com/packages/anvil?key={%ACME_KEY}&version={%VERSION}"
        },
        "require": {
            "ffraenz/private-composer-installer": "^5.0"
        }
    }
}

In Composer 1.0:

Composer resolves dependencies, iterates packages while dispatching PRE_PACKAGE_* before PRE_FILE_DOWNLOAD, then finally writes to the lockfile.

During the PRE_PACKAGE_* events, you inject the package's version into the URL. This filtered dist URL is used in the lockfile and serves as the cache key of FileDownloader and the processed URL of PreFileDownloadEvent.

During the PRE_FILE_DOWNLOAD event, you replace any other placeholders in the URL. This filtered processed URL is the target download.

2. Breakdown
  1. Dist URL: https://acme.com/packages/anvil?key={%ACME_KEY}&version={%VERSION}
  2. Resolves packages
  3. Dispatches PRE_PACKAGE_*
  4. Lockfile URL: https://acme.com/packages/anvil?key={%ACME_KEY}&version=1.0.0
  5. Cache Key: c49a8ac6c2b38148b07bded53fe49f1781fec37d
  6. Dispatches PRE_FILE_DOWNLOAD
  7. Download URL: https://acme.com/packages/anvil?key=foobar&version=1.0.0
  8. Cache Path: acme/anvil/c49a8ac6c2b38148b07bded53fe49f1781fec37d.zip
  9. Locks composer.lock

In Composer 2.0:

Composer resolves dependencies, writes to the lockfile, downloads the packages while dispatching PRE_FILE_DOWNLOAD, then iterates packages dispatching PRE_PACKAGE_*.

As a result, the package version is missing from the lockfile, the cache key, and the processed URL.

3. Breakdown
  1. Dist URL: https://acme.com/packages/anvil?key={%ACME_KEY}&version={%VERSION}
  2. Resolves packages
  3. Lockfile URL: https://acme.com/packages/anvil?key={%ACME_KEY}&version={%VERSION}
  4. Locks composer.lock
  5. Cache Key: ee70db77098cc143c9d23d7a1858573f1b03b960
  6. Dispatches PRE_FILE_DOWNLOAD
  7. Download URL: https://acme.com/packages/anvil?key=foobar&version={%VERSION}
  8. Cache Path: acme/anvil/ee70db77098cc143c9d23d7a1858573f1b03b960.zip
  9. Dispatches PRE_PACKAGE_*

As of d91f7fa, your plugin throws MissingEnvException at step 7 because it can't find VERSION among the environment variables.

Even if we ensure the version is injected at this step…

4. Snippet
public function handlePreFileDownload(PreFileDownloadEvent $event): void
{
    $processedUrl = $event->getProcessedUrl();

    if ($event->getType() === 'package') {
        $package = $event->getContext();
        if ($package instanceof PackageInterface) {
            $filteredUrl = $this->filterDistUrl($processedUrl, $package);
            if ($filteredUrl !== $processedUrl) {
                $processedUrl = $filteredUrl;
                $package->setDistUrl($filteredUrl);
            }
        }
    }

    $filteredProcessedUrl = $this->filterProcessedUrl($processedUrl);

…the lockfile URL and cache key are already set.

At this point, on my end, Composer fails to unzip the archive and I end up with an empty folder at vendor/acme/anvil.

This is the roadblock I'm at.

I think symfony/flex might have some practical ideas on how to move forward.


ffraenz and others added 3 commits September 14, 2020 13:07
…the plugin API

Co-authored-by: Chauncey McAskill <chauncey@mcaskill.ca>
Co-authored-by: Viktor Szépe <viktor@szepe.net>
Co-authored-by: Chauncey McAskill <chauncey@mcaskill.ca>
@ffraenz
Copy link
Owner Author

ffraenz commented Sep 14, 2020

Hi @mcaskill, I completely missed the change in the Composer 2 workflow that renders the current strategy completely useless. Thank you for taking the time to break it down in a very detailed way. I guess I need to go back to the drawing board to see what we can do about it.

@mcaskill
Copy link
Contributor

This recent issue is tangentially related with our situation: composer/composer#9209

@Seldaek
Copy link

Seldaek commented Sep 15, 2020

@mcaskill please report an issue on composer about the cache key issue, that definitely seems like something we'd want to fix if possible.

@szepeviktor
Copy link
Contributor

@ffraenz coverallsapp/github-action does not support PHP 😿

@szepeviktor
Copy link
Contributor

Recently this PR got a green way php-coveralls/php-coveralls#296

@alexkerber
Copy link

Any updates with this? Pretty much stuck with
The "ffraenz/private-composer-installer" plugin was skipped because it requires a Plugin API version ("^1.0.0") that does not match your Composer installation ("2.0.0"). You may need to run composer update with the "--no- plugins" option. Installing dependencies from lock file (including require- dev) Verifying lock file contents can be installed on current platform. Your lock file does not contain a compatible set of packages. Please run composer update.

@szepeviktor
Copy link
Contributor

  • name: "Merge intermediate results and finalize Coveralls report"

@ffraenz Actually you can use that GitHub action in the final step :)

@ffraenz
Copy link
Owner Author

ffraenz commented Oct 26, 2020

@alexkerber A release supporting Composer 2 will be available shortly.

@alexkerber
Copy link

@alexkerber A release supporting Composer 2 will be available shortly.

We are stuck with several projects because of this issue :)

@szepeviktor
Copy link
Contributor

kép

Unit tests somehow did not reach this file.

@ffraenz ffraenz merged commit ced8193 into dev Oct 26, 2020
@mcaskill
Copy link
Contributor

mcaskill commented Oct 26, 2020

Congratulations

@MikeiLL
Copy link

MikeiLL commented Jan 22, 2021

Any updates with this? Pretty much stuck with
The "ffraenz/private-composer-installer" plugin was skipped because it requires a Plugin API version ("^1.0.0") that does not match your Composer installation ("2.0.0"). etc...

Same roadblock here. For the time being I'm using


    {
    "type": "package",
    "package": {
      "name": "junaidbhura/advanced-custom-fields-pro",
      "version": "5.9.1",
      "type": "wordpress-plugin",
      "dist": {
        "type": "zip",
        "url": "https://www.advancedcustomfields.com"
      },
      "require": {
          "junaidbhura/composer-wp-pro-plugins": "*"
      }
    }
  }
  ],
  "require": {
    "junaidbhura/advanced-custom-fields-pro": "*",

As seen here.

@szepeviktor
Copy link
Contributor

If you remove the vendor directory (or the global plugin) you should be good to go.

@MikeiLL
Copy link

MikeiLL commented Jan 22, 2021

If you remove the vendor directory (or the global plugin) you should be good to go.

I had tried removing just vendor/composer and vendor/ffraenz, which seemed not to work.

Would I be using ffraenz/private-composer-installer version 5 and vlucas/phpdotenv v5?

Also what do you mean, please by or the global plugin?

@szepeviktor
Copy link
Contributor

Would I be using ffraenz/private-composer-installer version 5 and vlucas/phpdotenv v5?

https://github.com/ffraenz/private-composer-installer#dependencies

Also what do you mean, please by or the global plugin?

You may have installed this plugin globally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants