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

Questions over Composer package naming #8

Closed
asgrim opened this issue Mar 18, 2024 · 9 comments · Fixed by #12
Closed

Questions over Composer package naming #8

asgrim opened this issue Mar 18, 2024 · 9 comments · Fixed by #12
Labels
question Further information is requested

Comments

@asgrim
Copy link
Collaborator

asgrim commented Mar 18, 2024

To record a discussion so far on Composer package naming.

Current plan: the Composer package name is something like ext-debug in composer.json. However, as per composer/packagist#1440, this would need specific support to be added to Packagist.

How are names decided currently?

as far as I understand it, the ext author decides the name, but then it has to be approved and added to pecl.php.net by hand; that would be done by requesting on the pecl mailing list, so if there was an objectionable name for some reason, it would be discussed prior

What about using vendor/ prefixes?

An advantage of using vendor/package format, is it allows multiple implementations; for example if I wanted to fork xdebug, and install my custom xdebug, I could do pie install asgrim/xdebug instead of pie install xdebug/xdebug to get my own fork.

This is an option, but we would still need a way to either derive or determine the extension name. Some options

  • derive it from the package name, e.g. asgrim/xdebug and xdebug/xdebug both have extension name of xdebug (i.e. just remove the "vendor"). Drawback; this may make an easy way to conflict with naming, both unintentionally + intentionally
  • require a name in the new php-ext section in composer.json, e.g.
    {
        "name": "asgrim/example-pie-extension",
        "type": "php-ext",
        "license": "MIT",
        "description": "Example PIE extension",
        "require": {
            "php": ">=7.1,<8.4"
        },
        "php-ext": {
            "name": "example-pie-extension",
            "priority": 74,
            "configure-options": []
        }
    }
  • use provide; e.g.:
    {
        "name": "asgrim/example-pie-extension",
        "type": "php-ext",
        "license": "MIT",
        "description": "Example PIE extension",
        "require": {
            "php": ">=7.1,<8.4"
        },
        "provide": {
            "ext-example-pie-extension": "*"
        },
        "php-ext": {
            "priority": 74,
            "configure-options": []
        }
    }
    Drawback to this: what if an extension (for some reason) decides to list multiple provide (for example, it provides a psr/message implementation as well as itself being ext-my-psr-messages); especially, if it somehow provide multiple extensions? 🤯

Questions - retroactive vendor prefixes

If we do add vendor/ prefix, how will we decide vendor prefix for existing extensions? For some, it is easy where a vendor is already used by the author(s), e.g. ext-xdebug could use xdebug/xdebug, ext-datadog-trace could use datadog/datadog-trace, etc.; however, some it might be unclear; e.g. ssh2. One could assume it should become php/ssh2 but there are implications to this (since at this point, it could conceivably be that php/ is a special vendor, and may even already be reserved by Composer/Packagist, for instance (would need to check this).

General points raised

@asgrim asgrim added the question Further information is requested label Mar 18, 2024
@Seldaek
Copy link

Seldaek commented Mar 18, 2024

The php vendor is indeed reserved, but I'm happy to let the php project use it if needed for php project packages of course.

@asgrim
Copy link
Collaborator Author

asgrim commented Mar 18, 2024

I'm personally thinking that a reasonable approach for this is:

  • Add a new optional php-ext.name key to specify the extension name
  • If the key is not specified, derive the extension name from the package name, i.e. just remove the vendor prefix (e.g. asgrim/xdebug would be xdebug)

This gives us the flexibility of having vendor namespaces, as well as reducing the attack vector @naderman has raised

@alcaeus
Copy link

alcaeus commented Mar 18, 2024

I think using vendor fixes are preferable to using the old ext-<name> package format, especially as it avoids special logic to tie the ext- prefix to a specific type in composer.json. The packagist versions of extensions can then use provide to satisfy the composer dependencies of other libraries on an ext-<name> package. All in all, the proposal sounds good to me.

@stof
Copy link
Contributor

stof commented Mar 20, 2024

Using a different name for the registered PIE extension than for the platform package used by composer for requirements in PHP packages actually introduces a new attack vector for typosquatting: packagist would not have any way to decide what is the source of truth for ext-xdebug as many php-ext packages could be providing that name or be named */xdebug (depending on the selected solution).

Note that the composer way of using a fork is not to register that fork on packagist under a new name (renaming a composer package while satisfying existing requirements is possible, but it is not a simple task, especially if you want it to be retroactive). The way to achieve this in composer is to register additional package repositories instead, with higher priority.

And changing composer so that it deals with namespaced names for platform packages would be a huge BC break for the composer ecosystem (and probably not that easy to implement in Composer as a bunch of places are currently relying on the fact that platform packages are not namespaced to identify them as they require special handling anyway due to not being installed per project by composer).

@Seldaek
Copy link

Seldaek commented Mar 20, 2024

@stof the extensions are installed by pie, and not by composer. Composer will see them as regular ext-<name> packages in the PlatformRepository.

And on the pie side namespacing reduces the potential for typo squatting as it does for the other packages. Because you can't register ext-xdepug next to ext-xdebug, you'd have to register xdepug/xdebug which well is typo prone sure but a bit less, and anyway we have systems in place to catch this.

Note that the composer way of using a fork is not to register that fork on packagist under a new name (renaming a composer package while satisfying existing requirements is possible, but it is not a simple task, especially if you want it to be retroactive). The way to achieve this in composer is to register additional package repositories instead, with higher priority.

As for that, I disagree. If you're forking for the long term maintenance then yes submitting another package name is the way to do this, and you can't use replace/provide there because it won't be resolved by composer, so the extension name has to match the original.

If you're forking for a temporary bugfix or whatever, then yeah what you said is more accurate, but these aren't forks per se, and I'm not sure how these will be best handled by pie but I suspect perhaps it's just best left to whoever patches to figure it out how to install their patched extension. This is a special case and way less common than php userland patching I'm thinking.

@asgrim
Copy link
Collaborator Author

asgrim commented Mar 21, 2024

Thank you everyone for your feedback, it is greatly appreciated. I don't think there is a perfect solution to this; but given the way people are used to using Composer, I propose a combination of all 3 originally suggested options; with the known drawback that there is a slight disconnect between the Composer package name, and the "extension name":

  • a PIE package MUST have its "top level" name key in the format vendor/foo in its composer.json
  • a PIE package SHOULD optionally specify the extension name in php-ext.extension-name
    • if the php-ext.name is not defined, or empty, the name of the extension will be extracted from the "top level" name, by removing the vendor/ prefix
  • a PIE package MAY optionally specify a provide with ext-foo

a complete example:

{
    "name": "asgrim/example-pie-extension",
    "type": "php-ext",
    "license": "MIT",
    "description": "Example PIE extension",
    "require": {
        "php": ">=7.1,<8.4"
    },
    "provide": {
        "ext-example-pie-extension": "*"
    },
    "php-ext": {
        "extension-name": "ext-example-pie-extension",
        "priority": 74,
        "configure-options": []
    }
}

The only thing is; I am not sure what (if any) benefits we have from specifying provide, but it may have some semantic benefit later down the line, hence my suggestion to keep it optional for now.

If there are no further objections to this, I will update the design documentation accordingly and produce a PR this morning.

@Seldaek
Copy link

Seldaek commented Mar 21, 2024

Yeah I am not sure either about the provide, it's probably harmless/useless. Otherwise sounds good to me 👍🏻

@naderman
Copy link

The semantics of provide in Composer however are that you can install multiple packages providing the same thing in parallel. More accurate would be replace, which implies a conflict and would better model the reality that two extensions with the same name cannot coexist?

@asgrim
Copy link
Collaborator Author

asgrim commented Mar 21, 2024

The semantics of provide in Composer however are that you can install multiple packages providing the same thing in parallel. More accurate would be replace, which implies a conflict and would better model the reality that two extensions with the same name cannot coexist?

Thank you for correcting me there; will amend the PR now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants