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

Package naming in 2.0 and the asset ecosystem #8749

Closed
cebe opened this issue Apr 9, 2020 · 6 comments · Fixed by #8767
Closed

Package naming in 2.0 and the asset ecosystem #8749

cebe opened this issue Apr 9, 2020 · 6 comments · Fixed by #8767
Milestone

Comments

@cebe
Copy link
Contributor

cebe commented Apr 9, 2020

Hi, I wanted to make you aware of an impact of the package naming restriction in 2.0, which is going to affect users of https://asset-packagist.org/ and composer-asset-plugin. This is not about whether one should use these (I know your opinion on this @Seldaek 😃) but the fact that upgrading to Composer 2.0 becomes really hard (currently impossible) for people on this ecosystem.

The issue

Composer 2.0 will restrict the use of - (dashes). When converting npm package names which include a vendor name (vendor/package) we can not keep the / because having multiple / in a package name has obvious issues (as discussed in #7975). Currently the / is converted to -- (see docs). Double dashes however are not valid anymore in Composer 2.0.

How to solve this issue?

There are two ways to solve this:

  1. The easiest solution: allow multiple dashes in package names.
  2. Come up with a different replacement when converting package names in composer-asset-plugin.
    Given that we find a replacement that matches the naming restrictions and does not cause other problems (which is already quite hard), this also means a lot of upgrade work as people need to rename all their package requirements.
    We also we need a breaking change in the repository of asset-packagist as well as a new major version of composer-asset-plugin, which is incompatible with the current version, meaning a composer.json file will work either with version 1 or 2 but never with both. The fact that the plugin needs to be installed globally will make it very hard to work on two projects with different state.

Allowing multiple dashes in package names is not a huge problem for composer and does not introduce any overhead as far as I see. Compared to the amount of work needed to make the composer-asset-plugin/asset-packagist ecosystem compatible with Composer 2.0.

Thank you!

Btw, docs should be adjusted:

https://github.com/composer/composer/blob/master/doc/04-schema.md#name

(I can help with that once this issue is solved)

cebe added a commit to cebe/composer that referenced this issue Apr 9, 2020
Just to have this request complete, I'd like to show how small the change is to make everyone happy :)

fixes composer#8749
@Seldaek Seldaek added this to the 2.0-core milestone Apr 9, 2020
cebe added a commit to cebe/composer that referenced this issue Apr 10, 2020
@cebe
Copy link
Contributor Author

cebe commented Apr 11, 2020

should the regex be added to the json schema?

"name": {
"type": "string",
"description": "Package name, including 'vendor-name/' prefix."
},

@Seldaek
Copy link
Member

Seldaek commented Apr 12, 2020

Yup that'd probably make sense for 2.0, it might make it a hard fail by default as well and allow us to remove the rest of the validation code, not sure. But anyway should be a separate PR afterwards I guess.

@Seldaek Seldaek reopened this Apr 13, 2020
@Seldaek
Copy link
Member

Seldaek commented Apr 13, 2020

Keeping open as the schema change should probably still happen in master, and we should also check whether it errors correctly when a require or name is defined with an invalid package name.

@cebe
Copy link
Contributor Author

cebe commented Apr 13, 2020

While checking the docs about naming, I also found the following thing:

edited, made a separate issue: #8778

@cebe
Copy link
Contributor Author

cebe commented Apr 16, 2020

Thank you, @Seldaek!

@francoispluchino
Copy link
Contributor

@Seldaek Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants