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

Make satis compatible with composer 2.3, 2.4 and add plugin mode #701

Merged
merged 7 commits into from
Oct 11, 2022

Conversation

ralflang
Copy link
Contributor

@ralflang ralflang commented Aug 18, 2022

  • Fix compatibility issues with composer 2.3/2.4 in CLI mode
  • Integrate ralflang/satis-plugin natively
  • Change app kernel into a child of composer's app kernel and remove parts better inherited
  • Satis as a root project will work as before, auto-installs a version of composer/composer for autoloading reasons
  • Satis as a non-root project will give guidance that composer/composer is no longer automatically installed and what to do
  • updated phpstan tool to not spam the CI log with php notices
  • Updated unit tests to be phpstan l5 compliant after the composer/composer upgrade created breakage due to nested array type annotations

adresses #700, #694

- Integrate ralflang/satis-plugin natively
- Fix compatibility issues with composer 2.3/2.4 in CLI mode
- Change app kernel into a child of composer's app kernel and remove parts better inherited
- Satis as a root project will work as before, auto-installs a version of composer/composer for autoloading reasons
- Satis as a non-root project will give guidance that composer/composer is no longer automatically installed and what to do
@alcohol
Copy link
Member

alcohol commented Oct 10, 2022

Ah, sorry, can you fix the conflicts first?

@private-packagist
Copy link

composer.lock

Package changes

Package Operation From To Changes
composer/class-map-generator add - 1.0.0 view code
seld/signal-handler add - 2.0.1 view code
composer/composer upgrade 2.2.18 2.4.2 diff
composer/pcre upgrade 1.0.1 3.0.0 diff
seld/phar-utils upgrade 1.2.0 1.2.1 diff
symfony/console upgrade v5.4.12 v5.4.13 diff
symfony/filesystem upgrade v5.4.12 v5.4.13 diff
symfony/mime upgrade v5.4.12 v5.4.13 diff
symfony/string upgrade v5.4.12 v5.4.13 diff
twig/twig upgrade v3.4.2 ⚠️ v3.4.3 ✅ diff

Dev Package changes

Package Operation From To Changes
nikic/php-parser upgrade v4.14.0 v4.15.1 diff
phpunit/php-code-coverage upgrade 9.2.16 9.2.17 diff
phpunit/phpunit upgrade 9.5.23 9.5.25 diff
sebastian/comparator upgrade 4.0.6 4.0.8 diff
sebastian/exporter upgrade 4.0.4 4.0.5 diff
sebastian/type upgrade 3.1.0 3.2.0 diff

Settings · Docs · Powered by Private Packagist

@ralflang
Copy link
Contributor Author

Yes, I have merged the conflicts. Meanwhile PHPStan has some complaints and I think a re-test with different composer versions is due after work today. I will highlight you once I am done with that.

@alcohol
Copy link
Member

alcohol commented Oct 10, 2022

PHPStan passing isn't really a requirement, more of a guide / nice-to-have. But I'm fine with waiting till after your test session(s) before merging this PR.

- Work around a bug with RootPackageLoader's / VersionGuesser's internally created ProcessExecutor not being async-enabled
- This should maybe be fixed upstream instead?
@ralflang
Copy link
Contributor Author

@alcohol Test went through

In Standalone mode with "lowest" and "latest" dependencies allowed by constraints:

./bin/satis 
./bin/satis init
./bin/satis add  --type vcs --name "horde/text_diff" https://github.com/maintaina-com/text_diff
./bin/satis build satis.json /tmp/composer/

Lowest gives one deprecation warning in a dependency.

In plugin mode (only tested with latest dependencies)

composer satis:init
composer satis:add --type vcs --name "horde/text_diff" https://github.com/maintaina-com/text_diff
composer satis:build ./satis.json /tmp/composer/

I think we are good to go.

There are two places which smell like they might sense to be changed elsewhere.
For the sake of getting done I worked around them for now.

Composer/Application constructor does not accept optional name and version parameters.
This makes sense from Composer's POV as it thinks it is an app while satis CLI treats it as a library.
https://github.com/composer/satis/pull/701/files#diff-c3161949b1e6a9be81ac8e805ddd3d9f9a8da07e10e3932c297dfadf1ea47da7R37

If leaving out optional arguments to RootPackageLoader, it constructs its own version of VersionGuesser and in turn ProcessExecutor which it cannot really use.
https://github.com/composer/satis/pull/701/files#diff-1c8b1c04ba573f2bb91f1cfa0ce3bb2eca35a8004c5caa12abbf51d86496589dR203

Do you think it makes sense to open issues in the appropriate repos for further exploring these?

@alcohol alcohol merged commit bf82c45 into composer:main Oct 11, 2022
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.

2 participants