phar-io/manifest

v2.0.x does not support all Composer type definition

Closed this issue · 11 comments

Hello,

Latest version 2.0.3 does not support yet all Composer Type definition.

Example: we cannot build a manifest.xml file that reflect the project situation of a composer-plugin .

PS: of course we can build it, but the type is identified by custom.

We cannot use our own ManifestSerializer version.
Even if class is not declared as final, we cannot override the addContains() method that support all types supported (application, library, extension, ... custom), because it's private. At least a protected visibility will greatly help to overrride the default ManifestSerializer.

I'm working on version 3.0, addressing issues like that.

I'm reviewing this issue to decide what actually needs implementing.

Turns out, you're reporting two different problems under one ticket.

Composer Types

Latest version 2.0.3 does not support yet all Composer Type definition.

That is technically correct but not actually a problem, as there is no requirement to map all types:

Composer Phar Manifest
library Type::library()
project Type::application (1)
metapackage N/A (2)
composer-plugin Type::extension(new Application('composer'), ...)

(1) One could argue here that we're missing a counterpart for project types. But to me, the term project has no semantic value. So it cannot have a useful 1:1 map. From a PHAR's perspective, its content is either a library, an extension or an application. I don't believe anyone would just generically build a phar as an alternative to a zip or tar.xz archive and then would want to have a manifest included that the just have to delete when "unpharing". Even if someone would say deploy a website as a phar, it would rather resemble an application type.

2 A metapackage is empty, according to composer's documentation, and thus cannot be build as a phar as there are no files to be bundled.

I'm open to add additional types, if needed. One thing that is like to be included in 3.0 is a stub type, to allow building of phars only useful for IDE autocomplete. This will allow for instance PHPUnit to ship an executable application type with all dependencies and a 2nd phar sub to support autocomplete of public API components of PHPUnit. The type has already been added to the schema, but there is no code to handle it yet.

I'm not convinced we do need a generic fallback like custom, or unknown. But that might be subject to discuss.


Serializer

You actually found a bug regarding the Serializer: The default mapping to custom is invalid according to the schema and the Mapper will not be able to process it. I'll have to change that to throw an exception.

I'm also not convinced that we need a means to extend the Serializer by third parties at this time: The Serializer maps the Manifest Document into XML according to the schema. Given there is a 1:1 relation between the document and its xml representation without any means for custom, third party code to be used here anywhere, I can't see a use case for allowing this.

So actually, all the classes should be final. I'm considering to do just that for 3.0.

Composer Types

I heard and may understand your point of view. But as it was not supposed to be really clear, I suggest you to add a documentation (with examples) to your package

Serializer

If you don't want that third parties extends the Serializer, I agree with you that you should use the final keyword on class.

About composer-plugin, I need your review to be sure to understand what we are supposed to be !

I'm actually working on a new composer plugin project that is declared like this

{
    "name": "bartlett/manifests",
    "description": "Composer plugin that creates manifests for a PHP Archive (PHAR)",
    "type": "composer-plugin",
    "license": "MIT",
.... <more > ...
}

No version is specified on the composer.json.

With such implementation on my PhiveManifest component :

    use PharIo\Manifest\ApplicationName;
    use PharIo\Manifest\Type;
    use PharIo\Version\AnyVersionConstraint;


    $applicationName = new ApplicationName($rootComponent->name);

    switch (\strtolower($rootComponent->type)) {
        case 'application':
            $type = Type::application();
            break;
        case 'composer-plugin':
            $type = Type::extension($applicationName, new AnyVersionConstraint());
            break;
        case 'library':
        default:
            $type = Type::library();
    }

I got such results :

<?xml version="1.0" encoding="UTF-8"?>
<phar xmlns="https://phar.io/xml/manifest/1.0">
    <contains name="bartlett/manifests" version="0.0.0-dev" type="extension">
        <extension for="bartlett/manifests" compatible="*"/>
    </contains>
    <copyright>
        <license type="MIT" url="https://spdx.org/licenses/MIT.html#licenseText"/>
    </copyright>
    <requires>
        <php version="*"/>
    </requires>
    <bundles>
    </bundles>
</phar>

I think that the contains tag is quiet difficult to read/understand like that. Did I missed something (not understand what an extension for you is supposed to be ?)

Composer Types

I heard and may understand your point of view. But as it was not supposed to be really clear, I suggest you to add a documentation (with examples) to your package

Yes, this package likely needs more documentation.

Serializer

If you don't want that third parties extends the Serializer, I agree with you that you should use the final keyword on class.

I'll keep thinking about it :)

About composer-plugin, I need your review to be sure to understand what we are supposed to be !

I'm actually working on a new composer plugin project that is declared like this

{
    "name": "bartlett/manifests",
    "description": "Composer plugin that creates manifests for a PHP Archive (PHAR)",
    "type": "composer-plugin",
    "license": "MIT",
.... <more > ...
}

No version is specified on the composer.json.

With such implementation on my PhiveManifest component :

    use PharIo\Manifest\ApplicationName;
    use PharIo\Manifest\Type;
    use PharIo\Version\AnyVersionConstraint;


    $applicationName = new ApplicationName($rootComponent->name);

    switch (\strtolower($rootComponent->type)) {
        case 'application':
            $type = Type::application();
            break;
        case 'composer-plugin':
            $type = Type::extension($applicationName, new AnyVersionConstraint());
            break;
        case 'library':
        default:
            $type = Type::library();
    }

I got such results :

<?xml version="1.0" encoding="UTF-8"?>
<phar xmlns="https://phar.io/xml/manifest/1.0">
    <contains name="bartlett/manifests" version="0.0.0-dev" type="extension">
        <extension for="bartlett/manifests" compatible="*"/>
    </contains>
    <copyright>
        <license type="MIT" url="https://spdx.org/licenses/MIT.html#licenseText"/>
    </copyright>
    <requires>
        <php version="*"/>
    </requires>
    <bundles>
    </bundles>
</phar>

I think that the contains tag is quiet difficult to read/understand like that. Did I missed something (not understand what an extension for you is supposed to be ?)

It looks a bit like you're misinterpreting the purpose of these manifests. They are intended to be included into a PHAR (PHP archive) as a sort of meta data. Think of https://en.wikipedia.org/wiki/Bill_of_materials with some additional info that is useful only on a PHAR context. A PHAR, other than a git repository, is a snapshot in time. The version of your software is usually not tracked in the composer.json but via branch or git tags. A phar, being a snapshot, contains a specific version only.

So a PHAR contains some code, representing something with a name. The phar contains a specific version of that something and that something is of a specific type: A library, a (stand alone) application or an extension for "something else". In your case, it's an extension for composer. As it's quite unlikely that this "something else" could be just anything - again, yours is only for composer and not,say, for PHPUnit at the same point in time, you have to specify what specific "something else" the extension is for and what version of that "something else" it's compatible with. Semantically, the AnyVersionConstraint is a valid selector from the manifest's perspective, but I doubt that to be technically correct. Your plugin in the contained version is likely not going to work with Composer 0.1, Composer 1.0 or a possible Composer 125.5.1. So you should be more specific, e.g. by saying `^2.0' (== require 2.x.y).

The result could look like this:

...
<contains name="bartlett/manifests" version="0.0.0-dev" type="extension">
         <extension for="composer/composer" compatible="^2.0"/>
</contains>
...

So a PHAR contains some code, representing something with a name. The phar contains a specific version of that something and that something is of a specific type: A library, a (stand alone) application or an extension for "something else". In your case, it's an extension for composer. As it's quite unlikely that this "something else" could be just anything - again, yours is only for composer and not,say, for PHPUnit at the same point in time, you have to specify what specific "something else" the extension is for and what version of that "something else" it's compatible with. Semantically, the AnyVersionConstraint is a valid selector from the manifest's perspective, but I doubt that to be technically correct. Your plugin in the contained version is likely not going to work with Composer 0.1, Composer 1.0 or a possible Composer 125.5.1. So you should be more specific, e.g. by saying `^2.0' (== require 2.x.y).

The result could look like this:

...
<contains name="bartlett/manifests" version="0.0.0-dev" type="extension">
         <extension for="composer/composer" compatible="^2.0"/>
</contains>
...

Make sense, but how did you do that with API ?
PHPUnit itself provides only a manifest.xml test extension example, but no way how to build it with API.

If I said that, this is because, I don't understand how to do.
If I want to add a constraint to Composer ^2.0, I've tried with

use Composer\Plugin\PluginInterface;

use PharIo\Manifest\ApplicationName;
use PharIo\Manifest\Type;
use PharIo\Version\GreaterThanOrEqualToVersionConstraint;
use PharIo\Version\Version;


    case 'composer-plugin':
        $type = Type::extension(new ApplicationName('composer/composer'), new GreaterThanOrEqualToVersionConstraint('^2.0', new Version(PluginInterface::PLUGIN_API_VERSION)));
        break;

But for me, it has nosense !

use PharIo\Manifest\ApplicationName;
use PharIo\Manifest\AuthorCollection;
use PharIo\Manifest\BundledComponentCollection;
use PharIo\Manifest\CopyrightInformation;
use PharIo\Manifest\Extension;
use PharIo\Manifest\License;
use PharIo\Manifest\Manifest;
use PharIo\Manifest\ManifestSerializer;
use PharIo\Manifest\RequirementCollection;
use PharIo\Manifest\Url;
use PharIo\Version\Version;
use PharIo\Version\VersionConstraintParser;

$manifest = new Manifest(
    new ApplicationName('bartlett/manifests'),
    new Version('0.0.0-dev'),
    new Extension(
	    new ApplicationName('composer/composer'), (new VersionConstraintParser())->parse('^2.0')
    ),
    new CopyrightInformation(
        new AuthorCollection(),
        new License('BSD-3-Clause', new Url('https://spdx.org/licenses/BSD-3-Clause.html'))
    ),
    new RequirementCollection(),
    new BundledComponentCollection()
);

// ....

Thanks for example.
Documentation will be really appreciate to avoid to request your help !