paketo-buildpacks / composer-install

Apache License 2.0
1 stars 5 forks source link

composer check-platform-reqs behavior #50

Open till opened 2 years ago

till commented 2 years ago

Expected Behavior

  1. There is a way to disable this completely. (see https://github.com/paketo-buildpacks/composer/issues/27#issuecomment-1235314530)
  2. A composer install already checks platform requirements (unless you tell it not to). So if composer check-platform-reqs is used to generated an ini file to load extensions, it has to happen before.

Current Behavior

composer install runs successfully, but then composer check-platform-reqs fails.

Possible Solution

A new environment variable, $BP_COMPOSER_CHECK_PLATFORM_REQS=true which can be disabled (false).

Steps to Reproduce

Outlined here: https://github.com/paketo-buildpacks/composer/issues/27#issuecomment-1235314530

Motivations

See #46

effulgentsia commented 1 year ago

A composer install already checks platform requirements (unless you tell it not to). So if composer check-platform-reqs is used to generated an ini file to load extensions, it has to happen before.

Related to this, the fact that runComposerInstall currently runs before runCheckPlatformReqs means that if I have the following composer.json file (example shows gd, but could use any extension as an example):

{
    "require": {
        "ext-gd": "*"
    }
}

and a matching valid lock file (e.g., by running composer update), then when I run:

pack build --builder="paketobuildpacks/builder-jammy-full" --env BP_PHP_SERVER="httpd" my-test --clear-cache

Then the build fails, because runComposerInstall() fails due to the platform not yet having the gd extension before runCheckPlatformReqs() adds it.

This prevents Drupal from installing unless I add --env BP_COMPOSER_INSTALL_OPTIONS="--ignore-platform-req=ext-gd --ignore-platform-req=ext-pdo" to the pack build command.

Would it be helpful to open that as a separate issue, or is it essentially the same issue as this already existing one?

till commented 1 year ago

@effulgentsia I can't say, maybe @sophiewigmore wants to weigh in. For me it seems related.

If you have the bandwidth, can you push a test case that fails? I know that would help. Not entirely sure which repository though. Maybe the family pack?

sophiewigmore commented 1 year ago

Hey @effulgentsia, your issue definitely seems related and is fine here. It's helpful context.

Running composer check-platform-reqs before composer install seems reasonable. I have to admit I'm not sure of the historical context as to why it is currently being done in the opposite order. We'd need to a bit of investigation into that to understand if there would be any side effects on the behaviour we already support before we make that change. I haven't had a lot of time to look into this issue yet. As @till mentioned, a failing test case would be a great starting place :)

sophiewigmore commented 4 months ago

@effulgentsia @till better late than never I hope - but we just ran into this when we upgraded to Composer 2.7.7, because it completely fails (instead of just warning) when it can't find extensions. We've moved the platform req check BEFORE composer install, which I believe solves half of this issue. Check out #332 for more info. We haven't added the second part of this issue though, for opting out of that check