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

Allowing getcomposer 2.2 channel #25

Merged
merged 4 commits into from May 26, 2023
Merged

Conversation

benbor
Copy link
Collaborator

@benbor benbor commented Apr 4, 2022

Since composer create one more channels composer/composer#10682 we have to allow it.
I think, removing static validation is safe enough, just because there are realtime validation exists. A user may specify any option, that announced on composer.phar help self-update or installer.php help

Another option is to create whitelist and to update it each time, when composer made channel changes. Today the list is here https://getcomposer.org/versions :

  • preview
  • stable
  • snapshot
  • 1
  • 2
  • 2.2

ps A dynamic validation is left as is (the validation that such option is exists on self-update or installer)

@kamazee wdyt?

@kamazee
Copy link
Owner

kamazee commented Apr 4, 2022

The reason why it was there is to abstract away from CLI flags. If a user requests a specific composer version, and composer changes CLI flags to something different than --1 or --2, an updated wrapper would still be able to solve this puzzle and continue shipping exactly what is asked in the configuration. So, if static validation goes away, I guess we can't still say that we force a major version: we just pass a flag as is. This is more forward compatible but building a backward compatibility around it might be harder. Composer seems to be pretty stable, though, so it might be a way to go.

@benbor
Copy link
Collaborator Author

benbor commented May 8, 2023

So, if static validation goes away, I guess we can't still say that we force a major version: we just pass a flag as is. This is more forward compatible but building a backward compatibility around it might be harder. Composer seems to be pretty stable, though, so it might be a way to go.

As I see we have a lot of options:

  1. BC. Add flags and proxy whole arguments\options array as is. How to solve conflicts with existing forceMajorVersion?
  2. BC. Add channel (exactly as it named in Composer language). Validate that passed value doesn't have whitespacees and pass it as --$value. Also two extra options:
    2.1. Make channel validation remotely, making https://getcomposer.org/versions call.
    2.2. Hardcode whitelist (need composer wrapper update when changed)
  3. No BC. Leave forceMajorVersion name implement it as in 2. It is exactly what almost done in this PR (except a part with validation). Disadvantage: the option name confused a bit.

Plus, in case of BC we have to update composer-wrapper major version if we follow semver (but we don't :| so... have no idea how to handle it)

@kamazee please choose or provide strict requirement as you see it should be done

@kamazee
Copy link
Owner

kamazee commented May 12, 2023

Does "BC" stand for "BC (backward compatibility) break"? :)

I see option with introducing channel and deprecating forceMajorVersion the most viable (and simple) one. It's enough to just merge this PR and make forceMajorVersion a deprecated alias of channel, right?

@benbor
Copy link
Collaborator Author

benbor commented May 15, 2023

I see option with introducing channel and deprecating forceMajorVersion the most viable (and simple) one. It's enough to just merge this PR and make forceMajorVersion a deprecated alias of channel, right?

Actually, a few more steps are needed. Refactor names + validation issue. It can be any of 2.1, 2.2, declare something like we do not do any validation here in readme or something else.

So, still looking for your input @kamazee

@benbor
Copy link
Collaborator Author

benbor commented May 25, 2023

Updated codebase (except tests, yet). It works well on E2E, except one rare case:

>>>  touch -t 04010000 composer.phar && ls -la composer.phar && COMPOSER_CHANNEL=1 ./composer --version

-rwxr-xr-x  1 borisb  staff  2837334 Apr  1 00:00 composer.phar
Warning: You forced the install of 1.10.26 via --1, but 2.5.7 is the latest stable version. Updating to it via composer self-update --stable is recommended.
Upgrading to version 1.10.26 (1.x channel).
   
Use composer self-update --rollback to return to version 2.5.7
Composer version 1.10.26 2022-04-13 16:39:56


>>> touch -t 04010000 composer.phar && ls -la composer.phar && COMPOSER_CHANNEL=2.2 ./composer --version

-rwxr-xr-x  1 borisb  staff  1993462 Apr  1 00:00 composer.phar
Forcing channel 2.2 is requested but current composer version doesn't support --2.2 flag, so nothing will be forced.
A new stable major version of Composer is available (2.5.7), run "composer self-update --2" to update to it. See also https://getcomposer.org/2
You are already using composer version 1.10.26 (1.x channel).
Composer version 1.10.26 2022-04-13 16:39:56

Let me explain: composer1 self-update doesn't have yet --2.2 flag. Composer-installer, composer2 works well in the same scenario, because they have --2.2 flag.
I think we have to ignore such case.

@benbor benbor requested a review from kamazee May 25, 2023 14:04
@benbor benbor force-pushed the version-specifying-changes branch from 98eadbd to 17df7f4 Compare May 25, 2023 14:35
@benbor benbor force-pushed the version-specifying-changes branch from 17df7f4 to 18abf63 Compare May 26, 2023 07:37
@benbor benbor merged commit a19c67a into master May 26, 2023
2 checks passed
@benbor benbor deleted the version-specifying-changes branch May 26, 2023 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants