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

Add support for os-families-exclude in extensions composer.json #116

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alexandre-daubois
Copy link
Contributor

@alexandre-daubois alexandre-daubois commented Nov 20, 2024

Fix #106

Allows to specify OS families which the extension is incompatible with:

{
    "name": "myvendor/myext",
    "php-ext": {
        "extension-name": "myext",
        "configure-options": [
            // ...
        ],
        "os-families-exclude": ['Windows', 'Darwin']
    }
}

When the syntax is validated, I'll create a PR in Composer to allow the new key in the Package::$phpExt property.

Related to ThePHPF/pie-design#29

@alexandre-daubois alexandre-daubois changed the title Add support for os-families in extensions composer.json Add support for os-families-exclude in extensions composer.json Nov 20, 2024
@alexandre-daubois alexandre-daubois force-pushed the os-composer-json branch 2 times, most recently from a303ebf to 71003c8 Compare November 20, 2024 11:59
src/Command/CommandHelper.php Outdated Show resolved Hide resolved
return;
}

if (in_array(PHP_OS_FAMILY, $package->excludedOsFamilies, true)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This must use the TargetPlatform; since users of PIE can install a package for a different version of PHP than the one running PIE

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I thought first. Does it mean that user can even build and install extensions for a different OS than the one running Pie with a cross compilation system?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In practice, at the moment, no. Theoretically, it would be possible to change various command invocations and file operations to run against a remote PHP install. But, that hasn't been discussed though as whether it's something we even want to do.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right thanks, I'll do the update 👍

/** @param list<ConfigureOption> $configureOptions */
/**
* @param list<ConfigureOption> $configureOptions
* @param list<string> $excludedOsFamilies
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be a list<OsFamily> and create a new enum for this with the allowed values:

  • Windows
  • BSD
  • Darwin
  • Solaris
  • Linux
  • Unknown

ref: https://www.php.net/manual/en/reserved.constants.php#constant.php-os-family

@asgrim asgrim added the enhancement New feature or request label Nov 20, 2024
@alexandre-daubois alexandre-daubois force-pushed the os-composer-json branch 4 times, most recently from 51387f8 to 0c2998a Compare November 21, 2024 14:42
@alexandre-daubois
Copy link
Contributor Author

Revamped the whole PR to address your comments.

  • Things are done in ResolveDependencyWithComposer instead of CommandHelper
  • An enum holding OS families is added
  • It is added to the TargetPlatform
  • PhpBinaryPath is used to determine the OS family, just like the OS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to require an specific OS
2 participants