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

Allow multiple package versions inside of the package repository #6

Open
antonkomarev opened this issue Sep 10, 2019 · 4 comments
Open
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@antonkomarev
Copy link
Collaborator

antonkomarev commented Sep 10, 2019

The "package" key in a package repository may be set to an array to define multiple versions of a package: https://getcomposer.org/doc/05-repositories.md#package-2

Here is a payload for the test:

"repositories": [
    {
        "type": "package",
        "package": [
            {
                "name": "smarty/smarty",
                "version": "2.0.0",
                "dist": {
                    "url": "https://www.smarty.net/files/Smarty-2.0.0.zip",
                    "type": "zip"
                },
                "source": {
                    "url": "http://smarty-php.googlecode.com/svn/",
                    "type": "svn",
                    "reference": "tags/Smarty_2_0_0/distribution/"
                },
                "autoload": {
                    "classmap": ["libs/"]
                }
            },
            {
                "name": "smarty/smarty",
                "version": "3.0.0",
                "dist": {
                    "url": "https://www.smarty.net/files/Smarty-3.0.0.zip",
                    "type": "zip"
                },
                "source": {
                    "url": "http://smarty-php.googlecode.com/svn/",
                    "type": "svn",
                    "reference": "tags/Smarty_3_0_0/distribution/"
                },
                "autoload": {
                    "classmap": ["libs/"]
                }
            }
        ]
    }
],

As output I assumed to see repository with array of packages.

Personally I have never seen any project which defines repository packages in this way. Just keep in mind that these changes are breaking because getPackage returns ?ComposerJson right now, but may start to return an array of ComposerJson instances too. I don't really like it because return type becomes mixed ComposerJson|ComposerJson[]|null and will require additional checks in user application to determine what we have inside: is it array or one instance only.

As possible solutions to prevent mixed return type I think it could return array even when there is only one package instance defined. To prevent introduction of breaking changes in v1 this feature could be delayed for the v2 version.

@antonkomarev antonkomarev added the enhancement New feature or request label Sep 10, 2019
@antonkomarev antonkomarev changed the title Allow multiple package versions inside of the repository Allow multiple package versions inside of the package repository Sep 10, 2019
@antonkomarev
Copy link
Collaborator Author

antonkomarev commented Sep 10, 2019

Here is a link on section about package repository in schema specification.

"package": {
  "oneOf": [
    { "$ref": "#/definitions/inline-package" },
    {
      "type": "array",
      "items": { "$ref": "#/definitions/inline-package" }
    }
  ]
}

Specification says, it should be inline-package OR array of inline-package.

@antonkomarev
Copy link
Collaborator Author

antonkomarev commented Sep 10, 2019

And one more note about the package repository: it is the only one type of repositories which doesn't have url field. It has only 2 required fields: "required": ["type", "package"].

I think in v2 it will be good to revise Repository class implementation accordingly to the specification. Maybe it will be good to create separate class for each type of package: ComposerRepository, VcsRepository, PathRepository, ArtifactRepository, PearRepository, PackageRepository. Each of them has their own set of fields. For example I found that allow_ssl_downgrade introduced recently is applicable only to composer repositories.

@antonkomarev
Copy link
Collaborator Author

I've removed allow_ssl_downgrade flag in PR #7 because current implementation was wrong.

@MCStreetguy
Copy link
Owner

Sorry for responding this late, I really got little time at the moment.
I really like your proposal to split up the repository classes as this would give back more control of the actual structure inside, even though this would require a bit more logic during construction but this is an acceptable drawback I think.
This would be a v2 release (like you stated before) as It includes major changes to the current class structure and return types. I would add a mention for this limitation in the README file but I see no chance to fix the current behaviour in a v1 release in any way..

@MCStreetguy MCStreetguy added this to the v2.0 milestone Sep 14, 2019
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

No branches or pull requests

2 participants